From e9c2734441af0065c69fc1317965a6dd6c7f14e3 Mon Sep 17 00:00:00 2001 From: Jim Fehlig Date: Tue, 7 Jul 2015 12:29:24 -0600 Subject: [PATCH] libxl: rework setting the state of virDomainObj Set the state of virDomainObj in the functions that actually change the domain state, instead of the generic libxlDomainCleanup function. This approach gives functions calling libxlDomainCleanup more flexibility wrt when and how they change virDomainObj state via virDomainObjSetState. The prior approach of calling virDomainObjSetState in libxlDomainCleanup resulted in the following incorrect coding pattern in the various functions that change domain state libxlDomain call libxl function to do state transition emit lifecycle event libxlDomainCleanup virDomainObjSetState Once simple manifestation of this bug is seeing a domain running in virt-manager after selecting the shutdown button, even after the domain has long shutdown. --- src/libxl/libxl_domain.c | 23 ++++++++++++----------- src/libxl/libxl_domain.h | 3 +-- src/libxl/libxl_driver.c | 22 +++++++++++++++------- src/libxl/libxl_migration.c | 8 ++++++-- 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 8e8a29216f..224ff773f9 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -397,7 +397,6 @@ libxlDomainShutdownThread(void *opaque) libxlDriverPrivatePtr driver = shutdown_info->driver; virObjectEventPtr dom_event = NULL; libxl_shutdown_reason xl_reason = ev->u.domain_shutdown.shutdown_reason; - virDomainShutoffReason reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN; libxlDriverConfigPtr cfg; cfg = libxlDriverConfigGet(driver); @@ -406,12 +405,14 @@ libxlDomainShutdownThread(void *opaque) goto cleanup; if (xl_reason == LIBXL_SHUTDOWN_REASON_POWEROFF) { + virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, + VIR_DOMAIN_SHUTOFF_SHUTDOWN); + dom_event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN); switch ((virDomainLifecycleAction) vm->def->onPoweroff) { case VIR_DOMAIN_LIFECYCLE_DESTROY: - reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN; goto destroy; case VIR_DOMAIN_LIFECYCLE_RESTART: case VIR_DOMAIN_LIFECYCLE_RESTART_RENAME: @@ -421,12 +422,14 @@ libxlDomainShutdownThread(void *opaque) goto endjob; } } else if (xl_reason == LIBXL_SHUTDOWN_REASON_CRASH) { + virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, + VIR_DOMAIN_SHUTOFF_CRASHED); + dom_event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_CRASHED); switch ((virDomainLifecycleCrashAction) vm->def->onCrash) { case VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY: - reason = VIR_DOMAIN_SHUTOFF_CRASHED; goto destroy; case VIR_DOMAIN_LIFECYCLE_CRASH_RESTART: case VIR_DOMAIN_LIFECYCLE_CRASH_RESTART_RENAME: @@ -442,12 +445,14 @@ libxlDomainShutdownThread(void *opaque) goto restart; } } else if (xl_reason == LIBXL_SHUTDOWN_REASON_REBOOT) { + virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, + VIR_DOMAIN_SHUTOFF_SHUTDOWN); + dom_event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN); switch ((virDomainLifecycleAction) vm->def->onReboot) { case VIR_DOMAIN_LIFECYCLE_DESTROY: - reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN; goto destroy; case VIR_DOMAIN_LIFECYCLE_RESTART: case VIR_DOMAIN_LIFECYCLE_RESTART_RENAME: @@ -467,7 +472,7 @@ libxlDomainShutdownThread(void *opaque) dom_event = NULL; } libxlDomainDestroyInternal(driver, vm); - libxlDomainCleanup(driver, vm, reason); + libxlDomainCleanup(driver, vm); if (!vm->persistent) virDomainObjListRemove(driver->domains, vm); @@ -479,7 +484,7 @@ libxlDomainShutdownThread(void *opaque) dom_event = NULL; } libxlDomainDestroyInternal(driver, vm); - libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN); + libxlDomainCleanup(driver, vm); if (libxlDomainStart(driver, vm, false, -1) < 0) { virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to restart VM '%s': %s"), @@ -685,8 +690,7 @@ libxlDomainDestroyInternal(libxlDriverPrivatePtr driver, */ void libxlDomainCleanup(libxlDriverPrivatePtr driver, - virDomainObjPtr vm, - virDomainShutoffReason reason) + virDomainObjPtr vm) { libxlDomainObjPrivatePtr priv = vm->privateData; libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); @@ -709,9 +713,6 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, priv->deathW = NULL; } - if (vm->persistent) - virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); - if (virAtomicIntDecAndTest(&driver->nactive) && driver->inhibitCallback) driver->inhibitCallback(false, driver->inhibitOpaque); diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h index 8c73cc4f61..44b3e0bc41 100644 --- a/src/libxl/libxl_domain.h +++ b/src/libxl/libxl_domain.h @@ -110,8 +110,7 @@ libxlDomainDestroyInternal(libxlDriverPrivatePtr driver, void libxlDomainCleanup(libxlDriverPrivatePtr driver, - virDomainObjPtr vm, - virDomainShutoffReason reason); + virDomainObjPtr vm); /* * Note: Xen 4.3 removed the const from the event handler signature. diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index c0dd00b8f6..e72b12dfea 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -392,7 +392,7 @@ libxlReconnectDomain(virDomainObjPtr vm, return 0; out: - libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_UNKNOWN); + libxlDomainCleanup(driver, vm); if (!vm->persistent) virDomainObjListRemoveLocked(driver->domains, vm); else @@ -1346,16 +1346,19 @@ libxlDomainDestroyFlags(virDomainPtr dom, goto endjob; } - event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, - VIR_DOMAIN_EVENT_STOPPED_DESTROYED); - if (libxlDomainDestroyInternal(driver, vm) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to destroy domain '%d'"), vm->def->id); goto endjob; } - libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED); + virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, + VIR_DOMAIN_SHUTOFF_DESTROYED); + + event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_DESTROYED); + + libxlDomainCleanup(driver, vm); if (!vm->persistent) virDomainObjListRemove(driver->domains, vm); @@ -1689,6 +1692,9 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm, goto cleanup; } + virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, + VIR_DOMAIN_SHUTOFF_SAVED); + event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_SAVED); @@ -1698,7 +1704,7 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm, goto cleanup; } - libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_SAVED); + libxlDomainCleanup(driver, vm); vm->hasManagedSave = true; ret = 0; @@ -1909,7 +1915,9 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) goto unpause; } - libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_CRASHED); + libxlDomainCleanup(driver, vm); + virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, + VIR_DOMAIN_SHUTOFF_CRASHED); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_CRASHED); if (!vm->persistent) diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 39e4a65b93..aa9547b492 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -585,7 +585,9 @@ libxlDomainMigrationFinish(virConnectPtr dconn, cleanup: if (dom == NULL) { libxlDomainDestroyInternal(driver, vm); - libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); + libxlDomainCleanup(driver, vm); + virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, + VIR_DOMAIN_SHUTOFF_FAILED); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FAILED); if (!vm->persistent) @@ -624,7 +626,9 @@ libxlDomainMigrationConfirm(libxlDriverPrivatePtr driver, } libxlDomainDestroyInternal(driver, vm); - libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_MIGRATED); + libxlDomainCleanup(driver, vm); + virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, + VIR_DOMAIN_SHUTOFF_MIGRATED); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_MIGRATED);