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

Daniel P. Berrangé posted 22 patches 2 months, 1 week ago
[PATCH v2 05/22] qemu: support automatic VM managed save in system daemon
Posted by Daniel P. Berrangé 2 months, 1 week 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 = "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@redhat.com>
---
 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/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 642093c40b..605604a01a 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_poweroff"
 
    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 31172303dc..a674e11388 100644
--- a/src/qemu/qemu.conf.in
+++ b/src/qemu/qemu.conf.in
@@ -639,6 +639,48 @@
 #
 #auto_start_delay = 0
 
+# The settings for auto shutdown actions accept one of
+# four possible options:
+#
+# * "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
+#
+# 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).
+#
+# Defaults to "persistent" for session daemons and "none"
+# for system daemons. The values "all" and "transient" are
+# not permitted for this setting, since managed save is not
+# implemented for transient VMs.
+#
+# If 'libvirt-guests.service' is enabled, then this must be
+# set to 'none' for system daemons to avoid dueling actions
+#auto_shutdown_try_save = "persistent"
+
+# 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_poweroff = "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 b376841388..8554558201 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -304,6 +304,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_PERSISTENT;
+        cfg->autoShutdownTryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE;
+        cfg->autoShutdownPoweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_NONE;
+    }
+
     return g_steal_pointer(&cfg);
 }
 
@@ -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)
+        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;
+
+    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;
 }
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 3e1b41af73..3450f277f0 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -43,6 +43,7 @@
 #include "virfilecache.h"
 #include "virfirmware.h"
 #include "virinhibitor.h"
+#include "domain_driver.h"
 
 #define QEMU_DRIVER_NAME "QEMU"
 
@@ -201,6 +202,9 @@ struct _virQEMUDriverConfig {
     bool autoDumpBypassCache;
     bool autoStartBypassCache;
     unsigned int autoStartDelayMS;
+    virDomainDriverAutoShutdownScope autoShutdownTrySave;
+    virDomainDriverAutoShutdownScope autoShutdownTryShutdown;
+    virDomainDriverAutoShutdownScope autoShutdownPoweroff;
 
     char *lockManagerName;
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ec389453e7..cb7b03e391 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_PERSISTENT,
-        .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..bb216483a7 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" = "persistent" }
+{ "auto_shutdown_try_shutdown" = "none" }
+{ "auto_shutdown_poweroff" = "none" }
 { "hugetlbfs_mount" = "/dev/hugepages" }
 { "bridge_helper" = "qemu-bridge-helper" }
 { "set_process_name" = "1" }
-- 
2.48.1
Re: [PATCH v2 05/22] qemu: support automatic VM managed save in system daemon
Posted by Pavel Hrdina 2 months, 1 week ago
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@redhat.com>
> ---
>  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
Re: [PATCH v2 05/22] qemu: support automatic VM managed save in system daemon
Posted by Daniel P. Berrangé 2 months, 1 week ago
On Thu, Mar 13, 2025 at 01:31:41PM +0100, Pavel Hrdina wrote:
> 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@redhat.com>
> > ---
> >  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.

Doh, yes, we need g_clear_pointer(&autoShutdownScope, g_free) before
re-fetching the next value, or perhaps better just stop overloading
one variable for fetching multiple config parameter

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



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 v2 05/22] qemu: support automatic VM managed save in system daemon
Posted by Peter Krempa 2 months, 1 week ago
On Wed, Mar 12, 2025 at 17:17:45 +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@redhat.com>
> ---
>  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.in b/src/qemu/qemu.conf.in
> index 31172303dc..a674e11388 100644
> --- a/src/qemu/qemu.conf.in
> +++ b/src/qemu/qemu.conf.in
> @@ -639,6 +639,48 @@
>  #
>  #auto_start_delay = 0
>  
> +# The settings for auto shutdown actions accept one of
> +# four possible options:
> +#
> +# * "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
> +#
I'd suggest you change the commented-out empty line here to uncommented,
to separate the docs for auto_shutdown_try_save from the common section
above.

> +# 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).
> +#
> +# Defaults to "persistent" for session daemons and "none"
> +# for system daemons. The values "all" and "transient" are
> +# not permitted for this setting, since managed save is not
> +# implemented for transient VMs.
> +#
> +# If 'libvirt-guests.service' is enabled, then this must be
> +# set to 'none' for system daemons to avoid dueling actions
> +#auto_shutdown_try_save = "persistent"
> +

Reviewed-by: Peter Krempa <pkrempa@redhat.com>