[pve-devel] [PATCH v4 qemu-server 3/8] refactor: split check_running into _exists_ and _running_

Stefan Reiter s.reiter at proxmox.com
Tue Nov 19 12:23:46 CET 2019


vm_exists_on_node in PVE::QemuConfig checks if a config file for a vmid
exists

vm_running_locally in PVE::QemuServer::Helpers checks if a VM is running
on the local machine by probing its pidfile and checking /proc/.../cmdline

check_running is left in QemuServer for compatibility, but changed to
simply call the two new helper functions.

Both methods are also correctly mocked for testing snapshots.

Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
---

No users of check_running are changed for now, but this could be done in a
future series.

 PVE/QemuConfig.pm         | 15 ++++++++-
 PVE/QemuServer.pm         | 68 ++------------------------------------
 PVE/QemuServer/Helpers.pm | 69 +++++++++++++++++++++++++++++++++++++++
 test/snapshot-test.pm     | 16 +++++++--
 4 files changed, 100 insertions(+), 68 deletions(-)

diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index 2c23d67..c42091c 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -22,6 +22,19 @@ mkdir $lock_dir;
 
 my $MAX_UNUSED_DISKS = 256;
 
+sub assert_config_exists_on_node {
+    my ($vmid, $node) = @_;
+
+    $node //= $nodename;
+
+    my $filename = __PACKAGE__->config_file($vmid, $node);
+    my $exists = -f $filename;
+
+    my $type = guest_type();
+    die "unable to find configuration file for $type $vmid on node '$node'\n"
+	if !$exists;
+}
+
 # BEGIN implemented abstract methods from PVE::AbstractConfig
 
 sub guest_type {
@@ -167,7 +180,7 @@ sub __snapshot_save_vmstate {
 
 sub __snapshot_check_running {
     my ($class, $vmid) = @_;
-    return PVE::QemuServer::check_running($vmid);
+    return PVE::QemuServer::Helpers::vm_running_locally($vmid);
 }
 
 sub __snapshot_check_freeze_needed {
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 3e658bc..9d0df95 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2947,74 +2947,12 @@ sub check_local_storage_availability {
     return $nodehash
 }
 
-sub parse_cmdline {
-    my ($pid) = @_;
-
-    my $fh = IO::File->new("/proc/$pid/cmdline", "r");
-    if (defined($fh)) {
-	my $line = <$fh>;
-	$fh->close;
-	return undef if !$line;
-	my @param = split(/\0/, $line);
-
-	my $cmd = $param[0];
-	return if !$cmd || ($cmd !~ m|kvm$| && $cmd !~ m@(?:^|/)qemu-system-[^/]+$@);
-
-	my $phash = {};
-	my $pending_cmd;
-	for (my $i = 0; $i < scalar (@param); $i++) {
-	    my $p = $param[$i];
-	    next if !$p;
-
-	    if ($p =~ m/^--?(.*)$/) {
-		if ($pending_cmd) {
-		    $phash->{$pending_cmd} = {};
-		}
-		$pending_cmd = $1;
-	    } elsif ($pending_cmd) {
-		$phash->{$pending_cmd} = { value => $p };
-		$pending_cmd = undef;
-	    }
-	}
-
-	return $phash;
-    }
-    return undef;
-}
-
+# Compat only, use assert_config_exists_on_node and vm_running_locally where possible
 sub check_running {
     my ($vmid, $nocheck, $node) = @_;
 
-    my $filename = PVE::QemuConfig->config_file($vmid, $node);
-
-    die "unable to find configuration file for VM $vmid - no such machine\n"
-	if !$nocheck && ! -f $filename;
-
-    my $pidfile = PVE::QemuServer::Helpers::pidfile_name($vmid);
-
-    if (my $fd = IO::File->new("<$pidfile")) {
-	my $st = stat($fd);
-	my $line = <$fd>;
-	close($fd);
-
-	my $mtime = $st->mtime;
-	if ($mtime > time()) {
-	    warn "file '$filename' modified in future\n";
-	}
-
-	if ($line =~ m/^(\d+)$/) {
-	    my $pid = $1;
-	    my $cmdline = parse_cmdline($pid);
-	    if ($cmdline && defined($cmdline->{pidfile}) && $cmdline->{pidfile}->{value}
-		&& $cmdline->{pidfile}->{value} eq $pidfile) {
-		if (my $pinfo = PVE::ProcFSTools::check_process_running($pid)) {
-		    return $pid;
-		}
-	    }
-	}
-    }
-
-    return undef;
+    PVE::QemuConfig::assert_config_exists_on_node($vmid, $node) if !$nocheck;
+    return PVE::QemuServer::Helpers::vm_running_locally($vmid);
 }
 
 sub vzlist {
diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
index 97f33c4..115e801 100644
--- a/PVE/QemuServer/Helpers.pm
+++ b/PVE/QemuServer/Helpers.pm
@@ -3,7 +3,10 @@ package PVE::QemuServer::Helpers;
 use strict;
 use warnings;
 
+use File::stat;
+
 use PVE::INotify;
+use PVE::ProcFSTools;
 
 my $nodename = PVE::INotify::nodename();
 
@@ -28,4 +31,70 @@ sub vnc_socket {
     return "${var_run_tmpdir}/$vmid.vnc";
 }
 
+# Parse the cmdline of a running kvm/qemu process and return arguments as hash
+sub parse_cmdline {
+    my ($pid) = @_;
+
+    my $fh = IO::File->new("/proc/$pid/cmdline", "r");
+    if (defined($fh)) {
+	my $line = <$fh>;
+	$fh->close;
+	return undef if !$line;
+	my @param = split(/\0/, $line);
+
+	my $cmd = $param[0];
+	return if !$cmd || ($cmd !~ m|kvm$| && $cmd !~ m@(?:^|/)qemu-system-[^/]+$@);
+
+	my $phash = {};
+	my $pending_cmd;
+	for (my $i = 0; $i < scalar (@param); $i++) {
+	    my $p = $param[$i];
+	    next if !$p;
+
+	    if ($p =~ m/^--?(.*)$/) {
+		if ($pending_cmd) {
+		    $phash->{$pending_cmd} = {};
+		}
+		$pending_cmd = $1;
+	    } elsif ($pending_cmd) {
+		$phash->{$pending_cmd} = { value => $p };
+		$pending_cmd = undef;
+	    }
+	}
+
+	return $phash;
+    }
+    return undef;
+}
+
+sub vm_running_locally {
+    my ($vmid) = @_;
+
+    my $pidfile = pidfile_name($vmid);
+
+    if (my $fd = IO::File->new("<$pidfile")) {
+	my $st = stat($fd);
+	my $line = <$fd>;
+	close($fd);
+
+	my $mtime = $st->mtime;
+	if ($mtime > time()) {
+	    warn "file '$pidfile' modified in future\n";
+	}
+
+	if ($line =~ m/^(\d+)$/) {
+	    my $pid = $1;
+	    my $cmdline = parse_cmdline($pid);
+	    if ($cmdline && defined($cmdline->{pidfile}) && $cmdline->{pidfile}->{value}
+		&& $cmdline->{pidfile}->{value} eq $pidfile) {
+		if (my $pinfo = PVE::ProcFSTools::check_process_running($pid)) {
+		    return $pid;
+		}
+	    }
+	}
+    }
+
+    return undef;
+}
+
 1;
diff --git a/test/snapshot-test.pm b/test/snapshot-test.pm
index da55b2c..09d3e74 100644
--- a/test/snapshot-test.pm
+++ b/test/snapshot-test.pm
@@ -297,14 +297,22 @@ sub __snapshot_save_vmstate {
     $snap->{vmstate} = "somestorage:state-volume";
     $snap->{runningmachine} = "somemachine"
 }
+
+sub assert_config_exists_on_node {
+    my ($vmid, $node) = @_;
+    return -f cfs_config_path(undef, $vmid, $node);
+}
 # END mocked PVE::QemuConfig methods
 
-# BEGIN redefine PVE::QemuServer methods
+# BEGIN mocked PVE::QemuServer::Helpers methods
 
-sub check_running {
+sub vm_running_locally {
     return $running;
 }
 
+# END mocked PVE::QemuServer::Helpers methods
+
+# BEGIN redefine PVE::QemuServer methods
 
 sub do_snapshots_with_qemu {
     return 0;
@@ -369,6 +377,9 @@ sub vm_stop {
 PVE::Tools::run_command("rm -rf snapshot-working");
 PVE::Tools::run_command("cp -a snapshot-input snapshot-working");
 
+my $qemu_helpers_module = new Test::MockModule('PVE::QemuServer::Helpers');
+$qemu_helpers_module->mock('vm_running_locally', \&vm_running_locally);
+
 my $qemu_config_module = new Test::MockModule('PVE::QemuConfig');
 $qemu_config_module->mock('config_file_lock', \&config_file_lock);
 $qemu_config_module->mock('cfs_config_path', \&cfs_config_path);
@@ -376,6 +387,7 @@ $qemu_config_module->mock('load_config', \&load_config);
 $qemu_config_module->mock('write_config', \&write_config);
 $qemu_config_module->mock('has_feature', \&has_feature);
 $qemu_config_module->mock('__snapshot_save_vmstate', \&__snapshot_save_vmstate);
+$qemu_config_module->mock('assert_config_exists_on_node', \&assert_config_exists_on_node);
 
 # ignore existing replication config
 my $repl_config_module = new Test::MockModule('PVE::ReplicationConfig');
-- 
2.20.1





More information about the pve-devel mailing list