From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id ED7C91FF14F for ; Wed, 17 Jun 2026 14:14:03 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9AF1932CF3; Wed, 17 Jun 2026 14:14:03 +0200 (CEST) From: Arthur Bied-Charreton To: pve-devel@lists.proxmox.com Subject: [PATCH qemu-server v2] fix #7705: blockdev: detach backing chain on disk move/hot-unplug Date: Wed, 17 Jun 2026 14:12:13 +0200 Message-ID: <20260617121330.347174-1-a.bied-charreton@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.135 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 KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record Message-ID-Hash: CDEIW6RAU4M4UJN6VNKGSJWDGMBKST4B X-Message-ID-Hash: CDEIW6RAU4M4UJN6VNKGSJWDGMBKST4B X-MailFrom: abied-charreton@jett.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: When snapshots-as-volume-chains is enabled, moving or hot-unplugging a disk that holds snapshots does not detach the snapshot volumes. `Blockdev::detach` is called in both paths and only follows the `file` child, so the snapshot nodes are left behind. Since they were added explicitly via blockdev-add, they are also not GCed by QEMU and, as a result, keep their source volumes open. Deleting one of those snapshots after the move does not detach the orphaned block node because the disk is no longer attached to the running VM. QEMU keeps the volume open for as long as the VM runs, and a later attempt to remove the source fails with lvremove reporting the LV as still in use. Add 'follow-backing' option to `Blockdev::detach`. When enabled, the 'backing' children are detached in addition to the 'file' children, meaning the snapshot blockdevs are released. Enable 'follow-backing' at the following callsites: * qemu_handle_concluded_blockjob (on complete the source is detached along with the backing snapshot chain, on cancel/error it detaches the mirror target, which is flat so follow-backing is a no-op) * qemu_drivedel Signed-off-by: Arthur Bied-Charreton --- Changes since v1: - Update `Blockdev::detach`'s POD. src/PVE/QemuServer.pm | 6 +++++- src/PVE/QemuServer/BlockJob.pm | 4 +++- src/PVE/QemuServer/Blockdev.pm | 26 ++++++++++++++++++++------ 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm index 55e9f520..472bee7e 100644 --- a/src/PVE/QemuServer.pm +++ b/src/PVE/QemuServer.pm @@ -4082,7 +4082,11 @@ sub qemu_drivedel { # for the switch to -blockdev if (PVE::QemuServer::Machine::is_machine_version_at_least($machine_type, 10, 0)) { - PVE::QemuServer::Blockdev::detach(vm_qmp_peer($vmid), "drive-$deviceid"); + PVE::QemuServer::Blockdev::detach( + vm_qmp_peer($vmid), + "drive-$deviceid", + { 'follow-backing' => 1 }, + ); return 1; } else { my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_del drive-$deviceid", 10 * 60); diff --git a/src/PVE/QemuServer/BlockJob.pm b/src/PVE/QemuServer/BlockJob.pm index 921f046c..493dda33 100644 --- a/src/PVE/QemuServer/BlockJob.pm +++ b/src/PVE/QemuServer/BlockJob.pm @@ -34,7 +34,9 @@ sub qemu_handle_concluded_blockjob { $job->{'detach-node-name'} = $job->{'target-node-name'} if $qmp_info->{error} || $job->{cancel}; if (my $node_name = $job->{'detach-node-name'}) { - eval { PVE::QemuServer::Blockdev::detach($qmp_peer, $node_name); }; + eval { + PVE::QemuServer::Blockdev::detach($qmp_peer, $node_name, { 'follow-backing' => 1 }); + }; log_warn($@) if $@; } diff --git a/src/PVE/QemuServer/Blockdev.pm b/src/PVE/QemuServer/Blockdev.pm index 101c747c..8eafb9fb 100644 --- a/src/PVE/QemuServer/Blockdev.pm +++ b/src/PVE/QemuServer/Blockdev.pm @@ -637,7 +637,7 @@ sub attach { =head3 detach - detach($qmp_peer, $node_name); + detach($qmp_peer, $node_name, $opts); Detach the block device C<$node_name> from the QMP peer C<$qmp_peer>. Also removes associated child block nodes. @@ -650,15 +650,25 @@ Parameters: =item C<$node_name>: The node name identifying the block node in QEMU. +=item C<$opts> Additional options. Possible values: + +=over + +=item C<'follow-backing'>: also detach the 'backing' children + +=back + =back =cut sub detach { - my ($qmp_peer, $node_name) = @_; + my ($qmp_peer, $node_name, $opts) = @_; die "Blockdev::detach - no node name\n" if !$node_name; + my $follow_backing = $opts->{'follow-backing'}; + my $block_info = qmp_cmd($qmp_peer, "query-named-block-nodes"); $block_info = { map { $_->{'node-name'} => $_ } $block_info->@* }; @@ -667,12 +677,13 @@ sub detach { $remove_throttle_group_id = throttle_group_id($drive_id); } - while ($node_name) { - last if !$block_info->{$node_name}; # already gone + my @queue = ($node_name); + while (defined(my $node_name = shift @queue)) { + next if !$block_info->{$node_name}; # already gone my $res = qmp_cmd($qmp_peer, 'blockdev-del', 'node-name' => "$node_name", noerr => 1); if (my $err = $res->{error}) { - last if $err =~ m/Failed to find node with node-name/; # already gone + next if $err =~ m/Failed to find node with node-name/; # already gone die "deleting blockdev '$node_name' failed : $err\n"; } @@ -680,7 +691,10 @@ sub detach { # Recursively remove 'file' child nodes. QEMU will auto-remove implicitly added child nodes, # but e.g. the child of the top throttle node might have been explicitly added as a mirror # target, and needs to be removed manually. - $node_name = $children->{file}->{'node-name'}; + # 'backing' is only followed when requested, because callers tearing down a single chain node + # rely on the nodes below it surviving. + push @queue, $children->{file}->{'node-name'} if $children->{file}; + push @queue, $children->{backing}->{'node-name'} if $follow_backing && $children->{backing}; } if ($remove_throttle_group_id) { -- 2.47.3