Re: [PATCH 2/4] libxl: queue domain event earlier in shutdown handler

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 21.02.2014 00:02, Jim Fehlig wrote:
> The shutdown handler may restart a domain when handling a reboot
> event or when <on_*> is set to 'restart'.  Restarting consists of
> calling libxlVmCleanup followed by libxlVmStart.  libxlVmStart will
> emit a VIR_DOMAIN_EVENT_STARTED event, but the SHUTDOWN event is
> not emitted until exiting the shutdown handler, after the STARTED
> event.  Queue the event immediately after creation to avoid emitting
> it after the start event.
>
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
>   src/libxl/libxl_driver.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 721577d..e600de7 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -376,6 +376,8 @@ libxlDomainShutdownThread(void *opaque)
>           dom_event = virDomainEventLifecycleNewFromObj(vm,
>                                              VIR_DOMAIN_EVENT_STOPPED,
>                                              VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
> +        if (dom_event)
> +            libxlDomainEventQueue(driver, dom_event);
>           switch (vm->def->onPoweroff) {
>           case VIR_DOMAIN_LIFECYCLE_DESTROY:
>               reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
> @@ -390,6 +392,8 @@ libxlDomainShutdownThread(void *opaque)
>           dom_event = virDomainEventLifecycleNewFromObj(vm,
>                                              VIR_DOMAIN_EVENT_STOPPED,
>                                              VIR_DOMAIN_EVENT_STOPPED_CRASHED);
> +        if (dom_event)
> +            libxlDomainEventQueue(driver, dom_event);
>           switch (vm->def->onCrash) {
>           case VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY:
>               reason = VIR_DOMAIN_SHUTOFF_CRASHED;
> @@ -404,6 +408,8 @@ libxlDomainShutdownThread(void *opaque)
>           dom_event = virDomainEventLifecycleNewFromObj(vm,
>                                              VIR_DOMAIN_EVENT_STOPPED,
>                                              VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
> +        if (dom_event)
> +            libxlDomainEventQueue(driver, dom_event);
>           switch (vm->def->onReboot) {
>           case VIR_DOMAIN_LIFECYCLE_DESTROY:
>               reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
> @@ -437,8 +443,6 @@ restart:
>   cleanup:
>       if (vm)
>           virObjectUnlock(vm);
> -    if (dom_event)
> -        libxlDomainEventQueue(driver, dom_event);
>       libxl_event_free(ctx, ev);
>       VIR_FREE(shutdown_info);
>   }

Wouldn't it be better to enqueue events at the beginning of 'destroy' 
and 'restart' labels? I'm thinking about something among these lines:

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 0b9bf7d..c009407 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -425,6 +425,10 @@ libxlDomainShutdownThread(void *opaque)
     }

 destroy:
+    if (dom_event) {
+        libxlDomainEventQueue(driver, dom_event);
+        dom_event = NULL;
+    }
     libxl_domain_destroy(ctx, vm->def->id, NULL);
     if (libxlVmCleanupJob(driver, vm, reason)) {
         if (!vm->persistent) {
@@ -435,6 +439,10 @@ destroy:
     goto cleanup;

 restart:
+    if (dom_event) {
+        libxlDomainEventQueue(driver, dom_event);
+        dom_event = NULL;
+    }
     libxl_domain_destroy(ctx, vm->def->id, NULL);
     libxlVmCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
     libxlVmStart(driver, vm, 0, -1);


Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list