Re: [PATCH guest-common 2/2] fix #1964: replication config: add a setting to skip freezing guest filesystems

[Fiona Ebner <f.ebner@proxmox.com>]

A UI patch would be nice to go along with this.

Am 06.07.26 um 4:03 PM schrieb Maximiliano Sandoval:
> We log an entry when a freeze would have happened and we skip it based on
> replication.cfg.
> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
>  src/PVE/Replication.pm       | 7 +++++++
>  src/PVE/ReplicationConfig.pm | 8 ++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/src/PVE/Replication.pm b/src/PVE/Replication.pm
> index ba4c2c1..79f0be4 100644
> --- a/src/PVE/Replication.pm
> +++ b/src/PVE/Replication.pm
> @@ -300,6 +300,13 @@ sub replicate {
>  
>      my $conf = $guest_class->load_config($vmid);
>      my ($running, $freezefs) = $guest_class->__snapshot_check_freeze_needed($vmid, $conf, 0);

I think we should skip this call if the option is set. qemu-server's
guest_fs_freeze_applicable() might print an additional reason why freeze
is disabled (it does not actually end up in the replication log, because
the log function is not passed down the stack, but we probably should
change that - doesn't have to be this series), so that could be(come) a
bit confusing. The job-level setting has precedence for skipping, so I
think that should be what is logged.

There is __snapshot_check_running() so you can query the running state
independently.

It's not necessary to log that we skip freeze because of the setting if
the guest is not running.

> +    if ($jobcfg->{'skip-freeze-fs'}) {
> +        $logfunc->(
> +            "replication job configured with 'skip-freeze-fs' enabled, not freezing guest filesystems"
> +        ) if $freezefs;

Style nit: line too long, multiline post-if

> +        $freezefs = 0;
> +    }
> +
>      my $volumes = $guest_class->get_replicatable_volumes(
>          $storecfg,
>          $vmid,
> diff --git a/src/PVE/ReplicationConfig.pm b/src/PVE/ReplicationConfig.pm
> index cf9ee01..fcedbfe 100644
> --- a/src/PVE/ReplicationConfig.pm
> +++ b/src/PVE/ReplicationConfig.pm
> @@ -96,6 +96,13 @@ my $defaultData = {
>              format => 'pve-node',
>              optional => 1,
>          },
> +        'skip-freeze-fs' => {
> +            description =>
> +                "Wether to skip freezing & thawing the guest's filesystems during replication",

As you already noted: typo
Style nit: missing dot at the end, I would replace '&' with 'and', maybe
mention "for consistency"

I would suggest adding a verbose description to communicate how it
behaves if disabled, e.g. repeat the first line and then:

Disabling this setting does not enforce that a freeze happens for VMs.
For VMs, that also depends on the virtual machine's 'agent' settings for
the QEMU guest agent and the availability of the guest agent in the VM.

> +            type => 'boolean',
> +            optional => 1,
> +            default => 0,
> +        },
>      },
>  };
>  
> @@ -335,6 +342,7 @@ sub options {
>          schedule => { optional => 1 },
>          remove_job => { optional => 1 },
>          source => { optional => 1 },
> +        'skip-freeze-fs' => { optional => 1 },
>      };
>  }
>