[pve-devel] [PATCH FOLLOW-UP qemu-server 4/4] blockdev_external_snapshot: rework to avoid $running param

[Alexandre Derumier via pve-devel <pve-devel@lists.proxmox.com>]

[-- Attachment #1: Type: message/rfc822, Size: 7824 bytes --]

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH FOLLOW-UP qemu-server 4/4] blockdev_external_snapshot: rework to avoid $running param
Date: Wed, 16 Jul 2025 08:31:42 +0200
Message-ID: <20250716063153.1647681-8-alexandre.derumier@groupe-cyllene.com>

the new workflow is:

1)
PVE::Storage::volume_snapshot: rename the current to snap
                               + allocate the new current with snap backing

(the qemu process have still the inode opened)

2) replace the current to snap in the blockdev graph with reopen

3) add a new current blockdev

3) use block-snapshot to replace the active image with the new current blockdev
   with the qemu

also:

move PVE::Storage::volume_rename outside blockdev_replace,
and rename 'blockdev_rename()' to 'blockdev_replace()'.
as we only replace/reopen the blockdev nodes now.

Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 src/PVE/QemuServer.pm          | 38 ++++++----------------------------
 src/PVE/QemuServer/Blockdev.pm | 26 ++++++++++++++++-------
 2 files changed, 24 insertions(+), 40 deletions(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index c1e15675..eec887b3 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -4357,38 +4357,9 @@ sub qemu_volume_snapshot {
         my $snapshots = PVE::Storage::volume_snapshot_info($storecfg, $volid);
         my $parent_snap = $snapshots->{'current'}->{parent};
         my $machine_version = PVE::QemuServer::Machine::get_current_qemu_machine($vmid);
-
-        PVE::QemuServer::Blockdev::blockdev_rename(
-            $storecfg,
-            $vmid,
-            $machine_version,
-            $deviceid,
-            $drive,
-            'current',
-            $snap,
-            $parent_snap,
+        PVE::QemuServer::Blockdev::blockdev_external_snapshot(
+            $storecfg, $vmid, $machine_version, $deviceid, $drive, $snap, $parent_snap,
         );
-        eval {
-            PVE::QemuServer::Blockdev::blockdev_external_snapshot(
-                $storecfg, $vmid, $machine_version, $deviceid, $drive, $snap,
-            );
-        };
-        if ($@) {
-            warn $@ if $@;
-            print "Error creating snapshot. Revert rename\n";
-            eval {
-                PVE::QemuServer::Blockdev::blockdev_rename(
-                    $storecfg,
-                    $vmid,
-                    $machine_version,
-                    $deviceid,
-                    $drive,
-                    $snap,
-                    'current',
-                    $parent_snap,
-                );
-            };
-        }
     } elsif ($do_snapshots_type eq 'storage') {
         PVE::Storage::volume_snapshot($storecfg, $volid, $snap);
     }
@@ -4443,7 +4414,10 @@ sub qemu_volume_snapshot_delete {
                 $childsnap,
                 $snap,
             );
-            PVE::QemuServer::Blockdev::blockdev_rename(
+
+            PVE::Storage::rename_snapshot($storecfg, $volid, $snap, $childsnap);
+
+            PVE::QemuServer::Blockdev::blockdev_replace(
                 $storecfg,
                 $vmid,
                 $machine_version,
diff --git a/src/PVE/QemuServer/Blockdev.pm b/src/PVE/QemuServer/Blockdev.pm
index 73eb7c1e..1a87a2a3 100644
--- a/src/PVE/QemuServer/Blockdev.pm
+++ b/src/PVE/QemuServer/Blockdev.pm
@@ -798,14 +798,26 @@ sub set_io_throttle {
 }
 
 sub blockdev_external_snapshot {
-    my ($storecfg, $vmid, $machine_version, $deviceid, $drive, $snap, $size) = @_;
+    my ($storecfg, $vmid, $machine_version, $deviceid, $drive, $snap, $parent_snap) = @_;
 
     print "Creating a new current volume with $snap as backing snap\n";
 
     my $volid = $drive->{file};
 
-    #preallocate add a new current file with reference to backing-file
-    PVE::Storage::volume_snapshot($storecfg, $volid, $snap, 1);
+    #rename current to snap && preallocate add a new current file with reference to snap1 backing-file
+    PVE::Storage::volume_snapshot($storecfg, $volid, $snap);
+
+    #reopen current to snap
+    blockdev_replace(
+        $storecfg,
+        $vmid,
+        $machine_version,
+        $deviceid,
+        $drive,
+        'current',
+        $snap,
+        $parent_snap,
+    );
 
     #be sure to add drive in write mode
     delete($drive->{ro});
@@ -826,6 +838,7 @@ sub blockdev_external_snapshot {
 
     mon_cmd($vmid, 'blockdev-add', %$new_fmt_blockdev);
 
+    print "blockdev-snapshot: reopen current with $snap backing image\n";
     mon_cmd(
         $vmid, 'blockdev-snapshot',
         node => $snap_fmt_blockdev->{'node-name'},
@@ -849,7 +862,7 @@ sub blockdev_delete {
     PVE::Storage::volume_snapshot_delete($storecfg, $volid, $snap, 1);
 }
 
-sub blockdev_rename {
+sub blockdev_replace {
     my (
         $storecfg,
         $vmid,
@@ -861,7 +874,7 @@ sub blockdev_rename {
         $parent_snap,
     ) = @_;
 
-    print "rename $src_snap to $target_snap\n";
+    print "blockdev replace $src_snap by $target_snap\n";
 
     my $volid = $drive->{file};
 
@@ -878,9 +891,6 @@ sub blockdev_rename {
         { 'snapshot-name' => $src_snap },
     );
 
-    #rename the snapshot
-    PVE::Storage::rename_snapshot($storecfg, $volid, $src_snap, $target_snap);
-
     my $target_file_blockdev = generate_file_blockdev(
         $storecfg,
         $drive,
-- 
2.39.5



[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel