Re: [pve-devel] [RFC PATCH qemu-server 1/8] tests: cfg2cmd: pin QEMU version

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

Am 06.03.25 um 11:44 schrieb Dominik Csapak:
> but warn when we're out of date compared to the installed one, and die
> when we're one major (+1 minor) release behind.
> (the warning is not very visible when running tests or when building)
> 
> We don't want to depend on the installed QEMU version for such tests,
> otherwise a developer might need to adapt tests because the installed
> QEMU version is different to what is e.g. in the build environment for
> our packaging.
> 
> Also, this way, we have to show intent for bumping this and it will be
> obvious that we need to adapt tests because of a changed QEMU version.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  test/run_config2command_tests.pl | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl
> index 7e3d10e6..5fa6f2de 100755
> --- a/test/run_config2command_tests.pl
> +++ b/test/run_config2command_tests.pl
> @@ -20,6 +20,10 @@ use PVE::QemuServer::Monitor;
>  use PVE::QemuServer::QMPHelpers;
>  use PVE::QemuServer::CPUConfig;
>  
> +# bump when QEMU version changes
> +my $tested_version_major = 9;
> +my $tested_version_minor = 2;
> +
>  my $base_env = {
>      storage_config => {
>  	ids => {
> @@ -80,6 +84,7 @@ my $base_env = {
>  	}
>      },
>      vmid => 8006,
> +    tested_qemu_version => "$tested_version_major.$tested_version_minor",
>      real_qemu_version => PVE::QemuServer::Helpers::kvm_user_version(), # not yet mocked

I'd not keep the real_qemu_version in base_env anymore, but have it as a
stand-alone variable for the version comparison check.

>  };
>  
> @@ -184,7 +189,7 @@ sub parse_test($) {
>  }
>  
>  sub get_test_qemu_version {
> -    $current_test->{qemu_version} // $base_env->{real_qemu_version} // '2.12';
> +    $current_test->{qemu_version} // $base_env->{tested_qemu_version} // '2.12';
>  }
>  
>  my $qemu_server_module;
> @@ -528,3 +533,19 @@ if (my $file = shift) {
>  }
>  
>  done_testing();

Nit: Since the check below can die, I'd put it at the very beginning
rather than at the end.

> +
> +# reset warn

IMHO "Why is the reset needed?" is the interesting part worth commenting
here ;)

> +$SIG{__WARN__} = undef;
> +if ($base_env->{real_qemu_version} =~ m/^(\d+.\d+)/) {
> +	if (PVE::QemuServer::Helpers::version_cmp($1, $tested_version_major, $2, $tested_version_minor) < 0) {
> +	    warn "\nWARNING: installed QEMU version bigger than tested one, please bump!\n";
> +	}
> +
> +	# if we did not bump since the last major QEMU (+1 minor) release fail the test
> +	if (PVE::QemuServer::Helpers::version_cmp($1, $tested_version_major + 1, $2, 1) >= 0) {

I'd rather have a fixed difference between versions trigger the die.
Your check behaves the same when the tested version is 10.0 and when the
tested version is 10.2. In both cases, having the real version 11.1 will
trigger the die.

> +	    die "\nERROR: installed QEMU version one major release bigger than tested one, please bump!\n";
> +	}
> +
> +} else {
> +    die "\nERROR: invalid QEMU version\n";
> +}



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel