On Wed, Jan 08, 2025 at 19:42:45 +0000, Daniel P. Berrangé wrote:
> Currently the session daemon will try a managed save on all VMs,
> leaving them running if that fails.
>
> This limits the managed save just to persistent VMs, as there will
> usually not be any way to restore transient VMs later.
>
> It also enables graceful shutdown and then forced poweroff, should
> save fail for some reason.
>
> These new defaults can be overridden in the config file if needed.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> src/qemu/libvirtd_qemu.aug | 2 +-
> src/qemu/qemu.conf.in | 14 ++++++++------
> src/qemu/qemu_conf.c | 6 +++---
> src/qemu/test_libvirtd_qemu.aug.in | 6 +++---
> 4 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
> index e465a231fc..605604a01a 100644
> --- a/src/qemu/libvirtd_qemu.aug
> +++ b/src/qemu/libvirtd_qemu.aug
> @@ -100,7 +100,7 @@ module Libvirtd_qemu =
> | int_entry "auto_start_delay"
> | str_entry "auto_shutdown_try_save"
> | str_entry "auto_shutdown_try_shutdown"
> - | str_entry "auto_shutdown_powerdown"
> + | str_entry "auto_shutdown_poweroff"
>
> let process_entry = str_entry "hugetlbfs_mount"
> | str_entry "bridge_helper"
This hunk belongs to previous patch.
> diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in
> index d8890c4c32..82eae2eecd 100644
> --- a/src/qemu/qemu.conf.in
> +++ b/src/qemu/qemu.conf.in
> @@ -647,33 +647,35 @@
> # * "transient" - only try to save transient running VMs
> # * "all" - try to save all running VMs
> #
> -# Defaults to "all" for session daemons and "none"
> +# Defaults to "persistent" 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"
> +#auto_shutdown_try_save = "persistent"
As noted 'all' doesn't make sense for 'save' operation so I'd call that
a bugfix. Perhaps even worhty of a separate patch.
> # 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"
> +# 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_shutdown = "none"
> +#auto_shutdown_try_shutdown = "all"
>
> # 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"
> +# 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_powerdown = "none"
> +#auto_shutdown_poweroff = "all"
>
> # If provided by the host and a hugetlbfs mount point is configured,
> # a guest may request huge page backing. When this mount point is
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index a57eccf569..76bb3bd888 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -313,9 +313,9 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
> 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;
> + cfg->autoShutdownTrySave = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_PERSISTENT;
> + cfg->autoShutdownTryShutdown = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL;
> + cfg->autoShutdownPoweroff = VIR_DOMAIN_DRIVER_AUTO_SHUTDOWN_SCOPE_ALL;
> }
>
> return g_steal_pointer(&cfg);
> diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
> index e7137f686f..3bc8104d7a 100644
> --- a/src/qemu/test_libvirtd_qemu.aug.in
> +++ b/src/qemu/test_libvirtd_qemu.aug.in
> @@ -76,9 +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" }
> +{ "auto_shutdown_try_save" = "persistent" }
> +{ "auto_shutdown_try_shutdown" = "all" }
> +{ "auto_shutdown_poweroff" = "all" }
> { "hugetlbfs_mount" = "/dev/hugepages" }
> { "bridge_helper" = "qemu-bridge-helper" }
> { "set_process_name" = "1" }
For the rest:
Reviewed-by: Peter Krempa <pkrempa@redhat.com>