Re: [PATCH 10/26] hypervisor: custom shutdown actions for transient vs persistent VMs

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(a)redhat.com&gt; > --- > 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:

> + > if (virDomainShutdown(domains[i]) < 0) { > VIR_WARN("Unable to perform graceful shutdown of '%s': %s", > virDomainGetName(domains[i]), Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt;