Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_monitor_json.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 8e5af9f79a..8fb2bf4dc3 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8560,9 +8560,8 @@ int qemuMonitorJSONSetWatchdogAction(qemuMonitor *mon, const char *action) { - virJSONValue *cmd; - virJSONValue *reply = NULL; - int ret = -1; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; if (!(cmd = qemuMonitorJSONMakeCommand("watchdog-set-action", "s:action", action, @@ -8570,17 +8569,12 @@ qemuMonitorJSONSetWatchdogAction(qemuMonitor *mon, return -1; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) - goto cleanup; + return -1; if (qemuMonitorJSONCheckError(cmd, reply) < 0) - goto cleanup; - - ret = 0; + return -1; - cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + return 0; } -- 2.31.1

We've got multiple random open-coded versions. Switch to the helper function which doesn't report errors as they'd be mostly wrong as the operation was indeed successful. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 96 ++++++++++++------------------------------ 1 file changed, 26 insertions(+), 70 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2de17aaa4e..39347f84c7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1869,8 +1869,7 @@ static int qemuDomainSuspend(virDomainPtr dom) if (qemuProcessStopCPUs(driver, vm, reason, QEMU_ASYNC_JOB_NONE) < 0) goto endjob; } - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - goto endjob; + qemuDomainSaveStatus(vm); ret = 0; endjob: @@ -1927,8 +1926,7 @@ static int qemuDomainResume(virDomainPtr dom) goto endjob; } } - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - goto endjob; + qemuDomainSaveStatus(vm); ret = 0; endjob: @@ -2522,8 +2520,7 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr dom, int period, } def->memballoon->period = period; - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - goto endjob; + qemuDomainSaveStatus(vm); } if (persistentDef) { @@ -3623,7 +3620,6 @@ processGuestPanicEvent(virQEMUDriver *driver, { qemuDomainObjPrivate *priv = vm->privateData; virObjectEvent *event = NULL; - g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); bool removeInactive = false; unsigned long flags = VIR_DUMP_MEMORY_ONLY; @@ -3648,10 +3644,7 @@ processGuestPanicEvent(virQEMUDriver *driver, virObjectEventStateQueue(driver->domainEventState, event); - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) { - VIR_WARN("Unable to save status on vm %s after state change", - vm->def->name); - } + qemuDomainSaveStatus(vm); if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0) VIR_WARN("Unable to release lease on %s", vm->def->name); @@ -3704,7 +3697,6 @@ processDeviceDeletedEvent(virQEMUDriver *driver, virDomainObj *vm, const char *devAlias) { - g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); virDomainDeviceDef dev; VIR_DEBUG("Removing device %s from domain %p %s", @@ -3728,9 +3720,7 @@ processDeviceDeletedEvent(virQEMUDriver *driver, goto endjob; } - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - VIR_WARN("unable to save domain status after removing device %s", - devAlias); + qemuDomainSaveStatus(vm); endjob: qemuDomainObjEndJob(driver, vm); @@ -4045,7 +4035,6 @@ processSerialChangedEvent(virQEMUDriver *driver, const char *devAlias, bool connected) { - g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); virDomainChrDeviceState newstate; virObjectEvent *event = NULL; virDomainDeviceDef dev; @@ -4096,9 +4085,7 @@ processSerialChangedEvent(virQEMUDriver *driver, dev.data.chr->state = newstate; - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - VIR_WARN("unable to save status of domain %s after updating state of " - "channel %s", vm->def->name, devAlias); + qemuDomainSaveStatus(vm); if (STREQ_NULLABLE(dev.data.chr->target.name, "org.qemu.guest_agent.0")) { if (newstate == VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED) { @@ -4520,7 +4507,6 @@ qemuDomainPinVcpuLive(virDomainObj *vm, virDomainDef *def, int vcpu, virQEMUDriver *driver, - virQEMUDriverConfig *cfg, virBitmap *cpumap) { virBitmap *tmpmap = NULL; @@ -4572,8 +4558,7 @@ qemuDomainPinVcpuLive(virDomainObj *vm, vcpuinfo->cpumask = tmpmap; tmpmap = NULL; - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - goto cleanup; + qemuDomainSaveStatus(vm); if (g_snprintf(paramField, VIR_TYPED_PARAM_FIELD_LENGTH, VIR_DOMAIN_TUNABLE_CPU_VCPUPIN, vcpu) < 0) { @@ -4646,7 +4631,7 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, } if (def && - qemuDomainPinVcpuLive(vm, def, vcpu, driver, cfg, pcpumap) < 0) + qemuDomainPinVcpuLive(vm, def, vcpu, driver, pcpumap) < 0) goto endjob; if (persistentDef) { @@ -4787,8 +4772,7 @@ qemuDomainPinEmulator(virDomainPtr dom, virBitmapFree(def->cputune.emulatorpin); def->cputune.emulatorpin = virBitmapNewCopy(pcpumap); - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - goto endjob; + qemuDomainSaveStatus(vm); str = virBitmapFormat(pcpumap); if (virTypedParamsAddString(&eventParams, &eventNparams, @@ -5247,8 +5231,7 @@ qemuDomainPinIOThread(virDomainPtr dom, if (virProcessSetAffinity(iothrid->thread_id, pcpumap, false) < 0) goto endjob; - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - goto endjob; + qemuDomainSaveStatus(vm); if (g_snprintf(paramField, VIR_TYPED_PARAM_FIELD_LENGTH, VIR_DOMAIN_TUNABLE_CPU_IOTHREADSPIN, iothread_id) < 0) { @@ -5694,8 +5677,7 @@ qemuDomainChgIOThread(virQEMUDriver *driver, } - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - goto endjob; + qemuDomainSaveStatus(vm); } if (persistentDef) { @@ -7825,8 +7807,7 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObj *vm, * changed even if we failed to attach the device. For example, * a new controller may be created. */ - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - return -1; + qemuDomainSaveStatus(vm); } /* Finally, if no error until here, we can save config. */ @@ -7964,15 +7945,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, * device we're going to update. */ if ((ret = qemuDomainUpdateDeviceLive(vm, dev_copy, dom, force)) < 0) goto endjob; - /* - * update domain status forcibly because the domain status may be - * changed even if we failed to attach the device. For example, - * a new controller may be created. - */ - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) { - ret = -1; - goto endjob; - } + + qemuDomainSaveStatus(vm); } /* Finally, if no error until here, we can save config. */ @@ -8057,13 +8031,7 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriver *driver, if (rc == 0 && qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) goto cleanup; - /* - * update domain status forcibly because the domain status may be - * changed even if we failed to attach the device. For example, - * a new controller may be created. - */ - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - goto cleanup; + qemuDomainSaveStatus(vm); } /* Finally, if no error until here, we can save config. */ @@ -8436,9 +8404,7 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, if (def) { ret = virDomainCgroupSetupDomainBlkioParameters(priv->cgroup, def, params, nparams); - - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - goto endjob; + qemuDomainSaveStatus(vm); } if (ret < 0) goto endjob; @@ -8613,9 +8579,8 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, params, nparams) < 0) goto endjob; - if (def && - virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - goto endjob; + if (def) + qemuDomainSaveStatus(vm); if (persistentDef && virDomainDefSave(persistentDef, driver->xmlopt, cfg->configDir) < 0) @@ -8876,8 +8841,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom, -1, mode, nodeset) < 0) goto endjob; - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - goto endjob; + qemuDomainSaveStatus(vm); } if (persistentDef) { @@ -9068,8 +9032,7 @@ qemuDomainSetPerfEvents(virDomainPtr dom, VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO; } - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - goto endjob; + qemuDomainSaveStatus(vm); } if (persistentDef) { @@ -9520,8 +9483,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, } } - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - goto endjob; + qemuDomainSaveStatus(vm); if (eventNparams) { event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams); @@ -10418,8 +10380,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, goto endjob; } - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - goto endjob; + qemuDomainSaveStatus(vm); } if (persistentNet) { @@ -14444,7 +14405,6 @@ qemuDomainBlockJobAbort(virDomainPtr dom, { virQEMUDriver *driver = dom->conn->privateData; virDomainDiskDef *disk = NULL; - g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); bool pivot = !!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT); bool async = !!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC); g_autoptr(qemuBlockJobData) job = NULL; @@ -14508,7 +14468,7 @@ qemuDomainBlockJobAbort(virDomainPtr dom, job->state = QEMU_BLOCKJOB_STATE_ABORTING; } - ignore_value(virDomainObjSave(vm, driver->xmlopt, cfg->stateDir)); + qemuDomainSaveStatus(vm); if (!async) { qemuBlockJobUpdate(vm, job, QEMU_ASYNC_JOB_NONE); @@ -16191,8 +16151,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, qemuDomainSetGroupBlockIoTune(def, &info); - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - goto endjob; + qemuDomainSaveStatus(vm); if (eventNparams) { event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams); @@ -19686,8 +19645,7 @@ qemuDomainSetLifecycleAction(virDomainPtr dom, qemuDomainModifyLifecycleAction(def, type, action); - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - goto endjob; + qemuDomainSaveStatus(vm); } if (persistentDef) { @@ -20225,9 +20183,7 @@ qemuDomainAgentSetResponseTimeout(virDomainPtr dom, QEMU_DOMAIN_PRIVATE(vm)->agentTimeout = timeout; - if (virDomainObjIsActive(vm) && - virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) - goto cleanup; + qemuDomainSaveStatus(vm); ret = 0; -- 2.31.1

The qemu driver didn't ever implement any meaningful handling for the 'rename-restart' action. At this point the following handling would take place: 'on_reboot' set to 'rename-restart' is ignored on guest-initiated reboots, the guest simply reboots. For on_poweroff set to 'rename-restart' the following happens: guest initiated shutdown -> 'destroy' libvirt initiated shutdown -> 'reboot' In addition when 'on_reboot' is 'destroy' in addition to 'on_poweroff' being 'rename-restart' the guest is able to execute instructions after issuing a reset before libvirt terminates it. This will be addressed separately later. Forbid the flag in the qemu def validator and update the documentation to be factual. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- docs/formatdomain.rst | 6 +++--- src/qemu/qemu_validate.c | 32 ++++++++++++++++++++++++++++++++ src/qemu/qemu_validate.h | 5 +++++ 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 512e6abd27..fba4765e35 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -1743,12 +1743,12 @@ Each of these states allow for the same four possible actions. ``preserve`` The domain will be terminated and its resource preserved to allow analysis. ``rename-restart`` - The domain will be terminated and then restarted with a new name. + The domain will be terminated and then restarted with a new name. (Only + supported by the libxl hypervisor driver.) QEMU/KVM supports the ``on_poweroff`` and ``on_reboot`` events handling the ``destroy`` and ``restart`` actions. The ``preserve`` action for an -``on_reboot`` event is treated as a ``destroy`` and the ``rename-restart`` -action for an ``on_poweroff`` event is treated as a ``restart`` event. +``on_reboot`` event is treated as a ``destroy``. The ``on_crash`` event supports these additional actions :since:`since 0.8.4` . diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index e5c4e3af26..7bd8d8d9e7 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1063,6 +1063,35 @@ qemuValidateDomainDeviceInfo(virDomainDef *def G_GNUC_UNUSED, return 0; } + +int +qemuValidateLifecycleAction(virDomainLifecycleAction onPoweroff, + virDomainLifecycleAction onReboot, + virDomainLifecycleAction onCrash) +{ + /* The qemu driver doesn't yet implement any meaningful handling for + * VIR_DOMAIN_LIFECYCLE_ACTION_RESTART_RENAME */ + if (onPoweroff == VIR_DOMAIN_LIFECYCLE_ACTION_RESTART_RENAME || + onReboot == VIR_DOMAIN_LIFECYCLE_ACTION_RESTART_RENAME || + onCrash == VIR_DOMAIN_LIFECYCLE_ACTION_RESTART_RENAME) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("qemu driver doesn't support the 'rename-restart' action for 'on_reboot'/'on_poweroff'/'on_crash'")); + return -1; + } + + return 0; +} + + +static int +qemuValidateDomainLifecycleAction(const virDomainDef *def) +{ + return qemuValidateLifecycleAction(def->onPoweroff, + def->onReboot, + def->onCrash); +} + + int qemuValidateDomainDef(const virDomainDef *def, void *opaque, @@ -1157,6 +1186,9 @@ qemuValidateDomainDef(const virDomainDef *def, } } + if (qemuValidateDomainLifecycleAction(def) < 0) + return -1; + if (qemuValidateDomainDefCpu(driver, def, qemuCaps) < 0) return -1; diff --git a/src/qemu/qemu_validate.h b/src/qemu/qemu_validate.h index 19629ac3af..1e0546633c 100644 --- a/src/qemu/qemu_validate.h +++ b/src/qemu/qemu_validate.h @@ -38,3 +38,8 @@ qemuValidateDomainDeviceDef(const virDomainDeviceDef *dev, const virDomainDef *def, void *opaque, void *parseOpaque); + +int +qemuValidateLifecycleAction(virDomainLifecycleAction onPoweroff, + virDomainLifecycleAction onReboot, + virDomainLifecycleAction onCrash); -- 2.31.1

