From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id F37FF1FF0BC for ; Thu, 29 Oct 2026 15:36:42 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 48F23211F7; Thu, 29 Oct 2026 15:36:40 +0100 (CET) Message-ID: <08ebcb93-e097-4111-901c-ff39fd64f204@proxmox.com> Date: Thu, 29 Oct 2026 15:36:36 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH qemu-server 2/2] api: refuse moving disks if source VM is protected To: =?UTF-8?Q?Michael_K=C3=B6ppl?= , Jakob Klocker , pve-devel@lists.proxmox.com References: <20260921153545.406664-1-j.klocker@proxmox.com> <20260921153545.406664-3-j.klocker@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793284596301 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.323 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: OJ4FNI4HCJHVGWMU3XNDFPD6MMD7HKD5 X-Message-ID-Hash: OJ4FNI4HCJHVGWMU3XNDFPD6MMD7HKD5 X-MailFrom: f.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Am 22.10.26 um 3:15 PM schrieb Michael Köppl: > nit: the commit subject reads too broad. This commit only affects moving > disks to another VM. Moving disks to another storage is still possible > even with the --delete flag which removes the old disk. Mostly noticed > this because I initially thought this patch also covers the case where a > disk is moved to another storage. > > I'm not entirely sure how to best handle this, though. On the one hand, > moving to another storage with --delete doesn't really remove anything > from the VM in that since, since it attaches the new disk on the new > storage in its place. On the other hand, the behavior then differs > between CTs and VMs. I think the basis for this in the docs saying that > the protection option on CTs prevents "disk remove/update operation" > (which moving a volume to another storage is), whereas the VM docs only > say "This will disable the remove VM and remove disk operations". Still, > wanted to bring it up to clarify the scope of this. Maybe @Fiona can > share her thoughts on this. I suppose if the behavior here should match > the CT behavior, the docs would of course also have to be updated. > I also think it's a bit of a gray area and both sides can be argued for (more convenient, data is just moved - versus - original volume is removed, so something is removed). Slightly inclined towards the strict version like we have for containers, but if we do change it, it should be done for the next major release, because people might be relying on the current behavior. > On Mon Sep 21, 2026 at 5:35 PM CEST, Jakob Klocker wrote: >> Moving a disk removes it from the source VM, which, according to the >> documentation, the protection flag prevents. >> Refuse the move when the source is protected. >> >> Signed-off-by: Jakob Klocker >> --- >> src/PVE/API2/Qemu.pm | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm >> index 922c6599..3cf3517b 100644 >> --- a/src/PVE/API2/Qemu.pm >> +++ b/src/PVE/API2/Qemu.pm >> @@ -5024,6 +5024,11 @@ __PACKAGE__->register_method({ >> my $target_conf = PVE::QemuConfig->load_config($target_vmid); >> PVE::QemuConfig->check_lock($target_conf); >> >> + PVE::QemuConfig->check_protection( >> + $source_conf, >> + "Can't move disks from protected VMs\n", >> + ); >> + >> die "Can't move disks from or to template VMs\n" >> if ($source_conf->{template} || $target_conf->{template}); >> > >