[libvirt] [PATCH V2 0/3] libxl: add support for soft reset

Patches 1 and 2 are new to V2 and make slight improvements to libxlDomainShutdownThread. Patch 3 is adjusted to work with Xen 4.6. Jim Fehlig (3): libxl: remove redundant calls to virObjectEventStateQueue libxl: Remove some goto labels in libxlDomainShutdownThread libxl: add support for soft reset src/libxl/libxl_domain.c | 99 ++++++++++++++++++++++++++++------------ 1 file changed, 69 insertions(+), 30 deletions(-) -- 2.18.0

In libxlDomainShutdownThread, virObjectEventStateQueue is needlessly called in the destroy and restart labels. The cleanup label aready queues whatever event was created based on libxl_shutdown_reason. There is no need to handle destroy and restart differently. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 0032b9dd11..9ed6ee8fb3 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -538,8 +538,6 @@ libxlDomainShutdownThread(void *opaque) } destroy: - virObjectEventStateQueue(driver->domainEventState, dom_event); - dom_event = NULL; libxlDomainDestroyInternal(driver, vm); libxlDomainCleanup(driver, vm); if (!vm->persistent) @@ -548,8 +546,6 @@ libxlDomainShutdownThread(void *opaque) goto endjob; restart: - virObjectEventStateQueue(driver->domainEventState, dom_event); - dom_event = NULL; libxlDomainDestroyInternal(driver, vm); libxlDomainCleanup(driver, vm); if (libxlDomainStartNew(driver, vm, false) < 0) { -- 2.18.0

There are too many goto labels in libxlDomainShutdownThread. Convert the 'destroy' and 'restart' labels to helper functions, leaving only the commonly used pattern of 'endjob' and 'cleanup' labels. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 66 ++++++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 26 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 9ed6ee8fb3..4cdaee0e51 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -430,6 +430,30 @@ virDomainDefParserConfig libxlDomainDefParserConfig = { }; +static void +libxlDomainShutdownHandleDestroy(libxlDriverPrivatePtr driver, + virDomainObjPtr vm) +{ + libxlDomainDestroyInternal(driver, vm); + libxlDomainCleanup(driver, vm); + if (!vm->persistent) + virDomainObjListRemove(driver->domains, vm); +} + + +static void +libxlDomainShutdownHandleRestart(libxlDriverPrivatePtr driver, + virDomainObjPtr vm) +{ + libxlDomainDestroyInternal(driver, vm); + libxlDomainCleanup(driver, vm); + if (libxlDomainStartNew(driver, vm, false) < 0) { + VIR_ERROR(_("Failed to restart VM '%s': %s"), + vm->def->name, virGetLastErrorMessage()); + } +} + + struct libxlShutdownThreadInfo { libxlDriverPrivatePtr driver; @@ -468,10 +492,12 @@ libxlDomainShutdownThread(void *opaque) VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN); switch ((virDomainLifecycleAction) vm->def->onPoweroff) { case VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY: - goto destroy; + libxlDomainShutdownHandleDestroy(driver, vm); + goto endjob; case VIR_DOMAIN_LIFECYCLE_ACTION_RESTART: case VIR_DOMAIN_LIFECYCLE_ACTION_RESTART_RENAME: - goto restart; + libxlDomainShutdownHandleRestart(driver, vm); + goto endjob; case VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE: case VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_DESTROY: case VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_RESTART: @@ -487,19 +513,23 @@ libxlDomainShutdownThread(void *opaque) VIR_DOMAIN_EVENT_STOPPED_CRASHED); switch ((virDomainLifecycleAction) vm->def->onCrash) { case VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY: - goto destroy; + libxlDomainShutdownHandleDestroy(driver, vm); + goto endjob; case VIR_DOMAIN_LIFECYCLE_ACTION_RESTART: case VIR_DOMAIN_LIFECYCLE_ACTION_RESTART_RENAME: - goto restart; + libxlDomainShutdownHandleRestart(driver, vm); + goto endjob; case VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE: case VIR_DOMAIN_LIFECYCLE_ACTION_LAST: goto endjob; case VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_DESTROY: libxlDomainAutoCoreDump(driver, vm); - goto destroy; + libxlDomainShutdownHandleDestroy(driver, vm); + goto endjob; case VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_RESTART: libxlDomainAutoCoreDump(driver, vm); - goto restart; + libxlDomainShutdownHandleRestart(driver, vm); + goto endjob; } } else if (xl_reason == LIBXL_SHUTDOWN_REASON_REBOOT) { virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, @@ -510,10 +540,12 @@ libxlDomainShutdownThread(void *opaque) VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN); switch ((virDomainLifecycleAction) vm->def->onReboot) { case VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY: - goto destroy; + libxlDomainShutdownHandleDestroy(driver, vm); + goto endjob; case VIR_DOMAIN_LIFECYCLE_ACTION_RESTART: case VIR_DOMAIN_LIFECYCLE_ACTION_RESTART_RENAME: - goto restart; + libxlDomainShutdownHandleRestart(driver, vm); + goto endjob; case VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE: case VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_DESTROY: case VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_RESTART: @@ -531,26 +563,8 @@ libxlDomainShutdownThread(void *opaque) * Similar to the xl implementation, ignore SUSPEND. Any actions needed * after calling libxl_domain_suspend() are handled by it's callers. */ - goto endjob; } else { VIR_INFO("Unhandled shutdown_reason %d", xl_reason); - goto endjob; - } - - destroy: - libxlDomainDestroyInternal(driver, vm); - libxlDomainCleanup(driver, vm); - if (!vm->persistent) - virDomainObjListRemove(driver->domains, vm); - - goto endjob; - - restart: - libxlDomainDestroyInternal(driver, vm); - libxlDomainCleanup(driver, vm); - if (libxlDomainStartNew(driver, vm, false) < 0) { - VIR_ERROR(_("Failed to restart VM '%s': %s"), - vm->def->name, virGetLastErrorMessage()); } endjob: -- 2.18.0

The pvops Linux kernel implements machine_ops.crash_shutdown as static void xen_hvm_crash_shutdown(struct pt_regs *regs) { native_machine_crash_shutdown(regs); xen_reboot(SHUTDOWN_soft_reset); } but currently the libxl driver does not handle the soft reset shutdown event. As a result, the guest domain never proceeds past xen_reboot(), making it impossible for HVM domains to save a crash dump using kexec. This patch adds support for handling the soft reset event by calling libxl_domain_soft_reset() and re-enabling domain death events, which is similar to the xl tool handling of soft reset shutdown event. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- V2: Check for availability of soft reset with LIBXL_HAVE_SOFT_RESET src/libxl/libxl_domain.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 4cdaee0e51..47c1f49538 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -471,8 +471,10 @@ libxlDomainShutdownThread(void *opaque) virObjectEventPtr dom_event = NULL; libxl_shutdown_reason xl_reason = ev->u.domain_shutdown.shutdown_reason; libxlDriverConfigPtr cfg; + libxl_domain_config d_config; cfg = libxlDriverConfigGet(driver); + libxl_domain_config_init(&d_config); vm = virDomainObjListFindByID(driver->domains, ev->domid); if (!vm) { @@ -563,6 +565,33 @@ libxlDomainShutdownThread(void *opaque) * Similar to the xl implementation, ignore SUSPEND. Any actions needed * after calling libxl_domain_suspend() are handled by it's callers. */ +#ifdef LIBXL_HAVE_SOFT_RESET + } else if (xl_reason == LIBXL_SHUTDOWN_REASON_SOFT_RESET) { + libxlDomainObjPrivatePtr priv = vm->privateData; + if (libxl_retrieve_domain_configuration(cfg->ctx, vm->def->id, + &d_config) != 0) { + VIR_ERROR(_("Failed to retrieve config for VM '%s'. " + "Unable to perform soft reset. Destroying VM"), + vm->def->name); + libxlDomainShutdownHandleDestroy(driver, vm); + goto endjob; + } + + if (priv->deathW) { + libxl_evdisable_domain_death(cfg->ctx, priv->deathW); + priv->deathW = NULL; + } + + if (libxl_domain_soft_reset(cfg->ctx, &d_config, vm->def->id, + NULL, NULL) != 0) { + VIR_ERROR(_("Failed to soft reset VM '%s'. Destroying VM"), + vm->def->name); + libxlDomainShutdownHandleDestroy(driver, vm); + goto endjob; + } + libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, &priv->deathW); + libxl_domain_unpause(cfg->ctx, vm->def->id); +#endif } else { VIR_INFO("Unhandled shutdown_reason %d", xl_reason); } -- 2.18.0

On 10/31/2018 08:51 PM, Jim Fehlig wrote:
The pvops Linux kernel implements machine_ops.crash_shutdown as
static void xen_hvm_crash_shutdown(struct pt_regs *regs) { native_machine_crash_shutdown(regs); xen_reboot(SHUTDOWN_soft_reset); }
but currently the libxl driver does not handle the soft reset shutdown event. As a result, the guest domain never proceeds past xen_reboot(), making it impossible for HVM domains to save a crash dump using kexec.
This patch adds support for handling the soft reset event by calling libxl_domain_soft_reset() and re-enabling domain death events, which is similar to the xl tool handling of soft reset shutdown event.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> ---
V2: Check for availability of soft reset with LIBXL_HAVE_SOFT_RESET
src/libxl/libxl_domain.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 4cdaee0e51..47c1f49538 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -471,8 +471,10 @@ libxlDomainShutdownThread(void *opaque) virObjectEventPtr dom_event = NULL; libxl_shutdown_reason xl_reason = ev->u.domain_shutdown.shutdown_reason; libxlDriverConfigPtr cfg; + libxl_domain_config d_config;
cfg = libxlDriverConfigGet(driver); + libxl_domain_config_init(&d_config);
vm = virDomainObjListFindByID(driver->domains, ev->domid); if (!vm) { @@ -563,6 +565,33 @@ libxlDomainShutdownThread(void *opaque) * Similar to the xl implementation, ignore SUSPEND. Any actions needed * after calling libxl_domain_suspend() are handled by it's callers. */ +#ifdef LIBXL_HAVE_SOFT_RESET + } else if (xl_reason == LIBXL_SHUTDOWN_REASON_SOFT_RESET) { + libxlDomainObjPrivatePtr priv = vm->privateData;
I'd put an empty line here.
+ if (libxl_retrieve_domain_configuration(cfg->ctx, vm->def->id, + &d_config) != 0) { + VIR_ERROR(_("Failed to retrieve config for VM '%s'. " + "Unable to perform soft reset. Destroying VM"), + vm->def->name); + libxlDomainShutdownHandleDestroy(driver, vm); + goto endjob; + } + + if (priv->deathW) { + libxl_evdisable_domain_death(cfg->ctx, priv->deathW); + priv->deathW = NULL; + } + + if (libxl_domain_soft_reset(cfg->ctx, &d_config, vm->def->id, + NULL, NULL) != 0) { + VIR_ERROR(_("Failed to soft reset VM '%s'. Destroying VM"), + vm->def->name); + libxlDomainShutdownHandleDestroy(driver, vm); + goto endjob; + } + libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, &priv->deathW); + libxl_domain_unpause(cfg->ctx, vm->def->id); +#endif } else { VIR_INFO("Unhandled shutdown_reason %d", xl_reason); }
Michal

On 10/31/2018 08:51 PM, Jim Fehlig wrote:
Patches 1 and 2 are new to V2 and make slight improvements to libxlDomainShutdownThread. Patch 3 is adjusted to work with Xen 4.6.
Jim Fehlig (3): libxl: remove redundant calls to virObjectEventStateQueue libxl: Remove some goto labels in libxlDomainShutdownThread libxl: add support for soft reset
src/libxl/libxl_domain.c | 99 ++++++++++++++++++++++++++++------------ 1 file changed, 69 insertions(+), 30 deletions(-)
ACK Michal
participants (2)
-
Jim Fehlig
-
Michal Privoznik