Re: [PATCH pve-cluster 7/9] cluster: add helpers module with version comparison functions

["Daniel Kral" <d.kral@proxmox.com>]

Nice idea for making these helpers more accessible across the packages!

On Thu Jun 11, 2026 at 4:59 PM CEST, David Riley wrote:
> Move 'version_cmp' and 'pvecfg_min_version' over from qemu-server, to
> make them more accessible.
>
> Originally-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> Originally-by: Alexandre Derumier <aderumier@odiso.com>
> Signed-off-by: David Riley <d.riley@proxmox.com>
> ---
>  debian/pve-cluster.install |  1 +
>  src/PVE/Cluster/Helpers.pm | 51 ++++++++++++++++++++++++++++++++++++++
>  src/PVE/Cluster/Makefile   |  2 +-
>  3 files changed, 53 insertions(+), 1 deletion(-)
>  create mode 100644 src/PVE/Cluster/Helpers.pm

Hm, I think pve-common might be a more accessible location for such
helpers and PVE::Cluster::* modules (only Setup and IPCConst at the
moment) are only used by pve-cluster internally at the moment.

>
> diff --git a/debian/pve-cluster.install b/debian/pve-cluster.install
> index f66cd06..46d40c4 100644
> --- a/debian/pve-cluster.install
> +++ b/debian/pve-cluster.install
> @@ -5,4 +5,5 @@ usr/lib/
>  usr/share/man/man8/pmxcfs.8
>  usr/share/perl5/PVE/Cluster.pm
>  usr/share/perl5/PVE/Cluster/IPCConst.pm
> +usr/share/perl5/PVE/Cluster/Helpers.pm
>  usr/share/perl5/PVE/IPCC.pm
> diff --git a/src/PVE/Cluster/Helpers.pm b/src/PVE/Cluster/Helpers.pm
> new file mode 100644
> index 0000000..a4b41c9
> --- /dev/null
> +++ b/src/PVE/Cluster/Helpers.pm
> @@ -0,0 +1,51 @@
> +package PVE::Cluster::Helpers;
> +
> +use strict;
> +use warnings;

nit: new modules could already be introduced with 'use v5.36' so you can
     use subroutine signatures :) [0]

[0] https://perldoc.perl.org/perlsub#Signatures

> +
> +use JSON;
> +
> +use base 'Exporter';
> +our @EXPORT_OK = qw(pvecfg_min_version assert_min_cluster_version version_cmp);
> +
> +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'";
> +}
> +
> +# 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;
> +}

Preexisting, but as this will be used by assert_min_cluster_version(...)
in the next patch: the key-value pair 'version-info' contains the
broadcasted pve-manager version of each node.

For the pve-manager we sometimes use another revision number in addition
to the usual version triple in the version string, especially around the
time of a new major version (e.g. [1]).

After the major, minor and release/patch version is compared, the part
after should be compared lexically [2]. That means, that e.g. 9.0.0 is a
newer version than 9.0.0~22 as 9.0.0 is lexically first.

This is not needed right now, but might be worth to keep an eye on
already so this doesn't break especially for features that require
gatekeeping right around the release of a new major version.

[1] https://git.proxmox.com/?p=pve-manager.git;a=blob;f=debian/changelog;h=a792b2c51800b4e4d8c60b8b4235bbc85386eba6;hb=HEAD#l1039
[2] https://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-version

> +
> +1;
> +
> diff --git a/src/PVE/Cluster/Makefile b/src/PVE/Cluster/Makefile
> index 3f920cb..db685bc 100644
> --- a/src/PVE/Cluster/Makefile
> +++ b/src/PVE/Cluster/Makefile
> @@ -1,6 +1,6 @@
>  PVEDIR=$(DESTDIR)/usr/share/perl5/PVE
>  
> -SOURCES=IPCConst.pm Setup.pm
> +SOURCES=Helpers.pm IPCConst.pm Setup.pm
>  
>  .PHONY: install
>  install: $(SOURCES)