public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH v3 qemu-server 2/2] move helper to check running QEMU version out of the 'Machine' module
Date: Thu,  4 Jul 2024 11:32:12 +0200	[thread overview]
Message-ID: <20240704093212.27136-2-f.ebner@proxmox.com> (raw)
In-Reply-To: <20240704093212.27136-1-f.ebner@proxmox.com>

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@proxmox.com>
---

No changes in v3.

 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



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


  reply	other threads:[~2024-07-04  9:32 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-04  9:32 [pve-devel] [PATCH v3 qemu-server 1/2] migration: avoid crash with heavy IO on local VM disk Fiona Ebner
2024-07-04  9:32 ` Fiona Ebner [this message]
2024-07-30 19:25 ` [pve-devel] applied-series: " Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240704093212.27136-2-f.ebner@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal