public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH qemu-server] fix #7705: blockdev: detach backing chain on disk move/hot-unplug
@ 2026-06-15  7:34 Arthur Bied-Charreton
  2026-06-15  9:56 ` Arthur Bied-Charreton
  0 siblings, 1 reply; 2+ messages in thread
From: Arthur Bied-Charreton @ 2026-06-15  7:34 UTC (permalink / raw)
  To: pve-devel

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 <a.bied-charreton@proxmox.com>
---
 src/PVE/QemuServer.pm          |  6 +++++-
 src/PVE/QemuServer/BlockJob.pm |  4 +++-
 src/PVE/QemuServer/Blockdev.pm | 16 +++++++++++-----
 3 files changed, 19 insertions(+), 7 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 c8b0b030..eea3c509 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..a04e120c 100644
--- a/src/PVE/QemuServer/Blockdev.pm
+++ b/src/PVE/QemuServer/Blockdev.pm
@@ -655,10 +655,12 @@ Parameters:
 =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 +669,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 +683,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




^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH qemu-server] fix #7705: blockdev: detach backing chain on disk move/hot-unplug
  2026-06-15  7:34 [PATCH qemu-server] fix #7705: blockdev: detach backing chain on disk move/hot-unplug Arthur Bied-Charreton
@ 2026-06-15  9:56 ` Arthur Bied-Charreton
  0 siblings, 0 replies; 2+ messages in thread
From: Arthur Bied-Charreton @ 2026-06-15  9:56 UTC (permalink / raw)
  To: pve-devel

On Mon, Jun 15, 2026 at 09:34:06AM +0200, Arthur Bied-Charreton wrote:
> 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.
Forgot to mention that here but the bug report [0] includes a
reproducer, should fail before the fix and work afterwards :)

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=7705




^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-06-15  9:56 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-15  7:34 [PATCH qemu-server] fix #7705: blockdev: detach backing chain on disk move/hot-unplug Arthur Bied-Charreton
2026-06-15  9:56 ` Arthur Bied-Charreton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal