Re: [PATCH v2 05/22] qemu: support automatic VM managed save in system daemon

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 12, 2025 at 05:17:45PM +0000, Daniel P. Berrangé wrote:
> 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@xxxxxxxxxx>
> ---
>  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/qemu_conf.c b/src/qemu/qemu_conf.c
> index b376841388..8554558201 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c

[...]

> @@ -627,6 +642,8 @@ 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;
> @@ -663,6 +680,54 @@ 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)

I wanted to note the we would leak memory here when reusing the same
string but virConfGetValueString frees the value before assigning new
data to it. But it doesn't touch the variable if the config option is
missing so the next check for NULL can be invalid if
auto_shutdown_try_save is set but auto_shutdown_try_shutdown is not set.

> +        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;

Same here.

> +    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;
>  }

Pavel

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux