[PATCH 11/26] qemu: support automatic VM managed save in system daemon

Daniel P. Berrangé posted 26 patches 1 year, 1 month ago
There is a newer version of this series
[PATCH 11/26] qemu: support automatic VM managed save in system daemon
Posted by Daniel P. Berrangé 1 year, 1 month ago
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 = "all"
  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@redhat.com>
---
 src/qemu/libvirtd_qemu.aug         |  3 ++
 src/qemu/qemu.conf.in              | 37 ++++++++++++++++++++++
 src/qemu/qemu_conf.c               | 51 ++++++++++++++++++++++++++++++
 src/qemu/qemu_conf.h               |  3 ++
 src/qemu/qemu_driver.c             |  9 +++---
 src/qemu/test_libvirtd_qemu.aug.in |  3 ++
 6 files changed, 101 insertions(+), 5 deletions(-)

diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 642093c40b..e465a231fc 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -98,6 +98,9 @@ module Libvirtd_qemu =
                  | bool_entry "auto_dump_bypass_cache"
                  | bool_entry "auto_start_bypass_cache"
                  | int_entry "auto_start_delay"
+                 | str_entry "auto_shutdown_try_save"
+                 | str_entry "auto_shutdown_try_shutdown"
+                 | str_entry "auto_shutdown_powerdown"
 
    let process_entry = str_entry "hugetlbfs_mount"
                  | str_entry "bridge_helper"
diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in
index a3e9bbfcf3..d8890c4c32 100644
--- a/src/qemu/qemu.conf.in
+++ b/src/qemu/qemu.conf.in
@@ -638,6 +638,43 @@
 #
 #auto_start_delay = 0
 
+# Whether to perform managed save of running VMs if a host OS
+# shutdown is requested (system/session daemons), or the desktop
+# session terminates (session daemon only). Accepts
+#
+# * "none" - do not try to save any running VMs
+# * "persistent" - only try to save persistent running VMs
+# * "transient" - only try to save transient running VMs
+# * "all" - try to save all running VMs
+#
+# Defaults to "all" for session daemons and "none"
+# for system daemons
+#
+# If 'libvirt-guests.service' is enabled, then this must be
+# set to 'none' for system daemons to avoid dueling actions
+#auto_shutdown_try_save = "all"
+
+# As above, but with a graceful shutdown action instead of
+# managed save. If managed save is enabled, shutdown will
+# be tried only on failure to perform managed save.
+#
+# Defaults to "none"
+#
+# If 'libvirt-guests.service' is enabled, then this must be
+# set to 'none' for system daemons to avoid dueling actions
+#auto_shutdown_try_shutdown = "none"
+
+# As above, but with a forced poweroff instead of managed
+# save. If managed save or graceful shutdown are enabled,
+# forced poweroff will be tried only on failure of the
+# other options.
+#
+# Defaults to "none"
+#
+# If 'libvirt-guests.service' is enabled, then this must be
+# set to 'none' for system daemons to avoid dueling actions
+#auto_shutdown_powerdown = "none"
+
 # If provided by the host and a hugetlbfs mount point is configured,
 # a guest may request huge page backing.  When this mount point is
 # unspecified here, determination of a host mount point in /proc/mounts
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 0b6b923bcb..a57eccf569 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -303,6 +303,21 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
     cfg->dumpGuestCore = true;
 #endif
 
+    if (privileged) {
+        /*
+         * Defer to libvirt-guests.service.
+         *
+         * XXX, or query if libvirt-guests.service is enabled perhaps ?
+         */
+        cfg->autoShutdownTrySave = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE;
+        cfg->autoShutdownTryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE;
+        cfg->autoShutdownPoweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE;
+    } else {
+        cfg->autoShutdownTrySave = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL;
+        cfg->autoShutdownTryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE;
+        cfg->autoShutdownPoweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE;
+    }
+
     return g_steal_pointer(&cfg);
 }
 
