This series starts the work needed to obsolete the libvirt-guests.sh script which has grown a surprisingly large amount of functionality. Currently the virt daemons will acquire inhibitors to delay OS shutdown when VMs are running. The libvirt-guests.service unit can be used to call libvirt-guests.sh to shutdown running VMs on system shutdown. This split is a bad architecture because libvirt-guests.service will only run once the system has decided to initiate the shutdown sequence. When the user requests as shutdown while inhibitors are present, logind will emit a "PrepareForShutdown" signal over dbus. Applications are supposed to respond to this by preserving state & releasing their inhibitors, which in turns allows shutdown to be initiated. The remote daemon already has support for listening for the "PrepareForShutdown" signal, but only does this for session instances, not system instances. This series essentially takes that logic and expands it to run in the system instances too, thus conceptually making libvirt-guests.service obsolete. It is slightly more complicated than that though for many reasons... Saving running VMs can take a very long time. The inhibitor delay can be as low as 5 seconds, and when killing a service, systemd may not wait very long for it to terminate. libvirt-guests.service deals with this by setting TimeoutStopSecs=0 to make systemd wait forever. This is undesirable to set in libvirtd.service though, as we would like systemd to kill the daemon aggressively if it hangs. The series thus uses the notification protocol to request systemd give it more time to shutdown, as long as we're in the phase of saving running VMs. A bug in this code will still result in systemd waiting forever, which is no different from libvirt-guests.service, but a bug in any other part of the libvirt daemon shutdown code will result in systemd killing us. The existing logic for saving VMs in the session daemons had many feature gaps compared to libvirt-guests.sh. Thus there is code to add support * Requesting graceful OS shutdown if managed save failed * Force poweroff of VMs if no other action worked * Optionally enabling/disabling use of managed save, graceful shutdown and force poweroff, which is more flexible than ON_SHUTDOWN=nnn, as we can try the whole sequence of options * Ability to bypass cache in managed save * Support for one-time autostart of VMs as an official API To aid in testing this logic, virt-admin gains a new command 'virt-admin daemon-shutdown --preserve' All this new functionality is wired up into the QEMU driver, and is made easily accessible to other hypervisor drivers, so would easily be extendable to Xen, CH, LXC drivers, but this is not done in this series. IOW, libvirt-guests.service is not yet fully obsolete. The new functionality is also not enabled by default for the system daemon, it requires explicit admin changes to /etc/libvirt/qemu.conf to enable it. This is because it would clash with execution of the libvirt-guests.service if both were enabled. It is highly desirable that we enable this by default though, so we need to figure out a upgrade story wrt libvirt-guests.service. The only libvirt-guests.sh features not implemented are: * PARALLEL_SHUTDOWN=nn. When doing a graceful shutdown we initiate it on every single VM at once, and then monitor progress of all of them in parallel. * SYNC_TIME=nn When make not attempt to sync guest time when restoring from managed save. This ought to be fixed Daniel P. Berrangé (26): util: add APIs for more systemd notifications remote: notify systemd when reloading config hypervisor: introduce helper for autostart src: convert drivers over to use new autostart helper hypervisor: add support for delay interval during autostart qemu: add 'auto_start_delay' configuration parameter hypervisor: move support for auto-shutdown out of QEMU driver remote: always invoke virStateStop for all daemons hypervisor: expand available shutdown actions hypervisor: custom shutdown actions for transient vs persistent VMs qemu: support automatic VM managed save in system daemon qemu: improve shutdown defaults for session daemon qemu: configurable delay for shutdown before poweroff hypervisor: support bypassing cache for managed save qemu: add config parameter to control auto-save bypass cache src: add new APIs for marking a domain to autostart once conf: implement support for autostart once feature hypervisor: wire up support for auto restore of running domains qemu: wire up support for once only autostart qemu: add config to control if auto-shutdown VMs are restored rpc: move state stop into virNetDaemon class rpc: don't unconditionally quit after preserving state rpc: fix shutdown sequence when preserving state admin: add 'daemon-shutdown' command rpc: don't let systemd shutdown daemon while saving VMs hypervisor: send systemd status messages while saving include/libvirt/libvirt-admin.h | 13 ++ include/libvirt/libvirt-domain.h | 4 + src/admin/admin_protocol.x | 11 +- src/admin/admin_server_dispatch.c | 13 ++ src/admin/libvirt-admin.c | 33 ++++ src/admin/libvirt_admin_public.syms | 5 + src/bhyve/bhyve_driver.c | 53 ++---- src/conf/domain_conf.c | 6 +- src/conf/domain_conf.h | 1 + src/conf/virdomainobjlist.c | 7 +- src/driver-hypervisor.h | 10 ++ src/hypervisor/domain_driver.c | 250 ++++++++++++++++++++++++++++ src/hypervisor/domain_driver.h | 42 +++++ src/libvirt-domain.c | 87 ++++++++++ src/libvirt_private.syms | 10 +- src/libvirt_public.syms | 6 + src/libvirt_remote.syms | 2 +- src/libxl/libxl_driver.c | 36 ++-- src/lxc/lxc_driver.c | 13 +- src/lxc/lxc_process.c | 18 +- src/lxc/lxc_process.h | 2 + src/qemu/libvirtd_qemu.aug | 7 + src/qemu/qemu.conf.in | 59 +++++++ src/qemu/qemu_conf.c | 63 +++++++ src/qemu/qemu_conf.h | 7 + src/qemu/qemu_driver.c | 203 +++++++++++++--------- src/qemu/test_libvirtd_qemu.aug.in | 7 + src/remote/libvirtd.service.in | 2 +- src/remote/remote_daemon.c | 78 +++------ src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 30 +++- src/remote_protocol-structs | 12 ++ src/rpc/gendispatch.pl | 4 +- src/rpc/virnetdaemon.c | 212 +++++++++++++++++++---- src/rpc/virnetdaemon.h | 20 ++- src/util/virsystemd.c | 41 ++++- src/util/virsystemd.h | 6 +- src/virtd.service.in | 2 +- tools/virsh-domain-monitor.c | 5 + tools/virsh-domain.c | 39 ++++- tools/virt-admin.c | 41 +++++ 41 files changed, 1181 insertions(+), 281 deletions(-) -- 2.47.1 We have a way to notify systemd when we're done starting the daemon. Systemd supports many more notifications, however, and many of them are quite relevant to our needs: https://www.freedesktop.org/software/systemd/man/latest/sd_notify.html This renames the existing notification API to better reflect its semantics, and adds new APIs for reporting * Initiation of config file reload * Initiation of daemon shutdown process * Adhoc progress status messages * Request to extend service shutdown timeout Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/libvirt_private.syms | 6 +++++- src/rpc/virnetdaemon.c | 2 +- src/util/virsystemd.c | 41 +++++++++++++++++++++++++++++++++++++--- src/util/virsystemd.h | 6 +++++- 4 files changed, 49 insertions(+), 6 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index XXXXXXX..XXXXXXX 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -XXX,XX +XXX,XX @@ virSystemdHasResolved; virSystemdHasResolvedResetCachedValue; virSystemdMakeScopeName; virSystemdMakeSliceName; -virSystemdNotifyStartup; +virSystemdNotifyExtendTimeout; +virSystemdNotifyReady; +virSystemdNotifyReload; +virSystemdNotifyStatus; +virSystemdNotifyStopping; virSystemdResolvedRegisterNameServer; virSystemdTerminateMachine; diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -XXX,XX +XXX,XX @@ virNetDaemonRun(virNetDaemon *dmn) /* We are accepting connections now. Notify systemd * so it can start dependent services. */ - virSystemdNotifyStartup(); + virSystemdNotifyReady(); VIR_DEBUG("dmn=%p quit=%d", dmn, dmn->quit); while (!dmn->finished) { diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index XXXXXXX..XXXXXXX 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -XXX,XX +XXX,XX @@ int virSystemdTerminateMachine(const char *name) return 0; } -void -virSystemdNotifyStartup(void) +static void +virSystemdNotify(const char *msg) { #ifndef WIN32 const char *path; - const char *msg = "READY=1"; int fd; struct sockaddr_un un = { .sun_family = AF_UNIX, @@ -XXX,XX +XXX,XX @@ virSystemdNotifyStartup(void) .msg_iovlen = 1, }; + VIR_DEBUG("Notify '%s'", msg); + if (!(path = getenv("NOTIFY_SOCKET"))) { VIR_DEBUG("Skipping systemd notify, not requested"); return; @@ -XXX,XX +XXX,XX @@ virSystemdNotifyStartup(void) #endif /* !WIN32 */ } +void virSystemdNotifyReady(void) +{ + virSystemdNotify("READY=1"); +} + +void virSystemdNotifyReload(void) +{ + virSystemdNotify("RELOAD=1"); +} + +void virSystemdNotifyStopping(void) +{ + virSystemdNotify("STOPPING=1"); +} + +void virSystemdNotifyExtendTimeout(int secs) +{ + g_autofree char *msg = g_strdup_printf("EXTEND_TIMEOUT_USEC=%llu", + secs * 1000ull * 1000ul); + virSystemdNotify(msg); +} + +void virSystemdNotifyStatus(const char *fmt, ...) +{ + g_autofree char *msg1 = NULL; + g_autofree char *msg2 = NULL; + va_list ap; + va_start(ap, fmt); + msg1 = g_strdup_vprintf(fmt, ap); + va_end(ap); + msg2 = g_strdup_printf("STATUS=%s", msg1); + virSystemdNotify(msg2); +} + static int virSystemdPMSupportTarget(const char *methodName, bool *result) { diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h index XXXXXXX..XXXXXXX 100644 --- a/src/util/virsystemd.h +++ b/src/util/virsystemd.h @@ -XXX,XX +XXX,XX @@ int virSystemdCreateMachine(const char *name, int virSystemdTerminateMachine(const char *name); -void virSystemdNotifyStartup(void); +void virSystemdNotifyReady(void); +void virSystemdNotifyReload(void); +void virSystemdNotifyStopping(void); +void virSystemdNotifyExtendTimeout(int secs); +void virSystemdNotifyStatus(const char *fmt, ...) G_GNUC_PRINTF(1, 2); int virSystemdHasMachined(void); -- 2.47.1 Switch to the 'notify-reload' service type and send notifications to systemd when reloading configuration. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/libvirtd.service.in | 2 +- src/remote/remote_daemon.c | 2 ++ src/virtd.service.in | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in index XXXXXXX..XXXXXXX 100644 --- a/src/remote/libvirtd.service.in +++ b/src/remote/libvirtd.service.in @@ -XXX,XX +XXX,XX @@ After=xencommons.service Conflicts=xendomains.service [Service] -Type=notify +Type=notify-reload Environment=LIBVIRTD_ARGS="--timeout 120" EnvironmentFile=-@initconfdir@/libvirtd ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -XXX,XX +XXX,XX @@ static void daemonReloadHandlerThread(void *opaque G_GNUC_UNUSED) virHookCall(VIR_HOOK_DRIVER_DAEMON, "-", VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, "SIGHUP", NULL, NULL); + virSystemdNotifyReload(); if (virStateReload() < 0) { VIR_WARN("Error while reloading drivers"); } + virSystemdNotifyReady(); /* Drivers are initialized again. */ g_atomic_int_set(&driversInitialized, 1); diff --git a/src/virtd.service.in b/src/virtd.service.in index XXXXXXX..XXXXXXX 100644 --- a/src/virtd.service.in +++ b/src/virtd.service.in @@ -XXX,XX +XXX,XX @@ After=dbus.service After=apparmor.service [Service] -Type=notify +Type=notify-reload Environment=@SERVICE@_ARGS="--timeout 120" EnvironmentFile=-@initconfdir@/@service@ ExecStart=@sbindir@/@service@ $@SERVICE@_ARGS -- 2.47.1 There's a common pattern for autostart of iterating over VMs, acquiring a lock and ref count, then checking the autostart & is-active flags. Wrap this all up into a helper method. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/hypervisor/domain_driver.c | 40 ++++++++++++++++++++++++++++++++++ src/hypervisor/domain_driver.h | 17 +++++++++++++++ src/libvirt_private.syms | 1 + 3 files changed, 58 insertions(+) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -XXX,XX +XXX,XX @@ #include "viraccessapicheck.h" #include "datatypes.h" #include "driver.h" +#include "virlog.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN +VIR_LOG_INIT("hypervisor.domain_driver"); + char * virDomainDriverGenerateRootHash(const char *drivername, const char *root) @@ -XXX,XX +XXX,XX @@ virDomainDriverGetIOThreadsConfig(virDomainDef *targetDef, return ret; } + +static int +virDomainDriverAutoStartOne(virDomainObj *vm, + void *opaque) +{ + virDomainDriverAutoStartConfig *cfg = opaque; + + virObjectLock(vm); + virObjectRef(vm); + + VIR_DEBUG("Autostart %s: autostart=%d", + vm->def->name, vm->autostart); + + if (vm->autostart && !virDomainObjIsActive(vm)) { + virResetLastError(); + cfg->callback(vm, cfg->opaque); + } + + virDomainObjEndAPI(&vm); + virResetLastError(); + + return 0; +} + +void virDomainDriverAutoStart(virDomainObjList *domains, + virDomainDriverAutoStartConfig *cfg) +{ + bool autostart; + VIR_DEBUG("Run autostart stateDir=%s", cfg->stateDir); + if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0 || + !autostart) { + VIR_DEBUG("Autostart already processed"); + return; + } + + virDomainObjListForEach(domains, false, virDomainDriverAutoStartOne, cfg); +} diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -XXX,XX +XXX,XX @@ #include "node_device_conf.h" #include "virhostdev.h" #include "virpci.h" +#include "virdomainobjlist.h" char * virDomainDriverGenerateRootHash(const char *drivername, @@ -XXX,XX +XXX,XX @@ int virDomainDriverDelIOThreadCheck(virDomainDef *def, int virDomainDriverGetIOThreadsConfig(virDomainDef *targetDef, virDomainIOThreadInfoPtr **info, unsigned int bitmap_size); + +/* + * Will be called with 'vm' locked and ref count held, + * which will be released when this returns. + */ +typedef void (*virDomainDriverAutoStartCallback)(virDomainObj *vm, + void *opaque); + +typedef struct _virDomainDriverAutoStartConfig { + char *stateDir; + virDomainDriverAutoStartCallback callback; + void *opaque; +} virDomainDriverAutoStartConfig; + +void virDomainDriverAutoStart(virDomainObjList *domains, + virDomainDriverAutoStartConfig *cfg); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index XXXXXXX..XXXXXXX 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -XXX,XX +XXX,XX @@ virDomainCgroupSetupVcpuBW; # hypervisor/domain_driver.h virDomainDriverAddIOThreadCheck; +virDomainDriverAutoStart; virDomainDriverDelIOThreadCheck; virDomainDriverGenerateMachineName; virDomainDriverGenerateRootHash; -- 2.47.1 Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/bhyve/bhyve_driver.c | 53 ++++++++++++--------------------------- src/libxl/libxl_driver.c | 36 ++++++++------------------- src/lxc/lxc_driver.c | 13 +++++----- src/lxc/lxc_process.c | 18 ++------------ src/lxc/lxc_process.h | 2 ++ src/qemu/qemu_driver.c | 54 ++++++++++++---------------------------- 6 files changed, 53 insertions(+), 123 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -XXX,XX +XXX,XX @@ VIR_LOG_INIT("bhyve.bhyve_driver"); struct _bhyveConn *bhyve_driver = NULL; static int -bhyveAutostartDomain(virDomainObj *vm, void *opaque) +bhyveAutostartDomain(virDomainObj *vm, void *opaque G_GNUC_UNUSED) { - const struct bhyveAutostartData *data = opaque; int ret = 0; - VIR_LOCK_GUARD lock = virObjectLockGuard(vm); - - if (vm->autostart && !virDomainObjIsActive(vm)) { - virResetLastError(); - ret = virBhyveProcessStart(data->conn, vm, - VIR_DOMAIN_RUNNING_BOOTED, 0); - if (ret < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to autostart VM '%1$s': %2$s"), - vm->def->name, virGetLastErrorMessage()); - } - } - return ret; -} - -static void -bhyveAutostartDomains(struct _bhyveConn *driver) -{ - /* XXX: Figure out a better way todo this. The domain - * startup code needs a connection handle in order - * to lookup the bridge associated with a virtual - * network - */ - virConnectPtr conn = virConnectOpen("bhyve:///system"); - /* Ignoring NULL conn which is mostly harmless here */ - struct bhyveAutostartData data = { driver, conn }; - - virDomainObjListForEach(driver->domains, false, bhyveAutostartDomain, &data); + ret = virBhyveProcessStart(NULL, vm, + VIR_DOMAIN_RUNNING_BOOTED, 0); + if (ret < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to autostart VM '%1$s': %2$s"), + vm->def->name, virGetLastErrorMessage()); + } - virObjectUnref(conn); + return ret; } /** @@ -XXX,XX +XXX,XX @@ bhyveStateInitialize(bool privileged, virStateInhibitCallback callback G_GNUC_UNUSED, void *opaque G_GNUC_UNUSED) { - bool autostart = true; + virDomainDriverAutoStartConfig autostartCfg; if (root != NULL) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -XXX,XX +XXX,XX @@ bhyveStateInitialize(bool privileged, virBhyveProcessReconnectAll(bhyve_driver); - if (virDriverShouldAutostart(BHYVE_STATE_DIR, &autostart) < 0) - goto cleanup; - - if (autostart) - bhyveAutostartDomains(bhyve_driver); + autostartCfg = (virDomainDriverAutoStartConfig) { + .stateDir = BHYVE_STATE_DIR, + .callback = bhyveAutostartDomain, + .opaque = bhyve_driver, + }; + virDomainDriverAutoStart(bhyve_driver->domains, &autostartCfg); return VIR_DRV_STATE_INIT_COMPLETE; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -XXX,XX +XXX,XX @@ libxlDomObjFromDomain(virDomainPtr dom) return vm; } -static int +static void libxlAutostartDomain(virDomainObj *vm, void *opaque) { libxlDriverPrivate *driver = opaque; - int ret = -1; - - virObjectRef(vm); - virObjectLock(vm); - virResetLastError(); if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) - goto cleanup; + return; - if (vm->autostart && !virDomainObjIsActive(vm) && - libxlDomainStartNew(driver, vm, false) < 0) { + if (libxlDomainStartNew(driver, vm, false) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to autostart VM '%1$s': %2$s"), vm->def->name, virGetLastErrorMessage()); - goto endjob; } - ret = 0; - - endjob: virDomainObjEndJob(vm); - cleanup: - virDomainObjEndAPI(&vm); - - return ret; } @@ -XXX,XX +XXX,XX @@ libxlStateInitialize(bool privileged, { libxlDriverConfig *cfg; g_autofree char *driverConf = NULL; - bool autostart = true; + virDomainDriverAutoStartConfig autostartCfg; if (root != NULL) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -XXX,XX +XXX,XX @@ libxlStateInitialize(bool privileged, NULL, NULL) < 0) goto error; - if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0) - goto error; - - if (autostart) { - virDomainObjListForEach(libxl_driver->domains, false, - libxlAutostartDomain, - libxl_driver); - } + autostartCfg = (virDomainDriverAutoStartConfig) { + .stateDir = cfg->stateDir, + .callback = libxlAutostartDomain, + .opaque = libxl_driver, + }; + virDomainDriverAutoStart(libxl_driver->domains, &autostartCfg); virDomainObjListForEach(libxl_driver->domains, false, libxlDomainManagedSaveLoad, diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -XXX,XX +XXX,XX @@ lxcStateInitialize(bool privileged, void *opaque) { virLXCDriverConfig *cfg = NULL; - bool autostart = true; const char *defsecmodel; + virDomainDriverAutoStartConfig autostartCfg; if (root != NULL) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -XXX,XX +XXX,XX @@ lxcStateInitialize(bool privileged, NULL, NULL) < 0) goto cleanup; - if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0) - goto cleanup; - - if (autostart) - virLXCProcessAutostartAll(lxc_driver); + autostartCfg = (virDomainDriverAutoStartConfig) { + .stateDir = cfg->stateDir, + .callback = virLXCProcessAutostartDomain, + .opaque = NULL, + }; + virDomainDriverAutoStart(lxc_driver->domains, &autostartCfg); return VIR_DRV_STATE_INIT_COMPLETE; diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index XXXXXXX..XXXXXXX 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -XXX,XX +XXX,XX @@ int virLXCProcessStart(virLXCDriver * driver, } -static int +void virLXCProcessAutostartDomain(virDomainObj *vm, void *opaque G_GNUC_UNUSED) { - VIR_LOCK_GUARD lock = virObjectLockGuard(vm); virLXCDomainObjPrivate *priv = vm->privateData; virObjectEvent *event; int rc = 0; - if (!vm->autostart || - virDomainObjIsActive(vm)) - return 0; - rc = virLXCProcessStart(priv->driver, vm, 0, NULL, NULL, VIR_DOMAIN_RUNNING_BOOTED); virDomainAuditStart(vm, "booted", rc >= 0); @@ -XXX,XX +XXX,XX @@ virLXCProcessAutostartDomain(virDomainObj *vm, VIR_ERROR(_("Failed to autostart VM '%1$s': %2$s"), vm->def->name, virGetLastErrorMessage()); - return -1; + return; } event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, VIR_DOMAIN_EVENT_STARTED_BOOTED); virObjectEventStateQueue(priv->driver->domainEventState, event); - - return 0; -} - - -void -virLXCProcessAutostartAll(virLXCDriver *driver) -{ - virDomainObjListForEach(driver->domains, false, virLXCProcessAutostartDomain, NULL); } diff --git a/src/lxc/lxc_process.h b/src/lxc/lxc_process.h index XXXXXXX..XXXXXXX 100644 --- a/src/lxc/lxc_process.h +++ b/src/lxc/lxc_process.h @@ -XXX,XX +XXX,XX @@ int virLXCProcessStop(virLXCDriver *driver, unsigned int cleanupFlags); void virLXCProcessAutostartAll(virLXCDriver *driver); +void virLXCProcessAutostartDomain(virDomainObj *vm, + void *opaque); int virLXCProcessReconnectAll(virLXCDriver *driver, virDomainObjList *doms); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ qemuDomObjFromSnapshot(virDomainSnapshotPtr snapshot) -static int +static void qemuAutostartDomain(virDomainObj *vm, void *opaque) { virQEMUDriver *driver = opaque; int flags = 0; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - int ret = -1; if (cfg->autoStartBypassCache) flags |= VIR_DOMAIN_START_BYPASS_CACHE; - virObjectLock(vm); - virObjectRef(vm); - virResetLastError(); - if (vm->autostart && - !virDomainObjIsActive(vm)) { - if (qemuProcessBeginJob(vm, VIR_DOMAIN_JOB_OPERATION_START, - flags) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to start job on VM '%1$s': %2$s"), - vm->def->name, virGetLastErrorMessage()); - goto cleanup; - } + if (qemuProcessBeginJob(vm, VIR_DOMAIN_JOB_OPERATION_START, + flags) < 0) + return; - if (qemuDomainObjStart(NULL, driver, vm, flags, - VIR_ASYNC_JOB_START) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to autostart VM '%1$s': %2$s"), + if (qemuDomainObjStart(NULL, driver, vm, flags, + VIR_ASYNC_JOB_START) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to autostart VM '%1$s': %2$s"), vm->def->name, virGetLastErrorMessage()); - } - - qemuProcessEndJob(vm); } - ret = 0; - cleanup: - virDomainObjEndAPI(&vm); - return ret; -} - - -static void -qemuAutostartDomains(virQEMUDriver *driver) -{ - virDomainObjListForEach(driver->domains, false, qemuAutostartDomain, driver); + qemuProcessEndJob(vm); } @@ -XXX,XX +XXX,XX @@ qemuStateInitialize(bool privileged, virQEMUDriverConfig *cfg; uid_t run_uid = -1; gid_t run_gid = -1; - bool autostart = true; size_t i; const char *defsecmodel = NULL; g_autoptr(virIdentity) identity = virIdentityGetCurrent(); + virDomainDriverAutoStartConfig autostartCfg; qemu_driver = g_new0(virQEMUDriver, 1); @@ -XXX,XX +XXX,XX @@ qemuStateInitialize(bool privileged, qemuProcessReconnectAll(qemu_driver); - if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0) - goto error; - - if (autostart) - qemuAutostartDomains(qemu_driver); + autostartCfg = (virDomainDriverAutoStartConfig) { + .stateDir = cfg->stateDir, + .callback = qemuAutostartDomain, + .opaque = qemu_driver, + }; + virDomainDriverAutoStart(qemu_driver->domains, &autostartCfg); return VIR_DRV_STATE_INIT_COMPLETE; -- 2.47.1 This delay can reduce the CPU/IO load storm when autostarting many guests. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/hypervisor/domain_driver.c | 20 +++++++++++++++++--- src/hypervisor/domain_driver.h | 1 + 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -XXX,XX +XXX,XX @@ virDomainDriverGetIOThreadsConfig(virDomainDef *targetDef, return ret; } +typedef struct _virDomainDriverAutoStartState { + virDomainDriverAutoStartConfig *cfg; + bool first; +} virDomainDriverAutoStartState; + static int virDomainDriverAutoStartOne(virDomainObj *vm, void *opaque) { - virDomainDriverAutoStartConfig *cfg = opaque; + virDomainDriverAutoStartState *state = opaque; virObjectLock(vm); virObjectRef(vm); @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoStartOne(virDomainObj *vm, if (vm->autostart && !virDomainObjIsActive(vm)) { virResetLastError(); - cfg->callback(vm, cfg->opaque); + if (state->cfg->delayMS) { + if (!state->first) { + g_usleep(state->cfg->delayMS * 1000ull); + } else { + state->first = false; + } + } + + state->cfg->callback(vm, state->cfg->opaque); } virDomainObjEndAPI(&vm); @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoStartOne(virDomainObj *vm, void virDomainDriverAutoStart(virDomainObjList *domains, virDomainDriverAutoStartConfig *cfg) { + virDomainDriverAutoStartState state = { .cfg = cfg, .first = true }; bool autostart; VIR_DEBUG("Run autostart stateDir=%s", cfg->stateDir); if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0 || @@ -XXX,XX +XXX,XX @@ void virDomainDriverAutoStart(virDomainObjList *domains, return; } - virDomainObjListForEach(domains, false, virDomainDriverAutoStartOne, cfg); + virDomainObjListForEach(domains, false, virDomainDriverAutoStartOne, &state); } diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -XXX,XX +XXX,XX @@ typedef struct _virDomainDriverAutoStartConfig { char *stateDir; virDomainDriverAutoStartCallback callback; void *opaque; + int delayMS; /* milliseconds between starting each guest */ } virDomainDriverAutoStartConfig; void virDomainDriverAutoStart(virDomainObjList *domains, -- 2.47.1 This allows a user specified delay between autostart of each VM, giving parity with the equivalent feature of libvirt-guests. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf.in | 4 ++++ src/qemu/qemu_conf.c | 2 ++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 10 insertions(+) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -XXX,XX +XXX,XX @@ module Libvirtd_qemu = | str_entry "auto_dump_path" | bool_entry "auto_dump_bypass_cache" | bool_entry "auto_start_bypass_cache" + | int_entry "auto_start_delay" let process_entry = str_entry "hugetlbfs_mount" | str_entry "bridge_helper" diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu.conf.in +++ b/src/qemu/qemu.conf.in @@ -XXX,XX +XXX,XX @@ # #auto_start_bypass_cache = 0 +# Delay in milliseconds between starting each VM, during autostart +# +#auto_start_delay = 0 + # If provided by the host and a hugetlbfs mount point is configured, # a guest may request huge page backing. When this mount point is # unspecified here, determination of a host mount point in /proc/mounts diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -XXX,XX +XXX,XX @@ virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg, return -1; if (virConfGetValueBool(conf, "auto_start_bypass_cache", &cfg->autoStartBypassCache) < 0) return -1; + if (virConfGetValueInt(conf, "auto_start_delay", &cfg->autoStartDelayMS) < 0) + return -1; return 0; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -XXX,XX +XXX,XX @@ struct _virQEMUDriverConfig { char *autoDumpPath; bool autoDumpBypassCache; bool autoStartBypassCache; + int autoStartDelayMS; char *lockManagerName; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ qemuStateInitialize(bool privileged, .stateDir = cfg->stateDir, .callback = qemuAutostartDomain, .opaque = qemu_driver, + .delayMS = cfg->autoStartDelayMS, }; virDomainDriverAutoStart(qemu_driver->domains, &autostartCfg); diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -XXX,XX +XXX,XX @@ module Test_libvirtd_qemu = { "auto_dump_path" = "/var/lib/libvirt/qemu/dump" } { "auto_dump_bypass_cache" = "0" } { "auto_start_bypass_cache" = "0" } +{ "auto_start_delay" = "0" } { "hugetlbfs_mount" = "/dev/hugepages" } { "bridge_helper" = "qemu-bridge-helper" } { "set_process_name" = "1" } -- 2.47.1 This is a move of the code that currently exists in the QEMU driver, into the common layer that can be used by multiple drivers. The code currently supports performing managed save of all running guests, ignoring any failures. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/hypervisor/domain_driver.c | 48 ++++++++++++++++++++++++++++++++++ src/hypervisor/domain_driver.h | 8 +++++- src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 47 ++++----------------------------- 4 files changed, 61 insertions(+), 43 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -XXX,XX +XXX,XX @@ void virDomainDriverAutoStart(virDomainObjList *domains, virDomainObjListForEach(domains, false, virDomainDriverAutoStartOne, &state); } + + +void +virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) +{ + g_autoptr(virConnect) conn = NULL; + int numDomains = 0; + size_t i; + int state; + virDomainPtr *domains = NULL; + g_autofree unsigned int *flags = NULL; + + if (!(conn = virConnectOpen(cfg->uri))) + goto cleanup; + + if ((numDomains = virConnectListAllDomains(conn, + &domains, + VIR_CONNECT_LIST_DOMAINS_ACTIVE)) < 0) + goto cleanup; + + flags = g_new0(unsigned int, numDomains); + + /* First we pause all VMs to make them stop dirtying + pages, etc. We remember if any VMs were paused so + we can restore that on resume. */ + for (i = 0; i < numDomains; i++) { + 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; + } + virDomainSuspend(domains[i]); + } + + /* Then we save the VMs to disk */ + for (i = 0; i < numDomains; i++) + if (virDomainManagedSave(domains[i], flags[i]) < 0) + VIR_WARN("Unable to perform managed save of '%s': %s", + virDomainGetName(domains[i]), + virGetLastErrorMessage()); + + cleanup: + if (domains) { + for (i = 0; i < numDomains; i++) + virObjectUnref(domains[i]); + VIR_FREE(domains); + } +} diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -XXX,XX +XXX,XX @@ typedef void (*virDomainDriverAutoStartCallback)(virDomainObj *vm, void *opaque); typedef struct _virDomainDriverAutoStartConfig { - char *stateDir; + const char *stateDir; virDomainDriverAutoStartCallback callback; void *opaque; int delayMS; /* milliseconds between starting each guest */ @@ -XXX,XX +XXX,XX @@ typedef struct _virDomainDriverAutoStartConfig { void virDomainDriverAutoStart(virDomainObjList *domains, virDomainDriverAutoStartConfig *cfg); + +typedef struct _virDomainDriverAutoShutdownConfig { + const char *uri; +} virDomainDriverAutoShutdownConfig; + +void virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index XXXXXXX..XXXXXXX 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -XXX,XX +XXX,XX @@ virDomainCgroupSetupVcpuBW; # hypervisor/domain_driver.h virDomainDriverAddIOThreadCheck; +virDomainDriverAutoShutdown; virDomainDriverAutoStart; virDomainDriverDelIOThreadCheck; virDomainDriverGenerateMachineName; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ qemuStateReload(void) static int qemuStateStop(void) { - int ret = -1; - g_autoptr(virConnect) conn = NULL; - int numDomains = 0; - size_t i; - int state; - virDomainPtr *domains = NULL; - g_autofree unsigned int *flags = NULL; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(qemu_driver); + virDomainDriverAutoShutdownConfig ascfg = { + .uri = cfg->uri, + }; - if (!(conn = virConnectOpen(cfg->uri))) - goto cleanup; - - if ((numDomains = virConnectListAllDomains(conn, - &domains, - VIR_CONNECT_LIST_DOMAINS_ACTIVE)) < 0) - goto cleanup; - - flags = g_new0(unsigned int, numDomains); - - /* First we pause all VMs to make them stop dirtying - pages, etc. We remember if any VMs were paused so - we can restore that on resume. */ - for (i = 0; i < numDomains; i++) { - 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; - } - virDomainSuspend(domains[i]); - } - - ret = 0; - /* Then we save the VMs to disk */ - for (i = 0; i < numDomains; i++) - if (virDomainManagedSave(domains[i], flags[i]) < 0) - ret = -1; - - cleanup: - if (domains) { - for (i = 0; i < numDomains; i++) - virObjectUnref(domains[i]); - VIR_FREE(domains); - } + virDomainDriverAutoShutdown(&ascfg); - return ret; + return 0; } -- 2.47.1 Currently the virStateStop method is only wired up to run save for the unprivileged daemons, so there is no functional change. IOW, session exit, or host OS shutdown will trigger VM managed saved for QEMU session daemon, but not the system daemon. This changes the daemon code to always run virStateStop for all daemons. Instead the QEMU driver is responsible for skipping its own logic when running privileged...for now. This means that virStateStop will now be triggered by logind's PrepareForShutdown signal. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu_driver.c | 3 ++- src/remote/remote_daemon.c | 28 +++++++++++++++------------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ qemuStateStop(void) .uri = cfg->uri, }; - virDomainDriverAutoShutdown(&ascfg); + if (!qemu_driver->privileged) + virDomainDriverAutoShutdown(&ascfg); return 0; } diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -XXX,XX +XXX,XX @@ static void daemonRunStateInit(void *opaque) virStateShutdownPrepare, virStateShutdownWait); - /* Tie the non-privileged daemons to the session/shutdown lifecycle */ + /* Signal for VM shutdown when desktop session is terminated, in + * unprivileged daemons */ if (!virNetDaemonIsPrivileged(dmn)) { - sessionBus = virGDBusGetSessionBus(); if (sessionBus != NULL) g_dbus_connection_add_filter(sessionBus, handleSessionMessageFunc, dmn, NULL); + } - systemBus = virGDBusGetSystemBus(); - if (systemBus != NULL) - g_dbus_connection_signal_subscribe(systemBus, - "org.freedesktop.login1", - "org.freedesktop.login1.Manager", - "PrepareForShutdown", - NULL, - NULL, - G_DBUS_SIGNAL_FLAGS_NONE, - handleSystemMessageFunc, + /* Signal for VM shutdown when host OS shutdown is requested, in + * both privileged and unprivileged daemons */ + systemBus = virGDBusGetSystemBus(); + if (systemBus != NULL) + g_dbus_connection_signal_subscribe(systemBus, + "org.freedesktop.login1", + "org.freedesktop.login1.Manager", + "PrepareForShutdown", + NULL, + NULL, + G_DBUS_SIGNAL_FLAGS_NONE, + handleSystemMessageFunc, dmn, NULL); - } /* Only now accept clients from network */ virNetDaemonUpdateServices(dmn, true); -- 2.47.1 The auto shutdown code can currently only perform managed save, which may fail in some cases, for example when PCI devices are assigned. On failure, shutdown inhibitors remain in place which may be undesirable. This expands the logic to try a sequence of operations * Managed save * Graceful shutdown * Forced poweroff Each of these operations can be enabled or disabled, but they are always applied in this order. With the shutdown option, a configurable time is allowed for shutdown to complete, defaulting to 30 seconds, before moving onto the forced poweroff phase. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/hypervisor/domain_driver.c | 113 +++++++++++++++++++++++++++------ src/hypervisor/domain_driver.h | 4 ++ src/qemu/qemu_driver.c | 3 + 3 files changed, 100 insertions(+), 20 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) g_autoptr(virConnect) conn = NULL; int numDomains = 0; size_t i; - int state; virDomainPtr *domains = NULL; - g_autofree unsigned int *flags = NULL; + + VIR_DEBUG("Run autoshutdown uri=%s trySave=%d tryShutdown=%d poweroff=%d" + "waitShutdownSecs=%d", + cfg->uri, cfg->trySave, cfg->tryShutdown, cfg->poweroff, + cfg->waitShutdownSecs); + + /* + * Ideally guests will shutdown in a few seconds, but it would + * not be unsual for it to take a while longer, especially under + * load, or if the guest OS has inhibitors slowing down shutdown. + * + * If we wait too long, then guests which ignore the shutdown + * request will significantly delay host shutdown. + * + * Pick 30 seconds as a moderately safe default, assuming that + * most guests are well behaved. + */ + if (cfg->waitShutdownSecs <= 0) + cfg->waitShutdownSecs = 30; if (!(conn = virConnectOpen(cfg->uri))) goto cleanup; @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) VIR_CONNECT_LIST_DOMAINS_ACTIVE)) < 0) goto cleanup; - flags = g_new0(unsigned int, numDomains); + VIR_DEBUG("Auto shutdown with %d running domains", numDomains); + if (cfg->trySave) { + g_autofree unsigned int *flags = g_new0(unsigned int, numDomains); + for (i = 0; i < numDomains; i++) { + int state; + /* + * 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; + } + virDomainSuspend(domains[i]); + } + + for (i = 0; i < numDomains; i++) { + if (virDomainManagedSave(domains[i], flags[i]) < 0) { + VIR_WARN("Unable to perform managed save of '%s': %s", + virDomainGetName(domains[i]), + virGetLastErrorMessage()); + continue; + } + virObjectUnref(domains[i]); + domains[i] = NULL; + } + } + + if (cfg->tryShutdown) { + GTimer *timer = NULL; + for (i = 0; i < numDomains; i++) { + if (domains[i] == NULL) + continue; + if (virDomainShutdown(domains[i]) < 0) { + VIR_WARN("Unable to perform graceful shutdown of '%s': %s", + virDomainGetName(domains[i]), + virGetLastErrorMessage()); + break; + } + } + + timer = g_timer_new(); + while (1) { + bool anyRunning = false; + for (i = 0; i < numDomains; i++) { + if (!domains[i]) + continue; - /* First we pause all VMs to make them stop dirtying - pages, etc. We remember if any VMs were paused so - we can restore that on resume. */ - for (i = 0; i < numDomains; i++) { - 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 (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); } - virDomainSuspend(domains[i]); + g_timer_destroy(timer); } - /* Then we save the VMs to disk */ - for (i = 0; i < numDomains; i++) - if (virDomainManagedSave(domains[i], flags[i]) < 0) - VIR_WARN("Unable to perform managed save of '%s': %s", - virDomainGetName(domains[i]), - virGetLastErrorMessage()); + if (cfg->poweroff) { + for (i = 0; i < numDomains; i++) { + if (domains[i] == NULL) + continue; + virDomainDestroy(domains[i]); + } + } cleanup: if (domains) { - for (i = 0; i < numDomains; i++) - virObjectUnref(domains[i]); + for (i = 0; i < numDomains; i++) { + if (domains[i]) + virObjectUnref(domains[i]); + } VIR_FREE(domains); } } diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -XXX,XX +XXX,XX @@ void virDomainDriverAutoStart(virDomainObjList *domains, typedef struct _virDomainDriverAutoShutdownConfig { const char *uri; + bool trySave; + bool tryShutdown; + bool poweroff; + int waitShutdownSecs; } virDomainDriverAutoShutdownConfig; void virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ qemuStateStop(void) g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(qemu_driver); virDomainDriverAutoShutdownConfig ascfg = { .uri = cfg->uri, + .trySave = true, + .tryShutdown = false, + .poweroff = false, }; if (!qemu_driver->privileged) -- 2.47.1 It may be desirable to treat transient VMs differently from persistent VMs. For example, while performing managed save on persistent VMs makes sense, the same not usually true of transient VMs, since by their nature they will have no config to restore from. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/hypervisor/domain_driver.c | 46 +++++++++++++++++++++++++++++++--- src/hypervisor/domain_driver.h | 18 ++++++++++--- src/libvirt_private.syms | 2 ++ src/qemu/qemu_driver.c | 6 ++--- 4 files changed, 63 insertions(+), 9 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -XXX,XX +XXX,XX @@ VIR_LOG_INIT("hypervisor.domain_driver"); +VIR_ENUM_IMPL(virDomainDriverAutoShutdownScope, + VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_LAST, + "none", + "persistent", + "transient", + "all"); + char * virDomainDriverGenerateRootHash(const char *drivername, const char *root) @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) int numDomains = 0; size_t i; virDomainPtr *domains = NULL; + g_autofree bool *transient = NULL; VIR_DEBUG("Run autoshutdown uri=%s trySave=%d tryShutdown=%d poweroff=%d" "waitShutdownSecs=%d", @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) if (cfg->waitShutdownSecs <= 0) cfg->waitShutdownSecs = 30; + /* 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) + return; + if (!(conn = virConnectOpen(cfg->uri))) goto cleanup; @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) goto cleanup; VIR_DEBUG("Auto shutdown with %d running domains", numDomains); - if (cfg->trySave) { + + transient = g_new0(bool, numDomains); + for (i = 0; i < numDomains; i++) { + if (virDomainIsPersistent(domains[i]) == 0) + transient[i] = true; + } + + 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; + /* * Pause all VMs to make them stop dirtying pages, * so save is quicker. We remember if any VMs were @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) } } - if (cfg->tryShutdown) { + 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; + if (virDomainShutdown(domains[i]) < 0) { VIR_WARN("Unable to perform graceful shutdown of '%s': %s", virDomainGetName(domains[i]), @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) 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 { @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) g_timer_destroy(timer); } - if (cfg->poweroff) { + 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; + virDomainDestroy(domains[i]); } } diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -XXX,XX +XXX,XX @@ #include "virhostdev.h" #include "virpci.h" #include "virdomainobjlist.h" +#include "virenum.h" char * virDomainDriverGenerateRootHash(const char *drivername, @@ -XXX,XX +XXX,XX @@ typedef struct _virDomainDriverAutoStartConfig { void virDomainDriverAutoStart(virDomainObjList *domains, virDomainDriverAutoStartConfig *cfg); +typedef enum { + VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE, + VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT, + VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT, + VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL, + + VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_LAST, +} virDomainDriverAutoShutdownScope; + +VIR_ENUM_DECL(virDomainDriverAutoShutdownScope); + typedef struct _virDomainDriverAutoShutdownConfig { const char *uri; - bool trySave; - bool tryShutdown; - bool poweroff; + virDomainDriverAutoShutdownScope trySave; + virDomainDriverAutoShutdownScope tryShutdown; + virDomainDriverAutoShutdownScope poweroff; int waitShutdownSecs; } virDomainDriverAutoShutdownConfig; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index XXXXXXX..XXXXXXX 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -XXX,XX +XXX,XX @@ virDomainCgroupSetupVcpuBW; # hypervisor/domain_driver.h virDomainDriverAddIOThreadCheck; virDomainDriverAutoShutdown; +virDomainDriverAutoShutdownScopeTypeFromString; +virDomainDriverAutoShutdownScopeTypeToString; virDomainDriverAutoStart; virDomainDriverDelIOThreadCheck; virDomainDriverGenerateMachineName; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ qemuStateStop(void) g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(qemu_driver); virDomainDriverAutoShutdownConfig ascfg = { .uri = cfg->uri, - .trySave = true, - .tryShutdown = false, - .poweroff = false, + .trySave = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL, + .tryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE, + .poweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE }; if (!qemu_driver->privileged) -- 2.47.1 Currently automatic VM managed save is only performed in session daemons, on desktop session close, or host OS shutdown request. With this change it is possible to control shutdown behaviour for all daemons. A recommended setup might be: auto_shutdown_try_save = "persistent" auto_shutdown_try_shutdown = "all" auto_shutdown_poweroff = "all" Each setting accepts 'none', 'persistent', 'transient', and 'all' to control what types of guest it applies to. For historical compatibility, for the system daemon, the settings currently default to: auto_shutdown_try_save = "none" auto_shutdown_try_shutdown = "none" auto_shutdown_poweroff = "none" while for the session daemon they currently default to auto_shutdown_try_save = "all" auto_shutdown_try_shutdown = "none" auto_shutdown_poweroff = "none" The system daemon settings should NOT be enabled if the traditional libvirt-guests.service is already enabled. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/libvirtd_qemu.aug | 3 ++ src/qemu/qemu.conf.in | 37 ++++++++++++++++++++++ src/qemu/qemu_conf.c | 51 ++++++++++++++++++++++++++++++ src/qemu/qemu_conf.h | 3 ++ src/qemu/qemu_driver.c | 9 +++--- src/qemu/test_libvirtd_qemu.aug.in | 3 ++ 6 files changed, 101 insertions(+), 5 deletions(-) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -XXX,XX +XXX,XX @@ module Libvirtd_qemu = | bool_entry "auto_dump_bypass_cache" | bool_entry "auto_start_bypass_cache" | int_entry "auto_start_delay" + | str_entry "auto_shutdown_try_save" + | str_entry "auto_shutdown_try_shutdown" + | str_entry "auto_shutdown_powerdown" let process_entry = str_entry "hugetlbfs_mount" | str_entry "bridge_helper" diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu.conf.in +++ b/src/qemu/qemu.conf.in @@ -XXX,XX +XXX,XX @@ # #auto_start_delay = 0 +# Whether to perform managed save of running VMs if a host OS +# shutdown is requested (system/session daemons), or the desktop +# session terminates (session daemon only). Accepts +# +# * "none" - do not try to save any running VMs +# * "persistent" - only try to save persistent running VMs +# * "transient" - only try to save transient running VMs +# * "all" - try to save all running VMs +# +# Defaults to "all" for session daemons and "none" +# for system daemons +# +# If 'libvirt-guests.service' is enabled, then this must be +# set to 'none' for system daemons to avoid dueling actions +#auto_shutdown_try_save = "all" + +# As above, but with a graceful shutdown action instead of +# managed save. If managed save is enabled, shutdown will +# be tried only on failure to perform managed save. +# +# Defaults to "none" +# +# If 'libvirt-guests.service' is enabled, then this must be +# set to 'none' for system daemons to avoid dueling actions +#auto_shutdown_try_shutdown = "none" + +# As above, but with a forced poweroff instead of managed +# save. If managed save or graceful shutdown are enabled, +# forced poweroff will be tried only on failure of the +# other options. +# +# Defaults to "none" +# +# If 'libvirt-guests.service' is enabled, then this must be +# set to 'none' for system daemons to avoid dueling actions +#auto_shutdown_powerdown = "none" + # If provided by the host and a hugetlbfs mount point is configured, # a guest may request huge page backing. When this mount point is # unspecified here, determination of a host mount point in /proc/mounts diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -XXX,XX +XXX,XX @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged, cfg->dumpGuestCore = true; #endif + if (privileged) { + /* + * Defer to libvirt-guests.service. + * + * 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; + } else { + cfg->autoShutdownTrySave = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL; + cfg->autoShutdownTryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE; + cfg->autoShutdownPoweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE; + } + return g_steal_pointer(&cfg); } @@ -XXX,XX +XXX,XX @@ static int virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg, virConf *conf) { + g_autofree char *autoShutdownScope = NULL; + if (virConfGetValueString(conf, "save_image_format", &cfg->saveImageFormat) < 0) return -1; if (virConfGetValueString(conf, "dump_image_format", &cfg->dumpImageFormat) < 0) @@ -XXX,XX +XXX,XX @@ virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg, return -1; if (virConfGetValueInt(conf, "auto_start_delay", &cfg->autoStartDelayMS) < 0) return -1; + if (virConfGetValueString(conf, "auto_shutdown_try_save", &autoShutdownScope) < 0) + return -1; + + if (autoShutdownScope != NULL && + (cfg->autoShutdownTrySave = + virDomainDriverAutoShutdownScopeTypeFromString(autoShutdownScope)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("unknown auto_shutdown_try_save '%1$s'"), + autoShutdownScope); + return -1; + } + + if (virConfGetValueString(conf, "auto_shutdown_try_shutdown", &autoShutdownScope) < 0) + return -1; + + if (autoShutdownScope != NULL && + (cfg->autoShutdownTryShutdown = + virDomainDriverAutoShutdownScopeTypeFromString(autoShutdownScope)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("unknown auto_shutdown_try_shutdown '%1$s'"), + autoShutdownScope); + return -1; + } + if (virConfGetValueString(conf, "auto_shutdown_poweroff", &autoShutdownScope) < 0) + return -1; + + if (autoShutdownScope != NULL && + (cfg->autoShutdownPoweroff = + virDomainDriverAutoShutdownScopeTypeFromString(autoShutdownScope)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("unknown auto_shutdown_poweroff '%1$s'"), + autoShutdownScope); + return -1; + } return 0; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -XXX,XX +XXX,XX @@ struct _virQEMUDriverConfig { bool autoDumpBypassCache; bool autoStartBypassCache; int autoStartDelayMS; + int autoShutdownTrySave; + int autoShutdownTryShutdown; + int autoShutdownPoweroff; char *lockManagerName; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ qemuStateStop(void) g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(qemu_driver); virDomainDriverAutoShutdownConfig ascfg = { .uri = cfg->uri, - .trySave = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL, - .tryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE, - .poweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE + .trySave = cfg->autoShutdownTrySave, + .tryShutdown = cfg->autoShutdownTryShutdown, + .poweroff = cfg->autoShutdownPoweroff, }; - if (!qemu_driver->privileged) - virDomainDriverAutoShutdown(&ascfg); + virDomainDriverAutoShutdown(&ascfg); return 0; } diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -XXX,XX +XXX,XX @@ module Test_libvirtd_qemu = { "auto_dump_bypass_cache" = "0" } { "auto_start_bypass_cache" = "0" } { "auto_start_delay" = "0" } +{ "auto_shutdown_try_save" = "all" } +{ "auto_shutdown_try_shutdown" = "none" } +{ "auto_shutdown_poweroff" = "none" } { "hugetlbfs_mount" = "/dev/hugepages" } { "bridge_helper" = "qemu-bridge-helper" } { "set_process_name" = "1" } -- 2.47.1 Currently the session daemon will try a managed save on all VMs, leaving them running if that fails. This limits the managed save just to persistent VMs, as there will usually not be any way to restore transient VMs later. It also enables graceful shutdown and then forced poweroff, should save fail for some reason. These new defaults can be overridden in the config file if needed. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/libvirtd_qemu.aug | 2 +- src/qemu/qemu.conf.in | 14 ++++++++------ src/qemu/qemu_conf.c | 6 +++--- src/qemu/test_libvirtd_qemu.aug.in | 6 +++--- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -XXX,XX +XXX,XX @@ module Libvirtd_qemu = | int_entry "auto_start_delay" | str_entry "auto_shutdown_try_save" | str_entry "auto_shutdown_try_shutdown" - | str_entry "auto_shutdown_powerdown" + | str_entry "auto_shutdown_poweroff" let process_entry = str_entry "hugetlbfs_mount" | str_entry "bridge_helper" diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu.conf.in +++ b/src/qemu/qemu.conf.in @@ -XXX,XX +XXX,XX @@ # * "transient" - only try to save transient running VMs # * "all" - try to save all running VMs # -# Defaults to "all" for session daemons and "none" +# Defaults to "persistent" for session daemons and "none" # for system daemons # # If 'libvirt-guests.service' is enabled, then this must be # set to 'none' for system daemons to avoid dueling actions -#auto_shutdown_try_save = "all" +#auto_shutdown_try_save = "persistent" # As above, but with a graceful shutdown action instead of # managed save. If managed save is enabled, shutdown will # be tried only on failure to perform managed save. # -# Defaults to "none" +# Defaults to "all" for session daemons and "none" for +# system daemons # # If 'libvirt-guests.service' is enabled, then this must be # set to 'none' for system daemons to avoid dueling actions -#auto_shutdown_try_shutdown = "none" +#auto_shutdown_try_shutdown = "all" # As above, but with a forced poweroff instead of managed # save. If managed save or graceful shutdown are enabled, # forced poweroff will be tried only on failure of the # other options. # -# Defaults to "none" +# Defaults to "all" for session daemons and "none" for +# system daemons. # # If 'libvirt-guests.service' is enabled, then this must be # set to 'none' for system daemons to avoid dueling actions -#auto_shutdown_powerdown = "none" +#auto_shutdown_poweroff = "all" # If provided by the host and a hugetlbfs mount point is configured, # a guest may request huge page backing. When this mount point is diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -XXX,XX +XXX,XX @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged, cfg->autoShutdownTryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE; cfg->autoShutdownPoweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE; } else { - cfg->autoShutdownTrySave = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL; - cfg->autoShutdownTryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE; - cfg->autoShutdownPoweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE; + 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; } return g_steal_pointer(&cfg); diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -XXX,XX +XXX,XX @@ module Test_libvirtd_qemu = { "auto_dump_bypass_cache" = "0" } { "auto_start_bypass_cache" = "0" } { "auto_start_delay" = "0" } -{ "auto_shutdown_try_save" = "all" } -{ "auto_shutdown_try_shutdown" = "none" } -{ "auto_shutdown_poweroff" = "none" } +{ "auto_shutdown_try_save" = "persistent" } +{ "auto_shutdown_try_shutdown" = "all" } +{ "auto_shutdown_poweroff" = "all" } { "hugetlbfs_mount" = "/dev/hugepages" } { "bridge_helper" = "qemu-bridge-helper" } { "set_process_name" = "1" } -- 2.47.1 Allow users to control how many seconds libvirt waits for QEMU shutdown before force powering off a guest. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf.in | 4 ++++ src/qemu/qemu_conf.c | 4 ++++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 12 insertions(+) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -XXX,XX +XXX,XX @@ module Libvirtd_qemu = | str_entry "auto_shutdown_try_save" | str_entry "auto_shutdown_try_shutdown" | str_entry "auto_shutdown_poweroff" + | int_entry "auto_shutdown_wait" let process_entry = str_entry "hugetlbfs_mount" | str_entry "bridge_helper" diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu.conf.in +++ b/src/qemu/qemu.conf.in @@ -XXX,XX +XXX,XX @@ # set to 'none' for system daemons to avoid dueling actions #auto_shutdown_poweroff = "all" +# How may seconds to wait for running VMs to gracefully shutdown +# when 'auto_shutdown_try_shutdown' is enabled +#auto_shutdown_wait = 30 + # If provided by the host and a hugetlbfs mount point is configured, # a guest may request huge page backing. When this mount point is # unspecified here, determination of a host mount point in /proc/mounts diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -XXX,XX +XXX,XX @@ virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg, return -1; } + if (virConfGetValueInt(conf, "auto_shutdown_wait", + &cfg->autoShutdownWait) < 0) + return -1; + return 0; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -XXX,XX +XXX,XX @@ struct _virQEMUDriverConfig { int autoShutdownTrySave; int autoShutdownTryShutdown; int autoShutdownPoweroff; + int autoShutdownWait; char *lockManagerName; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ qemuStateStop(void) .trySave = cfg->autoShutdownTrySave, .tryShutdown = cfg->autoShutdownTryShutdown, .poweroff = cfg->autoShutdownPoweroff, + .waitShutdownSecs = cfg->autoShutdownWait, }; virDomainDriverAutoShutdown(&ascfg); diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -XXX,XX +XXX,XX @@ module Test_libvirtd_qemu = { "auto_shutdown_try_save" = "persistent" } { "auto_shutdown_try_shutdown" = "all" } { "auto_shutdown_poweroff" = "all" } +{ "auto_shutdown_wait" = "30" } { "hugetlbfs_mount" = "/dev/hugepages" } { "bridge_helper" = "qemu-bridge-helper" } { "set_process_name" = "1" } -- 2.47.1 Bypassing cache can make save performance more predictable and avoids trashing the OS cache with data that will not be read again. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/hypervisor/domain_driver.c | 7 +++++-- src/hypervisor/domain_driver.h | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) g_autofree bool *transient = NULL; VIR_DEBUG("Run autoshutdown uri=%s trySave=%d tryShutdown=%d poweroff=%d" - "waitShutdownSecs=%d", + "waitShutdownSecs=%d saveBypassCache=%d", cfg->uri, cfg->trySave, cfg->tryShutdown, cfg->poweroff, - cfg->waitShutdownSecs); + cfg->waitShutdownSecs, cfg->saveBypassCache); /* * Ideally guests will shutdown in a few seconds, but it would @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) if (state == VIR_DOMAIN_PAUSED) flags[i] = VIR_DOMAIN_SAVE_PAUSED; } + if (cfg->saveBypassCache) + flags[i] |= VIR_DOMAIN_SAVE_BYPASS_CACHE; + virDomainSuspend(domains[i]); } diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -XXX,XX +XXX,XX @@ typedef struct _virDomainDriverAutoShutdownConfig { virDomainDriverAutoShutdownScope tryShutdown; virDomainDriverAutoShutdownScope poweroff; int waitShutdownSecs; + bool saveBypassCache; } virDomainDriverAutoShutdownConfig; void virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg); -- 2.47.1 When doing managed save of VMs, triggered by OS shutdown, it may be desirable to control cache usage. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf.in | 8 ++++++++ src/qemu/qemu_conf.c | 3 +++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 15 insertions(+) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -XXX,XX +XXX,XX @@ module Libvirtd_qemu = | str_entry "auto_shutdown_try_shutdown" | str_entry "auto_shutdown_poweroff" | int_entry "auto_shutdown_wait" + | bool_entry "auto_save_bypass_cache" let process_entry = str_entry "hugetlbfs_mount" | str_entry "bridge_helper" diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu.conf.in +++ b/src/qemu/qemu.conf.in @@ -XXX,XX +XXX,XX @@ # when 'auto_shutdown_try_shutdown' is enabled #auto_shutdown_wait = 30 +# When a domain is configured to be auto-saved on shutdown, enabling +# this flag has the same effect as using the VIR_DOMAIN_SAVE_BYPASS_CACHE +# flag with the virDomainManagedSave API. That is, the system will +# avoid using the file system cache when writing any managed state +# file, but may cause slower operation. +# +#auto_save_bypass_cache = 0 + # If provided by the host and a hugetlbfs mount point is configured, # a guest may request huge page backing. When this mount point is # unspecified here, determination of a host mount point in /proc/mounts diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -XXX,XX +XXX,XX @@ virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg, if (virConfGetValueInt(conf, "auto_shutdown_wait", &cfg->autoShutdownWait) < 0) return -1; + if (virConfGetValueBool(conf, "auto_save_bypass_cache", + &cfg->autoSaveBypassCache) < 0) + return -1; return 0; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -XXX,XX +XXX,XX @@ struct _virQEMUDriverConfig { int autoShutdownTryShutdown; int autoShutdownPoweroff; int autoShutdownWait; + bool autoSaveBypassCache; char *lockManagerName; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ qemuStateStop(void) .tryShutdown = cfg->autoShutdownTryShutdown, .poweroff = cfg->autoShutdownPoweroff, .waitShutdownSecs = cfg->autoShutdownWait, + .saveBypassCache = cfg->autoSaveBypassCache, }; virDomainDriverAutoShutdown(&ascfg); diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -XXX,XX +XXX,XX @@ module Test_libvirtd_qemu = { "auto_shutdown_try_shutdown" = "all" } { "auto_shutdown_poweroff" = "all" } { "auto_shutdown_wait" = "30" } +{ "auto_save_bypass_cache" = "0" } { "hugetlbfs_mount" = "/dev/hugepages" } { "bridge_helper" = "qemu-bridge-helper" } { "set_process_name" = "1" } -- 2.47.1 When a domain is marked for autostart, it will be started on every subsequent host OS boot. There may be times when it is desirable to mark a domain to be autostarted, on the next boot only. Thus we add virDomainSetAutostartOnce / virDomainGetAutostartOnce. An alternative would have been to overload the existing virDomainSetAutostart method, to accept values '1' or '2' for the autostart flag. This was not done because it is expected that language bindings will have mapped the current autostart flag to a boolean, and thus turning it into an enum would create a compatibility problem. A further alternative would have been to create a new method virDomainSetAutostartFlags, with a VIR_DOMAIN_AUTOSTART_ONCE flag defined. This was not done because it is felt desirable to clearly separate the two flags. Setting the "once" flag should not interfere with existing autostart setting, whether it is enabled or disabled currently. The 'virsh autostart' command, however, is still overloaded by just adding a --once flag, while current state is added to 'virsh dominfo'. No ability to filter by 'autostart once' status is added to the domain list APIs. The most common use of autostart once will be to automatically set it on host shutdown, and it be cleared on host startup. Thus there would rarely be scenarios in which a running app will need to filter on this new flag. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- include/libvirt/libvirt-domain.h | 4 ++ src/driver-hypervisor.h | 10 ++++ src/libvirt-domain.c | 87 ++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++ src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 30 ++++++++++- src/remote_protocol-structs | 12 +++++ src/rpc/gendispatch.pl | 4 +- tools/virsh-domain-monitor.c | 5 ++ tools/virsh-domain.c | 39 ++++++++++---- 10 files changed, 187 insertions(+), 12 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index XXXXXXX..XXXXXXX 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -XXX,XX +XXX,XX @@ int virDomainGetAutostart (virDomainPtr domain, int *autostart); int virDomainSetAutostart (virDomainPtr domain, int autostart); +int virDomainGetAutostartOnce(virDomainPtr domain, + int *autostart); +int virDomainSetAutostartOnce(virDomainPtr domain, + int autostart); /** * virVcpuState: diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index XXXXXXX..XXXXXXX 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -XXX,XX +XXX,XX @@ typedef int (*virDrvDomainSetAutostart)(virDomainPtr domain, int autostart); +typedef int +(*virDrvDomainGetAutostartOnce)(virDomainPtr domain, + int *autostart); + +typedef int +(*virDrvDomainSetAutostartOnce)(virDomainPtr domain, + int autostart); + typedef char * (*virDrvDomainGetSchedulerType)(virDomainPtr domain, int *nparams); @@ -XXX,XX +XXX,XX @@ struct _virHypervisorDriver { virDrvDomainDetachDeviceAlias domainDetachDeviceAlias; virDrvDomainGetAutostart domainGetAutostart; virDrvDomainSetAutostart domainSetAutostart; + virDrvDomainGetAutostartOnce domainGetAutostartOnce; + virDrvDomainSetAutostartOnce domainSetAutostartOnce; virDrvDomainGetSchedulerType domainGetSchedulerType; virDrvDomainGetSchedulerParameters domainGetSchedulerParameters; virDrvDomainGetSchedulerParametersFlags domainGetSchedulerParametersFlags; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index XXXXXXX..XXXXXXX 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -XXX,XX +XXX,XX @@ virDomainSetAutostart(virDomainPtr domain, } +/** + * virDomainGetAutostartOnce: + * @domain: a domain object + * @autostart: the value returned + * + * Provides a boolean value indicating whether the domain + * is configured to be automatically started the next time + * the host machine boots only. + * + * Returns -1 in case of error, 0 in case of success + * + * Since: 11.0.0 + */ +int +virDomainGetAutostartOnce(virDomainPtr domain, + int *autostart) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "autostart=%p", autostart); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + virCheckNonNullArgGoto(autostart, error); + + conn = domain->conn; + + if (conn->driver->domainGetAutostartOnce) { + int ret; + ret = conn->driver->domainGetAutostartOnce(domain, autostart); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + + +/** + * virDomainSetAutostartOnce: + * @domain: a domain object + * @autostart: whether the domain should be automatically started 0 or 1 + * + * Configure the domain to be automatically started + * the next time the host machine boots only. + * + * Returns -1 in case of error, 0 in case of success + * + * Since: 11.0.0 + */ +int +virDomainSetAutostartOnce(virDomainPtr domain, + int autostart) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "autostart=%d", autostart); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + + if (conn->driver->domainSetAutostartOnce) { + int ret; + ret = conn->driver->domainSetAutostartOnce(domain, autostart); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + + /** * virDomainInjectNMI: * @domain: pointer to domain object, or NULL for Domain0 diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index XXXXXXX..XXXXXXX 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -XXX,XX +XXX,XX @@ LIBVIRT_10.2.0 { virDomainGraphicsReload; } LIBVIRT_10.1.0; +LIBVIRT_11.0.0 { + global: + virDomainGetAutostartOnce; + virDomainSetAutostartOnce; +} LIBVIRT_10.2.0; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -XXX,XX +XXX,XX @@ static virHypervisorDriver hypervisor_driver = { .domainDetachDeviceAlias = remoteDomainDetachDeviceAlias, /* 4.4.0 */ .domainGetAutostart = remoteDomainGetAutostart, /* 0.3.0 */ .domainSetAutostart = remoteDomainSetAutostart, /* 0.3.0 */ + .domainGetAutostartOnce = remoteDomainGetAutostartOnce, /* 11.0.0 */ + .domainSetAutostartOnce = remoteDomainSetAutostartOnce, /* 11.0.0 */ .domainGetSchedulerType = remoteDomainGetSchedulerType, /* 0.3.0 */ .domainGetSchedulerParameters = remoteDomainGetSchedulerParameters, /* 0.3.0 */ .domainGetSchedulerParametersFlags = remoteDomainGetSchedulerParametersFlags, /* 0.9.2 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index XXXXXXX..XXXXXXX 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -XXX,XX +XXX,XX @@ struct remote_domain_fd_associate_args { remote_nonnull_string name; unsigned int flags; }; + +struct remote_domain_get_autostart_once_args { + remote_nonnull_domain dom; +}; + +struct remote_domain_get_autostart_once_ret { + int autostart; +}; + +struct remote_domain_set_autostart_once_args { + remote_nonnull_domain dom; + int autostart; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -XXX,XX +XXX,XX @@ enum remote_procedure { * @generate: both * @acl: domain:write */ - REMOTE_PROC_DOMAIN_GRAPHICS_RELOAD = 448 + REMOTE_PROC_DOMAIN_GRAPHICS_RELOAD = 448, + + /** + * @generate: both + * @priority: high + * @acl: domain:read + */ + REMOTE_PROC_DOMAIN_GET_AUTOSTART_ONCE = 449, + + /** + * @generate: both + * @priority: high + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_SET_AUTOSTART_ONCE = 450 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index XXXXXXX..XXXXXXX 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -XXX,XX +XXX,XX @@ struct remote_domain_fd_associate_args { remote_nonnull_string name; u_int flags; }; +struct remote_domain_get_autostart_once_args { + remote_nonnull_domain dom; +}; +struct remote_domain_get_autostart_once_ret { + int autostart; +}; +struct remote_domain_set_autostart_once_args { + remote_nonnull_domain dom; + int autostart; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -XXX,XX +XXX,XX @@ enum remote_procedure { REMOTE_PROC_NETWORK_EVENT_CALLBACK_METADATA_CHANGE = 446, REMOTE_PROC_NODE_DEVICE_UPDATE = 447, REMOTE_PROC_DOMAIN_GRAPHICS_RELOAD = 448, + REMOTE_PROC_DOMAIN_GET_AUTOSTART_ONCE = 449, + REMOTE_PROC_DOMAIN_SET_AUTOSTART_ONCE = 450, }; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index XXXXXXX..XXXXXXX 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -XXX,XX +XXX,XX @@ elsif ($mode eq "server") { push(@ret_list, "ret->$1 = $1;"); $single_ret_var = $1; - if ($call->{ProcName} =~ m/GetAutostart$/) { + if ($call->{ProcName} =~ m/GetAutostart(Once)?$/) { $single_ret_by_ref = 1; } else { $single_ret_by_ref = 0; @@ -XXX,XX +XXX,XX @@ elsif ($mode eq "client") { } elsif ($ret_member =~ m/^int (\S+);/) { my $arg_name = $1; - if ($call->{ProcName} =~ m/GetAutostart$/) { + if ($call->{ProcName} =~ m/GetAutostart(Once)?$/) { push(@args_list, "int *$arg_name"); push(@ret_list, "if ($arg_name) *$arg_name = ret.$arg_name;"); push(@ret_list, "rv = 0;"); diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index XXXXXXX..XXXXXXX 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -XXX,XX +XXX,XX @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "%-15s %s\n", _("Autostart:"), autostart ? _("enable") : _("disable")); } + /* Check and display whether the domain autostarts next boot or not */ + if (!virDomainGetAutostartOnce(dom, &autostart)) { + vshPrint(ctl, "%-15s %s\n", _("Autostart Once:"), + autostart ? _("enable") : _("disable")); + } has_managed_save = virDomainHasManagedSaveImage(dom, 0); if (has_managed_save < 0) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index XXXXXXX..XXXXXXX 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -XXX,XX +XXX,XX @@ static const vshCmdOptDef opts_autostart[] = { .type = VSH_OT_BOOL, .help = N_("disable autostarting") }, + {.name = "once", + .type = VSH_OT_BOOL, + .help = N_("control next boot state") + }, {.name = NULL} }; @@ -XXX,XX +XXX,XX @@ cmdAutostart(vshControl *ctl, const vshCmd *cmd) g_autoptr(virshDomain) dom = NULL; const char *name; int autostart; + int once; if (!(dom = virshCommandOptDomain(ctl, cmd, &name))) return false; autostart = !vshCommandOptBool(cmd, "disable"); + once = vshCommandOptBool(cmd, "once"); + + if (once) { + if (virDomainSetAutostartOnce(dom, autostart) < 0) { + if (autostart) + vshError(ctl, _("Failed to mark domain '%1$s' as autostarted on next boot"), name); + else + vshError(ctl, _("Failed to unmark domain '%1$s' as autostarted on next boot"), name); + return false; + } - if (virDomainSetAutostart(dom, autostart) < 0) { if (autostart) - vshError(ctl, _("Failed to mark domain '%1$s' as autostarted"), name); + vshPrintExtra(ctl, _("Domain '%1$s' marked as autostarted on next boot\n"), name); else - vshError(ctl, _("Failed to unmark domain '%1$s' as autostarted"), name); - return false; - } + vshPrintExtra(ctl, _("Domain '%1$s' unmarked as autostarted on next boot\n"), name); + } else { + if (virDomainSetAutostart(dom, autostart) < 0) { + if (autostart) + vshError(ctl, _("Failed to mark domain '%1$s' as autostarted"), name); + else + vshError(ctl, _("Failed to unmark domain '%1$s' as autostarted"), name); + return false; + } - if (autostart) - vshPrintExtra(ctl, _("Domain '%1$s' marked as autostarted\n"), name); - else - vshPrintExtra(ctl, _("Domain '%1$s' unmarked as autostarted\n"), name); + if (autostart) + vshPrintExtra(ctl, _("Domain '%1$s' marked as autostarted\n"), name); + else + vshPrintExtra(ctl, _("Domain '%1$s' unmarked as autostarted\n"), name); + } return true; } -- 2.47.1 This is maintained in the same way as the autostart flag, using a symlink. The difference is that instead of '.xml', the symlink suffix is '.xml.once'. The link is also deleted immediately after it has been read. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/domain_conf.c | 6 +++++- src/conf/domain_conf.h | 1 + src/conf/virdomainobjlist.c | 7 ++++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index XXXXXXX..XXXXXXX 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -XXX,XX +XXX,XX @@ virDomainDeleteConfig(const char *configDir, { g_autofree char *configFile = NULL; g_autofree char *autostartLink = NULL; + g_autofree char *autostartOnceLink = NULL; configFile = virDomainConfigFile(configDir, dom->def->name); autostartLink = virDomainConfigFile(autostartDir, dom->def->name); + autostartOnceLink = g_strdup_printf("%s.once", autostartLink); - /* Not fatal if this doesn't work */ + /* Not fatal if these don't work */ unlink(autostartLink); + unlink(autostartOnceLink); dom->autostart = 0; + dom->autostartOnce = 0; if (unlink(configFile) < 0 && errno != ENOENT) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index XXXXXXX..XXXXXXX 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -XXX,XX +XXX,XX @@ struct _virDomainObj { virDomainStateReason state; unsigned int autostart : 1; + unsigned int autostartOnce : 1; unsigned int persistent : 1; unsigned int updated : 1; unsigned int removing : 1; diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index XXXXXXX..XXXXXXX 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -XXX,XX +XXX,XX @@ virDomainObjListLoadConfig(virDomainObjList *doms, { g_autofree char *configFile = NULL; g_autofree char *autostartLink = NULL; + g_autofree char *autostartOnceLink = NULL; g_autoptr(virDomainDef) def = NULL; virDomainObj *dom; - int autostart; + int autostart, autostartOnce; g_autoptr(virDomainDef) oldDef = NULL; configFile = virDomainConfigFile(configDir, name); @@ -XXX,XX +XXX,XX @@ virDomainObjListLoadConfig(virDomainObjList *doms, return NULL; autostartLink = virDomainConfigFile(autostartDir, name); + autostartOnceLink = g_strdup_printf("%s.once", autostartLink); autostart = virFileLinkPointsTo(autostartLink, configFile); + autostartOnce = virFileLinkPointsTo(autostartOnceLink, configFile); + unlink(autostartOnceLink); if (!(dom = virDomainObjListAddLocked(doms, &def, xmlopt, 0, &oldDef))) return NULL; dom->autostart = autostart; + dom->autostartOnce = autostartOnce; if (notify) (*notify)(dom, oldDef == NULL, opaque); -- 2.47.1 When performing auto-shutdown of running domains, there is now the option to mark them as "autostart once", so that their state is restored on next boot. This applies on top of the traditional autostart flag. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/hypervisor/domain_driver.c | 27 ++++++++++++++++++++++----- src/hypervisor/domain_driver.h | 1 + 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoStartOne(virDomainObj *vm, virObjectLock(vm); virObjectRef(vm); - VIR_DEBUG("Autostart %s: autostart=%d", - vm->def->name, vm->autostart); + VIR_DEBUG("Autostart %s: autostart=%d autostartOnce=%d", + vm->def->name, vm->autostart, vm->autostartOnce); - if (vm->autostart && !virDomainObjIsActive(vm)) { + if ((vm->autostart || vm->autostartOnce) + && !virDomainObjIsActive(vm)) { virResetLastError(); if (state->cfg->delayMS) { if (!state->first) { @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoStartOne(virDomainObj *vm, } state->cfg->callback(vm, state->cfg->opaque); + vm->autostartOnce = 0; } virDomainObjEndAPI(&vm); @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) g_autofree bool *transient = NULL; VIR_DEBUG("Run autoshutdown uri=%s trySave=%d tryShutdown=%d poweroff=%d" - "waitShutdownSecs=%d saveBypassCache=%d", + "waitShutdownSecs=%d saveBypassCache=%d autoRestore=%d", cfg->uri, cfg->trySave, cfg->tryShutdown, cfg->poweroff, - cfg->waitShutdownSecs, cfg->saveBypassCache); + cfg->waitShutdownSecs, cfg->saveBypassCache, cfg->autoRestore); /* * Ideally guests will shutdown in a few seconds, but it would @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) for (i = 0; i < numDomains; i++) { if (virDomainIsPersistent(domains[i]) == 0) transient[i] = true; + + 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 (cfg->trySave != VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE) { diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -XXX,XX +XXX,XX @@ typedef struct _virDomainDriverAutoShutdownConfig { virDomainDriverAutoShutdownScope poweroff; int waitShutdownSecs; bool saveBypassCache; + bool autoRestore; } virDomainDriverAutoShutdownConfig; void virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg); -- 2.47.1 Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu_driver.c | 95 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ static int qemuDomainSetAutostart(virDomainPtr dom, } +static int qemuDomainGetAutostartOnce(virDomainPtr dom, + int *autostart) +{ + virDomainObj *vm; + int ret = -1; + + if (!(vm = qemuDomainObjFromDomain(dom))) + goto cleanup; + + if (virDomainGetAutostartOnceEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + *autostart = vm->autostartOnce; + ret = 0; + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + +static int qemuDomainSetAutostartOnce(virDomainPtr dom, + int autostart) +{ + virQEMUDriver *driver = dom->conn->privateData; + virDomainObj *vm; + g_autofree char *configFile = NULL; + g_autofree char *autostartLink = NULL; + g_autofree char *autostartOnceLink = NULL; + int ret = -1; + g_autoptr(virQEMUDriverConfig) cfg = NULL; + + if (!(vm = qemuDomainObjFromDomain(dom))) + return -1; + + cfg = virQEMUDriverGetConfig(driver); + + if (virDomainSetAutostartOnceEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (!vm->persistent) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cannot set autostart for transient domain")); + goto cleanup; + } + + autostart = (autostart != 0); + + if (vm->autostartOnce != autostart) { + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) + goto cleanup; + + configFile = virDomainConfigFile(cfg->configDir, vm->def->name); + autostartLink = virDomainConfigFile(cfg->autostartDir, vm->def->name); + autostartOnceLink = g_strdup_printf("%s.once", autostartLink); + + if (autostart) { + if (g_mkdir_with_parents(cfg->autostartDir, 0777) < 0) { + virReportSystemError(errno, + _("cannot create autostart directory %1$s"), + cfg->autostartDir); + goto endjob; + } + + if (symlink(configFile, autostartOnceLink) < 0) { + virReportSystemError(errno, + _("Failed to create symlink '%1$s' to '%2$s'"), + autostartOnceLink, configFile); + goto endjob; + } + } else { + if (unlink(autostartOnceLink) < 0 && + errno != ENOENT && + errno != ENOTDIR) { + virReportSystemError(errno, + _("Failed to delete symlink '%1$s'"), + autostartOnceLink); + goto endjob; + } + } + + vm->autostartOnce = autostart; + + endjob: + virDomainObjEndJob(vm); + } + ret = 0; + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + + static char *qemuDomainGetSchedulerType(virDomainPtr dom, int *nparams) { @@ -XXX,XX +XXX,XX @@ static virHypervisorDriver qemuHypervisorDriver = { .domainSetLaunchSecurityState = qemuDomainSetLaunchSecurityState, /* 8.0.0 */ .domainFDAssociate = qemuDomainFDAssociate, /* 9.0.0 */ .domainGraphicsReload = qemuDomainGraphicsReload, /* 10.2.0 */ + .domainGetAutostartOnce = qemuDomainGetAutostartOnce, /* 11.0.0 */ + .domainSetAutostartOnce = qemuDomainSetAutostartOnce, /* 11.0.0 */ }; -- 2.47.1 If shutting down running VMs at host shutdown, it can be useful to automatically start them again on next boot. This adds a config parameter 'auto_shutdown_restore', which defaults to enabled, which leverages the autostart once feature. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf.in | 4 ++++ src/qemu/qemu_conf.c | 3 +++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 11 insertions(+) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -XXX,XX +XXX,XX @@ module Libvirtd_qemu = | str_entry "auto_shutdown_try_shutdown" | str_entry "auto_shutdown_poweroff" | int_entry "auto_shutdown_wait" + | bool_entry "auto_shutdown_restore" | bool_entry "auto_save_bypass_cache" let process_entry = str_entry "hugetlbfs_mount" diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu.conf.in +++ b/src/qemu/qemu.conf.in @@ -XXX,XX +XXX,XX @@ # when 'auto_shutdown_try_shutdown' is enabled #auto_shutdown_wait = 30 +# Whether VMs that are automatically shutdown are set to +# automatically restore on next boot +#auto_shutdown_restore = 1 + # When a domain is configured to be auto-saved on shutdown, enabling # this flag has the same effect as using the VIR_DOMAIN_SAVE_BYPASS_CACHE # flag with the virDomainManagedSave API. That is, the system will diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -XXX,XX +XXX,XX @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged, cfg->autoShutdownTryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL; cfg->autoShutdownPoweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL; } + cfg->autoShutdownRestore = true; return g_steal_pointer(&cfg); } @@ -XXX,XX +XXX,XX @@ virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg, if (virConfGetValueInt(conf, "auto_shutdown_wait", &cfg->autoShutdownWait) < 0) return -1; + if (virConfGetValueBool(conf, "auto_shutdown_restore", &cfg->autoShutdownRestore) < 0) + return -1; if (virConfGetValueBool(conf, "auto_save_bypass_cache", &cfg->autoSaveBypassCache) < 0) return -1; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -XXX,XX +XXX,XX @@ struct _virQEMUDriverConfig { int autoShutdownTryShutdown; int autoShutdownPoweroff; int autoShutdownWait; + bool autoShutdownRestore; bool autoSaveBypassCache; char *lockManagerName; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ qemuStateStop(void) .poweroff = cfg->autoShutdownPoweroff, .waitShutdownSecs = cfg->autoShutdownWait, .saveBypassCache = cfg->autoSaveBypassCache, + .autoRestore = cfg->autoShutdownRestore, }; virDomainDriverAutoShutdown(&ascfg); diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -XXX,XX +XXX,XX @@ module Test_libvirtd_qemu = { "auto_shutdown_try_shutdown" = "all" } { "auto_shutdown_poweroff" = "all" } { "auto_shutdown_wait" = "30" } +{ "auto_shutdown_restore" = "1" } { "auto_save_bypass_cache" = "0" } { "hugetlbfs_mount" = "/dev/hugepages" } { "bridge_helper" = "qemu-bridge-helper" } -- 2.47.1 Currently the remote daemon code is responsible for calling virStateStop in a background thread. The virNetDaemon code wants to synchronize with this during shutdown, however, so the virThreadPtr must be passed over. Even the limited synchronization done currently, however, is flawed and to fix this requires the virNetDaemon code to be responsible for calling virStateStop in a thread more directly. Thus the logic is moved over into virStateStop via a further callback to be registered by the remote daemon. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/libvirt_remote.syms | 2 +- src/remote/remote_daemon.c | 40 ++------------------------ src/rpc/virnetdaemon.c | 58 ++++++++++++++++++++++++++++---------- src/rpc/virnetdaemon.h | 20 +++++++++++-- 4 files changed, 64 insertions(+), 56 deletions(-) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index XXXXXXX..XXXXXXX 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -XXX,XX +XXX,XX @@ virNetDaemonIsPrivileged; virNetDaemonNew; virNetDaemonNewPostExecRestart; virNetDaemonPreExecRestart; +virNetDaemonPreserve; virNetDaemonQuit; virNetDaemonQuitExecRestart; virNetDaemonRemoveShutdownInhibition; virNetDaemonRun; virNetDaemonSetShutdownCallbacks; -virNetDaemonSetStateStopWorkerThread; virNetDaemonUpdateServices; diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -XXX,XX +XXX,XX @@ static void daemonInhibitCallback(bool inhibit, void *opaque) static GDBusConnection *sessionBus; static GDBusConnection *systemBus; -static void daemonStopWorker(void *opaque) -{ - virNetDaemon *dmn = opaque; - - VIR_DEBUG("Begin stop dmn=%p", dmn); - - ignore_value(virStateStop()); - - VIR_DEBUG("Completed stop dmn=%p", dmn); - - /* Exit daemon cleanly */ - virNetDaemonQuit(dmn); -} - - -/* We do this in a thread to not block the main loop */ -static void daemonStop(virNetDaemon *dmn) -{ - virThread *thr; - virObjectRef(dmn); - - thr = g_new0(virThread, 1); - - if (virThreadCreateFull(thr, true, - daemonStopWorker, - "daemon-stop", false, dmn) < 0) { - virObjectUnref(dmn); - g_free(thr); - return; - } - - virNetDaemonSetStateStopWorkerThread(dmn, &thr); -} - - static GDBusMessage * handleSessionMessageFunc(GDBusConnection *connection G_GNUC_UNUSED, GDBusMessage *message, @@ -XXX,XX +XXX,XX @@ handleSessionMessageFunc(GDBusConnection *connection G_GNUC_UNUSED, if (virGDBusMessageIsSignal(message, "org.freedesktop.DBus.Local", "Disconnected")) - daemonStop(dmn); + virNetDaemonPreserve(dmn); return message; } @@ -XXX,XX +XXX,XX @@ handleSystemMessageFunc(GDBusConnection *connection G_GNUC_UNUSED, VIR_DEBUG("dmn=%p", dmn); - daemonStop(dmn); + virNetDaemonPreserve(dmn); } @@ -XXX,XX +XXX,XX @@ static void daemonRunStateInit(void *opaque) g_atomic_int_set(&driversInitialized, 1); virNetDaemonSetShutdownCallbacks(dmn, + virStateStop, virStateShutdownPrepare, virStateShutdownWait); diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -XXX,XX +XXX,XX @@ struct _virNetDaemon { GHashTable *servers; virJSONValue *srvObject; + virNetDaemonShutdownCallback shutdownPreserveCb; virNetDaemonShutdownCallback shutdownPrepareCb; virNetDaemonShutdownCallback shutdownWaitCb; - virThread *stateStopThread; + virThread *shutdownPreserveThread; int finishTimer; bool quit; bool finished; @@ -XXX,XX +XXX,XX @@ virNetDaemonDispose(void *obj) virEventRemoveHandle(dmn->sigwatch); #endif /* !WIN32 */ - g_free(dmn->stateStopThread); + g_free(dmn->shutdownPreserveThread); g_clear_pointer(&dmn->servers, g_hash_table_unref); @@ -XXX,XX +XXX,XX @@ daemonShutdownWait(void *opaque) virHashForEach(dmn->servers, daemonServerShutdownWait, NULL); if (!dmn->shutdownWaitCb || dmn->shutdownWaitCb() >= 0) { - if (dmn->stateStopThread) - virThreadJoin(dmn->stateStopThread); + if (dmn->shutdownPreserveThread) + virThreadJoin(dmn->shutdownPreserveThread); graceful = true; } @@ -XXX,XX +XXX,XX @@ virNetDaemonRun(virNetDaemon *dmn) } -void -virNetDaemonSetStateStopWorkerThread(virNetDaemon *dmn, - virThread **thr) -{ - VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); - - VIR_DEBUG("Setting state stop worker thread on dmn=%p to thr=%p", dmn, thr); - dmn->stateStopThread = g_steal_pointer(thr); -} - - void virNetDaemonQuit(virNetDaemon *dmn) { @@ -XXX,XX +XXX,XX @@ virNetDaemonQuitExecRestart(virNetDaemon *dmn) } +static void virNetDaemonPreserveWorker(void *opaque) +{ + virNetDaemon *dmn = opaque; + + VIR_DEBUG("Begin stop dmn=%p", dmn); + + dmn->shutdownPreserveCb(); + + VIR_DEBUG("Completed stop dmn=%p", dmn); + + virNetDaemonQuit(dmn); + virObjectUnref(dmn); +} + + +void virNetDaemonPreserve(virNetDaemon *dmn) +{ + VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); + + if (!dmn->shutdownPreserveCb || + dmn->shutdownPreserveThread) + return; + + virObjectRef(dmn); + dmn->shutdownPreserveThread = g_new0(virThread, 1); + + if (virThreadCreateFull(dmn->shutdownPreserveThread, true, + virNetDaemonPreserveWorker, + "daemon-stop", false, dmn) < 0) { + virObjectUnref(dmn); + g_clear_pointer(&dmn->shutdownPreserveThread, g_free); + return; + } +} + + static int daemonServerClose(void *payload, const char *key G_GNUC_UNUSED, @@ -XXX,XX +XXX,XX @@ virNetDaemonHasClients(virNetDaemon *dmn) void virNetDaemonSetShutdownCallbacks(virNetDaemon *dmn, + virNetDaemonShutdownCallback preserveCb, virNetDaemonShutdownCallback prepareCb, virNetDaemonShutdownCallback waitCb) { VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); + dmn->shutdownPreserveCb = preserveCb; dmn->shutdownPrepareCb = prepareCb; dmn->shutdownWaitCb = waitCb; } diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h index XXXXXXX..XXXXXXX 100644 --- a/src/rpc/virnetdaemon.h +++ b/src/rpc/virnetdaemon.h @@ -XXX,XX +XXX,XX @@ int virNetDaemonAddSignalHandler(virNetDaemon *dmn, void virNetDaemonUpdateServices(virNetDaemon *dmn, bool enabled); -void virNetDaemonSetStateStopWorkerThread(virNetDaemon *dmn, - virThread **thr); - void virNetDaemonRun(virNetDaemon *dmn); void virNetDaemonQuit(virNetDaemon *dmn); void virNetDaemonQuitExecRestart(virNetDaemon *dmn); +void virNetDaemonPreserve(virNetDaemon *dmn); bool virNetDaemonHasClients(virNetDaemon *dmn); @@ -XXX,XX +XXX,XX @@ bool virNetDaemonHasServer(virNetDaemon *dmn, typedef int (*virNetDaemonShutdownCallback)(void); +/* + * @preserveCb: preserves any active state + * @prepareCb: start shutting down daemon + * @waitCb: wait for shutdown completion + * + * The sequence of operations during shutdown is as follows + * + * - Listener stops accepting new clients + * - Existing clients are closed + * - Delay until @preserveCb is complete (if running) + * - @prepareCb invoked + * - Server worker pool is drained in background + * - @waitCb is invoked in background + * - Main loop terminates + */ void virNetDaemonSetShutdownCallbacks(virNetDaemon *dmn, + virNetDaemonShutdownCallback preserveCb, virNetDaemonShutdownCallback prepareCb, virNetDaemonShutdownCallback waitCb); -- 2.47.1 The call to preserve state (ie running VMs) is triggered in response to the desktop session dbus terminating (session daemon), or logind sending a "PrepareForShutdown" signal. In the case of the latter, daemons should only save their state, not actually exit yet. Other things on the system may still expect the daemon to be running at this stage. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/remote_daemon.c | 4 +++- src/rpc/virnetdaemon.c | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -XXX,XX +XXX,XX @@ handleSessionMessageFunc(GDBusConnection *connection G_GNUC_UNUSED, if (virGDBusMessageIsSignal(message, "org.freedesktop.DBus.Local", - "Disconnected")) + "Disconnected")) { virNetDaemonPreserve(dmn); + virNetDaemonQuit(dmn); + } return message; } diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -XXX,XX +XXX,XX @@ static void virNetDaemonPreserveWorker(void *opaque) VIR_DEBUG("Completed stop dmn=%p", dmn); - virNetDaemonQuit(dmn); virObjectUnref(dmn); } -- 2.47.1 The preserving of state (ie running VMs) requires a fully functional daemon and hypervisor driver. If any part has started shutting down then saving state may fail, or worse, hang. The current shutdown sequence does not guarantee safe ordering, as we synchronize with the state saving thread only after the hypervisor driver has had its 'shutdownPrepare' callback invoked. In the case of QEMU this means that worker threads processing monitor events may well have been stopped. This implements a full state machine that has a well defined ordering that an earlier commit documented as the desired semantics. With this change, nothing will start shutting down if the state saving thread is still running. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/remote_daemon.c | 4 +- src/rpc/virnetdaemon.c | 103 ++++++++++++++++++++++++++----------- 2 files changed, 76 insertions(+), 31 deletions(-) diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -XXX,XX +XXX,XX @@ static void daemonRunStateInit(void *opaque) NULL, G_DBUS_SIGNAL_FLAGS_NONE, handleSystemMessageFunc, - dmn, - NULL); + dmn, + NULL); /* Only now accept clients from network */ virNetDaemonUpdateServices(dmn, true); diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -XXX,XX +XXX,XX @@ VIR_LOG_INIT("rpc.netdaemon"); +typedef enum { + VIR_NET_DAEMON_QUIT_NONE, + VIR_NET_DAEMON_QUIT_REQUESTED, + VIR_NET_DAEMON_QUIT_PRESERVING, + VIR_NET_DAEMON_QUIT_READY, + VIR_NET_DAEMON_QUIT_WAITING, + VIR_NET_DAEMON_QUIT_COMPLETED, +} virNetDaemonQuitPhase; + #ifndef WIN32 typedef struct _virNetDaemonSignal virNetDaemonSignal; struct _virNetDaemonSignal { @@ -XXX,XX +XXX,XX @@ struct _virNetDaemon { virNetDaemonShutdownCallback shutdownPrepareCb; virNetDaemonShutdownCallback shutdownWaitCb; virThread *shutdownPreserveThread; - int finishTimer; - bool quit; - bool finished; + int quitTimer; + virNetDaemonQuitPhase quit; bool graceful; bool execRestart; bool running; /* the daemon has reached the running phase */ @@ -XXX,XX +XXX,XX @@ virNetDaemonAutoShutdownTimer(int timerid G_GNUC_UNUSED, if (!dmn->autoShutdownInhibitions) { VIR_DEBUG("Automatic shutdown triggered"); - dmn->quit = true; + if (dmn->quit == VIR_NET_DAEMON_QUIT_NONE) { + VIR_DEBUG("Requesting daemon shutdown"); + dmn->quit = VIR_NET_DAEMON_QUIT_REQUESTED; + } } } @@ -XXX,XX +XXX,XX @@ daemonShutdownWait(void *opaque) bool graceful = false; virHashForEach(dmn->servers, daemonServerShutdownWait, NULL); - if (!dmn->shutdownWaitCb || dmn->shutdownWaitCb() >= 0) { - if (dmn->shutdownPreserveThread) - virThreadJoin(dmn->shutdownPreserveThread); - + if (!dmn->shutdownWaitCb || dmn->shutdownWaitCb() >= 0) graceful = true; - } VIR_WITH_OBJECT_LOCK_GUARD(dmn) { dmn->graceful = graceful; - virEventUpdateTimeout(dmn->finishTimer, 0); + dmn->quit = VIR_NET_DAEMON_QUIT_COMPLETED; + virEventUpdateTimeout(dmn->quitTimer, 0); + VIR_DEBUG("Shutdown wait completed graceful=%d", graceful); } } static void -virNetDaemonFinishTimer(int timerid G_GNUC_UNUSED, - void *opaque) +virNetDaemonQuitTimer(int timerid G_GNUC_UNUSED, + void *opaque) { virNetDaemon *dmn = opaque; VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); - dmn->finished = true; + dmn->quit = VIR_NET_DAEMON_QUIT_COMPLETED; + VIR_DEBUG("Shutdown wait timed out"); } @@ -XXX,XX +XXX,XX @@ virNetDaemonRun(virNetDaemon *dmn) goto cleanup; } - dmn->quit = false; - dmn->finishTimer = -1; - dmn->finished = false; + dmn->quit = VIR_NET_DAEMON_QUIT_NONE; + dmn->quitTimer = -1; dmn->graceful = false; dmn->running = true; @@ -XXX,XX +XXX,XX @@ virNetDaemonRun(virNetDaemon *dmn) virSystemdNotifyReady(); VIR_DEBUG("dmn=%p quit=%d", dmn, dmn->quit); - while (!dmn->finished) { + while (dmn->quit != VIR_NET_DAEMON_QUIT_COMPLETED) { virNetDaemonShutdownTimerUpdate(dmn); virObjectUnlock(dmn); @@ -XXX,XX +XXX,XX @@ virNetDaemonRun(virNetDaemon *dmn) virHashForEach(dmn->servers, daemonServerProcessClients, NULL); /* don't shutdown services when performing an exec-restart */ - if (dmn->quit && dmn->execRestart) + if (dmn->quit == VIR_NET_DAEMON_QUIT_REQUESTED && dmn->execRestart) goto cleanup; - if (dmn->quit && dmn->finishTimer == -1) { + if (dmn->quit == VIR_NET_DAEMON_QUIT_REQUESTED) { + VIR_DEBUG("Process quit request"); virHashForEach(dmn->servers, daemonServerClose, NULL); + + if (dmn->shutdownPreserveThread) { + VIR_DEBUG("Shutdown preserve thread running"); + dmn->quit = VIR_NET_DAEMON_QUIT_PRESERVING; + } else { + VIR_DEBUG("Ready to shutdown"); + dmn->quit = VIR_NET_DAEMON_QUIT_READY; + } + } + + if (dmn->quit == VIR_NET_DAEMON_QUIT_READY) { + VIR_DEBUG("Starting shutdown, running prepare"); if (dmn->shutdownPrepareCb && dmn->shutdownPrepareCb() < 0) break; - if ((dmn->finishTimer = virEventAddTimeout(30 * 1000, - virNetDaemonFinishTimer, - dmn, NULL)) < 0) { + if ((dmn->quitTimer = virEventAddTimeout(30 * 1000, + virNetDaemonQuitTimer, + dmn, NULL)) < 0) { VIR_WARN("Failed to register finish timer."); break; } @@ -XXX,XX +XXX,XX @@ virNetDaemonRun(virNetDaemon *dmn) VIR_WARN("Failed to register join thread."); break; } + + VIR_DEBUG("Waiting for shutdown completion"); + dmn->quit = VIR_NET_DAEMON_QUIT_WAITING; } } @@ -XXX,XX +XXX,XX @@ virNetDaemonQuit(virNetDaemon *dmn) VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); VIR_DEBUG("Quit requested %p", dmn); - dmn->quit = true; + dmn->quit = VIR_NET_DAEMON_QUIT_REQUESTED; } @@ -XXX,XX +XXX,XX @@ virNetDaemonQuitExecRestart(virNetDaemon *dmn) VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); VIR_DEBUG("Exec-restart requested %p", dmn); - dmn->quit = true; + dmn->quit = VIR_NET_DAEMON_QUIT_REQUESTED; dmn->execRestart = true; } @@ -XXX,XX +XXX,XX @@ static void virNetDaemonPreserveWorker(void *opaque) { virNetDaemon *dmn = opaque; - VIR_DEBUG("Begin stop dmn=%p", dmn); + VIR_DEBUG("Begin preserve dmn=%p", dmn); dmn->shutdownPreserveCb(); - VIR_DEBUG("Completed stop dmn=%p", dmn); + VIR_DEBUG("Completed preserve dmn=%p", dmn); + VIR_WITH_OBJECT_LOCK_GUARD(dmn) { + if (dmn->quit == VIR_NET_DAEMON_QUIT_PRESERVING) { + VIR_DEBUG("Marking shutdown as ready"); + dmn->quit = VIR_NET_DAEMON_QUIT_READY; + } + g_clear_pointer(&dmn->shutdownPreserveThread, g_free); + } + + VIR_DEBUG("End preserve dmn=%p", dmn); virObjectUnref(dmn); } @@ -XXX,XX +XXX,XX @@ static void virNetDaemonPreserveWorker(void *opaque) void virNetDaemonPreserve(virNetDaemon *dmn) { VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); + VIR_DEBUG("Preserve state request"); - if (!dmn->shutdownPreserveCb || - dmn->shutdownPreserveThread) + if (!dmn->shutdownPreserveCb) { + VIR_DEBUG("No preserve callback registered"); return; + } + if (dmn->shutdownPreserveThread) { + VIR_DEBUG("Preserve state thread already running"); + return; + } + + if (dmn->quit != VIR_NET_DAEMON_QUIT_NONE) { + VIR_WARN("Already initiated shutdown sequence, unable to preserve state"); + return; + } virObjectRef(dmn); dmn->shutdownPreserveThread = g_new0(virThread, 1); - if (virThreadCreateFull(dmn->shutdownPreserveThread, true, + if (virThreadCreateFull(dmn->shutdownPreserveThread, false, virNetDaemonPreserveWorker, "daemon-stop", false, dmn) < 0) { virObjectUnref(dmn); -- 2.47.1 The daemons are wired up to shutdown in responsible to UNIX process signals, as well as in response to login1 dbus signals, or loss of desktop session. The latter two options can optionally preserve state (ie running VMs). In non-systemd environments, as well as for testing, it would be useful to have a way to trigger shutdown with state preservation more directly. Thus a new admin protocol API is introduced virAdmConnectDaemonShutdown which will trigger a daemon shutdown, and preserve running VMs if the VIR_DAEMON_SHUTDOWN_PRESERVE flag is set. It has a corresponding 'virt-admin daemon-shutdown [--preserve]' command binding. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- include/libvirt/libvirt-admin.h | 13 +++++++++ src/admin/admin_protocol.x | 11 +++++++- src/admin/admin_server_dispatch.c | 13 +++++++++ src/admin/libvirt-admin.c | 33 +++++++++++++++++++++++ src/admin/libvirt_admin_public.syms | 5 ++++ src/rpc/virnetdaemon.c | 4 +++ tools/virt-admin.c | 41 +++++++++++++++++++++++++++++ 7 files changed, 119 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index XXXXXXX..XXXXXXX 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -XXX,XX +XXX,XX @@ int virAdmConnectSetDaemonTimeout(virAdmConnectPtr conn, unsigned int timeout, unsigned int flags); +/** + * virAdmConnectDaemonShutdownFlags: + * + * Since: 11.0.0 + */ +typedef enum { + /* Preserve state before shutting down daemon (Since: 11.0.0) */ + VIR_DAEMON_SHUTDOWN_PRESERVE = (1 << 0), +} virAdmConnectDaemonShutdownFlags; + +int virAdmConnectDaemonShutdown(virAdmConnectPtr conn, + unsigned int flags); + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index XXXXXXX..XXXXXXX 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -XXX,XX +XXX,XX @@ struct admin_connect_set_daemon_timeout_args { unsigned int flags; }; +struct admin_connect_daemon_shutdown_args { + unsigned int flags; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -XXX,XX +XXX,XX @@ enum admin_procedure { /** * @generate: both */ - ADMIN_PROC_CONNECT_SET_DAEMON_TIMEOUT = 19 + ADMIN_PROC_CONNECT_SET_DAEMON_TIMEOUT = 19, + + /** + * @generate: both + */ + ADMIN_PROC_CONNECT_DAEMON_SHUTDOWN = 20 }; diff --git a/src/admin/admin_server_dispatch.c b/src/admin/admin_server_dispatch.c index XXXXXXX..XXXXXXX 100644 --- a/src/admin/admin_server_dispatch.c +++ b/src/admin/admin_server_dispatch.c @@ -XXX,XX +XXX,XX @@ adminConnectSetDaemonTimeout(virNetDaemon *dmn, return virNetDaemonAutoShutdown(dmn, timeout); } +static int +adminConnectDaemonShutdown(virNetDaemon *dmn, + unsigned int flags) +{ + virCheckFlags(VIR_DAEMON_SHUTDOWN_PRESERVE, -1); + + if (flags & VIR_DAEMON_SHUTDOWN_PRESERVE) + virNetDaemonPreserve(dmn); + + virNetDaemonQuit(dmn); + + return 0; +} static int adminDispatchConnectGetLoggingOutputs(virNetServer *server G_GNUC_UNUSED, diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c index XXXXXXX..XXXXXXX 100644 --- a/src/admin/libvirt-admin.c +++ b/src/admin/libvirt-admin.c @@ -XXX,XX +XXX,XX @@ virAdmConnectSetDaemonTimeout(virAdmConnectPtr conn, return ret; } + + +/** + * virAdmConnectDaemonShutdown: + * @conn: pointer to an active admin connection + * @flags: optional extra falgs + * + * Trigger shutdown of the daemon, if @flags includes + * VIR_DAEMON_SHUTDOWN_PRESERVE then state will be + * preserved before shutting down + * + * Returns 0 on success, -1 on error. + * + * Since: 11.0.0 + */ +int +virAdmConnectDaemonShutdown(virAdmConnectPtr conn, + unsigned int flags) +{ + int ret; + + VIR_DEBUG("conn=%p, flags=0x%x", conn, flags); + + virResetLastError(); + virCheckAdmConnectReturn(conn, -1); + + if ((ret = remoteAdminConnectDaemonShutdown(conn, flags)) < 0) { + virDispatchError(NULL); + return -1; + } + + return ret; +} diff --git a/src/admin/libvirt_admin_public.syms b/src/admin/libvirt_admin_public.syms index XXXXXXX..XXXXXXX 100644 --- a/src/admin/libvirt_admin_public.syms +++ b/src/admin/libvirt_admin_public.syms @@ -XXX,XX +XXX,XX @@ LIBVIRT_ADMIN_8.6.0 { global: virAdmConnectSetDaemonTimeout; } LIBVIRT_ADMIN_3.0.0; + +LIBVIRT_ADMIN_11.0.0 { + global: + virAdmConnectDaemonShutdown; +} LIBVIRT_ADMIN_8.6.0; diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -XXX,XX +XXX,XX @@ virNetDaemonRun(virNetDaemon *dmn) } } + VIR_DEBUG("Main loop exited"); if (dmn->graceful) { virThreadJoin(&shutdownThread); + VIR_DEBUG("Graceful shutdown complete"); } else { VIR_WARN("Make forcefull daemon shutdown"); exit(EXIT_FAILURE); @@ -XXX,XX +XXX,XX @@ virNetDaemonSetShutdownCallbacks(virNetDaemon *dmn, { VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); + VIR_DEBUG("Shutdown callbacks preserve=%p prepare=%p wait=%p", + preserveCb, prepareCb, waitCb); dmn->shutdownPreserveCb = preserveCb; dmn->shutdownPrepareCb = prepareCb; dmn->shutdownWaitCb = waitCb; diff --git a/tools/virt-admin.c b/tools/virt-admin.c index XXXXXXX..XXXXXXX 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -XXX,XX +XXX,XX @@ cmdDaemonTimeout(vshControl *ctl, const vshCmd *cmd) } +/* -------------------------- + * Command daemon-shutdown + * -------------------------- + */ +static const vshCmdInfo info_daemon_shutdown = { + .help = N_("stop the daemon"), + .desc = N_("stop the daemon"), +}; + +static const vshCmdOptDef opts_daemon_shutdown[] = { + {.name = "preserve", + .type = VSH_OT_BOOL, + .required = false, + .positional = false, + .help = N_("preserve state before shutting down"), + }, + {.name = NULL} +}; + +static bool +cmdDaemonShutdown(vshControl *ctl, const vshCmd *cmd) +{ + vshAdmControl *priv = ctl->privData; + unsigned int flags = 0; + + if (vshCommandOptBool(cmd, "preserve")) + flags |= VIR_DAEMON_SHUTDOWN_PRESERVE; + + if (virAdmConnectDaemonShutdown(priv->conn, flags) < 0) + return false; + + return true; +} + + static void * vshAdmConnectionHandler(vshControl *ctl) { @@ -XXX,XX +XXX,XX @@ static const vshCmdDef managementCmds[] = { .info = &info_daemon_timeout, .flags = 0 }, + {.name = "daemon-shutdown", + .handler = cmdDaemonShutdown, + .opts = opts_daemon_shutdown, + .info = &info_daemon_shutdown, + .flags = 0 + }, {.name = NULL} }; -- 2.47.1 The service unit "TimeoutStopSec" setting controls how long systemd waits for a service to stop before aggressively killing it, defaulting to 30 seconds if not set. When we're processing shutdown of VMs in response to OS shutdown, we very likely need more than 30 seconds to complete this job, and can not stop the daemon during this time. To avoid being prematurely killed, setup a timer that repeatedly extends the "TimeoutStopSec" value while stop of running VMs is arranged. This does mean if libvirt hangs while stoppping VMs, systemd won't get to kill the libvirt daemon, but this is considered less harmful that forcefully killing running VMs. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnetdaemon.c | 62 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -XXX,XX +XXX,XX @@ struct _virNetDaemon { virNetDaemonShutdownCallback shutdownPrepareCb; virNetDaemonShutdownCallback shutdownWaitCb; virThread *shutdownPreserveThread; + unsigned long long preserveStart; + unsigned int preserveExtended; + int preserveTimer; int quitTimer; virNetDaemonQuitPhase quit; bool graceful; @@ -XXX,XX +XXX,XX @@ struct _virNetDaemon { static virClass *virNetDaemonClass; +/* + * The minimum additional shutdown time (secs) we should ask + * systemd to allow, while state preservation operations + * are running. A timer will run every 5 seconds, and + * ensure at least this much extra time is requested + */ +#define VIR_NET_DAEMON_PRESERVE_MIN_TIME 30 + static int daemonServerClose(void *payload, const char *key G_GNUC_UNUSED, @@ -XXX,XX +XXX,XX @@ virNetDaemonNew(void) if (virEventRegisterDefaultImpl() < 0) goto error; + dmn->preserveTimer = -1; dmn->autoShutdownTimerID = -1; #ifndef WIN32 @@ -XXX,XX +XXX,XX @@ daemonShutdownWait(void *opaque) } } +static void +virNetDaemonPreserveTimer(int timerid G_GNUC_UNUSED, + void *opaque) +{ + virNetDaemon *dmn = opaque; + VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); + unsigned long long now = g_get_monotonic_time(); + unsigned long long delta; + + if (dmn->quit != VIR_NET_DAEMON_QUIT_PRESERVING) + return; + + VIR_DEBUG("Started at %llu now %llu extended %u", + dmn->preserveStart, now, dmn->preserveExtended); + + /* Time since start of preserving state in usec */ + delta = now - dmn->preserveStart; + /* Converts to secs */ + delta /= (1000ull * 1000ull); + + /* Want extra seconds grace to ensure this timer fires + * again before system timeout expires, under high + * load conditions */ + delta += VIR_NET_DAEMON_PRESERVE_MIN_TIME; + + /* Deduct any extension we've previously asked for */ + delta -= dmn->preserveExtended; + + /* Tell systemd how much more we need to extend by */ + virSystemdNotifyExtendTimeout(delta); + dmn->preserveExtended += delta; + + VIR_DEBUG("Extended by %llu", delta); +} + + static void virNetDaemonQuitTimer(int timerid G_GNUC_UNUSED, void *opaque) @@ -XXX,XX +XXX,XX @@ virNetDaemonRun(virNetDaemon *dmn) if (dmn->quit == VIR_NET_DAEMON_QUIT_REQUESTED) { VIR_DEBUG("Process quit request"); + virSystemdNotifyStopping(); virHashForEach(dmn->servers, daemonServerClose, NULL); if (dmn->shutdownPreserveThread) { VIR_DEBUG("Shutdown preserve thread running"); dmn->quit = VIR_NET_DAEMON_QUIT_PRESERVING; + dmn->preserveStart = g_get_monotonic_time(); + dmn->preserveExtended = VIR_NET_DAEMON_PRESERVE_MIN_TIME; + virSystemdNotifyExtendTimeout(dmn->preserveExtended); + if ((dmn->preserveTimer = virEventAddTimeout(5 * 1000, + virNetDaemonPreserveTimer, + dmn, NULL)) < 0) { + VIR_WARN("Failed to register preservation timer"); + /* hope for the best */ + } } else { VIR_DEBUG("Ready to shutdown"); dmn->quit = VIR_NET_DAEMON_QUIT_READY; @@ -XXX,XX +XXX,XX @@ static void virNetDaemonPreserveWorker(void *opaque) dmn->quit = VIR_NET_DAEMON_QUIT_READY; } g_clear_pointer(&dmn->shutdownPreserveThread, g_free); + if (dmn->preserveTimer != -1) { + virEventRemoveTimeout(dmn->preserveTimer); + dmn->preserveTimer = -1; + } } VIR_DEBUG("End preserve dmn=%p", dmn); -- 2.47.1 Since processing running VMs on OS shutdown can take a while, it is beneficial to send systemd status messages about the progress. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/hypervisor/domain_driver.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -XXX,XX +XXX,XX @@ #include "datatypes.h" #include "driver.h" #include "virlog.h" +#include "virsystemd.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) (!transient[i] && cfg->trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT)) continue; + virSystemdNotifyStatus("Suspending '%s' (%zu of %d)", + virDomainGetName(domains[i]), i + 1, numDomains); /* * Pause all VMs to make them stop dirtying pages, * so save is quicker. We remember if any VMs were @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) } for (i = 0; i < numDomains; i++) { + virSystemdNotifyStatus("Saving '%s' (%zu of %d)", + virDomainGetName(domains[i]), i + 1, numDomains); + if (virDomainManagedSave(domains[i], flags[i]) < 0) { VIR_WARN("Unable to perform managed save of '%s': %s", virDomainGetName(domains[i]), @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) (!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); + if (virDomainShutdown(domains[i]) < 0) { VIR_WARN("Unable to perform graceful shutdown of '%s': %s", virDomainGetName(domains[i]), @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) } timer = g_timer_new(); + virSystemdNotifyStatus("Waiting %d secs for VM shutdown completion", + cfg->waitShutdownSecs); while (1) { bool anyRunning = false; for (i = 0; i < numDomains; i++) { @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) (!transient[i] && cfg->poweroff == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT)) continue; + virSystemdNotifyStatus("Destroying '%s' (%zu of %d)", + virDomainGetName(domains[i]), i + 1, numDomains); virDomainDestroy(domains[i]); } } + virSystemdNotifyStatus("Processed %d domains", numDomains); + cleanup: if (domains) { for (i = 0; i < numDomains; i++) { -- 2.47.1 This series starts the work needed to obsolete the libvirt-guests.sh script which has grown a surprisingly large amount of functionality. Currently the virt daemons will acquire inhibitors to delay OS shutdown when VMs are running. The libvirt-guests.service unit can be used to call libvirt-guests.sh to shutdown running VMs on system shutdown. This split is a bad architecture because libvirt-guests.service will only run once the system has decided to initiate the shutdown sequence. When the user requests as shutdown while inhibitors are present, logind will emit a "PrepareForShutdown" signal over dbus. Applications are supposed to respond to this by preserving state & releasing their inhibitors, which in turns allows shutdown to be initiated. The remote daemon already has support for listening for the "PrepareForShutdown" signal, but only does this for session instances, not system instances. This series essentially takes that logic and expands it to run in the system instances too, thus conceptually making libvirt-guests.service obsolete. It is slightly more complicated than that though for many reasons... Saving running VMs can take a very long time. The inhibitor delay can be as low as 5 seconds, and when killing a service, systemd may not wait very long for it to terminate. libvirt-guests.service deals with this by setting TimeoutStopSecs=0 to make systemd wait forever. This is undesirable to set in libvirtd.service though, as we would like systemd to kill the daemon aggressively if it hangs. The series thus uses the notification protocol to request systemd give it more time to shutdown, as long as we're in the phase of saving running VMs. A bug in this code will still result in systemd waiting forever, which is no different from libvirt-guests.service, but a bug in any other part of the libvirt daemon shutdown code will result in systemd killing us. The existing logic for saving VMs in the session daemons had many feature gaps compared to libvirt-guests.sh. Thus there is code to add support * Requesting graceful OS shutdown if managed save failed * Force poweroff of VMs if no other action worked * Optionally enabling/disabling use of managed save, graceful shutdown and force poweroff, which is more flexible than ON_SHUTDOWN=nnn, as we can try the whole sequence of options * Ability to bypass cache in managed save * Support for one-time autostart of VMs as an official API To aid in testing this logic, virt-admin gains a new command 'virt-admin daemon-shutdown --preserve' All this new functionality is wired up into the QEMU driver, and is made easily accessible to other hypervisor drivers, so would easily be extendable to Xen, CH, LXC drivers, but this is not done in this series. IOW, libvirt-guests.service is not yet fully obsolete. The new functionality is also not enabled by default for the system daemon, it requires explicit admin changes to /etc/libvirt/qemu.conf to enable it. This is because it would clash with execution of the libvirt-guests.service if both were enabled. It is highly desirable that we enable this by default though, so we need to figure out a upgrade story wrt libvirt-guests.service. The only libvirt-guests.sh features not implemented are: * PARALLEL_SHUTDOWN=nn. When doing a graceful shutdown we initiate it on every single VM at once, and then monitor progress of all of them in parallel. * SYNC_TIME=nn When make not attempt to sync guest time when restoring from managed save. This ought to be fixed Daniel P. Berrangé (22): hypervisor: move support for auto-shutdown out of QEMU driver remote: always invoke virStateStop for all daemons hypervisor: expand available shutdown actions hypervisor: custom shutdown actions for transient vs persistent VMs qemu: support automatic VM managed save in system daemon qemu: improve shutdown defaults for session daemon qemu: configurable delay for shutdown before poweroff hypervisor: support bypassing cache for managed save qemu: add config parameter to control auto-save bypass cache src: add new APIs for marking a domain to autostart once conf: implement support for autostart once feature hypervisor: wire up support for auto restore of running domains qemu: wire up support for once only autostart qemu: add config to control if auto-shutdown VMs are restored src: clarify semantics of the various virStateNNN methods rpc: rename virNetDaemonSetShutdownCallbacks rpc: move state stop into virNetDaemon class rpc: don't unconditionally quit after preserving state rpc: fix shutdown sequence when preserving state admin: add 'daemon-shutdown' command rpc: don't let systemd shutdown daemon while saving VMs hypervisor: emit systemd status & log messages while saving docs/manpages/virt-admin.rst | 13 ++ include/libvirt/libvirt-admin.h | 13 ++ include/libvirt/libvirt-domain.h | 4 + src/admin/admin_protocol.x | 11 +- src/admin/admin_server_dispatch.c | 13 ++ src/admin/libvirt-admin.c | 33 ++++ src/admin/libvirt_admin_public.syms | 5 + src/conf/domain_conf.c | 7 +- src/conf/domain_conf.h | 2 + src/conf/virdomainobjlist.c | 8 + src/driver-hypervisor.h | 10 ++ src/hypervisor/domain_driver.c | 246 +++++++++++++++++++++++++++- src/hypervisor/domain_driver.h | 27 +++ src/libvirt-domain.c | 87 ++++++++++ src/libvirt.c | 44 ++++- src/libvirt_private.syms | 3 + src/libvirt_public.syms | 6 + src/libvirt_remote.syms | 4 +- src/qemu/libvirtd_qemu.aug | 6 + src/qemu/qemu.conf.in | 62 +++++++ src/qemu/qemu_conf.c | 75 +++++++++ src/qemu/qemu_conf.h | 7 + src/qemu/qemu_driver.c | 150 ++++++++++++----- src/qemu/test_libvirtd_qemu.aug.in | 6 + src/remote/remote_daemon.c | 84 +++------- src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 30 +++- src/remote_protocol-structs | 12 ++ src/rpc/gendispatch.pl | 4 +- src/rpc/virnetdaemon.c | 218 +++++++++++++++++++----- src/rpc/virnetdaemon.h | 35 +++- tools/virsh-domain-monitor.c | 7 + tools/virsh-domain.c | 39 ++++- tools/virt-admin.c | 41 +++++ 34 files changed, 1146 insertions(+), 168 deletions(-) -- 2.48.1 This is a move of the code that currently exists in the QEMU driver, into the common layer that can be used by multiple drivers. The code currently supports performing managed save of all running guests, ignoring any failures. Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/hypervisor/domain_driver.c | 48 ++++++++++++++++++++++++++++++++++ src/hypervisor/domain_driver.h | 6 +++++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 47 ++++----------------------------- 4 files changed, 60 insertions(+), 42 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoStart(virDomainObjList *domains, virDomainObjListForEach(domains, false, virDomainDriverAutoStartOne, &state); } + + +void +virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) +{ + g_autoptr(virConnect) conn = NULL; + int numDomains = 0; + size_t i; + int state; + virDomainPtr *domains = NULL; + g_autofree unsigned int *flags = NULL; + + if (!(conn = virConnectOpen(cfg->uri))) + goto cleanup; + + if ((numDomains = virConnectListAllDomains(conn, + &domains, + VIR_CONNECT_LIST_DOMAINS_ACTIVE)) < 0) + goto cleanup; + + flags = g_new0(unsigned int, numDomains); + + /* First we pause all VMs to make them stop dirtying + pages, etc. We remember if any VMs were paused so + we can restore that on resume. */ + for (i = 0; i < numDomains; i++) { + 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; + } + virDomainSuspend(domains[i]); + } + + /* Then we save the VMs to disk */ + for (i = 0; i < numDomains; i++) + if (virDomainManagedSave(domains[i], flags[i]) < 0) + VIR_WARN("Unable to perform managed save of '%s': %s", + virDomainGetName(domains[i]), + virGetLastErrorMessage()); + + cleanup: + if (domains) { + for (i = 0; i < numDomains; i++) + virObjectUnref(domains[i]); + VIR_FREE(domains); + } +} diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -XXX,XX +XXX,XX @@ typedef struct _virDomainDriverAutoStartConfig { void virDomainDriverAutoStart(virDomainObjList *domains, virDomainDriverAutoStartConfig *cfg); + +typedef struct _virDomainDriverAutoShutdownConfig { + const char *uri; +} virDomainDriverAutoShutdownConfig; + +void virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index XXXXXXX..XXXXXXX 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -XXX,XX +XXX,XX @@ virDomainCgroupSetupVcpuBW; # hypervisor/domain_driver.h virDomainDriverAddIOThreadCheck; +virDomainDriverAutoShutdown; virDomainDriverAutoStart; virDomainDriverDelIOThreadCheck; virDomainDriverGenerateMachineName; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ qemuStateReload(void) static int qemuStateStop(void) { - int ret = -1; - g_autoptr(virConnect) conn = NULL; - int numDomains = 0; - size_t i; - int state; - virDomainPtr *domains = NULL; - g_autofree unsigned int *flags = NULL; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(qemu_driver); + virDomainDriverAutoShutdownConfig ascfg = { + .uri = cfg->uri, + }; - if (!(conn = virConnectOpen(cfg->uri))) - goto cleanup; - - if ((numDomains = virConnectListAllDomains(conn, - &domains, - VIR_CONNECT_LIST_DOMAINS_ACTIVE)) < 0) - goto cleanup; - - flags = g_new0(unsigned int, numDomains); - - /* First we pause all VMs to make them stop dirtying - pages, etc. We remember if any VMs were paused so - we can restore that on resume. */ - for (i = 0; i < numDomains; i++) { - 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; - } - virDomainSuspend(domains[i]); - } - - ret = 0; - /* Then we save the VMs to disk */ - for (i = 0; i < numDomains; i++) - if (virDomainManagedSave(domains[i], flags[i]) < 0) - ret = -1; - - cleanup: - if (domains) { - for (i = 0; i < numDomains; i++) - virObjectUnref(domains[i]); - VIR_FREE(domains); - } + virDomainDriverAutoShutdown(&ascfg); - return ret; + return 0; } -- 2.48.1 Currently the virStateStop method is only wired up to run save for the unprivileged daemons, so there is no functional change. IOW, session exit, or host OS shutdown will trigger VM managed saved for QEMU session daemon, but not the system daemon. This changes the daemon code to always run virStateStop for all daemons. Instead the QEMU driver is responsible for skipping its own logic when running privileged...for now. This means that virStateStop will now be triggered by logind's PrepareForShutdown signal. Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu_driver.c | 3 ++- src/remote/remote_daemon.c | 34 ++++++++++++++++++---------------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ qemuStateStop(void) .uri = cfg->uri, }; - virDomainDriverAutoShutdown(&ascfg); + if (!qemu_driver->privileged) + virDomainDriverAutoShutdown(&ascfg); return 0; } diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -XXX,XX +XXX,XX @@ static void daemonRunStateInit(void *opaque) virStateShutdownPrepare, virStateShutdownWait); - /* Tie the non-privileged daemons to the session/shutdown lifecycle */ + /* Signal for VM shutdown when desktop session is terminated, in + * unprivileged daemons */ if (!virNetDaemonIsPrivileged(dmn)) { - if (virGDBusHasSessionBus()) { sessionBus = virGDBusGetSessionBus(); if (sessionBus != NULL) g_dbus_connection_add_filter(sessionBus, handleSessionMessageFunc, dmn, NULL); } + } - if (virGDBusHasSystemBus()) { - systemBus = virGDBusGetSystemBus(); - if (systemBus != NULL) - g_dbus_connection_signal_subscribe(systemBus, - "org.freedesktop.login1", - "org.freedesktop.login1.Manager", - "PrepareForShutdown", - NULL, - NULL, - G_DBUS_SIGNAL_FLAGS_NONE, - handleSystemMessageFunc, - dmn, - NULL); - } + if (virGDBusHasSystemBus()) { + /* Signal for VM shutdown when host OS shutdown is requested, in + * both privileged and unprivileged daemons */ + systemBus = virGDBusGetSystemBus(); + if (systemBus != NULL) + g_dbus_connection_signal_subscribe(systemBus, + "org.freedesktop.login1", + "org.freedesktop.login1.Manager", + "PrepareForShutdown", + NULL, + NULL, + G_DBUS_SIGNAL_FLAGS_NONE, + handleSystemMessageFunc, + dmn, + NULL); } /* Only now accept clients from network */ -- 2.48.1 The auto shutdown code can currently only perform managed save, which may fail in some cases, for example when PCI devices are assigned. On failure, shutdown inhibitors remain in place which may be undesirable. This expands the logic to try a sequence of operations * Managed save * Graceful shutdown * Forced poweroff Each of these operations can be enabled or disabled, but they are always applied in this order. With the shutdown option, a configurable time is allowed for shutdown to complete, defaulting to 30 seconds, before moving onto the forced poweroff phase. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/hypervisor/domain_driver.c | 130 ++++++++++++++++++++++++++++----- src/hypervisor/domain_driver.h | 7 ++ src/qemu/qemu_driver.c | 3 + 3 files changed, 121 insertions(+), 19 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) g_autoptr(virConnect) conn = NULL; int numDomains = 0; size_t i; - int state; virDomainPtr *domains = NULL; - g_autofree unsigned int *flags = NULL; + + VIR_DEBUG("Run autoshutdown uri=%s trySave=%d tryShutdown=%d poweroff=%d waitShutdownSecs=%u", + cfg->uri, cfg->trySave, cfg->tryShutdown, cfg->poweroff, + cfg->waitShutdownSecs); + + /* + * Ideally guests will shutdown in a few seconds, but it would + * not be unsual for it to take a while longer, especially under + * load, or if the guest OS has inhibitors slowing down shutdown. + * + * If we wait too long, then guests which ignore the shutdown + * request will significantly delay host shutdown. + * + * Pick 30 seconds as a moderately safe default, assuming that + * most guests are well behaved. + */ + if (cfg->waitShutdownSecs <= 0) + cfg->waitShutdownSecs = 30; if (!(conn = virConnectOpen(cfg->uri))) goto cleanup; @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) VIR_CONNECT_LIST_DOMAINS_ACTIVE)) < 0) goto cleanup; - flags = g_new0(unsigned int, numDomains); + VIR_DEBUG("Auto shutdown with %d running domains", numDomains); + if (cfg->trySave) { + g_autofree unsigned int *flags = g_new0(unsigned int, numDomains); + for (i = 0; i < numDomains; i++) { + int state; + /* + * 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 (flags[i] & VIR_DOMAIN_SAVE_RUNNING) + virDomainSuspend(domains[i]); + } + + for (i = 0; i < numDomains; i++) { + if (virDomainManagedSave(domains[i], flags[i]) < 0) { + VIR_WARN("Unable to perform managed save of '%s': %s", + virDomainGetName(domains[i]), + virGetLastErrorMessage()); + if (flags[i] & VIR_DOMAIN_SAVE_RUNNING) + virDomainResume(domains[i]); + continue; + } + virObjectUnref(domains[i]); + domains[i] = NULL; + } + } + + if (cfg->tryShutdown) { + GTimer *timer = NULL; + for (i = 0; i < numDomains; i++) { + if (domains[i] == NULL) + continue; + if (virDomainShutdown(domains[i]) < 0) { + VIR_WARN("Unable to request graceful shutdown of '%s': %s", + virDomainGetName(domains[i]), + virGetLastErrorMessage()); + break; + } + } + + timer = g_timer_new(); + while (1) { + bool anyRunning = false; + for (i = 0; i < numDomains; i++) { + if (!domains[i]) + continue; - /* First we pause all VMs to make them stop dirtying - pages, etc. We remember if any VMs were paused so - we can restore that on resume. */ - for (i = 0; i < numDomains; i++) { - 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 (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); } - virDomainSuspend(domains[i]); + g_timer_destroy(timer); } - /* Then we save the VMs to disk */ - for (i = 0; i < numDomains; i++) - if (virDomainManagedSave(domains[i], flags[i]) < 0) - VIR_WARN("Unable to perform managed save of '%s': %s", - virDomainGetName(domains[i]), - virGetLastErrorMessage()); + if (cfg->poweroff) { + for (i = 0; i < numDomains; i++) { + if (domains[i] == NULL) + continue; + /* + * 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; + } + } cleanup: if (domains) { - for (i = 0; i < numDomains; i++) + /* 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("Domain '%s' not successfully shut off by any action", + virDomainGetName(domains[i])); virObjectUnref(domains[i]); + } VIR_FREE(domains); } } diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -XXX,XX +XXX,XX @@ void virDomainDriverAutoStart(virDomainObjList *domains, typedef struct _virDomainDriverAutoShutdownConfig { const char *uri; + bool trySave; + bool tryShutdown; + bool poweroff; + unsigned int waitShutdownSecs; /* Seconds to wait for VM to shutdown + * before moving onto next action. + * If 0 a default is used (currently 30 secs) + */ } virDomainDriverAutoShutdownConfig; void virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ qemuStateStop(void) g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(qemu_driver); virDomainDriverAutoShutdownConfig ascfg = { .uri = cfg->uri, + .trySave = true, + .tryShutdown = false, + .poweroff = false, }; if (!qemu_driver->privileged) -- 2.48.1 It may be desirable to treat transient VMs differently from persistent VMs. For example, while performing managed save on persistent VMs makes sense, the same not usually true of transient VMs, since by their nature they will have no config to restore from. This also lets us fix a long standing problem with incorrectly attempting to perform managed save on transient VMs. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/hypervisor/domain_driver.c | 62 +++++++++++++++++++++++++++++++--- src/hypervisor/domain_driver.h | 18 ++++++++-- src/libvirt_private.syms | 2 ++ src/qemu/qemu_driver.c | 6 ++-- 4 files changed, 77 insertions(+), 11 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -XXX,XX +XXX,XX @@ VIR_LOG_INIT("hypervisor.domain_driver"); +VIR_ENUM_IMPL(virDomainDriverAutoShutdownScope, + VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_LAST, + "none", + "persistent", + "transient", + "all"); + char * virDomainDriverGenerateRootHash(const char *drivername, const char *root) @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) int numDomains = 0; size_t i; virDomainPtr *domains = NULL; + g_autofree bool *transient = NULL; - VIR_DEBUG("Run autoshutdown uri=%s trySave=%d tryShutdown=%d poweroff=%d waitShutdownSecs=%u", - cfg->uri, cfg->trySave, cfg->tryShutdown, cfg->poweroff, + VIR_DEBUG("Run autoshutdown uri=%s trySave=%s tryShutdown=%s poweroff=%s waitShutdownSecs=%u", + cfg->uri, + virDomainDriverAutoShutdownScopeTypeToString(cfg->trySave), + virDomainDriverAutoShutdownScopeTypeToString(cfg->tryShutdown), + virDomainDriverAutoShutdownScopeTypeToString(cfg->poweroff), cfg->waitShutdownSecs); /* @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) if (cfg->waitShutdownSecs <= 0) cfg->waitShutdownSecs = 30; + /* 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) + return; + + if (cfg->trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL || + cfg->trySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT) { + /* virDomainManagedSave will return an error. We'll let + * the code run, since it'll just fall through to the next + * actions, but give a clear warning upfront */ + VIR_WARN("Managed save not supported for transient VMs"); + return; + } + if (!(conn = virConnectOpen(cfg->uri))) goto cleanup; @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) goto cleanup; VIR_DEBUG("Auto shutdown with %d running domains", numDomains); - if (cfg->trySave) { + + transient = g_new0(bool, numDomains); + for (i = 0; i < numDomains; i++) { + if (virDomainIsPersistent(domains[i]) == 0) + transient[i] = true; + } + + 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; + /* * Pause all VMs to make them stop dirtying pages, * so save is quicker. We remember if any VMs were @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) } } - if (cfg->tryShutdown) { + 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; + if (virDomainShutdown(domains[i]) < 0) { VIR_WARN("Unable to request graceful shutdown of '%s': %s", virDomainGetName(domains[i]), @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) 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 { @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) g_timer_destroy(timer); } - if (cfg->poweroff) { + 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; + /* * NB might fail if we gave up on waiting for * virDomainShutdown, but it then completed anyway, diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -XXX,XX +XXX,XX @@ #include "virhostdev.h" #include "virpci.h" #include "virdomainobjlist.h" +#include "virenum.h" char * virDomainDriverGenerateRootHash(const char *drivername, @@ -XXX,XX +XXX,XX @@ typedef struct _virDomainDriverAutoStartConfig { void virDomainDriverAutoStart(virDomainObjList *domains, virDomainDriverAutoStartConfig *cfg); +typedef enum { + VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE, + VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT, + VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT, + VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL, + + VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_LAST, +} virDomainDriverAutoShutdownScope; + +VIR_ENUM_DECL(virDomainDriverAutoShutdownScope); + typedef struct _virDomainDriverAutoShutdownConfig { const char *uri; - bool trySave; - bool tryShutdown; - bool poweroff; + virDomainDriverAutoShutdownScope trySave; + virDomainDriverAutoShutdownScope tryShutdown; + virDomainDriverAutoShutdownScope poweroff; unsigned int waitShutdownSecs; /* Seconds to wait for VM to shutdown * before moving onto next action. * If 0 a default is used (currently 30 secs) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index XXXXXXX..XXXXXXX 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -XXX,XX +XXX,XX @@ virDomainCgroupSetupVcpuBW; # hypervisor/domain_driver.h virDomainDriverAddIOThreadCheck; virDomainDriverAutoShutdown; +virDomainDriverAutoShutdownScopeTypeFromString; +virDomainDriverAutoShutdownScopeTypeToString; virDomainDriverAutoStart; virDomainDriverDelIOThreadCheck; virDomainDriverGenerateMachineName; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ qemuStateStop(void) g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(qemu_driver); virDomainDriverAutoShutdownConfig ascfg = { .uri = cfg->uri, - .trySave = true, - .tryShutdown = false, - .poweroff = false, + .trySave = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT, + .tryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE, + .poweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE }; if (!qemu_driver->privileged) -- 2.48.1 Currently automatic VM managed save is only performed in session daemons, on desktop session close, or host OS shutdown request. With this change it is possible to control shutdown behaviour for all daemons. A recommended setup might be: auto_shutdown_try_save = "persistent" auto_shutdown_try_shutdown = "all" auto_shutdown_poweroff = "all" Each setting accepts 'none', 'persistent', 'transient', and 'all' to control what types of guest it applies to. For historical compatibility, for the system daemon, the settings currently default to: auto_shutdown_try_save = "none" auto_shutdown_try_shutdown = "none" auto_shutdown_poweroff = "none" while for the session daemon they currently default to auto_shutdown_try_save = "persistent" auto_shutdown_try_shutdown = "none" auto_shutdown_poweroff = "none" The system daemon settings should NOT be enabled if the traditional libvirt-guests.service is already enabled. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/libvirtd_qemu.aug | 3 ++ src/qemu/qemu.conf.in | 42 +++++++++++++++++++ src/qemu/qemu_conf.c | 65 ++++++++++++++++++++++++++++++ src/qemu/qemu_conf.h | 4 ++ src/qemu/qemu_driver.c | 9 ++--- src/qemu/test_libvirtd_qemu.aug.in | 3 ++ 6 files changed, 121 insertions(+), 5 deletions(-) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -XXX,XX +XXX,XX @@ module Libvirtd_qemu = | bool_entry "auto_dump_bypass_cache" | bool_entry "auto_start_bypass_cache" | int_entry "auto_start_delay" + | str_entry "auto_shutdown_try_save" + | str_entry "auto_shutdown_try_shutdown" + | str_entry "auto_shutdown_poweroff" let process_entry = str_entry "hugetlbfs_mount" | str_entry "bridge_helper" diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu.conf.in +++ b/src/qemu/qemu.conf.in @@ -XXX,XX +XXX,XX @@ # #auto_start_delay = 0 +# The settings for auto shutdown actions accept one of +# four possible options: +# +# * "none" - do not try to save any running VMs +# * "persistent" - only try to save persistent running VMs +# * "transient" - only try to save transient running VMs +# * "all" - try to save all running VMs +# +# Whether to perform managed save of running VMs if a host OS +# shutdown is requested (system/session daemons), or the desktop +# session terminates (session daemon only). +# +# Defaults to "persistent" for session daemons and "none" +# for system daemons. The values "all" and "transient" are +# not permitted for this setting, since managed save is not +# implemented for transient VMs. +# +# If 'libvirt-guests.service' is enabled, then this must be +# set to 'none' for system daemons to avoid dueling actions +#auto_shutdown_try_save = "persistent" + +# As above, but with a graceful shutdown action instead of +# managed save. If managed save is enabled, shutdown will +# be tried only on failure to perform managed save. +# +# Defaults to "none" +# +# If 'libvirt-guests.service' is enabled, then this must be +# set to 'none' for system daemons to avoid dueling actions +#auto_shutdown_try_shutdown = "none" + +# As above, but with a forced poweroff instead of managed +# save. If managed save or graceful shutdown are enabled, +# forced poweroff will be tried only on failure of the +# other options. +# +# Defaults to "none" +# +# If 'libvirt-guests.service' is enabled, then this must be +# set to 'none' for system daemons to avoid dueling actions +#auto_shutdown_poweroff = "none" + # If provided by the host and a hugetlbfs mount point is configured, # a guest may request huge page backing. When this mount point is # unspecified here, determination of a host mount point in /proc/mounts diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -XXX,XX +XXX,XX @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged, cfg->dumpGuestCore = true; #endif + if (privileged) { + /* + * Defer to libvirt-guests.service. + * + * 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; + } else { + cfg->autoShutdownTrySave = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT; + cfg->autoShutdownTryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE; + cfg->autoShutdownPoweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE; + } + return g_steal_pointer(&cfg); } @@ -XXX,XX +XXX,XX @@ virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg, g_autofree char *savestr = NULL; g_autofree char *dumpstr = NULL; g_autofree char *snapstr = NULL; + g_autofree char *autoShutdownScope = NULL; + int autoShutdownScopeVal; if (virConfGetValueString(conf, "save_image_format", &savestr) < 0) return -1; @@ -XXX,XX +XXX,XX @@ virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg, return -1; if (virConfGetValueUInt(conf, "auto_start_delay", &cfg->autoStartDelayMS) < 0) return -1; + if (virConfGetValueString(conf, "auto_shutdown_try_save", &autoShutdownScope) < 0) + return -1; + + if (autoShutdownScope != NULL) { + if ((autoShutdownScopeVal = + virDomainDriverAutoShutdownScopeTypeFromString(autoShutdownScope)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("unknown auto_shutdown_try_save '%1$s'"), + autoShutdownScope); + return -1; + } + cfg->autoShutdownTrySave = autoShutdownScopeVal; + } + + if (cfg->autoShutdownTrySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL || + cfg->autoShutdownTrySave == VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_TRANSIENT) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("managed save cannot be requested for transient domains")); + return -1; + } + + if (virConfGetValueString(conf, "auto_shutdown_try_shutdown", &autoShutdownScope) < 0) + return -1; + + if (autoShutdownScope != NULL) { + if ((autoShutdownScopeVal = + virDomainDriverAutoShutdownScopeTypeFromString(autoShutdownScope)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("unknown auto_shutdown_try_shutdown '%1$s'"), + autoShutdownScope); + return -1; + } + cfg->autoShutdownTryShutdown = autoShutdownScopeVal; + } + + if (virConfGetValueString(conf, "auto_shutdown_poweroff", &autoShutdownScope) < 0) + return -1; + + if (autoShutdownScope != NULL) { + if ((autoShutdownScopeVal = + virDomainDriverAutoShutdownScopeTypeFromString(autoShutdownScope)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("unknown auto_shutdown_poweroff '%1$s'"), + autoShutdownScope); + return -1; + } + cfg->autoShutdownPoweroff = autoShutdownScopeVal; + } return 0; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -XXX,XX +XXX,XX @@ #include "virfilecache.h" #include "virfirmware.h" #include "virinhibitor.h" +#include "domain_driver.h" #define QEMU_DRIVER_NAME "QEMU" @@ -XXX,XX +XXX,XX @@ struct _virQEMUDriverConfig { bool autoDumpBypassCache; bool autoStartBypassCache; unsigned int autoStartDelayMS; + virDomainDriverAutoShutdownScope autoShutdownTrySave; + virDomainDriverAutoShutdownScope autoShutdownTryShutdown; + virDomainDriverAutoShutdownScope autoShutdownPoweroff; char *lockManagerName; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ qemuStateStop(void) g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(qemu_driver); virDomainDriverAutoShutdownConfig ascfg = { .uri = cfg->uri, - .trySave = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT, - .tryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE, - .poweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE + .trySave = cfg->autoShutdownTrySave, + .tryShutdown = cfg->autoShutdownTryShutdown, + .poweroff = cfg->autoShutdownPoweroff, }; - if (!qemu_driver->privileged) - virDomainDriverAutoShutdown(&ascfg); + virDomainDriverAutoShutdown(&ascfg); return 0; } diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -XXX,XX +XXX,XX @@ module Test_libvirtd_qemu = { "auto_dump_bypass_cache" = "0" } { "auto_start_bypass_cache" = "0" } { "auto_start_delay" = "0" } +{ "auto_shutdown_try_save" = "persistent" } +{ "auto_shutdown_try_shutdown" = "none" } +{ "auto_shutdown_poweroff" = "none" } { "hugetlbfs_mount" = "/dev/hugepages" } { "bridge_helper" = "qemu-bridge-helper" } { "set_process_name" = "1" } -- 2.48.1 Currently the session daemon will try a managed save on all VMs, leaving them running if that fails. This limits the managed save just to persistent VMs, as there will usually not be any way to restore transient VMs later. It also enables graceful shutdown and then forced poweroff, should save fail for some reason. These new defaults can be overridden in the config file if needed. Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu.conf.in | 10 ++++++---- src/qemu/qemu_conf.c | 4 ++-- src/qemu/test_libvirtd_qemu.aug.in | 4 ++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu.conf.in +++ b/src/qemu/qemu.conf.in @@ -XXX,XX +XXX,XX @@ # managed save. If managed save is enabled, shutdown will # be tried only on failure to perform managed save. # -# Defaults to "none" +# Defaults to "all" for session daemons and "none" for +# system daemons # # If 'libvirt-guests.service' is enabled, then this must be # set to 'none' for system daemons to avoid dueling actions -#auto_shutdown_try_shutdown = "none" +#auto_shutdown_try_shutdown = "all" # As above, but with a forced poweroff instead of managed # save. If managed save or graceful shutdown are enabled, # forced poweroff will be tried only on failure of the # other options. # -# Defaults to "none" +# Defaults to "all" for session daemons and "none" for +# system daemons. # # If 'libvirt-guests.service' is enabled, then this must be # set to 'none' for system daemons to avoid dueling actions -#auto_shutdown_poweroff = "none" +#auto_shutdown_poweroff = "all" # If provided by the host and a hugetlbfs mount point is configured, # a guest may request huge page backing. When this mount point is diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -XXX,XX +XXX,XX @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged, cfg->autoShutdownPoweroff = 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_NONE; - cfg->autoShutdownPoweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE; + cfg->autoShutdownTryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL; + cfg->autoShutdownPoweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL; } return g_steal_pointer(&cfg); diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -XXX,XX +XXX,XX @@ module Test_libvirtd_qemu = { "auto_start_bypass_cache" = "0" } { "auto_start_delay" = "0" } { "auto_shutdown_try_save" = "persistent" } -{ "auto_shutdown_try_shutdown" = "none" } -{ "auto_shutdown_poweroff" = "none" } +{ "auto_shutdown_try_shutdown" = "all" } +{ "auto_shutdown_poweroff" = "all" } { "hugetlbfs_mount" = "/dev/hugepages" } { "bridge_helper" = "qemu-bridge-helper" } { "set_process_name" = "1" } -- 2.48.1 Allow users to control how many seconds libvirt waits for QEMU shutdown before force powering off a guest. Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf.in | 6 ++++++ src/qemu/qemu_conf.c | 4 ++++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 14 insertions(+) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -XXX,XX +XXX,XX @@ module Libvirtd_qemu = | str_entry "auto_shutdown_try_save" | str_entry "auto_shutdown_try_shutdown" | str_entry "auto_shutdown_poweroff" + | int_entry "auto_shutdown_wait" let process_entry = str_entry "hugetlbfs_mount" | str_entry "bridge_helper" diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu.conf.in +++ b/src/qemu/qemu.conf.in @@ -XXX,XX +XXX,XX @@ # set to 'none' for system daemons to avoid dueling actions #auto_shutdown_poweroff = "all" +# How may seconds to wait for running VMs to gracefully shutdown +# when 'auto_shutdown_try_shutdown' is enabled. If set to 0 +# then an arbitrary built-in default value will be used (which +# is currently 30 secs) +#auto_shutdown_wait = 30 + # If provided by the host and a hugetlbfs mount point is configured, # a guest may request huge page backing. When this mount point is # unspecified here, determination of a host mount point in /proc/mounts diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -XXX,XX +XXX,XX @@ virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg, cfg->autoShutdownPoweroff = autoShutdownScopeVal; } + if (virConfGetValueUInt(conf, "auto_shutdown_wait", + &cfg->autoShutdownWait) < 0) + return -1; + return 0; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -XXX,XX +XXX,XX @@ struct _virQEMUDriverConfig { virDomainDriverAutoShutdownScope autoShutdownTrySave; virDomainDriverAutoShutdownScope autoShutdownTryShutdown; virDomainDriverAutoShutdownScope autoShutdownPoweroff; + unsigned int autoShutdownWait; char *lockManagerName; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ qemuStateStop(void) .trySave = cfg->autoShutdownTrySave, .tryShutdown = cfg->autoShutdownTryShutdown, .poweroff = cfg->autoShutdownPoweroff, + .waitShutdownSecs = cfg->autoShutdownWait, }; virDomainDriverAutoShutdown(&ascfg); diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -XXX,XX +XXX,XX @@ module Test_libvirtd_qemu = { "auto_shutdown_try_save" = "persistent" } { "auto_shutdown_try_shutdown" = "all" } { "auto_shutdown_poweroff" = "all" } +{ "auto_shutdown_wait" = "30" } { "hugetlbfs_mount" = "/dev/hugepages" } { "bridge_helper" = "qemu-bridge-helper" } { "set_process_name" = "1" } -- 2.48.1 Bypassing cache can make save performance more predictable and avoids trashing the OS cache with data that will not be read again. Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/hypervisor/domain_driver.c | 7 +++++-- src/hypervisor/domain_driver.h | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) virDomainPtr *domains = NULL; g_autofree bool *transient = NULL; - VIR_DEBUG("Run autoshutdown uri=%s trySave=%s tryShutdown=%s poweroff=%s waitShutdownSecs=%u", + VIR_DEBUG("Run autoshutdown uri=%s trySave=%s tryShutdown=%s poweroff=%s waitShutdownSecs=%u saveBypassCache=%d", cfg->uri, virDomainDriverAutoShutdownScopeTypeToString(cfg->trySave), virDomainDriverAutoShutdownScopeTypeToString(cfg->tryShutdown), virDomainDriverAutoShutdownScopeTypeToString(cfg->poweroff), - cfg->waitShutdownSecs); + cfg->waitShutdownSecs, cfg->saveBypassCache); /* * Ideally guests will shutdown in a few seconds, but it would @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) 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]); } diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -XXX,XX +XXX,XX @@ typedef struct _virDomainDriverAutoShutdownConfig { * before moving onto next action. * If 0 a default is used (currently 30 secs) */ + bool saveBypassCache; } virDomainDriverAutoShutdownConfig; void virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg); -- 2.48.1 When doing managed save of VMs, triggered by OS shutdown, it may be desirable to control cache usage. Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf.in | 8 ++++++++ src/qemu/qemu_conf.c | 3 +++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 15 insertions(+) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -XXX,XX +XXX,XX @@ module Libvirtd_qemu = | str_entry "auto_shutdown_try_shutdown" | str_entry "auto_shutdown_poweroff" | int_entry "auto_shutdown_wait" + | bool_entry "auto_save_bypass_cache" let process_entry = str_entry "hugetlbfs_mount" | str_entry "bridge_helper" diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu.conf.in +++ b/src/qemu/qemu.conf.in @@ -XXX,XX +XXX,XX @@ # is currently 30 secs) #auto_shutdown_wait = 30 +# When a domain is configured to be auto-saved on shutdown, enabling +# this flag has the same effect as using the VIR_DOMAIN_SAVE_BYPASS_CACHE +# flag with the virDomainManagedSave API. That is, the system will +# avoid using the file system cache when writing any managed state +# file, but may cause slower operation. +# +#auto_save_bypass_cache = 0 + # If provided by the host and a hugetlbfs mount point is configured, # a guest may request huge page backing. When this mount point is # unspecified here, determination of a host mount point in /proc/mounts diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -XXX,XX +XXX,XX @@ virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg, if (virConfGetValueUInt(conf, "auto_shutdown_wait", &cfg->autoShutdownWait) < 0) return -1; + if (virConfGetValueBool(conf, "auto_save_bypass_cache", + &cfg->autoSaveBypassCache) < 0) + return -1; return 0; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -XXX,XX +XXX,XX @@ struct _virQEMUDriverConfig { virDomainDriverAutoShutdownScope autoShutdownTryShutdown; virDomainDriverAutoShutdownScope autoShutdownPoweroff; unsigned int autoShutdownWait; + bool autoSaveBypassCache; char *lockManagerName; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ qemuStateStop(void) .tryShutdown = cfg->autoShutdownTryShutdown, .poweroff = cfg->autoShutdownPoweroff, .waitShutdownSecs = cfg->autoShutdownWait, + .saveBypassCache = cfg->autoSaveBypassCache, }; virDomainDriverAutoShutdown(&ascfg); diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -XXX,XX +XXX,XX @@ module Test_libvirtd_qemu = { "auto_shutdown_try_shutdown" = "all" } { "auto_shutdown_poweroff" = "all" } { "auto_shutdown_wait" = "30" } +{ "auto_save_bypass_cache" = "0" } { "hugetlbfs_mount" = "/dev/hugepages" } { "bridge_helper" = "qemu-bridge-helper" } { "set_process_name" = "1" } -- 2.48.1 When a domain is marked for autostart, it will be started on every subsequent host OS boot. There may be times when it is desirable to mark a domain to be autostarted, on the next boot only. Thus we add virDomainSetAutostartOnce / virDomainGetAutostartOnce. An alternative would have been to overload the existing virDomainSetAutostart method, to accept values '1' or '2' for the autostart flag. This was not done because it is expected that language bindings will have mapped the current autostart flag to a boolean, and thus turning it into an enum would create a compatibility problem. A further alternative would have been to create a new method virDomainSetAutostartFlags, with a VIR_DOMAIN_AUTOSTART_ONCE flag defined. This was not done because it is felt desirable to clearly separate the two flags. Setting the "once" flag should not interfere with existing autostart setting, whether it is enabled or disabled currently. The 'virsh autostart' command, however, is still overloaded by just adding a --once flag, while current state is added to 'virsh dominfo'. No ability to filter by 'autostart once' status is added to the domain list APIs. The most common use of autostart once will be to automatically set it on host shutdown, and it be cleared on host startup. Thus there would rarely be scenarios in which a running app will need to filter on this new flag. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- include/libvirt/libvirt-domain.h | 4 ++ src/driver-hypervisor.h | 10 ++++ src/libvirt-domain.c | 87 ++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++ src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 30 ++++++++++- src/remote_protocol-structs | 12 +++++ src/rpc/gendispatch.pl | 4 +- tools/virsh-domain-monitor.c | 7 +++ tools/virsh-domain.c | 39 ++++++++++---- 10 files changed, 189 insertions(+), 12 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index XXXXXXX..XXXXXXX 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -XXX,XX +XXX,XX @@ int virDomainGetAutostart (virDomainPtr domain, int *autostart); int virDomainSetAutostart (virDomainPtr domain, int autostart); +int virDomainGetAutostartOnce(virDomainPtr domain, + int *autostart); +int virDomainSetAutostartOnce(virDomainPtr domain, + int autostart); /** * virVcpuState: diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index XXXXXXX..XXXXXXX 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -XXX,XX +XXX,XX @@ typedef int (*virDrvDomainSetAutostart)(virDomainPtr domain, int autostart); +typedef int +(*virDrvDomainGetAutostartOnce)(virDomainPtr domain, + int *autostart); + +typedef int +(*virDrvDomainSetAutostartOnce)(virDomainPtr domain, + int autostart); + typedef char * (*virDrvDomainGetSchedulerType)(virDomainPtr domain, int *nparams); @@ -XXX,XX +XXX,XX @@ struct _virHypervisorDriver { virDrvDomainDetachDeviceAlias domainDetachDeviceAlias; virDrvDomainGetAutostart domainGetAutostart; virDrvDomainSetAutostart domainSetAutostart; + virDrvDomainGetAutostartOnce domainGetAutostartOnce; + virDrvDomainSetAutostartOnce domainSetAutostartOnce; virDrvDomainGetSchedulerType domainGetSchedulerType; virDrvDomainGetSchedulerParameters domainGetSchedulerParameters; virDrvDomainGetSchedulerParametersFlags domainGetSchedulerParametersFlags; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index XXXXXXX..XXXXXXX 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -XXX,XX +XXX,XX @@ virDomainSetAutostart(virDomainPtr domain, } +/** + * virDomainGetAutostartOnce: + * @domain: a domain object + * @autostart: the value returned + * + * Provides a boolean value indicating whether the domain + * is configured to be automatically started the next time + * the host machine boots only. + * + * Returns -1 in case of error, 0 in case of success + * + * Since: 11.2.0 + */ +int +virDomainGetAutostartOnce(virDomainPtr domain, + int *autostart) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "autostart=%p", autostart); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + virCheckNonNullArgGoto(autostart, error); + + conn = domain->conn; + + if (conn->driver->domainGetAutostartOnce) { + int ret; + ret = conn->driver->domainGetAutostartOnce(domain, autostart); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + + +/** + * virDomainSetAutostartOnce: + * @domain: a domain object + * @autostart: whether the domain should be automatically started 0 or 1 + * + * Configure the domain to be automatically started + * the next time the host machine boots only. + * + * Returns -1 in case of error, 0 in case of success + * + * Since: 11.2.0 + */ +int +virDomainSetAutostartOnce(virDomainPtr domain, + int autostart) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "autostart=%d", autostart); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + + if (conn->driver->domainSetAutostartOnce) { + int ret; + ret = conn->driver->domainSetAutostartOnce(domain, autostart); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + + /** * virDomainInjectNMI: * @domain: pointer to domain object, or NULL for Domain0 diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index XXXXXXX..XXXXXXX 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -XXX,XX +XXX,XX @@ LIBVIRT_10.2.0 { virDomainGraphicsReload; } LIBVIRT_10.1.0; +LIBVIRT_11.2.0 { + global: + virDomainGetAutostartOnce; + virDomainSetAutostartOnce; +} LIBVIRT_10.2.0; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -XXX,XX +XXX,XX @@ static virHypervisorDriver hypervisor_driver = { .domainDetachDeviceAlias = remoteDomainDetachDeviceAlias, /* 4.4.0 */ .domainGetAutostart = remoteDomainGetAutostart, /* 0.3.0 */ .domainSetAutostart = remoteDomainSetAutostart, /* 0.3.0 */ + .domainGetAutostartOnce = remoteDomainGetAutostartOnce, /* 11.2.0 */ + .domainSetAutostartOnce = remoteDomainSetAutostartOnce, /* 11.2.0 */ .domainGetSchedulerType = remoteDomainGetSchedulerType, /* 0.3.0 */ .domainGetSchedulerParameters = remoteDomainGetSchedulerParameters, /* 0.3.0 */ .domainGetSchedulerParametersFlags = remoteDomainGetSchedulerParametersFlags, /* 0.9.2 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index XXXXXXX..XXXXXXX 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -XXX,XX +XXX,XX @@ struct remote_domain_fd_associate_args { remote_nonnull_string name; unsigned int flags; }; + +struct remote_domain_get_autostart_once_args { + remote_nonnull_domain dom; +}; + +struct remote_domain_get_autostart_once_ret { + int autostart; +}; + +struct remote_domain_set_autostart_once_args { + remote_nonnull_domain dom; + int autostart; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -XXX,XX +XXX,XX @@ enum remote_procedure { * @generate: both * @acl: domain:write */ - REMOTE_PROC_DOMAIN_GRAPHICS_RELOAD = 448 + REMOTE_PROC_DOMAIN_GRAPHICS_RELOAD = 448, + + /** + * @generate: both + * @priority: high + * @acl: domain:read + */ + REMOTE_PROC_DOMAIN_GET_AUTOSTART_ONCE = 449, + + /** + * @generate: both + * @priority: high + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_SET_AUTOSTART_ONCE = 450 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index XXXXXXX..XXXXXXX 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -XXX,XX +XXX,XX @@ struct remote_domain_fd_associate_args { remote_nonnull_string name; u_int flags; }; +struct remote_domain_get_autostart_once_args { + remote_nonnull_domain dom; +}; +struct remote_domain_get_autostart_once_ret { + int autostart; +}; +struct remote_domain_set_autostart_once_args { + remote_nonnull_domain dom; + int autostart; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -XXX,XX +XXX,XX @@ enum remote_procedure { REMOTE_PROC_NETWORK_EVENT_CALLBACK_METADATA_CHANGE = 446, REMOTE_PROC_NODE_DEVICE_UPDATE = 447, REMOTE_PROC_DOMAIN_GRAPHICS_RELOAD = 448, + REMOTE_PROC_DOMAIN_GET_AUTOSTART_ONCE = 449, + REMOTE_PROC_DOMAIN_SET_AUTOSTART_ONCE = 450, }; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index XXXXXXX..XXXXXXX 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -XXX,XX +XXX,XX @@ elsif ($mode eq "server") { push(@ret_list, "ret->$1 = $1;"); $single_ret_var = $1; - if ($call->{ProcName} =~ m/GetAutostart$/) { + if ($call->{ProcName} =~ m/GetAutostart(Once)?$/) { $single_ret_by_ref = 1; } else { $single_ret_by_ref = 0; @@ -XXX,XX +XXX,XX @@ elsif ($mode eq "client") { } elsif ($ret_member =~ m/^int (\S+);/) { my $arg_name = $1; - if ($call->{ProcName} =~ m/GetAutostart$/) { + if ($call->{ProcName} =~ m/GetAutostart(Once)?$/) { push(@args_list, "int *$arg_name"); push(@ret_list, "if ($arg_name) *$arg_name = ret.$arg_name;"); push(@ret_list, "rv = 0;"); diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index XXXXXXX..XXXXXXX 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -XXX,XX +XXX,XX @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "%-15s %s\n", _("Autostart:"), autostart ? _("enable") : _("disable")); } + /* Check and display whether the domain autostarts next boot or not */ + if (!virDomainGetAutostartOnce(dom, &autostart)) { + vshPrint(ctl, "%-15s %s\n", _("Autostart Once:"), + autostart ? _("enable") : _("disable")); + } else { + vshResetLibvirtError(); + } has_managed_save = virDomainHasManagedSaveImage(dom, 0); if (has_managed_save < 0) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index XXXXXXX..XXXXXXX 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -XXX,XX +XXX,XX @@ static const vshCmdOptDef opts_autostart[] = { .type = VSH_OT_BOOL, .help = N_("disable autostarting") }, + {.name = "once", + .type = VSH_OT_BOOL, + .help = N_("control next boot state") + }, {.name = NULL} }; @@ -XXX,XX +XXX,XX @@ cmdAutostart(vshControl *ctl, const vshCmd *cmd) g_autoptr(virshDomain) dom = NULL; const char *name; int autostart; + int once; if (!(dom = virshCommandOptDomain(ctl, cmd, &name))) return false; autostart = !vshCommandOptBool(cmd, "disable"); + once = vshCommandOptBool(cmd, "once"); + + if (once) { + if (virDomainSetAutostartOnce(dom, autostart) < 0) { + if (autostart) + vshError(ctl, _("Failed to mark domain '%1$s' as autostarted on next boot"), name); + else + vshError(ctl, _("Failed to unmark domain '%1$s' as autostarted on next boot"), name); + return false; + } - if (virDomainSetAutostart(dom, autostart) < 0) { if (autostart) - vshError(ctl, _("Failed to mark domain '%1$s' as autostarted"), name); + vshPrintExtra(ctl, _("Domain '%1$s' marked as autostarted on next boot\n"), name); else - vshError(ctl, _("Failed to unmark domain '%1$s' as autostarted"), name); - return false; - } + vshPrintExtra(ctl, _("Domain '%1$s' unmarked as autostarted on next boot\n"), name); + } else { + if (virDomainSetAutostart(dom, autostart) < 0) { + if (autostart) + vshError(ctl, _("Failed to mark domain '%1$s' as autostarted"), name); + else + vshError(ctl, _("Failed to unmark domain '%1$s' as autostarted"), name); + return false; + } - if (autostart) - vshPrintExtra(ctl, _("Domain '%1$s' marked as autostarted\n"), name); - else - vshPrintExtra(ctl, _("Domain '%1$s' unmarked as autostarted\n"), name); + if (autostart) + vshPrintExtra(ctl, _("Domain '%1$s' marked as autostarted\n"), name); + else + vshPrintExtra(ctl, _("Domain '%1$s' unmarked as autostarted\n"), name); + } return true; } -- 2.48.1 This is maintained in the same way as the autostart flag, using a symlink. The difference is that instead of '.xml', the symlink suffix is '.xml.once'. The link is also deleted immediately after it has been read. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/domain_conf.c | 7 ++++++- src/conf/domain_conf.h | 2 ++ src/conf/virdomainobjlist.c | 8 ++++++++ src/hypervisor/domain_driver.c | 14 +++++++++++--- 4 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index XXXXXXX..XXXXXXX 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -XXX,XX +XXX,XX @@ static void virDomainObjDispose(void *obj) virDomainCheckpointObjListFree(dom->checkpoints); virDomainJobObjFree(dom->job); virObjectUnref(dom->closecallbacks); + g_free(dom->autostartOnceLink); } virDomainObj * @@ -XXX,XX +XXX,XX @@ virDomainDeleteConfig(const char *configDir, { g_autofree char *configFile = NULL; g_autofree char *autostartLink = NULL; + g_autofree char *autostartOnceLink = NULL; configFile = virDomainConfigFile(configDir, dom->def->name); autostartLink = virDomainConfigFile(autostartDir, dom->def->name); + autostartOnceLink = g_strdup_printf("%s.once", autostartLink); - /* Not fatal if this doesn't work */ + /* Not fatal if these don't work */ unlink(autostartLink); + unlink(autostartOnceLink); dom->autostart = 0; + dom->autostartOnce = 0; if (unlink(configFile) < 0 && errno != ENOENT) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index XXXXXXX..XXXXXXX 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -XXX,XX +XXX,XX @@ struct _virDomainObj { virDomainStateReason state; unsigned int autostart : 1; + unsigned int autostartOnce : 1; unsigned int persistent : 1; unsigned int updated : 1; unsigned int removing : 1; + char *autostartOnceLink; virDomainDef *def; /* The current definition */ virDomainDef *newDef; /* New definition to activate at shutdown */ diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index XXXXXXX..XXXXXXX 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -XXX,XX +XXX,XX @@ virDomainObjListLoadConfig(virDomainObjList *doms, { g_autofree char *configFile = NULL; g_autofree char *autostartLink = NULL; + g_autofree char *autostartOnceLink = NULL; g_autoptr(virDomainDef) def = NULL; virDomainObj *dom; int autostart; + int autostartOnce; g_autoptr(virDomainDef) oldDef = NULL; configFile = virDomainConfigFile(configDir, name); @@ -XXX,XX +XXX,XX @@ virDomainObjListLoadConfig(virDomainObjList *doms, return NULL; autostartLink = virDomainConfigFile(autostartDir, name); + autostartOnceLink = g_strdup_printf("%s.once", autostartLink); autostart = virFileLinkPointsTo(autostartLink, configFile); + autostartOnce = virFileLinkPointsTo(autostartOnceLink, configFile); if (!(dom = virDomainObjListAddLocked(doms, &def, xmlopt, 0, &oldDef))) return NULL; dom->autostart = autostart; + dom->autostartOnce = autostartOnce; + + if (autostartOnce) + dom->autostartOnceLink = g_steal_pointer(&autostartOnceLink); if (notify) (*notify)(dom, oldDef == NULL, opaque); diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoStartOne(virDomainObj *vm, virObjectLock(vm); virObjectRef(vm); - VIR_DEBUG("Autostart %s: autostart=%d", - vm->def->name, vm->autostart); + VIR_DEBUG("Autostart %s: autostart=%d autostartOnce=%d autostartOnceLink=%s", + vm->def->name, vm->autostart, vm->autostartOnce, + NULLSTR(vm->autostartOnceLink)); - if (vm->autostart && !virDomainObjIsActive(vm)) { + if ((vm->autostart || vm->autostartOnce) && + !virDomainObjIsActive(vm)) { virResetLastError(); if (state->cfg->delayMS) { if (!state->first) { @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoStartOne(virDomainObj *vm, } state->cfg->callback(vm, state->cfg->opaque); + vm->autostartOnce = 0; + } + + if (vm->autostartOnceLink) { + unlink(vm->autostartOnceLink); + g_clear_pointer(&vm->autostartOnceLink, g_free); } virDomainObjEndAPI(&vm); -- 2.48.1 When performing auto-shutdown of running domains, there is now the option to mark them as "autostart once", so that their state is restored on next boot. This applies on top of the traditional autostart flag. Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/hypervisor/domain_driver.c | 19 +++++++++++++++++-- src/hypervisor/domain_driver.h | 1 + 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) virDomainPtr *domains = NULL; g_autofree bool *transient = NULL; - VIR_DEBUG("Run autoshutdown uri=%s trySave=%s tryShutdown=%s poweroff=%s waitShutdownSecs=%u saveBypassCache=%d", + VIR_DEBUG("Run autoshutdown uri=%s trySave=%s tryShutdown=%s poweroff=%s waitShutdownSecs=%u saveBypassCache=%d autoRestore=%d", cfg->uri, virDomainDriverAutoShutdownScopeTypeToString(cfg->trySave), virDomainDriverAutoShutdownScopeTypeToString(cfg->tryShutdown), virDomainDriverAutoShutdownScopeTypeToString(cfg->poweroff), - cfg->waitShutdownSecs, cfg->saveBypassCache); + cfg->waitShutdownSecs, cfg->saveBypassCache, cfg->autoRestore); /* * Ideally guests will shutdown in a few seconds, but it would @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) for (i = 0; i < numDomains; i++) { if (virDomainIsPersistent(domains[i]) == 0) transient[i] = true; + + 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 (cfg->trySave != VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE) { diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -XXX,XX +XXX,XX @@ typedef struct _virDomainDriverAutoShutdownConfig { * If 0 a default is used (currently 30 secs) */ bool saveBypassCache; + bool autoRestore; } virDomainDriverAutoShutdownConfig; void virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg); -- 2.48.1 Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu_driver.c | 97 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ static int qemuDomainSetAutostart(virDomainPtr dom, } +static int +qemuDomainGetAutostartOnce(virDomainPtr dom, + int *autostart) +{ + virDomainObj *vm; + int ret = -1; + + if (!(vm = qemuDomainObjFromDomain(dom))) + goto cleanup; + + if (virDomainGetAutostartOnceEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + *autostart = vm->autostartOnce; + ret = 0; + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + +static int +qemuDomainSetAutostartOnce(virDomainPtr dom, + int autostart) +{ + virQEMUDriver *driver = dom->conn->privateData; + virDomainObj *vm; + g_autofree char *configFile = NULL; + g_autofree char *autostartLink = NULL; + g_autofree char *autostartOnceLink = NULL; + int ret = -1; + g_autoptr(virQEMUDriverConfig) cfg = NULL; + + if (!(vm = qemuDomainObjFromDomain(dom))) + return -1; + + cfg = virQEMUDriverGetConfig(driver); + + if (virDomainSetAutostartOnceEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (!vm->persistent) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cannot set autostart for transient domain")); + goto cleanup; + } + + autostart = (autostart != 0); + + if (vm->autostartOnce != autostart) { + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) + goto cleanup; + + configFile = virDomainConfigFile(cfg->configDir, vm->def->name); + autostartLink = virDomainConfigFile(cfg->autostartDir, vm->def->name); + autostartOnceLink = g_strdup_printf("%s.once", autostartLink); + + if (autostart) { + if (g_mkdir_with_parents(cfg->autostartDir, 0777) < 0) { + virReportSystemError(errno, + _("cannot create autostart directory %1$s"), + cfg->autostartDir); + goto endjob; + } + + if (symlink(configFile, autostartOnceLink) < 0) { + virReportSystemError(errno, + _("Failed to create symlink '%1$s' to '%2$s'"), + autostartOnceLink, configFile); + goto endjob; + } + } else { + if (unlink(autostartOnceLink) < 0 && + errno != ENOENT && + errno != ENOTDIR) { + virReportSystemError(errno, + _("Failed to delete symlink '%1$s'"), + autostartOnceLink); + goto endjob; + } + } + + vm->autostartOnce = autostart; + + endjob: + virDomainObjEndJob(vm); + } + ret = 0; + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + + static char *qemuDomainGetSchedulerType(virDomainPtr dom, int *nparams) { @@ -XXX,XX +XXX,XX @@ static virHypervisorDriver qemuHypervisorDriver = { .domainSetLaunchSecurityState = qemuDomainSetLaunchSecurityState, /* 8.0.0 */ .domainFDAssociate = qemuDomainFDAssociate, /* 9.0.0 */ .domainGraphicsReload = qemuDomainGraphicsReload, /* 10.2.0 */ + .domainGetAutostartOnce = qemuDomainGetAutostartOnce, /* 11.2.0 */ + .domainSetAutostartOnce = qemuDomainSetAutostartOnce, /* 11.2.0 */ }; -- 2.48.1 If shutting down running VMs at host shutdown, it can be useful to automatically start them again on next boot. This adds a config parameter 'auto_shutdown_restore', which defaults to enabled, which leverages the autostart once feature. Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf.in | 4 ++++ src/qemu/qemu_conf.c | 3 +++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 11 insertions(+) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -XXX,XX +XXX,XX @@ module Libvirtd_qemu = | str_entry "auto_shutdown_try_shutdown" | str_entry "auto_shutdown_poweroff" | int_entry "auto_shutdown_wait" + | bool_entry "auto_shutdown_restore" | bool_entry "auto_save_bypass_cache" let process_entry = str_entry "hugetlbfs_mount" diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu.conf.in +++ b/src/qemu/qemu.conf.in @@ -XXX,XX +XXX,XX @@ # is currently 30 secs) #auto_shutdown_wait = 30 +# Whether VMs that are automatically powered off or saved during +# host shutdown, should be set to restore on next boot +#auto_shutdown_restore = 1 + # When a domain is configured to be auto-saved on shutdown, enabling # this flag has the same effect as using the VIR_DOMAIN_SAVE_BYPASS_CACHE # flag with the virDomainManagedSave API. That is, the system will diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -XXX,XX +XXX,XX @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged, cfg->autoShutdownTryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL; cfg->autoShutdownPoweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL; } + cfg->autoShutdownRestore = true; return g_steal_pointer(&cfg); } @@ -XXX,XX +XXX,XX @@ virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg, if (virConfGetValueUInt(conf, "auto_shutdown_wait", &cfg->autoShutdownWait) < 0) return -1; + if (virConfGetValueBool(conf, "auto_shutdown_restore", &cfg->autoShutdownRestore) < 0) + return -1; if (virConfGetValueBool(conf, "auto_save_bypass_cache", &cfg->autoSaveBypassCache) < 0) return -1; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -XXX,XX +XXX,XX @@ struct _virQEMUDriverConfig { virDomainDriverAutoShutdownScope autoShutdownTryShutdown; virDomainDriverAutoShutdownScope autoShutdownPoweroff; unsigned int autoShutdownWait; + bool autoShutdownRestore; bool autoSaveBypassCache; char *lockManagerName; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -XXX,XX +XXX,XX @@ qemuStateStop(void) .poweroff = cfg->autoShutdownPoweroff, .waitShutdownSecs = cfg->autoShutdownWait, .saveBypassCache = cfg->autoSaveBypassCache, + .autoRestore = cfg->autoShutdownRestore, }; virDomainDriverAutoShutdown(&ascfg); diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -XXX,XX +XXX,XX @@ module Test_libvirtd_qemu = { "auto_shutdown_try_shutdown" = "all" } { "auto_shutdown_poweroff" = "all" } { "auto_shutdown_wait" = "30" } +{ "auto_shutdown_restore" = "1" } { "auto_save_bypass_cache" = "0" } { "hugetlbfs_mount" = "/dev/hugepages" } { "bridge_helper" = "qemu-bridge-helper" } -- 2.48.1 It is not documented what the various virStateNNN methods are each responsible for doing and the names give little guidance either. Provide some useful documentation comments to explain the intended usage of each. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/libvirt.c | 44 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index XXXXXXX..XXXXXXX 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -XXX,XX +XXX,XX @@ virStateInitialize(bool privileged, /** * virStateShutdownPrepare: * - * Run each virtualization driver's shutdown prepare method. + * Tell each driver to prepare for shutdown of the daemon. This should + * trigger any actions required to stop processing background work. + * + * This method is called directly from the main event loop thread, so + * the event loop will not execute while this method is running. After + * this method returns, the event loop will continue running until + * the virStateShutdownWait method completes. * * Returns 0 if all succeed, -1 upon any failure. */ @@ -XXX,XX +XXX,XX @@ virStateShutdownPrepare(void) /** * virStateShutdownWait: * - * Run each virtualization driver's shutdown wait method. + * Tell each driver to finalize any work required to enable + * graceful shutdown of the daemon. This method is invoked + * from a background thread, and when it completes, the event + * loop will terminate. As such drivers can register callbacks + * that will prevent the event loop terminating until actions + * initiated by virStateShutdownPrepare are complete. * * Returns 0 if all succeed, -1 upon any failure. */ @@ -XXX,XX +XXX,XX @@ virStateShutdownWait(void) /** * virStateCleanup: * - * Run each virtualization driver's cleanup method. + * Tell each driver to release all resources it holds in + * order to fully shutdown the daemon. When this is called + * the event loop will no longer be running. Thus any + * cleanup that depends on execution of the event loop + * must have been triggered by the virStateShutdownPrepare + * method. * * Returns 0 if all succeed, -1 upon any failure. */ @@ -XXX,XX +XXX,XX @@ virStateCleanup(void) /** * virStateReload: * - * Run each virtualization driver's reload method. + * Tell each driver to reload their global configuration + * file(s). * * Returns 0 if all succeed, -1 upon any failure. */ @@ -XXX,XX +XXX,XX @@ virStateReload(void) /** * virStateStop: * - * Run each virtualization driver's "stop" method. + * Tell each driver to prepare for system/session stop. + * + * In an unprivileged daemon, this indicates that the + * current user's primary login session is about to + * be terminated. + * + * In a privileged daemon, this indicates that the host + * OS is about to shutdown. + * + * This is a signal that the driver should stop and/or + * preserve any resources affected by the system/session + * stop. + * + * On host OS stop there is a very short wait for the + * stop action to complete. For any prolonged tasks + * the driver must acquire inhibitor locks, or send + * a request to systemd to extend the shutdown wait + * timeout. * * Returns 0 if successful, -1 on failure */ -- 2.48.1 The next patch will be introducing a new callback, so rename the method to virNetDaemonSetLifecycleCallbacks to reflect the more general usage. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/libvirt_remote.syms | 2 +- src/remote/remote_daemon.c | 6 +++--- src/rpc/virnetdaemon.c | 10 +++++----- src/rpc/virnetdaemon.h | 8 ++++---- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index XXXXXXX..XXXXXXX 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -XXX,XX +XXX,XX @@ virNetDaemonQuit; virNetDaemonQuitExecRestart; virNetDaemonRemoveShutdownInhibition; virNetDaemonRun; -virNetDaemonSetShutdownCallbacks; +virNetDaemonSetLifecycleCallbacks; virNetDaemonSetStateStopWorkerThread; virNetDaemonUpdateServices; diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -XXX,XX +XXX,XX @@ static void daemonRunStateInit(void *opaque) g_atomic_int_set(&driversInitialized, 1); - virNetDaemonSetShutdownCallbacks(dmn, - virStateShutdownPrepare, - virStateShutdownWait); + virNetDaemonSetLifecycleCallbacks(dmn, + virStateShutdownPrepare, + virStateShutdownWait); /* Signal for VM shutdown when desktop session is terminated, in * unprivileged daemons */ diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -XXX,XX +XXX,XX @@ struct _virNetDaemon { GHashTable *servers; virJSONValue *srvObject; - virNetDaemonShutdownCallback shutdownPrepareCb; - virNetDaemonShutdownCallback shutdownWaitCb; + virNetDaemonLifecycleCallback shutdownPrepareCb; + virNetDaemonLifecycleCallback shutdownWaitCb; virThread *stateStopThread; int finishTimer; bool quit; @@ -XXX,XX +XXX,XX @@ virNetDaemonHasClients(virNetDaemon *dmn) } void -virNetDaemonSetShutdownCallbacks(virNetDaemon *dmn, - virNetDaemonShutdownCallback prepareCb, - virNetDaemonShutdownCallback waitCb) +virNetDaemonSetLifecycleCallbacks(virNetDaemon *dmn, + virNetDaemonLifecycleCallback prepareCb, + virNetDaemonLifecycleCallback waitCb) { VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h index XXXXXXX..XXXXXXX 100644 --- a/src/rpc/virnetdaemon.h +++ b/src/rpc/virnetdaemon.h @@ -XXX,XX +XXX,XX @@ ssize_t virNetDaemonGetServers(virNetDaemon *dmn, virNetServer ***servers); bool virNetDaemonHasServer(virNetDaemon *dmn, const char *serverName); -typedef int (*virNetDaemonShutdownCallback)(void); +typedef int (*virNetDaemonLifecycleCallback)(void); -void virNetDaemonSetShutdownCallbacks(virNetDaemon *dmn, - virNetDaemonShutdownCallback prepareCb, - virNetDaemonShutdownCallback waitCb); +void virNetDaemonSetLifecycleCallbacks(virNetDaemon *dmn, + virNetDaemonLifecycleCallback prepareCb, + virNetDaemonLifecycleCallback waitCb); -- 2.48.1 Currently the remote daemon code is responsible for calling virStateStop in a background thread. The virNetDaemon code wants to synchronize with this during shutdown, however, so the virThreadPtr must be passed over. Even the limited synchronization done currently, however, is flawed and to fix this requires the virNetDaemon code to be responsible for calling virStateStop in a thread more directly. Thus the logic is moved over into virStateStop via a further callback to be registered by the remote daemon. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/libvirt_remote.syms | 2 +- src/remote/remote_daemon.c | 40 ++------------------------ src/rpc/virnetdaemon.c | 59 +++++++++++++++++++++++++++++++------- src/rpc/virnetdaemon.h | 27 +++++++++++++++-- 4 files changed, 77 insertions(+), 51 deletions(-) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index XXXXXXX..XXXXXXX 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -XXX,XX +XXX,XX @@ virNetDaemonQuitExecRestart; virNetDaemonRemoveShutdownInhibition; virNetDaemonRun; virNetDaemonSetLifecycleCallbacks; -virNetDaemonSetStateStopWorkerThread; +virNetDaemonStop; virNetDaemonUpdateServices; diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -XXX,XX +XXX,XX @@ static void daemonInhibitCallback(bool inhibit, void *opaque) static GDBusConnection *sessionBus; static GDBusConnection *systemBus; -static void daemonStopWorker(void *opaque) -{ - virNetDaemon *dmn = opaque; - - VIR_DEBUG("Begin stop dmn=%p", dmn); - - ignore_value(virStateStop()); - - VIR_DEBUG("Completed stop dmn=%p", dmn); - - /* Exit daemon cleanly */ - virNetDaemonQuit(dmn); -} - - -/* We do this in a thread to not block the main loop */ -static void daemonStop(virNetDaemon *dmn) -{ - virThread *thr; - virObjectRef(dmn); - - thr = g_new0(virThread, 1); - - if (virThreadCreateFull(thr, true, - daemonStopWorker, - "daemon-stop", false, dmn) < 0) { - virObjectUnref(dmn); - g_free(thr); - return; - } - - virNetDaemonSetStateStopWorkerThread(dmn, &thr); -} - - static GDBusMessage * handleSessionMessageFunc(GDBusConnection *connection G_GNUC_UNUSED, GDBusMessage *message, @@ -XXX,XX +XXX,XX @@ handleSessionMessageFunc(GDBusConnection *connection G_GNUC_UNUSED, if (virGDBusMessageIsSignal(message, "org.freedesktop.DBus.Local", "Disconnected")) - daemonStop(dmn); + virNetDaemonStop(dmn); return message; } @@ -XXX,XX +XXX,XX @@ handleSystemMessageFunc(GDBusConnection *connection G_GNUC_UNUSED, VIR_DEBUG("dmn=%p", dmn); - daemonStop(dmn); + virNetDaemonStop(dmn); } @@ -XXX,XX +XXX,XX @@ static void daemonRunStateInit(void *opaque) g_atomic_int_set(&driversInitialized, 1); virNetDaemonSetLifecycleCallbacks(dmn, + virStateStop, virStateShutdownPrepare, virStateShutdownWait); diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -XXX,XX +XXX,XX @@ struct _virNetDaemon { GHashTable *servers; virJSONValue *srvObject; + virNetDaemonLifecycleCallback stopCb; virNetDaemonLifecycleCallback shutdownPrepareCb; virNetDaemonLifecycleCallback shutdownWaitCb; virThread *stateStopThread; @@ -XXX,XX +XXX,XX @@ virNetDaemonRun(virNetDaemon *dmn) } } + VIR_DEBUG("Main loop exited"); if (dmn->graceful) { virThreadJoin(&shutdownThread); + VIR_DEBUG("Graceful shutdown complete"); } else { VIR_WARN("Make forcefull daemon shutdown"); exit(EXIT_FAILURE); @@ -XXX,XX +XXX,XX @@ virNetDaemonRun(virNetDaemon *dmn) void -virNetDaemonSetStateStopWorkerThread(virNetDaemon *dmn, - virThread **thr) +virNetDaemonQuit(virNetDaemon *dmn) { VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); - VIR_DEBUG("Setting state stop worker thread on dmn=%p to thr=%p", dmn, thr); - dmn->stateStopThread = g_steal_pointer(thr); + VIR_DEBUG("Quit requested %p", dmn); + dmn->quit = true; } void -virNetDaemonQuit(virNetDaemon *dmn) +virNetDaemonQuitExecRestart(virNetDaemon *dmn) { VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); - VIR_DEBUG("Quit requested %p", dmn); + VIR_DEBUG("Exec-restart requested %p", dmn); dmn->quit = true; + dmn->execRestart = true; +} + + +static void +virNetDaemonStopWorker(void *opaque) +{ + virNetDaemon *dmn = opaque; + + VIR_DEBUG("Begin stop dmn=%p", dmn); + + dmn->stopCb(); + + VIR_DEBUG("Completed stop dmn=%p", dmn); + + virNetDaemonQuit(dmn); + virObjectUnref(dmn); } void -virNetDaemonQuitExecRestart(virNetDaemon *dmn) +virNetDaemonStop(virNetDaemon *dmn) { VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); - VIR_DEBUG("Exec-restart requested %p", dmn); - dmn->quit = true; - dmn->execRestart = true; + if (!dmn->stopCb || + dmn->stateStopThread) + return; + + virObjectRef(dmn); + dmn->stateStopThread = g_new0(virThread, 1); + + if (virThreadCreateFull(dmn->stateStopThread, true, + virNetDaemonStopWorker, + "daemon-stop", false, dmn) < 0) { + virObjectUnref(dmn); + g_clear_pointer(&dmn->stateStopThread, g_free); + return; + } } @@ -XXX,XX +XXX,XX @@ virNetDaemonHasClients(virNetDaemon *dmn) void virNetDaemonSetLifecycleCallbacks(virNetDaemon *dmn, + virNetDaemonLifecycleCallback stopCb, virNetDaemonLifecycleCallback prepareCb, virNetDaemonLifecycleCallback waitCb) { VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); + VIR_DEBUG("Lifecycle callbacks stop=%p prepare=%p wait=%p", + stopCb, prepareCb, waitCb); + + /* Immutable once set */ + if (dmn->stopCb || dmn->shutdownPrepareCb || dmn->shutdownWaitCb) + return; + + dmn->stopCb = stopCb; dmn->shutdownPrepareCb = prepareCb; dmn->shutdownWaitCb = waitCb; } diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h index XXXXXXX..XXXXXXX 100644 --- a/src/rpc/virnetdaemon.h +++ b/src/rpc/virnetdaemon.h @@ -XXX,XX +XXX,XX @@ int virNetDaemonAddSignalHandler(virNetDaemon *dmn, void virNetDaemonUpdateServices(virNetDaemon *dmn, bool enabled); -void virNetDaemonSetStateStopWorkerThread(virNetDaemon *dmn, - virThread **thr); - void virNetDaemonRun(virNetDaemon *dmn); void virNetDaemonQuit(virNetDaemon *dmn); void virNetDaemonQuitExecRestart(virNetDaemon *dmn); +void virNetDaemonStop(virNetDaemon *dmn); bool virNetDaemonHasClients(virNetDaemon *dmn); @@ -XXX,XX +XXX,XX @@ bool virNetDaemonHasServer(virNetDaemon *dmn, typedef int (*virNetDaemonLifecycleCallback)(void); +/* + * @stopCb: preserves any active state on host shutdown / session exit + * @prepareCb: start shutting down daemon + * @waitCb: wait for shutdown completion + * + * This method may only be invoked once, the callbacks are immutable + * once set. + * + * On host shutdown (privileged) or session exit (unprivileged) + * the @stopCb will be invoked first. + * + * When the daemon shuts down, the sequence of operations is + * as follows + * + * - Listener stops accepting new clients + * - Existing clients are closed + * - Delay until @stopCb is complete (if still running) + * - @prepareCb invoked + * - Server worker pool is drained in background + * - @waitCb is invoked in background + * - Main loop terminates + */ void virNetDaemonSetLifecycleCallbacks(virNetDaemon *dmn, + virNetDaemonLifecycleCallback stopCb, virNetDaemonLifecycleCallback prepareCb, virNetDaemonLifecycleCallback waitCb); -- 2.48.1 The call to preserve state (ie running VMs) is triggered in response to the desktop session dbus terminating (session daemon), or logind sending a "PrepareForShutdown" signal. In the case of the latter, daemons should only save their state, not actually exit yet. Other things on the system may still expect the daemon to be running at this stage. Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/remote_daemon.c | 4 +++- src/rpc/virnetdaemon.c | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -XXX,XX +XXX,XX @@ handleSessionMessageFunc(GDBusConnection *connection G_GNUC_UNUSED, if (virGDBusMessageIsSignal(message, "org.freedesktop.DBus.Local", - "Disconnected")) + "Disconnected")) { virNetDaemonStop(dmn); + virNetDaemonQuit(dmn); + } return message; } diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -XXX,XX +XXX,XX @@ virNetDaemonStopWorker(void *opaque) VIR_DEBUG("Completed stop dmn=%p", dmn); - virNetDaemonQuit(dmn); virObjectUnref(dmn); } -- 2.48.1 The preserving of state (ie running VMs) requires a fully functional daemon and hypervisor driver. If any part has started shutting down then saving state may fail, or worse, hang. The current shutdown sequence does not guarantee safe ordering, as we synchronize with the state saving thread only after the hypervisor driver has had its 'shutdownPrepare' callback invoked. In the case of QEMU this means that worker threads processing monitor events may well have been stopped. This implements a full state machine that has a well defined ordering that an earlier commit documented as the desired semantics. With this change, nothing will start shutting down if the state saving thread is still running. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnetdaemon.c | 107 ++++++++++++++++++++++++++++++----------- 1 file changed, 80 insertions(+), 27 deletions(-) diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -XXX,XX +XXX,XX @@ VIR_LOG_INIT("rpc.netdaemon"); +/* The daemon shutdown process will step through + * each state listed below in the order they are + * declared. The 'STOPPING' state may be skipped + * over if the stateStopThread is not required + * for the particular shutdown scenari + */ +typedef enum { + VIR_NET_DAEMON_QUIT_NONE, /* Running normally */ + VIR_NET_DAEMON_QUIT_REQUESTED, /* Daemon shutdown requested */ + VIR_NET_DAEMON_QUIT_STOPPING, /* stateStopThread is running, so shutdown cannot request must be delayed */ + VIR_NET_DAEMON_QUIT_READY, /* Ready to initiate shutdown request by calling shutdownPrepareCb */ + VIR_NET_DAEMON_QUIT_WAITING, /* shutdownWaitCb is running, waiting for it to finished */ + VIR_NET_DAEMON_QUIT_COMPLETED, /* shutdownWaitCb is finished, event loop will now terminate */ +} virNetDaemonQuitPhase; + #ifndef WIN32 typedef struct _virNetDaemonSignal virNetDaemonSignal; struct _virNetDaemonSignal { @@ -XXX,XX +XXX,XX @@ struct _virNetDaemon { virNetDaemonLifecycleCallback shutdownPrepareCb; virNetDaemonLifecycleCallback shutdownWaitCb; virThread *stateStopThread; - int finishTimer; - bool quit; - bool finished; + int quitTimer; + virNetDaemonQuitPhase quit; bool graceful; bool execRestart; bool running; /* the daemon has reached the running phase */ @@ -XXX,XX +XXX,XX @@ virNetDaemonAutoShutdownTimer(int timerid G_GNUC_UNUSED, if (!dmn->autoShutdownInhibitions) { VIR_DEBUG("Automatic shutdown triggered"); - dmn->quit = true; + if (dmn->quit == VIR_NET_DAEMON_QUIT_NONE) { + VIR_DEBUG("Requesting daemon shutdown"); + dmn->quit = VIR_NET_DAEMON_QUIT_REQUESTED; + } } } @@ -XXX,XX +XXX,XX @@ daemonShutdownWait(void *opaque) bool graceful = false; virHashForEach(dmn->servers, daemonServerShutdownWait, NULL); - if (!dmn->shutdownWaitCb || dmn->shutdownWaitCb() >= 0) { - if (dmn->stateStopThread) - virThreadJoin(dmn->stateStopThread); - + if (!dmn->shutdownWaitCb || dmn->shutdownWaitCb() >= 0) graceful = true; - } VIR_WITH_OBJECT_LOCK_GUARD(dmn) { dmn->graceful = graceful; - virEventUpdateTimeout(dmn->finishTimer, 0); + dmn->quit = VIR_NET_DAEMON_QUIT_COMPLETED; + virEventUpdateTimeout(dmn->quitTimer, 0); + VIR_DEBUG("Shutdown wait completed graceful=%d", graceful); } } static void -virNetDaemonFinishTimer(int timerid G_GNUC_UNUSED, - void *opaque) +virNetDaemonQuitTimer(int timerid G_GNUC_UNUSED, + void *opaque) { virNetDaemon *dmn = opaque; VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); - dmn->finished = true; + dmn->quit = VIR_NET_DAEMON_QUIT_COMPLETED; + VIR_DEBUG("Shutdown wait timed out"); } @@ -XXX,XX +XXX,XX @@ virNetDaemonRun(virNetDaemon *dmn) goto cleanup; } - dmn->quit = false; - dmn->finishTimer = -1; - dmn->finished = false; + dmn->quit = VIR_NET_DAEMON_QUIT_NONE; + dmn->quitTimer = -1; dmn->graceful = false; dmn->running = true; @@ -XXX,XX +XXX,XX @@ virNetDaemonRun(virNetDaemon *dmn) virSystemdNotifyReady(); VIR_DEBUG("dmn=%p quit=%d", dmn, dmn->quit); - while (!dmn->finished) { + while (dmn->quit != VIR_NET_DAEMON_QUIT_COMPLETED) { virNetDaemonShutdownTimerUpdate(dmn); virObjectUnlock(dmn); @@ -XXX,XX +XXX,XX @@ virNetDaemonRun(virNetDaemon *dmn) virHashForEach(dmn->servers, daemonServerProcessClients, NULL); /* don't shutdown services when performing an exec-restart */ - if (dmn->quit && dmn->execRestart) + if (dmn->quit == VIR_NET_DAEMON_QUIT_REQUESTED && dmn->execRestart) goto cleanup; - if (dmn->quit && dmn->finishTimer == -1) { + if (dmn->quit == VIR_NET_DAEMON_QUIT_REQUESTED) { + VIR_DEBUG("Process quit request"); virHashForEach(dmn->servers, daemonServerClose, NULL); + + if (dmn->stateStopThread) { + VIR_DEBUG("State stop thread running"); + dmn->quit = VIR_NET_DAEMON_QUIT_STOPPING; + } else { + VIR_DEBUG("Ready to shutdown"); + dmn->quit = VIR_NET_DAEMON_QUIT_READY; + } + } + + if (dmn->quit == VIR_NET_DAEMON_QUIT_READY) { + VIR_DEBUG("Starting shutdown, running prepare"); if (dmn->shutdownPrepareCb && dmn->shutdownPrepareCb() < 0) break; - if ((dmn->finishTimer = virEventAddTimeout(30 * 1000, - virNetDaemonFinishTimer, - dmn, NULL)) < 0) { + if ((dmn->quitTimer = virEventAddTimeout(30 * 1000, + virNetDaemonQuitTimer, + dmn, NULL)) < 0) { VIR_WARN("Failed to register finish timer."); break; } @@ -XXX,XX +XXX,XX @@ virNetDaemonRun(virNetDaemon *dmn) VIR_WARN("Failed to register join thread."); break; } + + VIR_DEBUG("Waiting for shutdown completion"); + dmn->quit = VIR_NET_DAEMON_QUIT_WAITING; } } @@ -XXX,XX +XXX,XX @@ virNetDaemonQuit(virNetDaemon *dmn) VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); VIR_DEBUG("Quit requested %p", dmn); - dmn->quit = true; + if (dmn->quit == VIR_NET_DAEMON_QUIT_NONE) + dmn->quit = VIR_NET_DAEMON_QUIT_REQUESTED; } @@ -XXX,XX +XXX,XX @@ virNetDaemonQuitExecRestart(virNetDaemon *dmn) VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); VIR_DEBUG("Exec-restart requested %p", dmn); - dmn->quit = true; + if (dmn->quit == VIR_NET_DAEMON_QUIT_NONE) + dmn->quit = VIR_NET_DAEMON_QUIT_REQUESTED; dmn->execRestart = true; } @@ -XXX,XX +XXX,XX @@ virNetDaemonStopWorker(void *opaque) VIR_DEBUG("Completed stop dmn=%p", dmn); + VIR_WITH_OBJECT_LOCK_GUARD(dmn) { + if (dmn->quit == VIR_NET_DAEMON_QUIT_STOPPING) { + VIR_DEBUG("Marking shutdown as ready"); + dmn->quit = VIR_NET_DAEMON_QUIT_READY; + } + g_clear_pointer(&dmn->stateStopThread, g_free); + } + + VIR_DEBUG("End stop dmn=%p", dmn); virObjectUnref(dmn); } @@ -XXX,XX +XXX,XX @@ void virNetDaemonStop(virNetDaemon *dmn) { VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); + VIR_DEBUG("State stoprequest"); - if (!dmn->stopCb || - dmn->stateStopThread) + if (!dmn->stopCb) { + VIR_DEBUG("No stop callback registered"); return; + } + if (dmn->stateStopThread) { + VIR_DEBUG("State stop thread already running"); + return; + } + + if (dmn->quit != VIR_NET_DAEMON_QUIT_NONE) { + VIR_WARN("Already initiated shutdown sequence, unable to stop state"); + return; + } virObjectRef(dmn); dmn->stateStopThread = g_new0(virThread, 1); - if (virThreadCreateFull(dmn->stateStopThread, true, + if (virThreadCreateFull(dmn->stateStopThread, false, virNetDaemonStopWorker, "daemon-stop", false, dmn) < 0) { virObjectUnref(dmn); -- 2.48.1 The daemons are wired up to shutdown in responsible to UNIX process signals, as well as in response to login1 dbus signals, or loss of desktop session. The latter two options can optionally preserve state (ie running VMs). In non-systemd environments, as well as for testing, it would be useful to have a way to trigger shutdown with state preservation more directly. Thus a new admin protocol API is introduced virAdmConnectDaemonShutdown which will trigger a daemon shutdown, and preserve running VMs if the VIR_DAEMON_SHUTDOWN_PRESERVE flag is set. It has a corresponding 'virt-admin daemon-shutdown [--preserve]' command binding. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/manpages/virt-admin.rst | 13 +++++++++ include/libvirt/libvirt-admin.h | 13 +++++++++ src/admin/admin_protocol.x | 11 +++++++- src/admin/admin_server_dispatch.c | 13 +++++++++ src/admin/libvirt-admin.c | 33 +++++++++++++++++++++++ src/admin/libvirt_admin_public.syms | 5 ++++ tools/virt-admin.c | 41 +++++++++++++++++++++++++++++ 7 files changed, 128 insertions(+), 1 deletion(-) diff --git a/docs/manpages/virt-admin.rst b/docs/manpages/virt-admin.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/manpages/virt-admin.rst +++ b/docs/manpages/virt-admin.rst @@ -XXX,XX +XXX,XX @@ Sets the daemon timeout to the value of '--timeout' argument. Use ``--timeout 0` to disable auto-shutdown of the daemon. +daemon-shutdown +--------------- + +**Syntax:** + +:: + + daemon-shutdown [--preserve] + +Instruct the daemon to exit gracefully. If the ``--preserve`` flag is given, +it will save state in the same manner that would be done on a host OS shutdown +(privileged daemons) or a login session quit (unprivileged daemons). + SERVER COMMANDS =============== diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index XXXXXXX..XXXXXXX 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -XXX,XX +XXX,XX @@ int virAdmConnectSetDaemonTimeout(virAdmConnectPtr conn, unsigned int timeout, unsigned int flags); +/** + * virAdmConnectDaemonShutdownFlags: + * + * Since: 11.2.0 + */ +typedef enum { + /* Preserve state before shutting down daemon (Since: 11.2.0) */ + VIR_DAEMON_SHUTDOWN_PRESERVE = (1 << 0), +} virAdmConnectDaemonShutdownFlags; + +int virAdmConnectDaemonShutdown(virAdmConnectPtr conn, + unsigned int flags); + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index XXXXXXX..XXXXXXX 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -XXX,XX +XXX,XX @@ struct admin_connect_set_daemon_timeout_args { unsigned int flags; }; +struct admin_connect_daemon_shutdown_args { + unsigned int flags; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -XXX,XX +XXX,XX @@ enum admin_procedure { /** * @generate: both */ - ADMIN_PROC_CONNECT_SET_DAEMON_TIMEOUT = 19 + ADMIN_PROC_CONNECT_SET_DAEMON_TIMEOUT = 19, + + /** + * @generate: both + */ + ADMIN_PROC_CONNECT_DAEMON_SHUTDOWN = 20 }; diff --git a/src/admin/admin_server_dispatch.c b/src/admin/admin_server_dispatch.c index XXXXXXX..XXXXXXX 100644 --- a/src/admin/admin_server_dispatch.c +++ b/src/admin/admin_server_dispatch.c @@ -XXX,XX +XXX,XX @@ adminConnectSetDaemonTimeout(virNetDaemon *dmn, return virNetDaemonAutoShutdown(dmn, timeout); } +static int +adminConnectDaemonShutdown(virNetDaemon *dmn, + unsigned int flags) +{ + virCheckFlags(VIR_DAEMON_SHUTDOWN_PRESERVE, -1); + + if (flags & VIR_DAEMON_SHUTDOWN_PRESERVE) + virNetDaemonStop(dmn); + + virNetDaemonQuit(dmn); + + return 0; +} static int adminDispatchConnectGetLoggingOutputs(virNetServer *server G_GNUC_UNUSED, diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c index XXXXXXX..XXXXXXX 100644 --- a/src/admin/libvirt-admin.c +++ b/src/admin/libvirt-admin.c @@ -XXX,XX +XXX,XX @@ virAdmConnectSetDaemonTimeout(virAdmConnectPtr conn, return ret; } + + +/** + * virAdmConnectDaemonShutdown: + * @conn: pointer to an active admin connection + * @flags: optional extra falgs + * + * Trigger shutdown of the daemon, if @flags includes + * VIR_DAEMON_SHUTDOWN_PRESERVE then state will be + * preserved before shutting down + * + * Returns 0 on success, -1 on error. + * + * Since: 11.2.0 + */ +int +virAdmConnectDaemonShutdown(virAdmConnectPtr conn, + unsigned int flags) +{ + int ret; + + VIR_DEBUG("conn=%p, flags=0x%x", conn, flags); + + virResetLastError(); + virCheckAdmConnectReturn(conn, -1); + + if ((ret = remoteAdminConnectDaemonShutdown(conn, flags)) < 0) { + virDispatchError(NULL); + return -1; + } + + return ret; +} diff --git a/src/admin/libvirt_admin_public.syms b/src/admin/libvirt_admin_public.syms index XXXXXXX..XXXXXXX 100644 --- a/src/admin/libvirt_admin_public.syms +++ b/src/admin/libvirt_admin_public.syms @@ -XXX,XX +XXX,XX @@ LIBVIRT_ADMIN_8.6.0 { global: virAdmConnectSetDaemonTimeout; } LIBVIRT_ADMIN_3.0.0; + +LIBVIRT_ADMIN_11.2.0 { + global: + virAdmConnectDaemonShutdown; +} LIBVIRT_ADMIN_8.6.0; diff --git a/tools/virt-admin.c b/tools/virt-admin.c index XXXXXXX..XXXXXXX 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -XXX,XX +XXX,XX @@ cmdDaemonTimeout(vshControl *ctl, const vshCmd *cmd) } +/* -------------------------- + * Command daemon-shutdown + * -------------------------- + */ +static const vshCmdInfo info_daemon_shutdown = { + .help = N_("stop the daemon"), + .desc = N_("stop the daemon"), +}; + +static const vshCmdOptDef opts_daemon_shutdown[] = { + {.name = "preserve", + .type = VSH_OT_BOOL, + .required = false, + .positional = false, + .help = N_("preserve state before shutting down"), + }, + {.name = NULL} +}; + +static bool +cmdDaemonShutdown(vshControl *ctl, const vshCmd *cmd) +{ + vshAdmControl *priv = ctl->privData; + unsigned int flags = 0; + + if (vshCommandOptBool(cmd, "preserve")) + flags |= VIR_DAEMON_SHUTDOWN_PRESERVE; + + if (virAdmConnectDaemonShutdown(priv->conn, flags) < 0) + return false; + + return true; +} + + static void * vshAdmConnectionHandler(vshControl *ctl) { @@ -XXX,XX +XXX,XX @@ static const vshCmdDef managementCmds[] = { .info = &info_daemon_timeout, .flags = 0 }, + {.name = "daemon-shutdown", + .handler = cmdDaemonShutdown, + .opts = opts_daemon_shutdown, + .info = &info_daemon_shutdown, + .flags = 0 + }, {.name = NULL} }; -- 2.48.1 The service unit "TimeoutStopSec" setting controls how long systemd waits for a service to stop before aggressively killing it, defaulting to 30 seconds if not set. When we're processing shutdown of VMs in response to OS shutdown, we very likely need more than 30 seconds to complete this job, and can not stop the daemon during this time. To avoid being prematurely killed, setup a timer that repeatedly extends the "TimeoutStopSec" value while stop of running VMs is arranged. This does mean if libvirt hangs while stoppping VMs, systemd won't get to kill the libvirt daemon, but this is considered less harmful that forcefully killing running VMs. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnetdaemon.c | 53 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index XXXXXXX..XXXXXXX 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -XXX,XX +XXX,XX @@ struct _virNetDaemon { virNetDaemonLifecycleCallback shutdownPrepareCb; virNetDaemonLifecycleCallback shutdownWaitCb; virThread *stateStopThread; + int stopTimer; int quitTimer; virNetDaemonQuitPhase quit; bool graceful; @@ -XXX,XX +XXX,XX @@ struct _virNetDaemon { static virClass *virNetDaemonClass; +/* + * When running state stop operation which can be slow... + * + * How frequently we tell systemd to extend our stop time, + * and how much we ask for each time. The latter should + * exceed the former with a decent tolerance for high load + * scenarios + */ +#define VIR_NET_DAEMON_STOP_EXTEND_INTERVAL_MSEC (5 * 1000) +#define VIR_NET_DAEMON_STOP_EXTRA_TIME_SEC 10 + +/* + * When running daemon shutdown synchronization which + * ought to be moderately fast + */ +#define VIR_NET_DAEMON_SHUTDOWN_TIMEOUT_SEC 30 +#define VIR_NET_DAEMON_SHUTDOWN_TIMEOUT_MSEC (VIR_NET_DAEMON_SHUTDOWN_TIMEOUT_SEC * 1000) + + static int daemonServerClose(void *payload, const char *key G_GNUC_UNUSED, @@ -XXX,XX +XXX,XX @@ virNetDaemonNew(void) if (virEventRegisterDefaultImpl() < 0) goto error; + dmn->stopTimer = -1; dmn->autoShutdownTimerID = -1; #ifndef WIN32 @@ -XXX,XX +XXX,XX @@ daemonShutdownWait(void *opaque) } } +static void +virNetDaemonStopTimer(int timerid G_GNUC_UNUSED, + void *opaque) +{ + virNetDaemon *dmn = opaque; + VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); + + if (dmn->quit != VIR_NET_DAEMON_QUIT_STOPPING) + return; + + VIR_DEBUG("Extending stop timeout %u", + VIR_NET_DAEMON_STOP_EXTRA_TIME_SEC); + + virSystemdNotifyExtendTimeout(VIR_NET_DAEMON_STOP_EXTRA_TIME_SEC); +} + + static void virNetDaemonQuitTimer(int timerid G_GNUC_UNUSED, void *opaque) @@ -XXX,XX +XXX,XX @@ virNetDaemonRun(virNetDaemon *dmn) if (dmn->quit == VIR_NET_DAEMON_QUIT_REQUESTED) { VIR_DEBUG("Process quit request"); + virSystemdNotifyStopping(); virHashForEach(dmn->servers, daemonServerClose, NULL); if (dmn->stateStopThread) { VIR_DEBUG("State stop thread running"); dmn->quit = VIR_NET_DAEMON_QUIT_STOPPING; + virSystemdNotifyExtendTimeout(VIR_NET_DAEMON_STOP_EXTRA_TIME_SEC); + if ((dmn->stopTimer = virEventAddTimeout(VIR_NET_DAEMON_STOP_EXTEND_INTERVAL_MSEC, + virNetDaemonStopTimer, + dmn, NULL)) < 0) { + VIR_WARN("Failed to register stop timer"); + /* hope for the best */ + } } else { VIR_DEBUG("Ready to shutdown"); dmn->quit = VIR_NET_DAEMON_QUIT_READY; @@ -XXX,XX +XXX,XX @@ virNetDaemonRun(virNetDaemon *dmn) if (dmn->shutdownPrepareCb && dmn->shutdownPrepareCb() < 0) break; - if ((dmn->quitTimer = virEventAddTimeout(30 * 1000, + virSystemdNotifyExtendTimeout(VIR_NET_DAEMON_SHUTDOWN_TIMEOUT_SEC); + if ((dmn->quitTimer = virEventAddTimeout(VIR_NET_DAEMON_SHUTDOWN_TIMEOUT_MSEC, virNetDaemonQuitTimer, dmn, NULL)) < 0) { VIR_WARN("Failed to register finish timer."); @@ -XXX,XX +XXX,XX @@ virNetDaemonStopWorker(void *opaque) dmn->quit = VIR_NET_DAEMON_QUIT_READY; } g_clear_pointer(&dmn->stateStopThread, g_free); + if (dmn->stopTimer != -1) { + virEventRemoveTimeout(dmn->stopTimer); + dmn->stopTimer = -1; + } } VIR_DEBUG("End stop dmn=%p", dmn); -- 2.48.1 Since processing running VMs on OS shutdown can take a while, it is beneficial to send systemd status messages about the progress. The systemd status is a point-in-time message, with no ability to look at the history of received messages. So in the systemd status we include the progress information. For the same reason there is no benefit in sending failure messages, as they'll disappear as soon as a status is sent for the subsequent VM to be processed. The libvirt log statements can be viewed as a complete log record so don't need progress info, but do include warnings about failures (present from earlier commits). Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/hypervisor/domain_driver.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index XXXXXXX..XXXXXXX 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -XXX,XX +XXX,XX @@ #include "datatypes.h" #include "driver.h" #include "virlog.h" +#include "virsystemd.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) (!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 @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) } 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("Unable to perform managed save of '%s': %s", virDomainGetName(domains[i]), @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) (!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("Unable to request graceful shutdown of '%s': %s", virDomainGetName(domains[i]), @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) } timer = g_timer_new(); + virSystemdNotifyStatus("Waiting %d secs for VM shutdown completion", + cfg->waitShutdownSecs); + VIR_INFO("Waiting %d secs for VM shutdown completion", cfg->waitShutdownSecs); while (1) { bool anyRunning = false; for (i = 0; i < numDomains; i++) { @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) (!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, @@ -XXX,XX +XXX,XX @@ virDomainDriverAutoShutdown(virDomainDriverAutoShutdownConfig *cfg) } } + virSystemdNotifyStatus("Processed %d domains", numDomains); + VIR_INFO("Processed %d domains", numDomains); + cleanup: if (domains) { /* Anything non-NULL in this list indicates none of -- 2.48.1