Re: [PATCH qemu-server v2 4/4] qga: rename guest-fsfreeze to freeze-fs

[Maximiliano Sandoval <m.sandoval@proxmox.com>]

Thomas Lamprecht <t.lamprecht@proxmox.com> writes:

> These settings are already in the (QEMU guest) agent property, and we
> do not use the guest- prefix for any of the existing properties
> (fstrim or freeze-fs-on-backup), and moving freeze-fs-on-backup to a
> generic variant is less confusing if one just drops the "-on-backup"
> part, i.e. no point in inventing a new name schema just for the sake
> of it.
>
> We already rolled this the guest-fsfreeze name to pve-test, so lets
> add an alias, but as we do not provide API stability guarantees for
> test packages this is really just for convenience for anybody that
> tested this, I'm more than fine with dropping such a (never stable)
> released key again on a major release with the respective upgrade
> check in our pveXtoY tool.
>
> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>

For the sake of documenting, the rationale behind the "guest-fsfreeze"
name was using the guest agent command names, namely
"guest-fsfreeze-freeze" and "guest-fsfreeze-thaw", as a base.

For a long time I struggled to find the actual name of these commands
and their documentation since they are named ever-so-slightly
differently each time they appear on the docs or UI: "fsfreeze",
"freeze", "fs-freeze", etc.

If anything, I personally found the old name to be a source of confusion.

> ---
>  src/PVE/QemuServer/Agent.pm         | 9 +++++++--
>  src/test/cfg2cmd/qga-fs-freeze.conf | 2 +-
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/src/PVE/QemuServer/Agent.pm b/src/PVE/QemuServer/Agent.pm
> index b0abd7c0..23dbb3ff 100644
> --- a/src/PVE/QemuServer/Agent.pm
> +++ b/src/PVE/QemuServer/Agent.pm
> @@ -38,9 +38,14 @@ our $agent_fmt = {
>      'freeze-fs-on-backup' => {
>          type => 'boolean',
>          optional => 1,
> -        alias => 'guest-fsfreeze', # keep for old backup restore compatibility
> +        alias => 'freeze-fs', # keep for old backup restore compatibility
>      },
>      'guest-fsfreeze' => {
> +        type => 'boolean',
> +        optional => 1,
> +        alias => 'freeze-fs', # TODO: was only on test repo, drop with PVE 10.
> +    },
> +    'freeze-fs' => {
>          description => "Freeze guest filesystems through QGA for consistent disk state on"
>              . " operations such as snapshots, backups, replications and clones.",
>          verbose_description =>
> @@ -211,7 +216,7 @@ sub should_fs_freeze {
>  
>      my $agent = parse_guest_agent($conf);
>      return 0 if !$agent->{enabled};
> -    return $agent->{'guest-fsfreeze'} // 1;
> +    return $agent->{'freeze-fs'} // 1;
>  }
>  
>  =head3 guest_fsfreeze
> diff --git a/src/test/cfg2cmd/qga-fs-freeze.conf b/src/test/cfg2cmd/qga-fs-freeze.conf
> index c32e143e..809b7d43 100644
> --- a/src/test/cfg2cmd/qga-fs-freeze.conf
> +++ b/src/test/cfg2cmd/qga-fs-freeze.conf
> @@ -1,2 +1,2 @@
>  # TEST: Ensure agent sub-properties do not affect the QEMU command line.
> -agent: 1,guest-fsfreeze=1
> +agent: 1,freeze-fs=1

-- 
Maximiliano