[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