From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 147981FF2C7 for ; Wed, 24 Jul 2024 15:05:56 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 45CC51C3E3; Wed, 24 Jul 2024 15:06:31 +0200 (CEST) Message-ID: Date: Wed, 24 Jul 2024 15:05:57 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Proxmox VE development discussion , Markus Frank References: <20240529122348.1267369-1-m.frank@proxmox.com> <20240529122348.1267369-4-m.frank@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20240529122348.1267369-4-m.frank@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL -0.060 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH qemu-server v11 3/5] migration: add check_non_migratable_resources function X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" Am 29.05.24 um 14:23 schrieb Markus Frank: > diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm > index 2a1d4d7..8f36cf8 100644 > --- a/PVE/API2/Qemu.pm > +++ b/PVE/API2/Qemu.pm > @@ -4504,7 +4504,7 @@ __PACKAGE__->register_method({ > $res->{running} = PVE::QemuServer::check_running($vmid) ? 1:0; > > my ($local_resources, $mapped_resources, $missing_mappings_by_node) = > - PVE::QemuServer::check_local_resources($vmconf, 1); > + PVE::QemuServer::check_local_resources($vmconf, $res->{running}, 1); > delete $missing_mappings_by_node->{$localnode}; > > my $vga = PVE::QemuServer::parse_vga($vmconf->{vga}); > @@ -5192,6 +5192,9 @@ __PACKAGE__->register_method({ > die "unable to use snapshot name 'pending' (reserved name)\n" > if lc($snapname) eq 'pending'; > > + my $vmconf = PVE::QemuConfig->load_config($vmid); > + PVE::QemuServer::check_non_migratable_resources($vmconf, $param->{vmstate}, 0); This can be fine as an early check. But there should be another check after the 'snapshot' lock was set in the config (or right before while still holding the config file lock). Otherwise, there is no guarantee that the config is the same at the time of the check and during the actual snapshot operation. My suggestion is to introduce a new abstract method (named something like __snapshot_assert_no_blockers()) in AbstractConfig that is called during __snapshot_prepare() which QemuConfig can override. Like that we can do the check before writing the modified config for the snapshot. > + > my $realcmd = sub { > PVE::Cluster::log_msg('info', $authuser, "snapshot VM $vmid: $snapname"); > PVE::QemuConfig->snapshot_create($vmid, $snapname, $param->{vmstate}, > diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm > index 31a7ee9..9f342bf 100644 > --- a/PVE/QemuServer.pm > +++ b/PVE/QemuServer.pm > @@ -2563,13 +2563,32 @@ sub config_list { > return $res; > } > > +sub check_non_migratable_resources { > + my ($conf, $state, $noerr) = @_; > + > + my @non_migr_res = (); Style nit: I'd prefer writing out the name or use something like "blockers" (inside the function where it's clear what is meant) if it really is too long > + if ($state && $conf->{amd_sev}) { > + push @non_migr_res, "amd_sev"; > + } > + > + if (scalar @non_migr_res && !$noerr) { Style nit: I'd prefer parentheses for scalar(@stuff) > + die "Cannot live-migrate, snapshot (with RAM), or hibernate a VM with:" > + ." @non_migr_res\n"; > + } > + > + return @non_migr_res; > +} > + _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel