From: Peter Krempa <pkrempa(a)redhat.com&gt; The parameter overrides the maximum number of threads for the machine. Fixes: d5572f62e32 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virsystemd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 92d2890360..4f8424ae32 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -357,6 +357,7 @@ virSystemdGetMachineUnitByPID(pid_t pid) * @nnicindexes: number of network interface indexes in list * @nicindexes: list of network interface indexes * @partition: name of the slice to place the machine in + * @maxthreads: maximum number of threads the VM process can use * * Returns 0 on success, -1 on fatal error, or -2 if systemd-machine is not available */ -- 2.49.0

From: Peter Krempa <pkrempa(a)redhat.com&gt; The function is called just from one place within the module where it's defined. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/hypervisor/domain_cgroup.c | 2 +- src/hypervisor/domain_cgroup.h | 10 ---------- src/libvirt_private.syms | 1 - 3 files changed, 1 insertion(+), 12 deletions(-) diff --git a/src/hypervisor/domain_cgroup.c b/src/hypervisor/domain_cgroup.c index fda495faf5..fecc0f7966 100644 --- a/src/hypervisor/domain_cgroup.c +++ b/src/hypervisor/domain_cgroup.c @@ -342,7 +342,7 @@ virDomainCgroupSetupCpuCgroup(virDomainObj *vm, } -int +static int virDomainCgroupInitCgroup(const char *prefix, virDomainObj *vm, size_t nnicindexes, diff --git a/src/hypervisor/domain_cgroup.h b/src/hypervisor/domain_cgroup.h index f8d261a080..6e5c98004e 100644 --- a/src/hypervisor/domain_cgroup.h +++ b/src/hypervisor/domain_cgroup.h @@ -52,16 +52,6 @@ virDomainCgroupSetupCpusetCgroup(virCgroup *cgroup); int virDomainCgroupSetupCpuCgroup(virDomainObj *vm, virCgroup *cgroup); -int -virDomainCgroupInitCgroup(const char *prefix, - virDomainObj *vm, - size_t nnicindexes, - int *nicindexes, - virCgroup **cgroup, - int cgroupControllers, - unsigned int maxThreadsPerProc, - bool privileged, - char *machineName); void virDomainCgroupRestoreCgroupState(virDomainObj *vm, virCgroup *cgroup); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a8ebf9efd8..8f1489ecc8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1641,7 +1641,6 @@ virSetConnectStorage; virDomainCgroupConnectCgroup; virDomainCgroupEmulatorAllNodesAllow; virDomainCgroupEmulatorAllNodesRestore; -virDomainCgroupInitCgroup; virDomainCgroupRemoveCgroup; virDomainCgroupSetMemoryLimitParameters; virDomainCgroupSetupBlkio; -- 2.49.0

From: Peter Krempa <pkrempa(a)redhat.com&gt; Rather than having a bunch of extra variables save the configuration of the daemon auto shutdown in virDomainDriverAutoShutdownConfig which is also used when initiating the shutdown. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_conf.c | 30 +++++++++++++++--------------- src/qemu/qemu_conf.h | 7 +------ src/qemu/qemu_driver.c | 12 +++--------- 3 files changed, 19 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 9bf12fc179..482e19b502 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -320,15 +320,15 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged, * * XXX, or query if libvirt-guests.service is enabled perhaps ? */ - cfg->autoShutdownTrySave = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE; - cfg->autoShutdownTryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE; - cfg->autoShutdownPoweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE; + cfg->autoShutdown.trySave = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE; + cfg->autoShutdown.tryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE; + cfg->autoShutdown.poweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE; } else { - cfg->autoShutdownTrySave = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT; - cfg->autoShutdownTryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL; - cfg->autoShutdownPoweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL; + cfg->autoShutdown.trySave = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT; + cfg->autoShutdown.tryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL; + cfg->autoShutdown.poweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL; } - cfg->autoShutdownRestore = true; + cfg->autoShutdown.autoRestore = true; return g_steal_pointer(&cfg); } @@ -719,11 +719,11 @@ virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg, autoShutdownTrySave); return -1; } - cfg->autoShutdownTrySave = autoShutdownVal; + cfg->autoShutdown.trySave = autoShutdownVal; } - if (cfg->autoShutdownTrySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL || - cfg->autoShutdownTrySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT) { + if (cfg->autoShutdown.trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL || + cfg->autoShutdown.trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("managed save cannot be requested for transient domains")); return -1; @@ -740,7 +740,7 @@ virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg, autoShutdownTryShutdown); return -1; } - cfg->autoShutdownTryShutdown = autoShutdownVal; + cfg->autoShutdown.tryShutdown = autoShutdownVal; } if (virConfGetValueString(conf, "auto_shutdown_poweroff", &autoShutdownPoweroff) < 0) @@ -754,16 +754,16 @@ virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg, autoShutdownPoweroff); return -1; } - cfg->autoShutdownPoweroff = autoShutdownVal; + cfg->autoShutdown.poweroff = autoShutdownVal; } if (virConfGetValueUInt(conf, "auto_shutdown_wait", - &cfg->autoShutdownWait) < 0) + &cfg->autoShutdown.waitShutdownSecs) < 0) return -1; - if (virConfGetValueBool(conf, "auto_shutdown_restore", &cfg->autoShutdownRestore) < 0) + if (virConfGetValueBool(conf, "auto_shutdown_restore", &cfg->autoShutdown.autoRestore) < 0) return -1; if (virConfGetValueBool(conf, "auto_save_bypass_cache", - &cfg->autoSaveBypassCache) < 0) + &cfg->autoShutdown.saveBypassCache) < 0) return -1; return 0; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 1ce9dbe4a8..ff376aed4d 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -208,12 +208,7 @@ struct _virQEMUDriverConfig { bool autoDumpBypassCache; bool autoStartBypassCache; unsigned int autoStartDelayMS; - virDomainDriverAutoShutdownScope autoShutdownTrySave; - virDomainDriverAutoShutdownScope autoShutdownTryShutdown; - virDomainDriverAutoShutdownScope autoShutdownPoweroff; - unsigned int autoShutdownWait; - bool autoShutdownRestore; - bool autoSaveBypassCache; + virDomainDriverAutoShutdownConfig autoShutdown; char *lockManagerName; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9b583ad7aa..4dbd5ec2fc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -964,15 +964,9 @@ static int qemuStateStop(void) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(qemu_driver); - virDomainDriverAutoShutdownConfig ascfg = { - .uri = cfg->uri, - .trySave = cfg->autoShutdownTrySave, - .tryShutdown = cfg->autoShutdownTryShutdown, - .poweroff = cfg->autoShutdownPoweroff, - .waitShutdownSecs = cfg->autoShutdownWait, - .saveBypassCache = cfg->autoSaveBypassCache, - .autoRestore = cfg->autoShutdownRestore, - }; + virDomainDriverAutoShutdownConfig ascfg = cfg->autoShutdown; + + ascfg.uri = cfg->uri; virDomainDriverAutoShutdown(&ascfg); -- 2.49.0

From: Peter Krempa <pkrempa(a)redhat.com&gt; Extract the checker that determines whether the daemon auto shutdown functionality is active to a separate helper 'virDomainDriverAutoShutdownActive'. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/hypervisor/domain_driver.c | 13 ++++++++++--- src/hypervisor/domain_driver.h | 1 + src/libvirt_private.syms | 1 + 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index 62bbe176ae..353b8875ec 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -729,6 +729,15 @@ virDomainDriverAutoStart(virDomainObjList *domains, } +bool +virDomainDriverAutoShutdownActive(virDomainDriverAutoShutdownConfig *cfg) +{ + return cfg->trySave != VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE || + cfg->tryShutdown != VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE || + cfg->poweroff != VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE; +} + + void virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) { @@ -773,9 +782,7 @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) } /* Short-circuit if all actions are disabled */ - if (cfg->trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE && - cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE && - cfg->poweroff == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE) + if (!virDomainDriverAutoShutdownActive(cfg)) return; if (!(conn = virConnectOpen(cfg->uri))) diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index d90466b942..af1c4eaed6 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -116,4 +116,5 @@ typedef struct _virDomainDriverAutoShutdownConfig { bool autoRestore; } virDomainDriverAutoShutdownConfig; +bool virDomainDriverAutoShutdownActive(virDomainDriverAutoShutdownConfig *cfg); void virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8f1489ecc8..1b9be478e4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1658,6 +1658,7 @@ virDomainCgroupSetupVcpuBW; # hypervisor/domain_driver.h virDomainDriverAddIOThreadCheck; virDomainDriverAutoShutdown; +virDomainDriverAutoShutdownActive; virDomainDriverAutoShutdownScopeTypeFromString; virDomainDriverAutoShutdownScopeTypeToString; virDomainDriverAutoStart; -- 2.49.0

From: Peter Krempa <pkrempa(a)redhat.com&gt; The existing dependency order of the 'machined' unit file for the domain we're starting ("After libvirtd/virtqemud"->thus shuts down *before* the daemon) is intended to work with 'libvirt-guests.service' which requires the daemon to be around to shut down the VMs. If we want to use the integrated auto shutdown done by the daemon itself we need to be able to instruct the domains (thus the corresponding machined units to shut down *after* virtqemud/libvirt. This means that we need to be able to invert the ordering relationship to "Before". This patch adds a parameter to virSystemdCreateMachine so that when starting the VM we'll be able to tell the daemon to use the proper relationship. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/vircgroup.c | 3 ++- src/util/virsystemd.c | 27 +++++++++++++++++++++------ src/util/virsystemd.h | 3 ++- tests/virsystemdtest.c | 15 +++++++++------ 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 1daa95e178..fc5dca4858 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1293,7 +1293,8 @@ virCgroupNewMachineSystemd(const char *name, nnicindexes, nicindexes, partition, - maxthreads)) < 0) + maxthreads, + false)) < 0) return rv; if (controllers != -1) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 4f8424ae32..bd174c683e 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -358,6 +358,8 @@ virSystemdGetMachineUnitByPID(pid_t pid) * @nicindexes: list of network interface indexes * @partition: name of the slice to place the machine in * @maxthreads: maximum number of threads the VM process can use + * @daemonDomainShutdown: shutdown of domains on host shutdown is done by the + * daemon instead of the libvirt-guests script * * Returns 0 on success, -1 on fatal error, or -2 if systemd-machine is not available */ @@ -370,7 +372,8 @@ int virSystemdCreateMachine(const char *name, size_t nnicindexes, int *nicindexes, const char *partition, - unsigned int maxthreads) + unsigned int maxthreads, + bool daemonDomainShutdown) { int rc; GDBusConnection *conn; @@ -462,11 +465,23 @@ int virSystemdCreateMachine(const char *name, uuid, 16, sizeof(unsigned char)); gnicindexes = g_variant_new_fixed_array(G_VARIANT_TYPE("i"), nicindexes, nnicindexes, sizeof(int)); - gprops = g_variant_new_parsed("[('Slice', <%s>)," - " ('After', <['libvirtd.service', %s]>)," - " ('Before', <['virt-guest-shutdown.target']>)]", - slicename, - servicename); + + if (daemonDomainShutdown) { + /* When domains are shut down by the daemon rather than the + * "libvirt-guests" script we need ensure that their unit + * is ordered so that it's shutdown after the libvirt daemon itself */ + gprops = g_variant_new_parsed("[('Slice', <%s>)," + " ('Before', <['libvirtd.service', %s]>)]", + slicename, + servicename); + } else { + gprops = g_variant_new_parsed("[('Slice', <%s>)," + " ('After', <['libvirtd.service', %s]>)," + " ('Before', <['virt-guest-shutdown.target']>)]", + slicename, + servicename); + } + message = g_variant_new("(s@ayssus@ai@a(sv))", name, guuid, diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h index 98460dbc3a..620d9a9645 100644 --- a/src/util/virsystemd.h +++ b/src/util/virsystemd.h @@ -40,7 +40,8 @@ int virSystemdCreateMachine(const char *name, size_t nnicindexes, int *nicindexes, const char *partition, - unsigned int maxthreads); + unsigned int maxthreads, + bool daemonDomainShutdown); int virSystemdTerminateMachine(const char *name); diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c index 004b0549ce..24c118a409 100644 --- a/tests/virsystemdtest.c +++ b/tests/virsystemdtest.c @@ -170,7 +170,8 @@ static int testCreateContainer(const void *opaque G_GNUC_UNUSED) 123, true, 0, NULL, - "highpriority.slice", 0) < 0) { + "highpriority.slice", 0, + false) < 0) { fprintf(stderr, "%s", "Failed to create LXC machine\n"); return -1; } @@ -203,7 +204,9 @@ static int testCreateMachine(const void *opaque G_GNUC_UNUSED) 123, false, 0, NULL, - NULL, 0) < 0) { + NULL, + 0, + true) < 0) { fprintf(stderr, "%s", "Failed to create KVM machine\n"); return -1; } @@ -240,7 +243,7 @@ static int testCreateNoSystemd(const void *opaque G_GNUC_UNUSED) 123, false, 0, NULL, - NULL, 0)) == 0) { + NULL, 0, false)) == 0) { g_unsetenv("FAIL_NO_SERVICE"); fprintf(stderr, "%s", "Unexpected create machine success\n"); return -1; @@ -274,7 +277,7 @@ static int testCreateSystemdNotRunning(const void *opaque G_GNUC_UNUSED) 123, false, 0, NULL, - NULL, 0)) == 0) { + NULL, 0, false)) == 0) { g_unsetenv("FAIL_NOT_REGISTERED"); fprintf(stderr, "%s", "Unexpected create machine success\n"); return -1; @@ -308,7 +311,7 @@ static int testCreateBadSystemd(const void *opaque G_GNUC_UNUSED) 123, false, 0, NULL, - NULL, 0)) == 0) { + NULL, 0, false)) == 0) { g_unsetenv("FAIL_BAD_SERVICE"); fprintf(stderr, "%s", "Unexpected create machine success\n"); return -1; @@ -343,7 +346,7 @@ static int testCreateNetwork(const void *opaque G_GNUC_UNUSED) 123, true, nnicindexes, nicindexes, - "highpriority.slice", 2) < 0) { + "highpriority.slice", 2, false) < 0) { fprintf(stderr, "%s", "Failed to create LXC machine\n"); return -1; } -- 2.49.0

From: Peter Krempa <pkrempa(a)redhat.com&gt; Plumb the new argument across the cgroup helpers up to the domain driver code. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/ch/ch_process.c | 2 ++ src/hypervisor/domain_cgroup.c | 4 ++++ src/hypervisor/domain_cgroup.h | 1 + src/lxc/lxc_cgroup.c | 1 + src/qemu/qemu_cgroup.c | 1 + src/util/vircgroup.c | 6 +++++- src/util/vircgroup.h | 1 + 7 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index 95c808cb41..cc84823fdc 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -973,6 +973,7 @@ virCHProcessStart(virCHDriver *driver, cfg->cgroupControllers, 0, /*maxThreadsPerProc*/ priv->driver->privileged, + false, priv->machineName) < 0) goto cleanup; @@ -1147,6 +1148,7 @@ virCHProcessStartRestore(virCHDriver *driver, virDomainObj *vm, const char *from cfg->cgroupControllers, 0, /*maxThreadsPerProc*/ priv->driver->privileged, + false, priv->machineName) < 0) goto cleanup; diff --git a/src/hypervisor/domain_cgroup.c b/src/hypervisor/domain_cgroup.c index fecc0f7966..8787165f48 100644 --- a/src/hypervisor/domain_cgroup.c +++ b/src/hypervisor/domain_cgroup.c @@ -351,6 +351,7 @@ virDomainCgroupInitCgroup(const char *prefix, int cgroupControllers, unsigned int maxThreadsPerProc, bool privileged, + bool daemonDomainShutdown, char *machineName) { if (!privileged) @@ -384,6 +385,7 @@ virDomainCgroupInitCgroup(const char *prefix, vm->def->resource->partition, cgroupControllers, maxThreadsPerProc, + daemonDomainShutdown, cgroup) < 0) { if (virCgroupNewIgnoreError()) return 0; @@ -513,6 +515,7 @@ virDomainCgroupSetupCgroup(const char *prefix, int cgroupControllers, unsigned int maxThreadsPerProc, bool privileged, + bool daemonDomainShutdown, char *machineName) { if (vm->pid == 0) { @@ -529,6 +532,7 @@ virDomainCgroupSetupCgroup(const char *prefix, cgroupControllers, maxThreadsPerProc, privileged, + daemonDomainShutdown, machineName) < 0) return -1; diff --git a/src/hypervisor/domain_cgroup.h b/src/hypervisor/domain_cgroup.h index 6e5c98004e..7769572a2c 100644 --- a/src/hypervisor/domain_cgroup.h +++ b/src/hypervisor/domain_cgroup.h @@ -71,6 +71,7 @@ virDomainCgroupSetupCgroup(const char *prefix, int cgroupControllers, unsigned int maxThreadsPerProc, bool privileged, + bool daemonDomainShutdown, char *machineName); void virDomainCgroupEmulatorAllNodesDataFree(virCgroupEmulatorAllNodesData *data); diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 7c889667ba..f566a5468e 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -400,6 +400,7 @@ virCgroup *virLXCCgroupCreate(virDomainDef *def, def->resource->partition, -1, 0, + false, &cgroup) < 0) return NULL; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 48af467bf9..04d6370011 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -919,6 +919,7 @@ qemuSetupCgroup(virDomainObj *vm, cfg->cgroupControllers, cfg->maxThreadsPerProc, priv->driver->privileged, + false, priv->machineName) < 0) return -1; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index fc5dca4858..532a7e5690 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1274,6 +1274,7 @@ virCgroupNewMachineSystemd(const char *name, const char *partition, int controllers, unsigned int maxthreads, + bool daemonDomainShutdown, virCgroup **group) { int rv; @@ -1294,7 +1295,7 @@ virCgroupNewMachineSystemd(const char *name, nicindexes, partition, maxthreads, - false)) < 0) + daemonDomainShutdown)) < 0) return rv; if (controllers != -1) @@ -1407,6 +1408,7 @@ virCgroupNewMachine(const char *name, const char *partition, int controllers, unsigned int maxthreads, + bool daemonDomainShutdown, virCgroup **group) { int rv; @@ -1424,6 +1426,7 @@ virCgroupNewMachine(const char *name, partition, controllers, maxthreads, + daemonDomainShutdown, group)) == 0) return 0; @@ -3144,6 +3147,7 @@ virCgroupNewMachine(const char *name G_GNUC_UNUSED, const char *partition G_GNUC_UNUSED, int controllers G_GNUC_UNUSED, unsigned int maxthreads G_GNUC_UNUSED, + bool daemonDomainShutdown G_GNUC_UNUSED, virCgroup **group G_GNUC_UNUSED) { virReportSystemError(ENXIO, "%s", diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index adf3850b22..2a7aa3306c 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -101,6 +101,7 @@ int virCgroupNewMachine(const char *name, const char *partition, int controllers, unsigned int maxthreads, + bool daemonDomainShutdown, virCgroup **group) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); -- 2.49.0

From: Peter Krempa <pkrempa(a)redhat.com&gt; When auto-shutdown via the qemu driver is requested (rather than via libvirt guests) we need to start the VMs in a way that they will be kept around for libvirt to terminate them. This involves inverting the dependancy relationship for the machined unit file. Since the setup is done at startup of the VM, add a disclaimer to qemu.conf that switching between the two modes with VMs running will not work properly. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu.conf.in | 15 ++++++++++++++- src/qemu/qemu_cgroup.c | 8 +++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in index 221bfa8095..6358a45ae2 100644 --- a/src/qemu/qemu.conf.in +++ b/src/qemu/qemu.conf.in @@ -663,7 +663,10 @@ # implemented for transient VMs. # # If 'libvirt-guests.service' is enabled, then this must be -# set to 'none' for system daemons to avoid dueling actions +# set to 'none' for system daemons to avoid dueling actions. +# Warning: Switching between 'libvirt-guests.service' and this option +# causes VMs running at that point to misbehave on host shutdown unless +# they are restarted, or saved and restored. #auto_shutdown_try_save = "persistent" # As above, but with a graceful shutdown action instead of @@ -675,6 +678,9 @@ # # If 'libvirt-guests.service' is enabled, then this must be # set to 'none' for system daemons to avoid dueling actions +# Warning: Switching between 'libvirt-guests.service' and this option +# causes VMs running at that point to misbehave on host shutdown unless +# they are restarted, or saved and restored. #auto_shutdown_try_shutdown = "all" # As above, but with a forced poweroff instead of managed @@ -687,6 +693,13 @@ # # If 'libvirt-guests.service' is enabled, then this must be # set to 'none' for system daemons to avoid dueling actions +# +# Warning: Switching between 'libvirt-guests.service' and this option +# causes VMs running at that point to misbehave on host shutdown unless +# they are restarted, or saved and restored. +# +# When using any 'auto_shutdown_try_save', 'auto_shutdown_try_shutdown' this +# feature should to be enabled as well to ensure proper cleanup of the VMs. #auto_shutdown_poweroff = "all" # How may seconds to wait for running VMs to gracefully shutdown diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 04d6370011..25e42ebfc6 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -910,6 +910,12 @@ qemuSetupCgroup(virDomainObj *vm, { qemuDomainObjPrivate *priv = vm->privateData; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver); + /* When users wants to auto-shutdown the VMs via the qemu daemon itself + * we need to instruct machined to create dependencies for the units + * in such way that the VMs will not be killed before the auto shutdown + * code is reached. + */ + bool daemonAutoShutdown = virDomainDriverAutoShutdownActive(&cfg->autoShutdown); if (virDomainCgroupSetupCgroup("qemu", vm, @@ -919,7 +925,7 @@ qemuSetupCgroup(virDomainObj *vm, cfg->cgroupControllers, cfg->maxThreadsPerProc, priv->driver->privileged, - false, + daemonAutoShutdown, priv->machineName) < 0) return -1; -- 2.49.0

From: Peter Krempa <pkrempa(a)redhat.com&gt; 'virDomainDriverAutoShutdown' grew into an unwieldy function. Extract the code for each of the save/shutdown/poweroff steps into helpers and call them. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/hypervisor/domain_driver.c | 285 +++++++++++++++++++-------------- 1 file changed, 161 insertions(+), 124 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index 353b8875ec..cce6c64d1b 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -738,6 +738,164 @@ virDomainDriverAutoShutdownActive(virDomainDriverAutoShutdownConfig *cfg) } +static void +virDomainDriverAutoShutdownDoSave(virDomainPtr *domains, + bool *transient, + size_t numDomains, + virDomainDriverAutoShutdownConfig *cfg) +{ + g_autofree unsigned int *flags = g_new0(unsigned int, numDomains); + size_t i; + + if (cfg->trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE) + return; + + for (i = 0; i < numDomains; i++) { + int state; + + if ((transient[i] && cfg->trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT) || + (!transient[i] && cfg->trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT)) + continue; + + virSystemdNotifyStatus("Suspending '%s' (%zu of %zu)", + virDomainGetName(domains[i]), i + 1, numDomains); + VIR_INFO("Suspending '%s'", virDomainGetName(domains[i])); + + /* + * Pause all VMs to make them stop dirtying pages, + * so save is quicker. We remember if any VMs were + * paused so we can restore that on resume. + */ + flags[i] = VIR_DOMAIN_SAVE_RUNNING; + if (virDomainGetState(domains[i], &state, NULL, 0) == 0) { + if (state == VIR_DOMAIN_PAUSED) + flags[i] = VIR_DOMAIN_SAVE_PAUSED; + } + if (cfg->saveBypassCache) + flags[i] |= VIR_DOMAIN_SAVE_BYPASS_CACHE; + + if (flags[i] & VIR_DOMAIN_SAVE_RUNNING) + virDomainSuspend(domains[i]); + } + + for (i = 0; i < numDomains; i++) { + virSystemdNotifyStatus("Saving '%s' (%zu of %zu)", + virDomainGetName(domains[i]), i + 1, numDomains); + VIR_INFO("Saving '%s'", virDomainGetName(domains[i])); + + if (virDomainManagedSave(domains[i], flags[i]) < 0) { + VIR_WARN("auto-shutdown: unable to perform managed save of '%s': %s", + domains[i]->name, + virGetLastErrorMessage()); + if (flags[i] & VIR_DOMAIN_SAVE_RUNNING) + virDomainResume(domains[i]); + continue; + } + virObjectUnref(domains[i]); + domains[i] = NULL; + } +} + + +static void +virDomainDriverAutoShutdownDoShutdown(virDomainPtr *domains, + bool *transient, + size_t numDomains, + virDomainDriverAutoShutdownConfig *cfg) +{ + GTimer *timer = NULL; + size_t i; + + if (cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE) + return; + + for (i = 0; i < numDomains; i++) { + if (domains[i] == NULL) + continue; + + if ((transient[i] && cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT) || + (!transient[i] && cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT)) + continue; + + virSystemdNotifyStatus("Shutting down '%s' (%zu of %zu)", + virDomainGetName(domains[i]), i + 1, numDomains); + VIR_INFO("Shutting down '%s'", virDomainGetName(domains[i])); + + if (virDomainShutdown(domains[i]) < 0) { + VIR_WARN("auto-shutdown: unable to request graceful shutdown of '%s': %s", + domains[i]->name, + virGetLastErrorMessage()); + break; + } + } + + timer = g_timer_new(); + virSystemdNotifyStatus("Waiting %u secs for VM shutdown completion", + cfg->waitShutdownSecs); + VIR_INFO("Waiting %u secs for VM shutdown completion", cfg->waitShutdownSecs); + while (1) { + bool anyRunning = false; + for (i = 0; i < numDomains; i++) { + if (!domains[i]) + continue; + + if ((transient[i] && cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT) || + (!transient[i] && cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT)) + continue; + + if (virDomainIsActive(domains[i]) == 1) { + anyRunning = true; + } else { + virObjectUnref(domains[i]); + domains[i] = NULL; + } + } + + if (!anyRunning) + break; + if (g_timer_elapsed(timer, NULL) > cfg->waitShutdownSecs) + break; + g_usleep(1000*500); + } + g_timer_destroy(timer); +} + + +static void +virDomainDriverAutoShutdownDoPoweroff(virDomainPtr *domains, + bool *transient, + size_t numDomains, + virDomainDriverAutoShutdownConfig *cfg) +{ + size_t i; + + if (cfg->poweroff == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE) + return; + + for (i = 0; i < numDomains; i++) { + if (domains[i] == NULL) + continue; + + if ((transient[i] && cfg->poweroff == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT) || + (!transient[i] && cfg->poweroff == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT)) + continue; + + virSystemdNotifyStatus("Destroying '%s' (%zu of %zu)", + virDomainGetName(domains[i]), i + 1, numDomains); + VIR_INFO("Destroying '%s'", virDomainGetName(domains[i])); + /* + * NB might fail if we gave up on waiting for + * virDomainShutdown, but it then completed anyway, + * hence we're not checking for failure + */ + virDomainDestroy(domains[i]); + + virObjectUnref(domains[i]); + domains[i] = NULL; + } +} + + void virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) { @@ -816,130 +974,9 @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) } } - if (cfg->trySave != VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE) { - g_autofree unsigned int *flags = g_new0(unsigned int, numDomains); - for (i = 0; i < numDomains; i++) { - int state; - - if ((transient[i] && cfg->trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT) || - (!transient[i] && cfg->trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT)) - continue; - - virSystemdNotifyStatus("Suspending '%s' (%zu of %d)", - virDomainGetName(domains[i]), i + 1, numDomains); - VIR_INFO("Suspending '%s'", virDomainGetName(domains[i])); - - /* - * Pause all VMs to make them stop dirtying pages, - * so save is quicker. We remember if any VMs were - * paused so we can restore that on resume. - */ - flags[i] = VIR_DOMAIN_SAVE_RUNNING; - if (virDomainGetState(domains[i], &state, NULL, 0) == 0) { - if (state == VIR_DOMAIN_PAUSED) - flags[i] = VIR_DOMAIN_SAVE_PAUSED; - } - if (cfg->saveBypassCache) - flags[i] |= VIR_DOMAIN_SAVE_BYPASS_CACHE; - - if (flags[i] & VIR_DOMAIN_SAVE_RUNNING) - virDomainSuspend(domains[i]); - } - - for (i = 0; i < numDomains; i++) { - virSystemdNotifyStatus("Saving '%s' (%zu of %d)", - virDomainGetName(domains[i]), i + 1, numDomains); - VIR_INFO("Saving '%s'", virDomainGetName(domains[i])); - - if (virDomainManagedSave(domains[i], flags[i]) < 0) { - VIR_WARN("auto-shutdown: unable to perform managed save of '%s': %s", - domains[i]->name, - virGetLastErrorMessage()); - if (flags[i] & VIR_DOMAIN_SAVE_RUNNING) - virDomainResume(domains[i]); - continue; - } - virObjectUnref(domains[i]); - domains[i] = NULL; - } - } - - if (cfg->tryShutdown != VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE) { - GTimer *timer = NULL; - for (i = 0; i < numDomains; i++) { - if (domains[i] == NULL) - continue; - - if ((transient[i] && cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT) || - (!transient[i] && cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT)) - continue; - - virSystemdNotifyStatus("Shutting down '%s' (%zu of %d)", - virDomainGetName(domains[i]), i + 1, numDomains); - VIR_INFO("Shutting down '%s'", virDomainGetName(domains[i])); - - if (virDomainShutdown(domains[i]) < 0) { - VIR_WARN("auto-shutdown: unable to request graceful shutdown of '%s': %s", - domains[i]->name, - virGetLastErrorMessage()); - break; - } - } - - timer = g_timer_new(); - virSystemdNotifyStatus("Waiting %u secs for VM shutdown completion", - cfg->waitShutdownSecs); - VIR_INFO("Waiting %u secs for VM shutdown completion", cfg->waitShutdownSecs); - while (1) { - bool anyRunning = false; - for (i = 0; i < numDomains; i++) { - if (!domains[i]) - continue; - - if ((transient[i] && cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT) || - (!transient[i] && cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT)) - continue; - - if (virDomainIsActive(domains[i]) == 1) { - anyRunning = true; - } else { - virObjectUnref(domains[i]); - domains[i] = NULL; - } - } - - if (!anyRunning) - break; - if (g_timer_elapsed(timer, NULL) > cfg->waitShutdownSecs) - break; - g_usleep(1000*500); - } - g_timer_destroy(timer); - } - - if (cfg->poweroff != VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE) { - for (i = 0; i < numDomains; i++) { - if (domains[i] == NULL) - continue; - - if ((transient[i] && cfg->poweroff == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT) || - (!transient[i] && cfg->poweroff == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT)) - continue; - - virSystemdNotifyStatus("Destroying '%s' (%zu of %d)", - virDomainGetName(domains[i]), i + 1, numDomains); - VIR_INFO("Destroying '%s'", virDomainGetName(domains[i])); - /* - * NB might fail if we gave up on waiting for - * virDomainShutdown, but it then completed anyway, - * hence we're not checking for failure - */ - virDomainDestroy(domains[i]); - - virObjectUnref(domains[i]); - domains[i] = NULL; - } - } + virDomainDriverAutoShutdownDoSave(domains, transient, numDomains, cfg); + virDomainDriverAutoShutdownDoShutdown(domains, transient, numDomains, cfg); + virDomainDriverAutoShutdownDoPoweroff(domains, transient, numDomains, cfg); virSystemdNotifyStatus("Processed %d domains", numDomains); VIR_INFO("Processed %d domains", numDomains); -- 2.49.0

From: Peter Krempa <pkrempa(a)redhat.com&gt; Commit 84bb136c31e added code that intended to skip the save of transient domains but did so only in the setup part where we pause the VMS. The second loop that actually attempts to save the VM was not modified so we'd still try saving them: Jul 03 14:15:13 andariel virtqemud[247210]: auto-shutdown: unable to perform managed save of 'cd3': Requested operation is not valid: cannot do managed save for transient domain Fixes: 84bb136c31e Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/hypervisor/domain_driver.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index cce6c64d1b..d8ccee40d5 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -779,6 +779,10 @@ virDomainDriverAutoShutdownDoSave(virDomainPtr *domains, } for (i = 0; i < numDomains; i++) { + if ((transient[i] && cfg->trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT) || + (!transient[i] && cfg->trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT)) + continue; + virSystemdNotifyStatus("Saving '%s' (%zu of %zu)", virDomainGetName(domains[i]), i + 1, numDomains); VIR_INFO("Saving '%s'", virDomainGetName(domains[i])); -- 2.49.0

From: Peter Krempa <pkrempa(a)redhat.com&gt; Decide separately and record what shutdown modes are to be applied on given VM object rather than spreading out the logic through the code. This centralization simplifies the conditions in the worker functions and also: - provides easy way to check if the auto-shutdown code will be acting on domain object (will be used to fix attempt to auto-restore of VMs which were not selected to be acted on - will simplify further work where the desired shutdown action will be picked per-VM This refactor also fixes a bug where if restoring of the state is applied also on VMs that are not selected for action based on current logic. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/hypervisor/domain_driver.c | 176 +++++++++++++++++++-------------- 1 file changed, 100 insertions(+), 76 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index d8ccee40d5..7f958c087f 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -738,25 +738,32 @@ virDomainDriverAutoShutdownActive(virDomainDriverAutoShutdownConfig *cfg) } +enum { + VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_MODE_SAVE = 1 >> 1, + VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_MODE_SHUTDOWN = 1 >> 2, + VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_MODE_POWEROFF = 1 >> 3, + VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_MODE_RESTORE = 1 >> 4, +} virDomainDriverAutoShutdownModeFlag; + + static void virDomainDriverAutoShutdownDoSave(virDomainPtr *domains, - bool *transient, + unsigned int *modes, size_t numDomains, virDomainDriverAutoShutdownConfig *cfg) { g_autofree unsigned int *flags = g_new0(unsigned int, numDomains); + bool hasSave = false; size_t i; - if (cfg->trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE) - return; - for (i = 0; i < numDomains; i++) { int state; - if ((transient[i] && cfg->trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT) || - (!transient[i] && cfg->trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT)) + if (!(modes[i] & VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_MODE_SAVE)) continue; + hasSave = true; + virSystemdNotifyStatus("Suspending '%s' (%zu of %zu)", virDomainGetName(domains[i]), i + 1, numDomains); VIR_INFO("Suspending '%s'", virDomainGetName(domains[i])); @@ -778,9 +785,11 @@ virDomainDriverAutoShutdownDoSave(virDomainPtr *domains, virDomainSuspend(domains[i]); } + if (!hasSave) + return; + for (i = 0; i < numDomains; i++) { - if ((transient[i] && cfg->trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT) || - (!transient[i] && cfg->trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT)) + if (!(modes[i] & VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_MODE_SAVE)) continue; virSystemdNotifyStatus("Saving '%s' (%zu of %zu)", @@ -795,31 +804,27 @@ virDomainDriverAutoShutdownDoSave(virDomainPtr *domains, virDomainResume(domains[i]); continue; } - virObjectUnref(domains[i]); - domains[i] = NULL; + + modes[i] = 0; } } static void virDomainDriverAutoShutdownDoShutdown(virDomainPtr *domains, - bool *transient, + unsigned int *modes, size_t numDomains, virDomainDriverAutoShutdownConfig *cfg) { GTimer *timer = NULL; + bool hasShutdown = false; size_t i; - if (cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE) - return; - for (i = 0; i < numDomains; i++) { - if (domains[i] == NULL) + if (!(modes[i] & VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_MODE_SHUTDOWN)) continue; - if ((transient[i] && cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT) || - (!transient[i] && cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT)) - continue; + hasShutdown = true; virSystemdNotifyStatus("Shutting down '%s' (%zu of %zu)", virDomainGetName(domains[i]), i + 1, numDomains); @@ -833,25 +838,24 @@ virDomainDriverAutoShutdownDoShutdown(virDomainPtr *domains, } } + if (!hasShutdown) + return; + timer = g_timer_new(); virSystemdNotifyStatus("Waiting %u secs for VM shutdown completion", cfg->waitShutdownSecs); VIR_INFO("Waiting %u secs for VM shutdown completion", cfg->waitShutdownSecs); + while (1) { bool anyRunning = false; for (i = 0; i < numDomains; i++) { - if (!domains[i]) - continue; - - if ((transient[i] && cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT) || - (!transient[i] && cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT)) + if (!(modes[i] & VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_MODE_SHUTDOWN)) continue; if (virDomainIsActive(domains[i]) == 1) { anyRunning = true; } else { - virObjectUnref(domains[i]); - domains[i] = NULL; + modes[i] = 0; } } @@ -867,21 +871,13 @@ virDomainDriverAutoShutdownDoShutdown(virDomainPtr *domains, static void virDomainDriverAutoShutdownDoPoweroff(virDomainPtr *domains, - bool *transient, - size_t numDomains, - virDomainDriverAutoShutdownConfig *cfg) + unsigned int *modes, + size_t numDomains) { size_t i; - if (cfg->poweroff == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE) - return; - for (i = 0; i < numDomains; i++) { - if (domains[i] == NULL) - continue; - - if ((transient[i] && cfg->poweroff == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT) || - (!transient[i] && cfg->poweroff == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT)) + if (!(modes[i] & VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_MODE_POWEROFF)) continue; virSystemdNotifyStatus("Destroying '%s' (%zu of %zu)", @@ -894,11 +890,49 @@ virDomainDriverAutoShutdownDoPoweroff(virDomainPtr *domains, */ virDomainDestroy(domains[i]); - virObjectUnref(domains[i]); - domains[i] = NULL; + modes[i] = 0; } } +static unsigned int +virDomainDriverAutoShutdownGetMode(virDomainPtr domain, + virDomainDriverAutoShutdownConfig *cfg) +{ + unsigned int mode = 0; + + if (virDomainIsPersistent(domain) == 0) { + if (cfg->trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL || + cfg->trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT) + mode |= VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_MODE_SAVE; + + if (cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL || + cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT) + mode |= VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_MODE_SHUTDOWN; + + if (cfg->poweroff == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL || + cfg->poweroff == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT) + mode |= VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_MODE_POWEROFF; + + /* Don't restore VMs which weren't selected for auto-shutdown */ + if (mode != 0 && cfg->autoRestore) + mode |= VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_MODE_RESTORE; + } else { + if (cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL || + cfg->tryShutdown == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT) + mode |= VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_MODE_SHUTDOWN; + + if (cfg->poweroff == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL || + cfg->poweroff == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT) + mode |= VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_MODE_POWEROFF; + + if (cfg->autoRestore) + VIR_DEBUG("Cannot auto-restore transient VM '%s'", + virDomainGetName(domain)); + } + + return mode; +} + void virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) @@ -907,7 +941,7 @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) int numDomains = 0; size_t i; virDomainPtr *domains = NULL; - g_autofree bool *transient = NULL; + g_autofree unsigned int *modes = NULL; VIR_DEBUG("Run autoshutdown uri=%s trySave=%s tryShutdown=%s poweroff=%s waitShutdownSecs=%u saveBypassCache=%d autoRestore=%d", cfg->uri, @@ -948,58 +982,48 @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) return; if (!(conn = virConnectOpen(cfg->uri))) - goto cleanup; + return; if ((numDomains = virConnectListAllDomains(conn, &domains, VIR_CONNECT_LIST_DOMAINS_ACTIVE)) < 0) - goto cleanup; + return; VIR_DEBUG("Auto shutdown with %d running domains", numDomains); - transient = g_new0(bool, numDomains); + modes = g_new0(unsigned int, numDomains); + for (i = 0; i < numDomains; i++) { - if (virDomainIsPersistent(domains[i]) == 0) - transient[i] = true; + modes[i] = virDomainDriverAutoShutdownGetMode(domains[i], cfg); - if (cfg->autoRestore) { - if (transient[i]) { - VIR_DEBUG("Cannot auto-restore transient VM %s", - virDomainGetName(domains[i])); - } else { - VIR_DEBUG("Mark %s for autostart on next boot", - virDomainGetName(domains[i])); - if (virDomainSetAutostartOnce(domains[i], 1) < 0) { - VIR_WARN("Unable to mark domain '%s' for auto restore: %s", - virDomainGetName(domains[i]), - virGetLastErrorMessage()); - } + if (modes[i] == 0) { + /* VM wasn't selected for any of the shutdown modes. There's not + * much we can do about that as the host is powering off, logging + * at least lets admins know */ + VIR_WARN("auto-shutdown: domain '%s' not successfully shut off by any action", + domains[i]->name); + } + + if (modes[i] & VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_MODE_RESTORE) { + VIR_DEBUG("Mark %s for autostart on next boot", + virDomainGetName(domains[i])); + if (virDomainSetAutostartOnce(domains[i], 1) < 0) { + VIR_WARN("Unable to mark domain '%s' for auto restore: %s", + virDomainGetName(domains[i]), + virGetLastErrorMessage()); } } } - virDomainDriverAutoShutdownDoSave(domains, transient, numDomains, cfg); - virDomainDriverAutoShutdownDoShutdown(domains, transient, numDomains, cfg); - virDomainDriverAutoShutdownDoPoweroff(domains, transient, numDomains, cfg); + virDomainDriverAutoShutdownDoSave(domains, modes, numDomains, cfg); + virDomainDriverAutoShutdownDoShutdown(domains, modes, numDomains, cfg); + virDomainDriverAutoShutdownDoPoweroff(domains, modes, numDomains); virSystemdNotifyStatus("Processed %d domains", numDomains); VIR_INFO("Processed %d domains", numDomains); - cleanup: - if (domains) { - /* Anything non-NULL in this list indicates none of - * the configured ations were successful in processing - * the domain. There's not much we can do about that - * as the host is powering off, logging at least lets - * admins know - */ - for (i = 0; i < numDomains; i++) { - if (domains[i] == NULL) - continue; - VIR_WARN("auto-shutdown: domain '%s' not successfully shut off by any action", - domains[i]->name); - virObjectUnref(domains[i]); - } - VIR_FREE(domains); - } + for (i = 0; i < numDomains; i++) + virObjectUnref(domains[i]); + + VIR_FREE(domains); } -- 2.49.0

On Fri, Jul 04, 2025 at 15:05:10 +0200, Pavel Hrdina wrote: Oops! something looked weird here but I couldn't tell what when looking at the patch. And for those curious, no I didn't test this. It's quite a hassle because the "host" running this needs to actually reboot. I spend quite a few reboots already when validating the ordering fix and simply didn't bother, especially since I was developing per-VM config of shutdown behaviour which I didn't yet finish, and thus also didn't test. This patch was supposed to be a refactor for it but turned out a bugfix (for the resume thing.) I promise I'll test it before pushing