@@ -626,6 +641,8 @@ static int
 virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg,
                                  virConf *conf)
 {
+    g_autofree char *autoShutdownScope = NULL;
+
     if (virConfGetValueString(conf, "save_image_format", &cfg->saveImageFormat) < 0)
         return -1;
     if (virConfGetValueString(conf, "dump_image_format", &cfg->dumpImageFormat) < 0)
@@ -640,6 +657,40 @@ virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg,
         return -1;
     if (virConfGetValueInt(conf, "auto_start_delay", &cfg->autoStartDelayMS) < 0)
         return -1;
+    if (virConfGetValueString(conf, "auto_shutdown_try_save", &autoShutdownScope) < 0)
+        return -1;
+
+    if (autoShutdownScope != NULL &&
+        (cfg->autoShutdownTrySave =
+         virDomainDriverAutoShutdownScopeTypeFromString(autoShutdownScope)) < 0) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("unknown auto_shutdown_try_save '%1$s'"),
+                       autoShutdownScope);
+        return -1;
+    }
+
+    if (virConfGetValueString(conf, "auto_shutdown_try_shutdown", &autoShutdownScope) < 0)
+        return -1;
+
+    if (autoShutdownScope != NULL &&
+        (cfg->autoShutdownTryShutdown =
+         virDomainDriverAutoShutdownScopeTypeFromString(autoShutdownScope)) < 0) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("unknown auto_shutdown_try_shutdown '%1$s'"),
+                       autoShutdownScope);
+        return -1;
+    }
+    if (virConfGetValueString(conf, "auto_shutdown_poweroff", &autoShutdownScope) < 0)
+        return -1;
+
+    if (autoShutdownScope != NULL &&
+        (cfg->autoShutdownPoweroff =
+         virDomainDriverAutoShutdownScopeTypeFromString(autoShutdownScope)) < 0) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("unknown auto_shutdown_poweroff '%1$s'"),
+                       autoShutdownScope);
+        return -1;
+    }
 
     return 0;
 }
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 61a2bdce51..5d9ace6dcc 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -201,6 +201,9 @@ struct _virQEMUDriverConfig {
     bool autoDumpBypassCache;
     bool autoStartBypassCache;
     int autoStartDelayMS;
+    int autoShutdownTrySave;
+    int autoShutdownTryShutdown;
+    int autoShutdownPoweroff;
 
     char *lockManagerName;
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8e15a77be2..641b45fd8f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -947,13 +947,12 @@ qemuStateStop(void)
     g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(qemu_driver);
     virDomainDriverAutoShutdownConfig ascfg = {
         .uri = cfg->uri,
-        .trySave = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL,
-        .tryShutdown =  VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE,
-        .poweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE
+        .trySave = cfg->autoShutdownTrySave,
+        .tryShutdown = cfg->autoShutdownTryShutdown,
+        .poweroff = cfg->autoShutdownPoweroff,
     };
 
-    if (!qemu_driver->privileged)
-        virDomainDriverAutoShutdown(&ascfg);
+    virDomainDriverAutoShutdown(&ascfg);
 
     return 0;
 }
diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
index c2a1d7d829..e7137f686f 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -76,6 +76,9 @@ module Test_libvirtd_qemu =
 { "auto_dump_bypass_cache" = "0" }
 { "auto_start_bypass_cache" = "0" }
 { "auto_start_delay" = "0" }
+{ "auto_shutdown_try_save" = "all" }
+{ "auto_shutdown_try_shutdown" = "none" }
+{ "auto_shutdown_poweroff" = "none" }
 { "hugetlbfs_mount" = "/dev/hugepages" }
 { "bridge_helper" = "qemu-bridge-helper" }
 { "set_process_name" = "1" }
-- 
2.47.1
Re: [PATCH 11/26] qemu: support automatic VM managed save in system daemon
Posted by Peter Krempa 1 year ago
On Wed, Jan 08, 2025 at 19:42:44 +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 = "all"
>   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@redhat.com>
> ---
>  src/qemu/libvirtd_qemu.aug         |  3 ++
>  src/qemu/qemu.conf.in              | 37 ++++++++++++++++++++++
>  src/qemu/qemu_conf.c               | 51 ++++++++++++++++++++++++++++++
>  src/qemu/qemu_conf.h               |  3 ++
>  src/qemu/qemu_driver.c             |  9 +++---
>  src/qemu/test_libvirtd_qemu.aug.in |  3 ++
>  6 files changed, 101 insertions(+), 5 deletions(-)
> 
> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
> index 642093c40b..e465a231fc 100644
> --- a/src/qemu/libvirtd_qemu.aug
> +++ b/src/qemu/libvirtd_qemu.aug
> @@ -98,6 +98,9 @@ module Libvirtd_qemu =
>                   | bool_entry "auto_dump_bypass_cache"
>                   | bool_entry "auto_start_bypass_cache"
>                   | int_entry "auto_start_delay"
> +                 | str_entry "auto_shutdown_try_save"
> +                 | str_entry "auto_shutdown_try_shutdown"
> +                 | str_entry "auto_shutdown_powerdown"

Don't forget to merge in the appropriate hunk from the next patch to
make tests pass.

>  
>     let process_entry = str_entry "hugetlbfs_mount"
>                   | str_entry "bridge_helper"
> diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in
> index a3e9bbfcf3..d8890c4c32 100644
> --- a/src/qemu/qemu.conf.in
> +++ b/src/qemu/qemu.conf.in
> @@ -638,6 +638,43 @@
>  #
>  #auto_start_delay = 0
>  
> +# Whether to perform managed save of running VMs if a host OS
> +# shutdown is requested (system/session daemons), or the desktop
> +# session terminates (session daemon only). Accepts
> +#
> +# * "none" - do not try to save any running VMs
> +# * "persistent" - only try to save persistent running VMs
> +# * "transient" - only try to save transient running VMs
> +# * "all" - try to save all running VMs
> +#
> +# Defaults to "all" for session daemons and "none"
> +# for system daemons
> +#
> +# If 'libvirt-guests.service' is enabled, then this must be
> +# set to 'none' for system daemons to avoid dueling actions
> +#auto_shutdown_try_save = "all"

As demonstrated in previous patch, the 'transient' and 'all' make no
sense here, as all you'll get is an error per semantics of managed save.

The only way we could make saving of transient VMs work is to invoke the
normal save API and dump them in a directory configured for this reason.

But then you get problem of what to do at startup as IIUC the native
replacement to libvirt-guests will only support startup via 'autostart',
thus that'd overcomplicate things a bit more.

I'd suggest to simply don't do save for transient VMs.

> +
> +# As above, but with a graceful shutdown action instead of
> +# managed save. If managed save is enabled, shutdown will
> +# be tried only on failure to perform managed save.
> +#
> +# Defaults to "none"
> +#
> +# If 'libvirt-guests.service' is enabled, then this must be
> +# set to 'none' for system daemons to avoid dueling actions
> +#auto_shutdown_try_shutdown = "none"
> +
> +# As above, but with a forced poweroff instead of managed
> +# save. If managed save or graceful shutdown are enabled,
> +# forced poweroff will be tried only on failure of the
> +# other options.
> +#
> +# Defaults to "none"
> +#
> +# If 'libvirt-guests.service' is enabled, then this must be
> +# set to 'none' for system daemons to avoid dueling actions
> +#auto_shutdown_powerdown = "none"
> +
>  # If provided by the host and a hugetlbfs mount point is configured,
>  # a guest may request huge page backing.  When this mount point is
>  # unspecified here, determination of a host mount point in /proc/mounts
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 0b6b923bcb..a57eccf569 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -303,6 +303,21 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
>      cfg->dumpGuestCore = true;
>  #endif
>  
> +    if (privileged) {
> +        /*
> +         * Defer to libvirt-guests.service.
> +         *
> +         * XXX, or query if libvirt-guests.service is enabled perhaps ?
> +         */
> +        cfg->autoShutdownTrySave = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE;
> +        cfg->autoShutdownTryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE;
> +        cfg->autoShutdownPoweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE;
> +    } else {
> +        cfg->autoShutdownTrySave = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL;
> +        cfg->autoShutdownTryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE;
> +        cfg->autoShutdownPoweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE;
> +    }
> +
>      return g_steal_pointer(&cfg);
>  }
>  
> @@ -626,6 +641,8 @@ static int
>  virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg,
>                                   virConf *conf)
>  {
> +    g_autofree char *autoShutdownScope = NULL;
> +
>      if (virConfGetValueString(conf, "save_image_format", &cfg->saveImageFormat) < 0)
>          return -1;
>      if (virConfGetValueString(conf, "dump_image_format", &cfg->dumpImageFormat) < 0)
> @@ -640,6 +657,40 @@ virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg,
>          return -1;
>      if (virConfGetValueInt(conf, "auto_start_delay", &cfg->autoStartDelayMS) < 0)
>          return -1;
> +    if (virConfGetValueString(conf, "auto_shutdown_try_save", &autoShutdownScope) < 0)
> +        return -1;
> +
> +    if (autoShutdownScope != NULL &&
> +        (cfg->autoShutdownTrySave =
> +         virDomainDriverAutoShutdownScopeTypeFromString(autoShutdownScope)) < 0) {
> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("unknown auto_shutdown_try_save '%1$s'"),
> +                       autoShutdownScope);
> +        return -1;
> +    }

