Re: [PATCH pve-common v3 01/12] tools: add helpers for version comparison

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

Am 24.07.26 um 4:26 PM schrieb David Riley:
> Move 'version_cmp' and 'pvecfg_min_version' over from qemu-server, to
> make them more accessible.
> 
> Prepare for #7294 to enable centralized cluster wide version
> assertions.
> 
> Originally-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> Originally-by: Alexandre Derumier <aderumier@odiso.com>
> Signed-off-by: David Riley <d.riley@proxmox.com>
> Reviewed-by: Daniel Kral <d.kral@proxmox.com>
> 
> Link: https://bugzilla.proxmox.com/show_bug.cgi?id=7294
> ---
>  src/PVE/Tools.pm | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
> index 2915b61..445cb74 100644
> --- a/src/PVE/Tools.pm
> +++ b/src/PVE/Tools.pm
> @@ -53,6 +53,8 @@ our @EXPORT_OK = qw(
>      extract_sensitive_params
>      file_copy
>      get_host_arch
> +    pvecfg_min_version
> +    version_cmp
>      CLONE_NEWNS
>      CLONE_NEWUTS
>      CLONE_NEWIPC
> @@ -1628,4 +1630,43 @@ sub is_deeply {
>      return 1;
>  }
>  
> +sub pvecfg_min_version {
> +    my ($verstr, $major, $minor, $release) = @_;
> +
> +    return 0 if !$verstr;
> +
> +    if ($verstr =~ m/^(\d+)\.(\d+)(?:[.-](\d+))?/) {
> +        return 1 if version_cmp($1, $major, $2, $minor, $3 // 0, $release) >= 0;
> +        return 0;
> +    }
> +
> +    die "internal error: cannot check version of invalid string '$verstr'";
> +}

I feel like this one would be better placed in pve-cluster and not
separated from get_node_pvecfg_version(). I require these helpers more
centrally in one of my series [0] as well (and Lukas needs it too, which
prompted me to reply here now), where I moved the helper to pve-cluster,
and also added a new node_pvecfg_version_at_least() helper, since that
one is nicer for the usage in QemuMigrate.pm.

Could you check out if that also works well for your use case
here/review the relevant patches? Then we could think about applying
those already.

[0]:
https://lore.proxmox.com/pve-devel/20260918160841.128088-1-f.ebner@proxmox.com/

> +
> +# gets in pairs the versions you want to compares, i.e.:
> +# ($a-major, $b-major, $a-minor, $b-minor, $a-extra, $b-extra, ...)
> +# returns 0 if same, -1 if $a is older than $b, +1 if $a is newer than $b
> +sub version_cmp {
> +    my @versions = @_;
> +
> +    my $size = scalar(@versions);
> +
> +    return 0 if $size == 0;
> +
> +    if ($size & 1) {
> +        my (undef, $fn, $line) = caller(0);
> +        die "cannot compare odd count of versions, called from $fn:$line\n";
> +    }
> +
> +    for (my $i = 0; $i < $size; $i += 2) {
> +        my ($left, $right) = splice(@versions, 0, 2);
> +        $left //= 0;
> +        $right //= 0;
> +
> +        return 1 if $left > $right;
> +        return -1 if $left < $right;
> +    }
> +    return 0;
> +}
> +
>  1;