The qemu driver didn't ever implement any meaningful handling for the 'preserve' action. Forbid the flag in the qemu def validator and update the documentation to be factual. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- docs/formatdomain.rst | 3 +-- src/qemu/qemu_validate.c | 10 ++++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index fba4765e35..a894a51c00 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -1747,8 +1747,7 @@ Each of these states allow for the same four possible actions. supported by the libxl hypervisor driver.) QEMU/KVM supports the ``on_poweroff`` and ``on_reboot`` events handling the -``destroy`` and ``restart`` actions. The ``preserve`` action for an -``on_reboot`` event is treated as a ``destroy``. +``destroy`` and ``restart`` actions. The ``on_crash`` event supports these additional actions :since:`since 0.8.4` . diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 7bd8d8d9e7..d27c3b6c6c 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1079,6 +1079,16 @@ qemuValidateLifecycleAction(virDomainLifecycleAction onPoweroff, return -1; } + /* The qemu driver doesn't yet implement any meaningful handling for + * VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE */ + if (onPoweroff == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE || + onReboot == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE || + onCrash == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("qemu driver doesn't support the 'preserve' action for 'on_reboot'/'on_poweroff'/'on_crash'")); + return -1; + } + return 0; } -- 2.31.1

The 'set-action' QMP command allows modifying the behaviour when the guest resets. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 + 6 files changed, 7 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 286d34ae54..70c3ec2f0c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -637,6 +637,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "confidential-guest-support", /* QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT */ "query-display-options", /* QEMU_CAPS_QUERY_DISPLAY_OPTIONS */ "s390-pv-guest", /* QEMU_CAPS_S390_PV_GUEST */ + "set-action", /* QEMU_CAPS_SET_ACTION */ ); @@ -1180,6 +1181,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { "query-display-options", QEMU_CAPS_QUERY_DISPLAY_OPTIONS }, { "blockdev-reopen", QEMU_CAPS_BLOCKDEV_REOPEN }, { "set-numa-node", QEMU_CAPS_NUMA }, + { "set-action", QEMU_CAPS_SET_ACTION }, }; struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 17fe91dae9..bc762d1916 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -617,6 +617,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT, /* -machine confidential-guest-support */ QEMU_CAPS_QUERY_DISPLAY_OPTIONS, /* 'query-display-options' qmp command present */ QEMU_CAPS_S390_PV_GUEST, /* -object s390-pv-guest,... */ + QEMU_CAPS_SET_ACTION, /* 'set-action' QMP command */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml index c65884deca..dcc41ed067 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml @@ -210,6 +210,7 @@ <flag name='input-linux'/> <flag name='confidential-guest-support'/> <flag name='query-display-options'/> + <flag name='set-action'/> <version>6000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml b/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml index 7124e5194a..ebcca6e114 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml @@ -164,6 +164,7 @@ <flag name='confidential-guest-support'/> <flag name='query-display-options'/> <flag name='s390-pv-guest'/> + <flag name='set-action'/> <version>6000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml index 7e06d93c0f..4951644354 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml @@ -254,6 +254,7 @@ <flag name='input-linux'/> <flag name='confidential-guest-support'/> <flag name='query-display-options'/> + <flag name='set-action'/> <version>6000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml index 1fad52ba90..1ba344ea1c 100644 --- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml @@ -256,6 +256,7 @@ <flag name='virtio-vga-gl'/> <flag name='confidential-guest-support'/> <flag name='query-display-options'/> + <flag name='set-action'/> <version>6000090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> -- 2.31.1

If current qemu supports 'set-action' use it instead of the single-use command. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 54 +++++++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c3c49fe080..64580b573b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3117,8 +3117,6 @@ qemuDomainAttachWatchdog(virQEMUDriver *driver, int ret = -1; qemuDomainObjPrivate *priv = vm->privateData; virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_WATCHDOG, { .watchdog = watchdog } }; - virDomainWatchdogAction actualAction = watchdog->action; - const char *actionStr = NULL; g_autofree char *watchdogstr = NULL; bool releaseAddress = false; int rv; @@ -3146,17 +3144,59 @@ qemuDomainAttachWatchdog(virQEMUDriver *driver, if (!(watchdogstr = qemuBuildWatchdogDevStr(vm->def, watchdog, priv->qemuCaps))) goto cleanup; + qemuDomainObjEnterMonitor(driver, vm); + /* QEMU doesn't have a 'dump' action; we tell qemu to 'pause', then libvirt listens for the watchdog event, and we perform the dump ourselves. so convert 'dump' to 'pause' for the qemu cli */ - if (actualAction == VIR_DOMAIN_WATCHDOG_ACTION_DUMP) - actualAction = VIR_DOMAIN_WATCHDOG_ACTION_PAUSE; + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SET_ACTION)) { + qemuMonitorActionWatchdog watchdogaction = QEMU_MONITOR_ACTION_WATCHDOG_KEEP; - actionStr = virDomainWatchdogActionTypeToString(actualAction); + switch (watchdog->action) { + case VIR_DOMAIN_WATCHDOG_ACTION_RESET: + watchdogaction = QEMU_MONITOR_ACTION_WATCHDOG_RESET; + break; - qemuDomainObjEnterMonitor(driver, vm); + case VIR_DOMAIN_WATCHDOG_ACTION_SHUTDOWN: + watchdogaction = QEMU_MONITOR_ACTION_WATCHDOG_SHUTDOWN; + break; + + case VIR_DOMAIN_WATCHDOG_ACTION_POWEROFF: + watchdogaction = QEMU_MONITOR_ACTION_WATCHDOG_POWEROFF; + break; + + case VIR_DOMAIN_WATCHDOG_ACTION_PAUSE: + case VIR_DOMAIN_WATCHDOG_ACTION_DUMP: + watchdogaction = QEMU_MONITOR_ACTION_WATCHDOG_PAUSE; + break; + + case VIR_DOMAIN_WATCHDOG_ACTION_NONE: + watchdogaction = QEMU_MONITOR_ACTION_WATCHDOG_NONE; + break; + + case VIR_DOMAIN_WATCHDOG_ACTION_INJECTNMI: + watchdogaction = QEMU_MONITOR_ACTION_WATCHDOG_INJECT_NMI; + break; + + case VIR_DOMAIN_WATCHDOG_ACTION_LAST: + default: + break; + }; + + rv = qemuMonitorSetAction(priv->mon, + QEMU_MONITOR_ACTION_SHUTDOWN_KEEP, + QEMU_MONITOR_ACTION_REBOOT_KEEP, + watchdogaction, + QEMU_MONITOR_ACTION_PANIC_KEEP); + } else { + virDomainWatchdogAction actualAction = watchdog->action; - rv = qemuMonitorSetWatchdogAction(priv->mon, actionStr); + if (actualAction == VIR_DOMAIN_WATCHDOG_ACTION_DUMP) + actualAction = VIR_DOMAIN_WATCHDOG_ACTION_PAUSE; + + rv = qemuMonitorSetWatchdogAction(priv->mon, + virDomainWatchdogActionTypeToString(actualAction)); + } if (rv >= 0) rv = qemuMonitorAddDevice(priv->mon, watchdogstr); -- 2.31.1

Save further readers the headache of determining what it actually does and note that it's not used with qemu version supporting the 'set-action' command. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_domain.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index abc3c7158d..f3c6bb3390 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -154,6 +154,14 @@ struct _qemuDomainObjPrivate { bool fakeReboot; bool pausedShutdown; + /* allowReboot: + * + * Unused with new QEMU versions which have QEMU_CAPS_SET_ACTION. + * + * Otherwise if it's set to VIR_TRISTATE_BOOL_YES, QEMU was started with + * -no-shutdown, and if set to VIR_TRISTATE_BOOL_NO qemu was started with + * -no-reboot instead. + */ virTristateBool allowReboot; int jobs_queued; -- 2.31.1

Directly use 'priv->allowReboot' as we now document what the behaiour is to avoid another lookup. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 4 +--- src/qemu/qemu_domain.c | 16 ---------------- src/qemu/qemu_domain.h | 3 --- src/qemu/qemu_process.c | 3 +-- 4 files changed, 2 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2756f46b51..5b743dd1ad 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6165,11 +6165,9 @@ qemuBuildPMCommandLine(virCommand *cmd, { virQEMUCaps *qemuCaps = priv->qemuCaps; - /* Only add -no-reboot option if each event destroys domain */ if (priv->allowReboot == VIR_TRISTATE_BOOL_NO) virCommandAddArg(cmd, "-no-reboot"); - - if (qemuDomainIsUsingNoShutdown(priv)) + else virCommandAddArg(cmd, "-no-shutdown"); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_ACPI)) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9baa4b5d90..ce355d37a0 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11149,22 +11149,6 @@ qemuDomainRunningReasonToResumeEvent(virDomainRunningReason reason) } -/* qemuDomainIsUsingNoShutdown: - * @priv: Domain private data - * - * We can receive an event when QEMU stops. If we use no-shutdown, then - * we can watch for this event and do a soft/warm reboot. - * - * Returns: @true when -no-shutdown either should be or was added to the - * command line. - */ -bool -qemuDomainIsUsingNoShutdown(qemuDomainObjPrivate *priv) -{ - return priv->allowReboot == VIR_TRISTATE_BOOL_YES; -} - - bool qemuDomainDiskIsMissingLocalOptional(virDomainDiskDef *disk) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index f3c6bb3390..cb1cd968d5 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1001,9 +1001,6 @@ void qemuDomainStorageIdReset(qemuDomainObjPrivate *priv); virDomainEventResumedDetailType qemuDomainRunningReasonToResumeEvent(virDomainRunningReason reason); -bool -qemuDomainIsUsingNoShutdown(qemuDomainObjPrivate *priv); - bool qemuDomainDiskIsMissingLocalOptional(virDomainDiskDef *disk); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 5958a48182..c9cdff4e82 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8751,8 +8751,7 @@ qemuProcessReconnect(void *opaque) * domain crashed; otherwise, if the monitor was started, * then we can blame ourselves, else we failed before the * monitor started so we don't really know. */ - if (!priv->mon && tryMonReconn && - qemuDomainIsUsingNoShutdown(priv)) + if (!priv->mon && tryMonReconn && priv->allowReboot == VIR_TRISTATE_BOOL_YES) state = VIR_DOMAIN_SHUTOFF_CRASHED; else if (priv->mon) state = VIR_DOMAIN_SHUTOFF_DAEMON; -- 2.31.1

The RESET event is delivered by qemu only when the guest OS is actually allowed to reboot ('-no-reboot' or equivalent is not used) and due to the nature of async handling of the events VM is actually already executing guest code after the reboot, until our code gets to killing it. In general it should have been impossible to reach a state where the reboot action is 'destroy' but we didn't use '-no-reboot' but due to various bugs it was. Due to the fact that this was not a desired operation and additionally guest code already is executing I think the best option is not to kill the VM any more (possible data loss?) and rely for the proper fix where we use the new 'set-action' QMP command to enable an equivalent behaviour to '-no-reboot' during runtime. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_process.c | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c9cdff4e82..a839d587c2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -443,27 +443,6 @@ qemuProcessHandleReset(qemuMonitor *mon G_GNUC_UNUSED, if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) VIR_WARN("Failed to save status on vm %s", vm->def->name); - if (vm->def->onReboot == VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY || - vm->def->onReboot == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE) { - - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; - - if (!virDomainObjIsActive(vm)) { - VIR_DEBUG("Ignoring RESET event from inactive domain %s", - vm->def->name); - goto endjob; - } - - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED, - QEMU_ASYNC_JOB_NONE, 0); - virDomainAuditStop(vm, "destroyed"); - qemuDomainRemoveInactive(driver, vm); - endjob: - qemuDomainObjEndJob(driver, vm); - } - - cleanup: virObjectUnlock(vm); virObjectEventStateQueue(driver->domainEventState, event); } -- 2.31.1

