On Thu, Jan 24, 2013 at 12:45:21PM -0700, Jim Fehlig wrote:
> Jim Fehlig wrote:
> > Since libxl provides the domain ID in the event handler callback,
> > find the domain object based on the ID. This approach prevents
> > processing the callback on a domain that has already been reaped.
> > ---
> > src/libxl/libxl_driver.c | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> > index 7484b83..e28b641 100644
> > --- a/src/libxl/libxl_driver.c
> > +++ b/src/libxl/libxl_driver.c
> > @@ -656,22 +656,22 @@ libxlVmReap(libxlDriverPrivatePtr driver,
> > * Handle previously registered event notification from libxenlight
> > */
> > static void
> > -libxlEventHandler(void *data, const libxl_event *event)
> > +libxlEventHandler(void *data ATTRIBUTE_UNUSED, const libxl_event *event)
> > {
> > libxlDriverPrivatePtr driver = libxl_driver;
> > - virDomainObjPtr vm = data;
> > + virDomainObjPtr vm = NULL;
> > virDomainEventPtr dom_event = NULL;
> >
> > libxlDriverLock(driver);
> >
>
> On xen-unstable, I noticed an occasional deadlock here when libxl
> invokes the event handler with a SUSPEND shutdown reason. The driver
> lock is already held by the caller of libxl_domain_suspend(). Inspired
> by the xl implementation, I've updated this patch to ignore the SUSPEND
> shutdown reason before acquiring the driver lock.
>
> Regards,
> Jim
>
>
> >From b9b3cd0259168ccf10f525d1b459bfe790162be3 Mon Sep 17 00:00:00 2001
> From: Jim Fehlig <jfehlig@xxxxxxxx>
> Date: Mon, 21 Jan 2013 10:36:03 -0700
> Subject: [PATCH 6/6] libxl: Domain event handler improvements
>
> Since libxl provides the domain ID in the event handler callback,
> find the domain object based on the ID. This approach prevents
> processing the callback on a domain that has already been reaped.
>
> Also, similar to the xl implementation, ignore the SUSPEND shutdown
> reason. By calling libxl_domain_suspend(), we know a shutdown
> event with SUSPEND reason will be generated, but it can be safely
> ignored since any subsequent cleanup will be done by the callers.
> ---
> src/libxl/libxl_driver.c | 26 ++++++++++++++++----------
> 1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 7484b83..39875aa 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -656,26 +656,32 @@ libxlVmReap(libxlDriverPrivatePtr driver,
> * Handle previously registered event notification from libxenlight
> */
> static void
> -libxlEventHandler(void *data, const libxl_event *event)
> +libxlEventHandler(void *data ATTRIBUTE_UNUSED, const libxl_event *event)
> {
> libxlDriverPrivatePtr driver = libxl_driver;
> - virDomainObjPtr vm = data;
> + virDomainObjPtr vm = NULL;
> virDomainEventPtr dom_event = NULL;
> -
> - libxlDriverLock(driver);
> - virObjectLock(vm);
> - libxlDriverUnlock(driver);
> + libxl_shutdown_reason xl_reason = event->u.domain_shutdown.shutdown_reason;
>
> if (event->type == LIBXL_EVENT_TYPE_DOMAIN_SHUTDOWN) {
> virDomainShutoffReason reason;
>
> - if (event->domid != vm->def->id)
> + /* Similar to the xl implementation, ignore SUSPEND. Any actions needed
> + after calling libxl_domain_suspend() are handled by it's callers. */
> + if (xl_reason == LIBXL_SHUTDOWN_REASON_SUSPEND)
> + goto cleanup;
> +
> + libxlDriverLock(driver);
> + vm = virDomainFindByID(&driver->domains, event->domid);
> + libxlDriverUnlock(driver);
> +
> + if (!vm)
> goto cleanup;
>
> - switch (event->u.domain_shutdown.shutdown_reason) {
> + switch (xl_reason) {
> case LIBXL_SHUTDOWN_REASON_POWEROFF:
> case LIBXL_SHUTDOWN_REASON_CRASH:
> - if (event->u.domain_shutdown.shutdown_reason == LIBXL_SHUTDOWN_REASON_CRASH) {
> + if (xl_reason == LIBXL_SHUTDOWN_REASON_CRASH) {
> dom_event = virDomainEventNewFromObj(vm,
> VIR_DOMAIN_EVENT_STOPPED,
> VIR_DOMAIN_EVENT_STOPPED_CRASHED);
> @@ -694,7 +700,7 @@ libxlEventHandler(void *data, const libxl_event *event)
> libxlVmStart(driver, vm, 0, -1);
> break;
> default:
> - VIR_INFO("Unhandled shutdown_reason %d", event->u.domain_shutdown.shutdown_reason);
> + VIR_INFO("Unhandled shutdown_reason %d", xl_reason);
> break;
> }
> }
> --
> 1.8.0.1
>
ACK,
Daniel
--
Daniel Veillard | Open Source and Standards, Red Hat
veillard@xxxxxxxxxx | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list