all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH qemu-server 2/2] explain 'nocheck' in more places
Date: Mon, 21 Nov 2022 13:16:05 +0100	[thread overview]
Message-ID: <20221121121605.1217-2-f.gruenbichler@proxmox.com> (raw)
In-Reply-To: <20221121121605.1217-1-f.gruenbichler@proxmox.com>

was only explained in git history and vm_stop, add comments in other
relevant places to avoid future breakage.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 PVE/API2/Qemu.pm   | 2 ++
 PVE/CLI/qm.pm      | 2 ++
 PVE/QemuMigrate.pm | 3 ++-
 PVE/QemuServer.pm  | 9 +++++++++
 4 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 6bdcce21..badfc37b 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -3200,6 +3200,8 @@ __PACKAGE__->register_method({
 	raise_param_exc({ skiplock => "Only root may use this option." })
 	    if $skiplock && $authuser ne 'root@pam';
 
+	# nocheck is used as part of migration when config file might be still
+	# be on source node
 	my $nocheck = extract_param($param, 'nocheck');
 	raise_param_exc({ nocheck => "Only root may use this option." })
 	    if $nocheck && $authuser ne 'root@pam';
diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
index 66feecce..3e0f1289 100755
--- a/PVE/CLI/qm.pm
+++ b/PVE/CLI/qm.pm
@@ -426,6 +426,8 @@ __PACKAGE__->register_method ({
 		last;
 	    } elsif ($line =~ /^resume (\d+)$/) {
 		my $vmid = $1;
+		# check_running and vm_resume with nocheck, since local node
+		# might not have processed config move/rename yet
 		if (PVE::QemuServer::check_running($vmid, 1)) {
 		    eval { PVE::QemuServer::vm_resume($vmid, 1, 1); };
 		    if ($@) {
diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 5941cce6..5e466d95 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -1500,6 +1500,7 @@ sub phase3_cleanup {
 		    $self->{errors} = 1;
 		}
 	    } else {
+		# nocheck in case target node hasn't processed the config move/rename yet
 		my $cmd = [@{$self->{rem_ssh}}, 'qm', 'resume', $vmid, '--skiplock', '--nocheck'];
 		my $logf = sub {
 		    my $line = shift;
@@ -1561,7 +1562,7 @@ sub phase3_cleanup {
 	}
     };
 
-    # always stop local VM
+    # always stop local VM with nocheck, since config is moved already
     eval { PVE::QemuServer::vm_stop($self->{storecfg}, $vmid, 1, 1); };
     if (my $err = $@) {
 	$self->log('err', "stopping vm failed - $err");
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 29110c0f..e1ece548 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2788,6 +2788,12 @@ sub check_local_storage_availability {
 sub check_running {
     my ($vmid, $nocheck, $node) = @_;
 
+    # $nocheck is set when called during a migration, in which case the config
+    # file might still or already reside on the *other* node
+    # - because rename has already happened, and current node is source
+    # - because rename hasn't happened yet, and current node is target
+    # - because rename has happened, current node is target, but hasn't yet
+    # processed it yet
     PVE::QemuConfig::assert_config_exists_on_node($vmid, $node) if !$nocheck;
     return PVE::QemuServer::Helpers::vm_running_locally($vmid);
 }
@@ -6359,6 +6365,9 @@ sub vm_suspend {
     }
 }
 
+# $nocheck is set when called as part of a migration - in this context the
+# location of the config file (source or target node) is not deterministic,
+# since migration cannot wait for pmxcfs to process the rename
 sub vm_resume {
     my ($vmid, $skiplock, $nocheck) = @_;
 
-- 
2.30.2





  reply	other threads:[~2022-11-21 12:16 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-21 12:16 [pve-devel] [PATCH qemu-server 1/2] vm_resume: fix nocheck/migrate handling Fabian Grünbichler
2022-11-21 12:16 ` Fabian Grünbichler [this message]
2022-11-21 13:08 ` [pve-devel] applied: " 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=20221121121605.1217-2-f.gruenbichler@proxmox.com \
    --to=f.gruenbichler@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal