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

[David Riley <d.riley@proxmox.com>]

On 10/8/26 12:31 PM, Fiona Ebner wrote:
> 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/

Thanks for the heads up. I'll take a look at your patches and adapt my series accordingly in
version 4.

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