The '-no-shutdown' flag prevents qemu from terminating if a shutdown was requested. Libvirt will handle the termination of the qemu process anyways and using this consistently will allow greater flexibility for the virDomainSetLifecycleAction API as well as will allow using the 'system-reset' QMP command during startup to reinitiate devices exported to the firmware. This efectively partially reverts 0e034efaf9b963760516a65413fd9771034357aa Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 12 +++++++++--- src/qemu/qemu_process.c | 4 +++- .../misc-no-reboot.x86_64-latest.args | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5b743dd1ad..b230314f7f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6165,10 +6165,16 @@ qemuBuildPMCommandLine(virCommand *cmd, { virQEMUCaps *qemuCaps = priv->qemuCaps; - if (priv->allowReboot == VIR_TRISTATE_BOOL_NO) - virCommandAddArg(cmd, "-no-reboot"); - else + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SET_ACTION)) { + /* with new qemu we always want '-no-shutdown' on startup and we set + * all the other behaviour later during startup */ virCommandAddArg(cmd, "-no-shutdown"); + } else { + if (priv->allowReboot == VIR_TRISTATE_BOOL_NO) + virCommandAddArg(cmd, "-no-reboot"); + else + virCommandAddArg(cmd, "-no-shutdown"); + } if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_ACPI)) { if (def->features[VIR_DOMAIN_FEATURE_ACPI] != VIR_TRISTATE_SWITCH_ON) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c41c6ab793..bbcef47885 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8765,7 +8765,9 @@ qemuProcessReconnect(void *opaque) * domain crashed; otherwise, if the monitor was started, * then we can blame ourselves, else we failed before the * monitor started so we don't really know. */ - if (!priv->mon && tryMonReconn && priv->allowReboot == VIR_TRISTATE_BOOL_YES) + if (!priv->mon && tryMonReconn && + (priv->allowReboot == VIR_TRISTATE_BOOL_YES || + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SET_ACTION))) state = VIR_DOMAIN_SHUTOFF_CRASHED; else if (priv->mon) state = VIR_DOMAIN_SHUTOFF_DAEMON; diff --git a/tests/qemuxml2argvdata/misc-no-reboot.x86_64-latest.args b/tests/qemuxml2argvdata/misc-no-reboot.x86_64-latest.args index 197b6375a7..f34e9c8708 100644 --- a/tests/qemuxml2argvdata/misc-no-reboot.x86_64-latest.args +++ b/tests/qemuxml2argvdata/misc-no-reboot.x86_64-latest.args @@ -23,7 +23,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ -mon chardev=charmonitor,id=monitor,mode=control \ -rtc base=utc \ --no-reboot \ +-no-shutdown \ -no-acpi \ -boot strict=on \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ -- 2.31.1

When qemu supports 'set-action' command we can update what happens on reboot. Additionally we can fully relax the checks as we now properly update the lifecycle actions. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 69 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3c57d67d1c..961f521f4b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19629,6 +19629,58 @@ qemuDomainModifyLifecycleAction(virDomainDef *def, } +static int +qemuDomainModifyLifecycleActionLive(virDomainObj *vm, + virDomainLifecycle type, + virDomainLifecycleAction action) +{ + qemuMonitorActionReboot monReboot = QEMU_MONITOR_ACTION_REBOOT_KEEP; + qemuDomainObjPrivate *priv = vm->privateData; + virQEMUDriver *driver = priv->driver; + int rc; + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SET_ACTION)) + return 0; + + /* For now we only update 'reboot' action here as we want to keep the + * shutdown action as is (we're emulating the outcome anyways)) */ + if (type != VIR_DOMAIN_LIFECYCLE_REBOOT || + vm->def->onReboot == action) + return 0; + + + switch (action) { + case VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY: + monReboot = QEMU_MONITOR_ACTION_REBOOT_SHUTDOWN; + break; + + case VIR_DOMAIN_LIFECYCLE_ACTION_RESTART: + monReboot = QEMU_MONITOR_ACTION_REBOOT_RESET; + break; + + case VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE: + case VIR_DOMAIN_LIFECYCLE_ACTION_RESTART_RENAME: + case VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_DESTROY: + case VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_RESTART: + case VIR_DOMAIN_LIFECYCLE_ACTION_LAST: + return 0; + } + + + qemuDomainObjEnterMonitor(driver, vm); + + rc = qemuMonitorSetAction(priv->mon, + QEMU_MONITOR_ACTION_SHUTDOWN_KEEP, + monReboot, + QEMU_MONITOR_ACTION_WATCHDOG_KEEP, + QEMU_MONITOR_ACTION_PANIC_KEEP); + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + return -1; + + return 0; +} + static int qemuDomainSetLifecycleAction(virDomainPtr dom, @@ -19671,14 +19723,19 @@ qemuDomainSetLifecycleAction(virDomainPtr dom, goto endjob; if (def) { - if (priv->allowReboot == VIR_TRISTATE_BOOL_NO || - (type == VIR_DOMAIN_LIFECYCLE_REBOOT && - def->onReboot != action)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("cannot update lifecycle action because QEMU was started with incompatible -no-reboot setting")); - goto endjob; + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SET_ACTION)) { + if (priv->allowReboot == VIR_TRISTATE_BOOL_NO || + (type == VIR_DOMAIN_LIFECYCLE_REBOOT && + def->onReboot != action)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("cannot update lifecycle action because QEMU was started with incompatible -no-reboot setting")); + goto endjob; + } } + if (qemuDomainModifyLifecycleActionLive(vm, type, action) < 0) + goto endjob; + qemuDomainModifyLifecycleAction(def, type, action); qemuDomainSaveStatus(vm); -- 2.31.1