On Thu, Jan 30, 2025 at 05:44:08PM +0100, Peter Krempa wrote:
On Wed, Jan 08, 2025 at 19:42:43 +0000, Daniel P. Berrangé wrote:
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 ea3f1cbfcd..4fecaf7e5c 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -35,6 +35,13 @@
VIR_LOG_INIT("hypervisor.domain_driver");
+VIR_ENUM_IMPL(virDomainDriverAutoShutdownScope, + VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_LAST, + "none", + "persistent", + "transient", + "all");
since we have the 'ToString' function ...
+ char * virDomainDriverGenerateRootHash(const char *drivername, const char *root) @@ -715,6 +722,7 @@ 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",
... consider logging the string here instead of the number.
@@ -735,6 +743,12 @@ 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;
@@ -744,10 +758,22 @@ 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 @@ -773,11 +799,16 @@ 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;
Trying to 'managedSave' a transient VM will/should fail as managedSave doesn't make sense with transient VMS:
Hmmm, true, we should reject 'trySave == transient' right at the start. Also if virDomainManagedSave() fails, we need to add a call to virDomainResume, otherwise the fallthrough to virDomainShutdown is useless as the VM will be paused still.
+ if (virDomainShutdown(domains[i]) < 0) { VIR_WARN("Unable to perform graceful shutdown of '%s': %s", virDomainGetName(domains[i]),
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|