[pve-devel] [RFC v2 qemu-server 4/4] move helper to check running QEMU version out of the 'Machine' module

[Fiona Ebner]

The version of the running QEMU binary is not related to the machine
version and so it's a bit confusing to have the helper in the
'Machine' module. It cannot live in the 'Helpers' module, because that
would lead to a cyclic inclusion Helpers <-> Monitor. Thus,
'QMPHelpers' is chosen as the new home.

Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
---

New in v2.

 PVE/QemuMigrate.pm                    |  3 ++-
 PVE/QemuServer/Machine.pm             | 12 ------------
 PVE/QemuServer/QMPHelpers.pm          | 13 +++++++++++++
 test/MigrationTest/QemuMigrateMock.pm |  4 ++++
 test/run_config2command_tests.pl      |  4 ++--
 5 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 34fc46ee..e71face4 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -30,6 +30,7 @@ use PVE::QemuServer::Helpers qw(min_version);
 use PVE::QemuServer::Machine;
 use PVE::QemuServer::Monitor qw(mon_cmd);
 use PVE::QemuServer::Memory qw(get_current_memory);
+use PVE::QemuServer::QMPHelpers;
 use PVE::QemuServer;
 
 use PVE::AbstractMigrate;
@@ -1140,7 +1141,7 @@ sub phase2 {
 	    PVE::QemuServer::qemu_drive_mirror($vmid, $drive, $nbd_uri, $vmid, undef, $self->{storage_migration_jobs}, 'skip', undef, $bwlimit, $bitmap);
 	}
 
-	if (PVE::QemuServer::Machine::runs_at_least_qemu_version($vmid, 8, 2)) {
+	if (PVE::QemuServer::QMPHelpers::runs_at_least_qemu_version($vmid, 8, 2)) {
 	    $self->log('info', "switching mirror jobs to actively synced mode");
 	    PVE::QemuServer::qemu_drive_mirror_switch_to_active_mode(
 		$vmid,
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index cc92e7e6..a3917dae 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -161,18 +161,6 @@ sub can_run_pve_machine_version {
     return 0;
 }
 
-# dies if a) VM not running or not exisiting b) Version query failed
-# So, any defined return value is valid, any invalid state can be caught by eval
-sub runs_at_least_qemu_version {
-    my ($vmid, $major, $minor, $extra) = @_;
-
-    my $v = PVE::QemuServer::Monitor::mon_cmd($vmid, 'query-version');
-    die "could not query currently running version for VM $vmid\n" if !defined($v);
-    $v = $v->{qemu};
-
-    return PVE::QemuServer::Helpers::version_cmp($v->{major}, $major, $v->{minor}, $minor, $v->{micro}, $extra) >= 0;
-}
-
 sub qemu_machine_pxe {
     my ($vmid, $conf) = @_;
 
diff --git a/PVE/QemuServer/QMPHelpers.pm b/PVE/QemuServer/QMPHelpers.pm
index d3a52327..0269ea46 100644
--- a/PVE/QemuServer/QMPHelpers.pm
+++ b/PVE/QemuServer/QMPHelpers.pm
@@ -3,6 +3,7 @@ package PVE::QemuServer::QMPHelpers;
 use warnings;
 use strict;
 
+use PVE::QemuServer::Helpers;
 use PVE::QemuServer::Monitor qw(mon_cmd);
 
 use base 'Exporter';
@@ -45,4 +46,16 @@ sub qemu_objectdel {
     return 1;
 }
 
+# dies if a) VM not running or not exisiting b) Version query failed
+# So, any defined return value is valid, any invalid state can be caught by eval
+sub runs_at_least_qemu_version {
+    my ($vmid, $major, $minor, $extra) = @_;
+
+    my $v = PVE::QemuServer::Monitor::mon_cmd($vmid, 'query-version');
+    die "could not query currently running version for VM $vmid\n" if !defined($v);
+    $v = $v->{qemu};
+
+    return PVE::QemuServer::Helpers::version_cmp($v->{major}, $major, $v->{minor}, $minor, $v->{micro}, $extra) >= 0;
+}
+
 1;
diff --git a/test/MigrationTest/QemuMigrateMock.pm b/test/MigrationTest/QemuMigrateMock.pm
index f5b44424..11c58c08 100644
--- a/test/MigrationTest/QemuMigrateMock.pm
+++ b/test/MigrationTest/QemuMigrateMock.pm
@@ -188,6 +188,10 @@ $qemu_server_machine_module->mock(
 	    if !defined($vm_status->{runningmachine});
 	return $vm_status->{runningmachine};
     },
+);
+
+my $qemu_server_qmphelpers_module = Test::MockModule->new("PVE::QemuServer::QMPHelpers");
+$qemu_server_qmphelpers_module->mock(
     runs_at_least_qemu_version => sub {
 	return 1;
     },
diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl
index 7212acc4..d48ef562 100755
--- a/test/run_config2command_tests.pl
+++ b/test/run_config2command_tests.pl
@@ -16,7 +16,7 @@ use PVE::SysFSTools;
 use PVE::QemuConfig;
 use PVE::QemuServer;
 use PVE::QemuServer::Monitor;
-use PVE::QemuServer::Machine;
+use PVE::QemuServer::QMPHelpers;
 use PVE::QemuServer::CPUConfig;
 
 my $base_env = {
@@ -472,7 +472,7 @@ sub do_test($) {
     # check if QEMU version set correctly and test version_cmp
     (my $qemu_major = get_test_qemu_version()) =~ s/\..*$//;
     die "runs_at_least_qemu_version returned false, maybe error in version_cmp?"
-	if !PVE::QemuServer::Machine::runs_at_least_qemu_version($vmid, $qemu_major);
+	if !PVE::QemuServer::QMPHelpers::runs_at_least_qemu_version($vmid, $qemu_major);
 
     $cmdline =~ s/ -/ \\\n  -/g; # same as qm showcmd --pretty
     $cmdline .= "\n";
-- 
2.39.2