Here it'll need to reject the two unsupported options as well.

> +
> +    if (virConfGetValueString(conf, "auto_shutdown_try_shutdown", &autoShutdownScope) < 0)
> +        return -1;
> +
> +    if (autoShutdownScope != NULL &&
> +        (cfg->autoShutdownTryShutdown =
> +         virDomainDriverAutoShutdownScopeTypeFromString(autoShutdownScope)) < 0) {
> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("unknown auto_shutdown_try_shutdown '%1$s'"),
> +                       autoShutdownScope);
> +        return -1;
> +    }
> +    if (virConfGetValueString(conf, "auto_shutdown_poweroff", &autoShutdownScope) < 0)
> +        return -1;
> +
> +    if (autoShutdownScope != NULL &&
> +        (cfg->autoShutdownPoweroff =
> +         virDomainDriverAutoShutdownScopeTypeFromString(autoShutdownScope)) < 0) {
> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("unknown auto_shutdown_poweroff '%1$s'"),
> +                       autoShutdownScope);
> +        return -1;
> +    }
>  
>      return 0;
>  }
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index 61a2bdce51..5d9ace6dcc 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -201,6 +201,9 @@ struct _virQEMUDriverConfig {
>      bool autoDumpBypassCache;
>      bool autoStartBypassCache;
>      int autoStartDelayMS;
> +    int autoShutdownTrySave;
> +    int autoShutdownTryShutdown;
> +    int autoShutdownPoweroff;

Preferrably use the enum type, although that'll make the parser a bit
more verbose due to absence of something like virXMLPropEnum.

With saving of transient VMs rejected:

Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Re: [PATCH 11/26] qemu: support automatic VM managed save in system daemon
Posted by Daniel P. Berrangé 1 year ago
On Thu, Jan 30, 2025 at 06:21:07PM +0100, Peter Krempa wrote:
> On Wed, Jan 08, 2025 at 19:42:44 +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 = "all"
> >   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@redhat.com>
> > ---
> >  src/qemu/libvirtd_qemu.aug         |  3 ++
> >  src/qemu/qemu.conf.in              | 37 ++++++++++++++++++++++
> >  src/qemu/qemu_conf.c               | 51 ++++++++++++++++++++++++++++++
> >  src/qemu/qemu_conf.h               |  3 ++
> >  src/qemu/qemu_driver.c             |  9 +++---
> >  src/qemu/test_libvirtd_qemu.aug.in |  3 ++
> >  6 files changed, 101 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
> > index 642093c40b..e465a231fc 100644
> > --- a/src/qemu/libvirtd_qemu.aug
> > +++ b/src/qemu/libvirtd_qemu.aug
> > @@ -98,6 +98,9 @@ module Libvirtd_qemu =
> >                   | bool_entry "auto_dump_bypass_cache"
> >                   | bool_entry "auto_start_bypass_cache"
> >                   | int_entry "auto_start_delay"
> > +                 | str_entry "auto_shutdown_try_save"
> > +                 | str_entry "auto_shutdown_try_shutdown"
> > +                 | str_entry "auto_shutdown_powerdown"
> 
> Don't forget to merge in the appropriate hunk from the next patch to
> make tests pass.
> 
> >  
> >     let process_entry = str_entry "hugetlbfs_mount"
> >                   | str_entry "bridge_helper"
> > diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in
> > index a3e9bbfcf3..d8890c4c32 100644
> > --- a/src/qemu/qemu.conf.in
> > +++ b/src/qemu/qemu.conf.in
> > @@ -638,6 +638,43 @@
> >  #
> >  #auto_start_delay = 0
> >  
> > +# Whether to perform managed save of running VMs if a host OS
> > +# shutdown is requested (system/session daemons), or the desktop
> > +# session terminates (session daemon only). Accepts
> > +#
> > +# * "none" - do not try to save any running VMs
> > +# * "persistent" - only try to save persistent running VMs
> > +# * "transient" - only try to save transient running VMs
> > +# * "all" - try to save all running VMs
> > +#
> > +# Defaults to "all" for session daemons and "none"
> > +# for system daemons
> > +#
> > +# If 'libvirt-guests.service' is enabled, then this must be
> > +# set to 'none' for system daemons to avoid dueling actions
> > +#auto_shutdown_try_save = "all"
> 
> As demonstrated in previous patch, the 'transient' and 'all' make no
> sense here, as all you'll get is an error per semantics of managed save.
> 
> The only way we could make saving of transient VMs work is to invoke the
> normal save API and dump them in a directory configured for this reason.
> 
> But then you get problem of what to do at startup as IIUC the native
> replacement to libvirt-guests will only support startup via 'autostart',
> thus that'd overcomplicate things a bit more.
> 
> I'd suggest to simply don't do save for transient VMs.

Yes, I'm going to block 'all' and 'transient' and rephase
these inline config file docs.


> > +    if (virConfGetValueString(conf, "auto_shutdown_try_save", &autoShutdownScope) < 0)
> > +        return -1;
> > +
> > +    if (autoShutdownScope != NULL &&
> > +        (cfg->autoShutdownTrySave =
> > +         virDomainDriverAutoShutdownScopeTypeFromString(autoShutdownScope)) < 0) {
> > +        virReportError(VIR_ERR_INVALID_ARG,
> > +                       _("unknown auto_shutdown_try_save '%1$s'"),
> > +                       autoShutdownScope);
> > +        return -1;
> > +    }
> 
> Here it'll need to reject the two unsupported options as well.

yep


> > diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> > index 61a2bdce51..5d9ace6dcc 100644
> > --- a/src/qemu/qemu_conf.h
> > +++ b/src/qemu/qemu_conf.h
> > @@ -201,6 +201,9 @@ struct _virQEMUDriverConfig {
> >      bool autoDumpBypassCache;
> >      bool autoStartBypassCache;
> >      int autoStartDelayMS;
> > +    int autoShutdownTrySave;
> > +    int autoShutdownTryShutdown;
> > +    int autoShutdownPoweroff;
> 
> Preferrably use the enum type, although that'll make the parser a bit
> more verbose due to absence of something like virXMLPropEnum.

It is pretty easy actually, so done that too


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 :|
Re: [PATCH 11/26] qemu: support automatic VM managed save in system daemon
Posted by Peter Krempa 1 year ago
On Wed, Jan 08, 2025 at 19:42:44 +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 = "all"
>   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@redhat.com>
> ---
>  src/qemu/libvirtd_qemu.aug         |  3 ++

Proper review later, but for now this fails
'check-augeas-libvirtd_qemu':

Listing only the last 100 lines from a long log.
  { "migrate_tls_x509_verify" = "1" }
  { "migrate_tls_x509_secret_uuid" = "00000000-0000-0000-0000-000000000000" }
  { "migrate_tls_force" = "0" }
  { "backup_tls_x509_cert_dir" = "/etc/pki/libvirt-backup" }
  { "backup_tls_x509_verify" = "1" }
  { "backup_tls_x509_secret_uuid" = "00000000-0000-0000-0000-000000000000" }
  { "nographics_allow_host_audio" = "1" }
  { "remote_display_port_min" = "5900" }
  { "remote_display_port_max" = "65535" }
  { "remote_websocket_port_min" = "5700" }
  { "remote_websocket_port_max" = "65535" }
  { "security_driver" = "selinux" }
  { "security_default_confined" = "1" }
  { "security_require_confined" = "1" }
  { "user" = "qemu" }
  { "group" = "qemu" }
  { "dynamic_ownership" = "1" }
  { "remember_owner" = "1" }
  { "cgroup_controllers"
    { "1" = "cpu" }
    { "2" = "devices" }
    { "3" = "memory" }
    { "4" = "blkio" }
    { "5" = "cpuset" }
    { "6" = "cpuacct" }
  }
  { "cgroup_device_acl"
    { "1" = "/dev/null" }
    { "2" = "/dev/full" }
    { "3" = "/dev/zero" }
    { "4" = "/dev/random" }
    { "5" = "/dev/urandom" }
    { "6" = "/dev/ptmx" }
    { "7" = "/dev/kvm" }
    { "8" = "/dev/userfaultfd" }
  }
  { "save_image_format" = "raw" }
  { "dump_image_format" = "raw" }
  { "snapshot_image_format" = "raw" }
  { "auto_dump_path" = "/var/lib/libvirt/qemu/dump" }
  { "auto_dump_bypass_cache" = "0" }
  { "auto_start_bypass_cache" = "0" }
  { "auto_start_delay" = "0" }
  { "auto_shutdown_try_save" = "all" }
  { "auto_shutdown_try_shutdown" = "none" }
  { "auto_shutdown_powerdown" = "none" }
  { "hugetlbfs_mount" = "/dev/hugepages" }
  { "bridge_helper" = "qemu-bridge-helper" }
  { "set_process_name" = "1" }
  { "max_processes" = "0" }
  { "max_files" = "0" }
  { "max_threads_per_process" = "0" }
  { "max_core" = "unlimited" }
  { "dump_guest_core" = "1" }
  { "mac_filter" = "1" }
  { "relaxed_acs_check" = "1" }
  { "lock_manager" = "lockd" }
  { "max_queued" = "0" }
  { "keepalive_interval" = "5" }
  { "keepalive_count" = "5" }
  { "seccomp_sandbox" = "1" }
  { "migration_address" = "0.0.0.0" }
  { "migration_host" = "host.example.com" }
  { "migration_port_min" = "49152" }
  { "migration_port_max" = "49215" }
  { "log_timestamp" = "0" }
  { "nvram"
    { "1" = "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd" }
    { "2" = "/usr/share/OVMF/OVMF_CODE.secboot.fd:/usr/share/OVMF/OVMF_VARS.fd" }
    { "3" = "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd" }
    { "4" = "/usr/share/AAVMF/AAVMF32_CODE.fd:/usr/share/AAVMF/AAVMF32_VARS.fd" }
  }
  { "stdio_handler" = "logd" }
  { "gluster_debug_level" = "9" }
  { "virtiofsd_debug" = "1" }
  { "namespaces"
    { "1" = "mount" }
  }
  { "memory_backing_dir" = "/var/lib/libvirt/qemu/ram" }
  { "pr_helper" = "qemu-pr-helper" }
  { "slirp_helper" = "/usr/bin/slirp-helper" }
  { "dbus_daemon" = "dbus-daemon" }
  { "swtpm_user" = "tss" }
  { "swtpm_group" = "tss" }
  { "capability_filters"
    { "1" = "capname" }
  }
  { "deprecation_behavior" = "none" }
  { "sched_core" = "none" }
  { "storage_use_nbdkit" = "0" }
  { "shared_filesystems"
    { "1" = "/path/to/images" }
    { "2" = "/path/to/nvram" }
    { "3" = "/path/to/swtpm" }
  }
}

stderr:
Syntax error in lens definition
Failed to load /home/pipo/build/libvirt/gcc/src/test_libvirtd_qemu.aug
Re: [PATCH 11/26] qemu: support automatic VM managed save in system daemon
Posted by Peter Krempa 1 year ago
On Wed, Jan 29, 2025 at 17:24:47 +0100, Peter Krempa wrote:
> On Wed, Jan 08, 2025 at 19:42:44 +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 = "all"
> >   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@redhat.com>
> > ---
> >  src/qemu/libvirtd_qemu.aug         |  3 ++
> 
> Proper review later, but for now this fails
> 'check-augeas-libvirtd_qemu':

It fixed itself after applying next patch so some hunks will be
misplaced.