From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 0B1931FF153 for ; Mon, 22 Jun 2026 11:31:32 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 389A5489F; Mon, 22 Jun 2026 11:31:30 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Mon, 22 Jun 2026 11:31:23 +0200 Message-Id: Subject: Re: [PATCH pve-cluster 7/9] cluster: add helpers module with version comparison functions From: "Daniel Kral" To: "David Riley" , X-Mailer: aerc 0.21.0-145-gf21bb67f8cab-dirty References: <20260611145935.147788-1-d.riley@proxmox.com> <20260611145935.147788-8-d.riley@proxmox.com> In-Reply-To: <20260611145935.147788-8-d.riley@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1782120673948 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.074 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: CJZZC5S7KDZJXTGOBDIICW4GV5NGYXFK X-Message-ID-Hash: CJZZC5S7KDZJXTGOBDIICW4GV5NGYXFK X-MailFrom: d.kral@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 > Originally-by: Alexandre Derumier > Signed-off-by: David Riley > --- > 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 =3D qw(pvecfg_min_version assert_min_cluster_version vers= ion_cmp); > + > +sub pvecfg_min_version { > + my ($verstr, $major, $minor, $release) =3D @_; > + > + return 0 if !$verstr; > + > + if ($verstr =3D~ m/^(\d+)\.(\d+)(?:[.-](\d+))?/) { > + return 1 if version_cmp($1, $major, $2, $minor, $3 // 0, $releas= e) >=3D 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 =3D @_; > + > + my $size =3D scalar(@versions); > + > + return 0 if $size =3D=3D 0; > + > + if ($size & 1) { > + my (undef, $fn, $line) =3D caller(0); > + die "cannot compare odd count of versions, called from $fn:$line= \n"; > + } > + > + for (my $i =3D 0; $i < $size; $i +=3D 2) { > + my ($left, $right) =3D splice(@versions, 0, 2); > + $left //=3D 0; > + $right //=3D 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=3Dpve-manager.git;a=3Dblob;f=3Ddebian/change= log;h=3Da792b2c51800b4e4d8c60b8b4235bbc85386eba6;hb=3DHEAD#l1039 [2] https://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-vers= ion > + > +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=3D$(DESTDIR)/usr/share/perl5/PVE > =20 > -SOURCES=3DIPCConst.pm Setup.pm > +SOURCES=3DHelpers.pm IPCConst.pm Setup.pm > =20 > .PHONY: install > install: $(SOURCES)