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");
+
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",
@@ -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;
+
if (virDomainShutdown(domains[i]) < 0) {
VIR_WARN("Unable to perform graceful shutdown of '%s': %s",
virDomainGetName(domains[i]),
@@ -793,6 +824,10 @@ 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 {
@@ -810,10 +845,15 @@ 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 25c4b0c664..acb7a41b5d 100644
--- a/src/hypervisor/domain_driver.h
+++ b/src/hypervisor/domain_driver.h
@@ -24,6 +24,7 @@
#include "virhostdev.h"
#include "virpci.h"
#include "virdomainobjlist.h"
+#include "virenum.h"
char *
virDomainDriverGenerateRootHash(const char *drivername,
@@ -90,11 +91,22 @@ 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 781bb566d2..301240452d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1641,6 +1641,8 @@ 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 2c97a6fb98..8e15a77be2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -947,9 +947,9 @@ 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.1On 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:
static int
qemuDomainManagedSaveHelper(virQEMUDriver *driver,
virDomainObj *vm,
const char *dxml,
unsigned int flags)
{
g_autoptr(virQEMUDriverConfig) cfg = NULL;
g_autoptr(virCommand) compressor = NULL;
g_autofree char *path = NULL;
int format;
if (virDomainObjCheckActive(vm) < 0)
return -1;
if (!vm->persistent) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("cannot do managed save for transient domain"));
return -1;
}
> +
> 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>
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 :|
© 2016 - 2026 Red Hat, Inc.