Re: [libvirt] [PATCH 2/4] libxl: queue domain event earlier in shutdown handler
Michal Privoznik wrote:
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(a)suse.com>
> ---
> 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:
Yes, agreed. As discussed on IRC, better to enqueue the event at the
labels since it is one less call to libxlDomainEventQueue, and improves
the code to handle any future <on_foo> action.
Regards,
Jim
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