Mailing List Archive

[RESEND PATCH 12/12] golang/xenlight: add NotifyDomainDeath method to Context
Add a helper function to wait for domain death events, and then write
the events to a provided channel. This handles the enabling/disabling of
the event type, freeing the event, and converting it to a Go type. The
caller can then handle the event however they need to. This function
will run until a provided context.Context is cancelled.

NotifyDomainDeath spawns two goroutines that return when the
context.Context is done. The first will make sure that the domain death
event is disabled, and that the corresponding event queue is cleared.
The second calls libxl_event_wait, and writes the event to the provided
channel.

With this, callers should be able to manage a full domain life cycle.
Add to the comment of DomainCreateNew so that package uses know they
should use this method in conjunction with DomainCreateNew.

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
---
tools/golang/xenlight/xenlight.go | 83 ++++++++++++++++++++++++++++++-
1 file changed, 82 insertions(+), 1 deletion(-)

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index 6fb22665cc..8406883433 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -53,6 +53,7 @@ import "C"
*/

import (
+ "context"
"fmt"
"os"
"os/signal"
@@ -1340,7 +1341,9 @@ func (ctx *Context) DeviceUsbdevRemove(domid Domid, usbdev *DeviceUsbdev) error
return nil
}

-// DomainCreateNew creates a new domain.
+// DomainCreateNew creates a new domain. Callers of DomainCreateNew are
+// responsible for handling the death of the resulting domain. This should be
+// done using NotifyDomainDeath.
func (ctx *Context) DomainCreateNew(config *DomainConfig) (Domid, error) {
var cdomid C.uint32_t
var cconfig C.libxl_domain_config
@@ -1358,6 +1361,84 @@ func (ctx *Context) DomainCreateNew(config *DomainConfig) (Domid, error) {
return Domid(cdomid), nil
}

+// NotifyDomainDeath registers an event handler for domain death events for a
+// given domnid, and writes events received to ec. NotifyDomainDeath returns an
+// error if it cannot register the event handler, but other errors encountered
+// are just logged. The goroutine spawned by calling NotifyDomainDeath runs
+// until the provided context.Context's Done channel is closed.
+func (ctx *Context) NotifyDomainDeath(c context.Context, domid Domid, ec chan<- Event) error {
+ var deathw *C.libxl_evgen_domain_death
+
+ ret := C.libxl_evenable_domain_death(ctx.ctx, C.uint32_t(domid), 0, &deathw)
+ if ret != 0 {
+ return Error(ret)
+ }
+
+ // Spawn a goroutine that is responsible for cleaning up when the
+ // passed context.Context's Done channel is closed.
+ go func() {
+ <-c.Done()
+
+ ctx.logd("cleaning up domain death event handler for domain %d", domid)
+
+ // Disable the event generation.
+ C.libxl_evdisable_domain_death(ctx.ctx, deathw)
+
+ // Make sure any events that were generated get cleaned up so they
+ // do not linger in the libxl event queue.
+ var evc *C.libxl_event
+ for {
+ ret := C.libxl_event_check(ctx.ctx, &evc, C.LIBXL_EVENTMASK_ALL, nil, nil)
+ if ret != 0 {
+ return
+ }
+ C.libxl_event_free(ctx.ctx, evc)
+ }
+ }()
+
+ go func() {
+ var (
+ ev Event
+ evc *C.libxl_event
+ )
+
+ for {
+ select {
+ case <-c.Done():
+ return
+ default:
+ // Go on and check for an event...
+ }
+
+ ret := C.libxl_event_wait(ctx.ctx, &evc, C.LIBXL_EVENTMASK_ALL, nil, nil)
+ if ret != 0 {
+ ctx.logw("unexpected error waiting for event: %s", Error(ret))
+ continue
+ }
+
+ // Try to convert the event to Go, and then free the
+ // C.libxl_event no matter what.
+ err := ev.fromC(evc)
+ C.libxl_event_free(ctx.ctx, evc)
+ if err != nil {
+ ctx.logw("error converting event from C: %v", err)
+ continue
+ }
+
+ ctx.logd("received domain death event (domid=%v, type=%v)", ev.Domid, ev.Type)
+
+ // Write the event to the channel
+ select {
+ case ec <- ev:
+ case <-c.Done():
+ return
+ }
+ }
+ }()
+
+ return nil
+}
+
// DomainDestroy destroys a domain given a domid.
func (ctx *Context) DomainDestroy(domid Domid) error {
ret := C.libxl_domain_destroy(ctx.ctx, C.uint32_t(domid), nil)
--
2.17.1
Re: [RESEND PATCH 12/12] golang/xenlight: add NotifyDomainDeath method to Context [ In reply to ]
> On May 24, 2021, at 9:36 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
>
> Add a helper function to wait for domain death events, and then write
> the events to a provided channel. This handles the enabling/disabling of
> the event type, freeing the event, and converting it to a Go type. The
> caller can then handle the event however they need to. This function
> will run until a provided context.Context is cancelled.
>
> NotifyDomainDeath spawns two goroutines that return when the
> context.Context is done. The first will make sure that the domain death
> event is disabled, and that the corresponding event queue is cleared.
> The second calls libxl_event_wait, and writes the event to the provided
> channel.
>
> With this, callers should be able to manage a full domain life cycle.
> Add to the comment of DomainCreateNew so that package uses know they
> should use this method in conjunction with DomainCreateNew.
>
> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
> ---
> tools/golang/xenlight/xenlight.go | 83 ++++++++++++++++++++++++++++++-
> 1 file changed, 82 insertions(+), 1 deletion(-)
>
> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> index 6fb22665cc..8406883433 100644
> --- a/tools/golang/xenlight/xenlight.go
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -53,6 +53,7 @@ import "C"
> */
>
> import (
> + "context"
> "fmt"
> "os"
> "os/signal"
> @@ -1340,7 +1341,9 @@ func (ctx *Context) DeviceUsbdevRemove(domid Domid, usbdev *DeviceUsbdev) error
> return nil
> }
>
> -// DomainCreateNew creates a new domain.
> +// DomainCreateNew creates a new domain. Callers of DomainCreateNew are
> +// responsible for handling the death of the resulting domain. This should be
> +// done using NotifyDomainDeath.
> func (ctx *Context) DomainCreateNew(config *DomainConfig) (Domid, error) {
> var cdomid C.uint32_t
> var cconfig C.libxl_domain_config
> @@ -1358,6 +1361,84 @@ func (ctx *Context) DomainCreateNew(config *DomainConfig) (Domid, error) {
> return Domid(cdomid), nil
> }
>
> +// NotifyDomainDeath registers an event handler for domain death events for a
> +// given domnid, and writes events received to ec. NotifyDomainDeath returns an
> +// error if it cannot register the event handler, but other errors encountered
> +// are just logged. The goroutine spawned by calling NotifyDomainDeath runs
> +// until the provided context.Context's Done channel is closed.
> +func (ctx *Context) NotifyDomainDeath(c context.Context, domid Domid, ec chan<- Event) error {
> + var deathw *C.libxl_evgen_domain_death
> +
> + ret := C.libxl_evenable_domain_death(ctx.ctx, C.uint32_t(domid), 0, &deathw)
> + if ret != 0 {
> + return Error(ret)
> + }
> +
> + // Spawn a goroutine that is responsible for cleaning up when the
> + // passed context.Context's Done channel is closed.
> + go func() {
> + <-c.Done()
> +
> + ctx.logd("cleaning up domain death event handler for domain %d", domid)
> +
> + // Disable the event generation.
> + C.libxl_evdisable_domain_death(ctx.ctx, deathw)
> +
> + // Make sure any events that were generated get cleaned up so they
> + // do not linger in the libxl event queue.
> + var evc *C.libxl_event
> + for {
> + ret := C.libxl_event_check(ctx.ctx, &evc, C.LIBXL_EVENTMASK_ALL, nil, nil)
> + if ret != 0 {
> + return
> + }
> + C.libxl_event_free(ctx.ctx, evc)

I have to admit, I don’t really understand how the libxl event stuff is supposed to work. But it looks like this will drain all events of any type, for any domain, associated with this context?

So if you had two domains, and called NotifyDomainDeath() on both with different contexts, and you closed the one context, you might miss events from the other context?

Or, suppose you did this:
* ctx.NotifyDomainDeath(ctx1, dom1, ec1)
* ctx.NotifyDiskEject(ctx2, dom1, ec2)
* ctx1CancelFunc()

Wouldn’t there be a risk that the disk eject message would get lost?

Ian, any suggestions for the right way to use these functions in this scenario?

-George
Re: [RESEND PATCH 12/12] golang/xenlight: add NotifyDomainDeath method to Context [ In reply to ]
> On Jun 18, 2021, at 7:28 PM, George Dunlap <george.dunlap@citrix.com> wrote:
>
>
>
>> On May 24, 2021, at 9:36 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
>>
>> Add a helper function to wait for domain death events, and then write
>> the events to a provided channel. This handles the enabling/disabling of
>> the event type, freeing the event, and converting it to a Go type. The
>> caller can then handle the event however they need to. This function
>> will run until a provided context.Context is cancelled.
>>
>> NotifyDomainDeath spawns two goroutines that return when the
>> context.Context is done. The first will make sure that the domain death
>> event is disabled, and that the corresponding event queue is cleared.
>> The second calls libxl_event_wait, and writes the event to the provided
>> channel.
>>
>> With this, callers should be able to manage a full domain life cycle.
>> Add to the comment of DomainCreateNew so that package uses know they
>> should use this method in conjunction with DomainCreateNew.
>>
>> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
>> ---
>> tools/golang/xenlight/xenlight.go | 83 ++++++++++++++++++++++++++++++-
>> 1 file changed, 82 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
>> index 6fb22665cc..8406883433 100644
>> --- a/tools/golang/xenlight/xenlight.go
>> +++ b/tools/golang/xenlight/xenlight.go
>> @@ -53,6 +53,7 @@ import "C"
>> */
>>
>> import (
>> + "context"
>> "fmt"
>> "os"
>> "os/signal"
>> @@ -1340,7 +1341,9 @@ func (ctx *Context) DeviceUsbdevRemove(domid Domid, usbdev *DeviceUsbdev) error
>> return nil
>> }
>>
>> -// DomainCreateNew creates a new domain.
>> +// DomainCreateNew creates a new domain. Callers of DomainCreateNew are
>> +// responsible for handling the death of the resulting domain. This should be
>> +// done using NotifyDomainDeath.
>> func (ctx *Context) DomainCreateNew(config *DomainConfig) (Domid, error) {
>> var cdomid C.uint32_t
>> var cconfig C.libxl_domain_config
>> @@ -1358,6 +1361,84 @@ func (ctx *Context) DomainCreateNew(config *DomainConfig) (Domid, error) {
>> return Domid(cdomid), nil
>> }
>>
>> +// NotifyDomainDeath registers an event handler for domain death events for a
>> +// given domnid, and writes events received to ec. NotifyDomainDeath returns an
>> +// error if it cannot register the event handler, but other errors encountered
>> +// are just logged. The goroutine spawned by calling NotifyDomainDeath runs
>> +// until the provided context.Context's Done channel is closed.
>> +func (ctx *Context) NotifyDomainDeath(c context.Context, domid Domid, ec chan<- Event) error {
>> + var deathw *C.libxl_evgen_domain_death
>> +
>> + ret := C.libxl_evenable_domain_death(ctx.ctx, C.uint32_t(domid), 0, &deathw)
>> + if ret != 0 {
>> + return Error(ret)
>> + }
>> +
>> + // Spawn a goroutine that is responsible for cleaning up when the
>> + // passed context.Context's Done channel is closed.
>> + go func() {
>> + <-c.Done()
>> +
>> + ctx.logd("cleaning up domain death event handler for domain %d", domid)
>> +
>> + // Disable the event generation.
>> + C.libxl_evdisable_domain_death(ctx.ctx, deathw)
>> +
>> + // Make sure any events that were generated get cleaned up so they
>> + // do not linger in the libxl event queue.
>> + var evc *C.libxl_event
>> + for {
>> + ret := C.libxl_event_check(ctx.ctx, &evc, C.LIBXL_EVENTMASK_ALL, nil, nil)
>> + if ret != 0 {
>> + return
>> + }
>> + C.libxl_event_free(ctx.ctx, evc)
>
> I have to admit, I don’t really understand how the libxl event stuff is supposed to work. But it looks like this will drain all events of any type, for any domain, associated with this context?
>
> So if you had two domains, and called NotifyDomainDeath() on both with different contexts, and you closed the one context, you might miss events from the other context?
>
> Or, suppose you did this:
> * ctx.NotifyDomainDeath(ctx1, dom1, ec1)
> * ctx.NotifyDiskEject(ctx2, dom1, ec2)
> * ctx1CancelFunc()
>
> Wouldn’t there be a risk that the disk eject message would get lost?
>
> Ian, any suggestions for the right way to use these functions in this scenario?

It looks like one option would be to add a “predicate” function filter, to filter by type and domid.

It looks like the other option would be to try to use libxl_event_register_callbacks(). We could have the C callback pass all the events to a goroutine which would act as a dispatcher.

-George
Re: [RESEND PATCH 12/12] golang/xenlight: add NotifyDomainDeath method to Context [ In reply to ]
On Fri, Jun 18, 2021 at 07:31:46PM +0000, George Dunlap wrote:
>
>
> > On Jun 18, 2021, at 7:28 PM, George Dunlap <george.dunlap@citrix.com> wrote:
> >
> >
> >
> >> On May 24, 2021, at 9:36 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
> >>
> >> Add a helper function to wait for domain death events, and then write
> >> the events to a provided channel. This handles the enabling/disabling of
> >> the event type, freeing the event, and converting it to a Go type. The
> >> caller can then handle the event however they need to. This function
> >> will run until a provided context.Context is cancelled.
> >>
> >> NotifyDomainDeath spawns two goroutines that return when the
> >> context.Context is done. The first will make sure that the domain death
> >> event is disabled, and that the corresponding event queue is cleared.
> >> The second calls libxl_event_wait, and writes the event to the provided
> >> channel.
> >>
> >> With this, callers should be able to manage a full domain life cycle.
> >> Add to the comment of DomainCreateNew so that package uses know they
> >> should use this method in conjunction with DomainCreateNew.
> >>
> >> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
> >> ---
> >> tools/golang/xenlight/xenlight.go | 83 ++++++++++++++++++++++++++++++-
> >> 1 file changed, 82 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> >> index 6fb22665cc..8406883433 100644
> >> --- a/tools/golang/xenlight/xenlight.go
> >> +++ b/tools/golang/xenlight/xenlight.go
> >> @@ -53,6 +53,7 @@ import "C"
> >> */
> >>
> >> import (
> >> + "context"
> >> "fmt"
> >> "os"
> >> "os/signal"
> >> @@ -1340,7 +1341,9 @@ func (ctx *Context) DeviceUsbdevRemove(domid Domid, usbdev *DeviceUsbdev) error
> >> return nil
> >> }
> >>
> >> -// DomainCreateNew creates a new domain.
> >> +// DomainCreateNew creates a new domain. Callers of DomainCreateNew are
> >> +// responsible for handling the death of the resulting domain. This should be
> >> +// done using NotifyDomainDeath.
> >> func (ctx *Context) DomainCreateNew(config *DomainConfig) (Domid, error) {
> >> var cdomid C.uint32_t
> >> var cconfig C.libxl_domain_config
> >> @@ -1358,6 +1361,84 @@ func (ctx *Context) DomainCreateNew(config *DomainConfig) (Domid, error) {
> >> return Domid(cdomid), nil
> >> }
> >>
> >> +// NotifyDomainDeath registers an event handler for domain death events for a
> >> +// given domnid, and writes events received to ec. NotifyDomainDeath returns an
> >> +// error if it cannot register the event handler, but other errors encountered
> >> +// are just logged. The goroutine spawned by calling NotifyDomainDeath runs
> >> +// until the provided context.Context's Done channel is closed.
> >> +func (ctx *Context) NotifyDomainDeath(c context.Context, domid Domid, ec chan<- Event) error {
> >> + var deathw *C.libxl_evgen_domain_death
> >> +
> >> + ret := C.libxl_evenable_domain_death(ctx.ctx, C.uint32_t(domid), 0, &deathw)
> >> + if ret != 0 {
> >> + return Error(ret)
> >> + }
> >> +
> >> + // Spawn a goroutine that is responsible for cleaning up when the
> >> + // passed context.Context's Done channel is closed.
> >> + go func() {
> >> + <-c.Done()
> >> +
> >> + ctx.logd("cleaning up domain death event handler for domain %d", domid)
> >> +
> >> + // Disable the event generation.
> >> + C.libxl_evdisable_domain_death(ctx.ctx, deathw)
> >> +
> >> + // Make sure any events that were generated get cleaned up so they
> >> + // do not linger in the libxl event queue.
> >> + var evc *C.libxl_event
> >> + for {
> >> + ret := C.libxl_event_check(ctx.ctx, &evc, C.LIBXL_EVENTMASK_ALL, nil, nil)
> >> + if ret != 0 {
> >> + return
> >> + }
> >> + C.libxl_event_free(ctx.ctx, evc)
> >
> > I have to admit, I don’t really understand how the libxl event stuff is supposed to work. But it looks like this will drain all events of any type, for any domain, associated with this context?
> >
> > So if you had two domains, and called NotifyDomainDeath() on both with different contexts, and you closed the one context, you might miss events from the other context?
> >
> > Or, suppose you did this:
> > * ctx.NotifyDomainDeath(ctx1, dom1, ec1)
> > * ctx.NotifyDiskEject(ctx2, dom1, ec2)
> > * ctx1CancelFunc()
> >
> > Wouldn’t there be a risk that the disk eject message would get lost?
> >
> > Ian, any suggestions for the right way to use these functions in this scenario?
>
> It looks like one option would be to add a “predicate” function filter, to filter by type and domid.
>
> It looks like the other option would be to try to use libxl_event_register_callbacks(). We could have the C callback pass all the events to a goroutine which would act as a dispatcher.
>
After a brief look at the documentation within libxl_event.h, it seems
using predicate functions would be the easiest solution given the
current layout. Though I will look closer at using
libxl_event_register_callbacks before sending a v2.

Thanks,
NR