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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
