Re: [PATCH v2 qemu-server 3/5] helpers: add helper for min package version comparison

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

Am 19.08.26 um 4:46 PM schrieb Christian Ebner:
> To be used for checking the minimum version of qemu-kvm package
> version, where the binary version check is not enough to detect
> versions having the pbs-restore `no-cache` flag.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> changes:
> - new in version 2
> 
>  src/PVE/QemuServer/Helpers.pm | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/src/PVE/QemuServer/Helpers.pm b/src/PVE/QemuServer/Helpers.pm
> index 658400f2..860c0a9c 100644
> --- a/src/PVE/QemuServer/Helpers.pm
> +++ b/src/PVE/QemuServer/Helpers.pm
> @@ -21,6 +21,7 @@ our @EXPORT_OK = qw(
>      parse_number_sets
>      windows_version
>      get_host_arch
> +    min_package_version
>  );
>  
>  my $nodename = PVE::INotify::nodename();
> @@ -89,6 +90,19 @@ sub kvm_user_version {
>      return $kvm_user_version->{$binary}->{$version_type};
>  }
>  
> +sub min_package_version {

I'd rather have the helper be binary_package_version_at_least() and take
the $binary instead of $verstr. It can call kvm_user_version itself.
Then there is less potential for somebody to pass along the wrong kind
of version.

> +    my ($verstr, $major, $minor, $patch, $subver, $rel) = @_;

s/$patch/$micro/

No need for $subver, see the previous patch

> +
> +    if ($verstr =~ m/^(\d+)\.(\d+)(?:\.(\d+))?(?:\.(\d+))?-(\d+)/) {
> +        return 1
> +            if version_cmp($1, $major, $2, $minor, $3 // 0, $patch, $4 // 0, $subver, $5, $rel) >=
> +            0;

Style nit: multi-line post-if

> +        return 0;
> +    }
> +
> +    die "internal error: cannot check version of invalid string '$verstr'";
> +}
> +
>  # Paths and directories
>  
>  # FIXME: MAJOR VERSION: use /run/qemu-server everywhere instead of mixing /run and /var/run and rely