[pve-devel] [PATCH FOLLOW-UP qemu-server 1/4] api2: move_disk: use parse_volname to find old volume format

[pve-devel] [PATCH FOLLOW-UP qemu-server 1/4] api2: move_disk: use parse_volname to find old volume format

[Alexandre Derumier via pve-devel]

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

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH FOLLOW-UP qemu-server 1/4] api2: move_disk: use parse_volname to find old volume format
Date: Wed, 16 Jul 2025 08:31:36 +0200
Message-ID: <20250716063153.1647681-2-alexandre.derumier@groupe-cyllene.com>

Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 src/PVE/API2/Qemu.pm | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index 2e6358e4..dbc08737 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -4677,11 +4677,8 @@ __PACKAGE__->register_method({
             die "you can't move a cdrom\n" if PVE::QemuServer::drive_is_cdrom($drive, 1);
 
             my $old_volid = $drive->{file};
-            my $oldfmt;
             my ($oldstoreid, $oldvolname) = PVE::Storage::parse_volume_id($old_volid);
-            if ($oldvolname =~ m/\.(raw|qcow2|vmdk)$/) {
-                $oldfmt = $1;
-            }
+            my $oldfmt = (PVE::Storage::parse_volname($storecfg, $old_volid))[6];
 
             die "you can't move to the same storage with same format\n"
                 if $oldstoreid eq $storeid && (!$format || !$oldfmt || $oldfmt eq $format);
-- 
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

[pve-devel] [PATCH FOLLOW-UP storage 01/14] helpers: make qemu_img* storage config independent

[Alexandre Derumier via pve-devel]

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

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH FOLLOW-UP storage 01/14] helpers: make qemu_img* storage config independent
Date: Wed, 16 Jul 2025 08:31:37 +0200
Message-ID: <20250716063153.1647681-3-alexandre.derumier@groupe-cyllene.com>

From: Fabian Grünbichler <f.gruenbichler@proxmox.com>

by moving the preallocation handling to the call site, and preparing
them for taking further options like cluster size in the future.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 src/PVE/Storage/LVMPlugin.pm | 10 ++++--
 src/PVE/Storage/Plugin.pm    | 66 ++++++++++++++++++++++--------------
 2 files changed, 48 insertions(+), 28 deletions(-)

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index a91801f..f90df18 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -579,11 +579,14 @@ my sub lvm_qcow2_format {
     $class->activate_volume($storeid, $scfg, $name);
     my $path = $class->path($scfg, $name, $storeid);
 
+    my $options = {
+        preallocation => PVE::Storage::Plugin::preallocation_cmd_opt($scfg, $fmt),
+    };
     if ($backing_snap) {
         my $backing_path = $class->path($scfg, $name, $storeid, $backing_snap);
-        PVE::Storage::Plugin::qemu_img_create_qcow2_backed($scfg, $path, $backing_path, $fmt);
+        PVE::Storage::Plugin::qemu_img_create_qcow2_backed($path, $backing_path, $fmt, $options);
     } else {
-        PVE::Storage::Plugin::qemu_img_create($scfg, $fmt, $size, $path);
+        PVE::Storage::Plugin::qemu_img_create($fmt, $size, $path, $options);
     }
 }
 
@@ -868,7 +871,8 @@ sub volume_resize {
     );
 
     if (!$running && $format eq 'qcow2') {
-        PVE::Storage::Plugin::qemu_img_resize($scfg, $path, $format, $size, 10);
+        my $preallocation = PVE::Storage::Plugin::preallocation_cmd_opt($scfg, $fmt);
+        PVE::Storage::Plugin::qemu_img_resize($path, $format, $size, $preallocation, 10);
     }
 
     return 1;
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 0b7989b..6eb91b7 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -615,7 +615,7 @@ sub parse_config {
     return $cfg;
 }
 
-sub preallocation_cmd_option {
+sub preallocation_cmd_opt {
     my ($scfg, $fmt) = @_;
 
     my $prealloc = $scfg->{preallocation};
@@ -626,7 +626,7 @@ sub preallocation_cmd_option {
         die "preallocation mode '$prealloc' not supported by format '$fmt'\n"
             if !$QCOW2_PREALLOCATION->{$prealloc};
 
-        return "preallocation=$prealloc";
+        return $prealloc;
     } elsif ($fmt eq 'raw') {
         $prealloc = $prealloc // 'off';
         $prealloc = 'off' if $prealloc eq 'metadata';
@@ -634,7 +634,7 @@ sub preallocation_cmd_option {
         die "preallocation mode '$prealloc' not supported by format '$fmt'\n"
             if !$RAW_PREALLOCATION->{$prealloc};
 
-        return "preallocation=$prealloc";
+        return $prealloc;
     }
 
     return;
@@ -644,19 +644,21 @@ sub preallocation_cmd_option {
 
 =head3 qemu_img_create
 
-    qemu_img_create($scfg, $fmt, $size, $path)
+    qemu_img_create($fmt, $size, $path, $options)
 
 Create a new qemu image with a specific format C<$format> and size C<$size> for a target C<$path>.
 
+C<$options> currently allows setting the C<preallocation> value
+
 =cut
 
 sub qemu_img_create {
-    my ($scfg, $fmt, $size, $path) = @_;
+    my ($fmt, $size, $path, $options) = @_;
 
     my $cmd = ['/usr/bin/qemu-img', 'create'];
 
-    my $prealloc_opt = preallocation_cmd_option($scfg, $fmt);
-    push @$cmd, '-o', $prealloc_opt if defined($prealloc_opt);
+    push @$cmd, '-o', "preallocation=$options->{preallocation}"
+        if defined($options->{preallocation});
 
     push @$cmd, '-f', $fmt, $path, "${size}K";
 
@@ -667,14 +669,16 @@ sub qemu_img_create {
 
 =head3 qemu_img_create_qcow2_backed
 
-    qemu_img_create_qcow2_backed($scfg, $path, $backing_path, $backing_format)
+    qemu_img_create_qcow2_backed($path, $backing_path, $backing_format, $options)
 
 Create a new qemu qcow2 image C<$path> using an existing backing image C<$backing_path> with backing_format C<$backing_format>.
 
+C<$options> currently allows setting the C<preallocation> value.
+
 =cut
 
 sub qemu_img_create_qcow2_backed {
-    my ($scfg, $path, $backing_path, $backing_format) = @_;
+    my ($path, $backing_path, $backing_format, $options) = @_;
 
     my $cmd = [
         '/usr/bin/qemu-img',
@@ -688,10 +692,10 @@ sub qemu_img_create_qcow2_backed {
         $path,
     ];
 
-    my $options = $QCOW2_CLUSTERS->{backed};
+    my $opts = $QCOW2_CLUSTERS->{backed};
 
-    push @$options, preallocation_cmd_option($scfg, 'qcow2');
-    push @$cmd, '-o', join(',', @$options) if @$options > 0;
+    push @$opts, $options->{preallocation} if defined($options->{preallocation});
+    push @$cmd, '-o', join(',', @$opts) if @$opts > 0;
 
     run_command($cmd, errmsg => "unable to create image");
 }
@@ -722,20 +726,21 @@ sub qemu_img_info {
 
 =head3 qemu_img_measure
 
-    qemu_img_measure($size, $fmt, $timeout, $is_backed)
+    qemu_img_measure($size, $fmt, $timeout, $options)
 
 Returns a json with the maximum size including all metadatas overhead for an image with format C<$fmt> and original size C<$size>Kb.
-If the image is backed C<$is_backed>, we use different cluster size informations.
+
+C<$options> allows specifying qemu-img options that might affect the sizing calculation, such as cluster size.
+
 =cut
 
 sub qemu_img_measure {
-    my ($size, $fmt, $timeout, $is_backed) = @_;
+    my ($size, $fmt, $timeout, $options) = @_;
 
     die "format is missing" if !$fmt;
 
     my $cmd = ['/usr/bin/qemu-img', 'measure', '--output=json', '--size', "${size}K", '-O', $fmt];
-    if ($is_backed) {
-        my $options = $QCOW2_CLUSTERS->{backed};
+    if ($options) {
         push $cmd->@*, '-o', join(',', @$options) if @$options > 0;
     }
     return PVE::Storage::Common::run_qemu_img_json($cmd, $timeout);
@@ -745,20 +750,21 @@ sub qemu_img_measure {
 
 =head3 qemu_img_resize
 
-    qemu_img_resize($scfg, $path, $format, $size, $timeout)
+    qemu_img_resize($scfg, $path, $format, $size, $preallocation, $timeout)
 
 Resize a qemu image C<$path> with format C<$format> to a target Kb size C<$size>.
 Default timeout C<$timeout> is 10s if not specified.
+C<$preallocation> allows to specify the preallocation option for the resize operation.
+
 =cut
 
 sub qemu_img_resize {
-    my ($scfg, $path, $format, $size, $timeout) = @_;
+    my ($scfg, $path, $format, $size, $preallocation, $timeout) = @_;
 
     die "format is missing" if !$format;
 
-    my $prealloc_opt = preallocation_cmd_option($scfg, $format);
     my $cmd = ['/usr/bin/qemu-img', 'resize'];
-    push $cmd->@*, "--$prealloc_opt" if $prealloc_opt;
+    push $cmd->@*, "--preallocation=$preallocation" if $preallocation;
     push $cmd->@*, '-f', $format, $path, $size;
 
     $timeout = 10 if !$timeout;
@@ -1067,7 +1073,10 @@ sub clone_image {
     # Note: we use relative paths, so we need to call chdir before qemu-img
     eval {
         local $CWD = $imagedir;
-        qemu_img_create_qcow2_backed($scfg, $path, "../$basevmid/$basename", $format);
+        my $options = {
+            preallocation => preallocation_cmd_opt($scfg, $format),
+        };
+        qemu_img_create_qcow2_backed($path, "../$basevmid/$basename", $format, $options);
     };
     my $err = $@;
 
@@ -1105,7 +1114,10 @@ sub alloc_image {
         umask $old_umask;
         die $err if $err;
     } else {
-        eval { qemu_img_create($scfg, $fmt, $size, $path) };
+        my $preallocation = preallocation_cmd_opt($scfg, $fmt);
+        my $options = {};
+        $options->{preallocation} = $preallocation if $preallocation;
+        eval { qemu_img_create($fmt, $size, $path, $options) };
         if ($@) {
             unlink $path;
             rmdir $imagedir;
@@ -1122,9 +1134,12 @@ my sub alloc_backed_image {
     my $path = $class->path($scfg, $volname, $storeid);
     my ($vmid, $backing_format) = ($class->parse_volname($volname))[2, 6];
 
+    my $preallocation = preallocation_cmd_opt($scfg, $backing_format);
+    my $options = {};
+    $options->{preallocation} = $preallocation if $preallocation;
     my $backing_volname = get_snap_name($class, $volname, $backing_snap);
     #qemu_img use relative path from base image for the backing_volname by default
-    eval { qemu_img_create_qcow2_backed($scfg, $path, $backing_volname, $backing_format) };
+    eval { qemu_img_create_qcow2_backed($path, $backing_volname, $backing_format, $options) };
     if ($@) {
         unlink $path;
         die "$@";
@@ -1371,7 +1386,8 @@ sub volume_resize {
 
     my $format = ($class->parse_volname($volname))[6];
 
-    qemu_img_resize($scfg, $path, $format, $size, 10);
+    my $preallocation = preallocation_cmd_opt($scfg, $format);
+    qemu_img_resize($path, $format, $size, $preallocation, 10);
 
     return undef;
 }
-- 
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

[pve-devel] [PATCH FOLLOW-UP qemu-server 2/4] blockdev_rename: remove old left-over rename()

[Alexandre Derumier via pve-devel]

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

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH FOLLOW-UP qemu-server 2/4] blockdev_rename: remove old left-over rename()
Date: Wed, 16 Jul 2025 08:31:38 +0200
Message-ID: <20250716063153.1647681-4-alexandre.derumier@groupe-cyllene.com>

Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 src/PVE/QemuServer/Blockdev.pm | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/PVE/QemuServer/Blockdev.pm b/src/PVE/QemuServer/Blockdev.pm
index f5c07e30..dd975e3c 100644
--- a/src/PVE/QemuServer/Blockdev.pm
+++ b/src/PVE/QemuServer/Blockdev.pm
@@ -912,8 +912,6 @@ sub blockdev_rename {
             generate_throttle_blockdev($drive_id, $target_fmt_blockdev->{'node-name'});
         mon_cmd($vmid, 'blockdev-reopen', options => [$throttle_blockdev]);
     } else {
-        rename($src_file_blockdev->{filename}, $target_file_blockdev->{filename});
-
         #intermediate snapshot
         mon_cmd($vmid, 'blockdev-add', %$target_fmt_blockdev);
 
-- 
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

[pve-devel] [PATCH FOLLOW-UP storage 02/14] helpers: move qemu_img* to Common module

[Alexandre Derumier via pve-devel]

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

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH FOLLOW-UP storage 02/14] helpers: move qemu_img* to Common module
Date: Wed, 16 Jul 2025 08:31:39 +0200
Message-ID: <20250716063153.1647681-5-alexandre.derumier@groupe-cyllene.com>

From: Fabian Grünbichler <f.gruenbichler@proxmox.com>

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 src/PVE/Storage/Common.pm    | 147 ++++++++++++++++++++++++++++++---
 src/PVE/Storage/LVMPlugin.pm |  10 +--
 src/PVE/Storage/Plugin.pm    | 153 +++--------------------------------
 3 files changed, 152 insertions(+), 158 deletions(-)

diff --git a/src/PVE/Storage/Common.pm b/src/PVE/Storage/Common.pm
index aa89e68..71d123a 100644
--- a/src/PVE/Storage/Common.pm
+++ b/src/PVE/Storage/Common.pm
@@ -111,18 +111,7 @@ sub deallocate : prototype($$$) {
     }
 }
 
-=pod
-
-=head3 run_qemu_img_json
-
-    run_qemu_img_json($cmd, $timeout)
-
-Execute qemu_img command C<$cmd> with a timeout C<$timeout>.
-Parse the output result and return a json.
-
-=cut
-
-sub run_qemu_img_json {
+my sub run_qemu_img_json {
     my ($cmd, $timeout) = @_;
     my $json = '';
     my $err_output = '';
@@ -143,4 +132,138 @@ sub run_qemu_img_json {
     }
     return $json;
 }
+
+=pod
+
+=head3 qemu_img_create
+
+    qemu_img_create($fmt, $size, $path, $options)
+
+Create a new qemu image with a specific format C<$format> and size C<$size> for a target C<$path>.
+
+C<$options> currently allows setting the C<preallocation> value
+
+=cut
+
+sub qemu_img_create {
+    my ($fmt, $size, $path, $options) = @_;
+
+    my $cmd = ['/usr/bin/qemu-img', 'create'];
+
+    push @$cmd, '-o', "preallocation=$options->{preallocation}"
+        if defined($options->{preallocation});
+
+    push @$cmd, '-f', $fmt, $path, "${size}K";
+
+    run_command($cmd, errmsg => "unable to create image");
+}
+
+=pod
+
+=head3 qemu_img_create_qcow2_backed
+
+    qemu_img_create_qcow2_backed($path, $backing_path, $backing_format, $options)
+
+Create a new qemu qcow2 image C<$path> using an existing backing image C<$backing_path> with backing_format C<$backing_format>.
+
+C<$options> currently allows setting the C<preallocation> value.
+
+=cut
+
+sub qemu_img_create_qcow2_backed {
+    my ($path, $backing_path, $backing_format, $options) = @_;
+
+    my $cmd = [
+        '/usr/bin/qemu-img',
+        'create',
+        '-F',
+        $backing_format,
+        '-b',
+        $backing_path,
+        '-f',
+        'qcow2',
+        $path,
+    ];
+
+    # TODO make this configurable for all volumes/types and pass in via $options
+    my $opts = ['extended_l2=on', 'cluster_size=128k'];
+
+    push @$opts, "preallocation=$options->{preallocation}"
+        if defined($options->{preallocation});
+    push @$cmd, '-o', join(',', @$opts) if @$opts > 0;
+
+    run_command($cmd, errmsg => "unable to create image");
+}
+
+=pod
+
+=head3 qemu_img_info
+
+    qemu_img_info($filename, $file_format, $timeout, $follow_backing_files)
+
+Returns a json with qemu image C<$filename> informations with format <$file_format>.
+If C<$follow_backing_files> option is defined, return a json with the whole chain
+of backing files images.
+
+=cut
+
+sub qemu_img_info {
+    my ($filename, $file_format, $timeout, $follow_backing_files) = @_;
+
+    my $cmd = ['/usr/bin/qemu-img', 'info', '--output=json', $filename];
+    push $cmd->@*, '-f', $file_format if $file_format;
+    push $cmd->@*, '--backing-chain' if $follow_backing_files;
+
+    return run_qemu_img_json($cmd, $timeout);
+}
+
+=pod
+
+=head3 qemu_img_measure
+
+    qemu_img_measure($size, $fmt, $timeout, $options)
+
+Returns a json with the maximum size including all metadatas overhead for an image with format C<$fmt> and original size C<$size>Kb.
+
+C<$options> allows specifying qemu-img options that might affect the sizing calculation, such as cluster size.
+
+=cut
+
+sub qemu_img_measure {
+    my ($size, $fmt, $timeout, $options) = @_;
+
+    die "format is missing" if !$fmt;
+
+    my $cmd = ['/usr/bin/qemu-img', 'measure', '--output=json', '--size', "${size}K", '-O', $fmt];
+    if ($options) {
+        push $cmd->@*, '-o', join(',', @$options) if @$options > 0;
+    }
+    return run_qemu_img_json($cmd, $timeout);
+}
+
+=pod
+
+=head3 qemu_img_resize
+
+    qemu_img_resize($scfg, $path, $format, $size, $preallocation, $timeout)
+
+Resize a qemu image C<$path> with format C<$format> to a target Kb size C<$size>.
+Default timeout C<$timeout> is 10s if not specified.
+C<$preallocation> allows to specify the preallocation option for the resize operation.
+
+=cut
+
+sub qemu_img_resize {
+    my ($scfg, $path, $format, $size, $preallocation, $timeout) = @_;
+
+    die "format is missing" if !$format;
+
+    my $cmd = ['/usr/bin/qemu-img', 'resize'];
+    push $cmd->@*, "--preallocation=$preallocation" if $preallocation;
+    push $cmd->@*, '-f', $format, $path, $size;
+
+    $timeout = 10 if !$timeout;
+    run_command($cmd, timeout => $timeout);
+}
+
 1;
diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index f90df18..4b60e32 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -584,9 +584,9 @@ my sub lvm_qcow2_format {
     };
     if ($backing_snap) {
         my $backing_path = $class->path($scfg, $name, $storeid, $backing_snap);
-        PVE::Storage::Plugin::qemu_img_create_qcow2_backed($path, $backing_path, $fmt, $options);
+        PVE::Storage::Common::qemu_img_create_qcow2_backed($path, $backing_path, $fmt, $options);
     } else {
-        PVE::Storage::Plugin::qemu_img_create($fmt, $size, $path, $options);
+        PVE::Storage::Common::qemu_img_create($fmt, $size, $path, $options);
     }
 }
 
@@ -598,7 +598,7 @@ my sub calculate_lvm_size {
 
     my $options = $backing_snap ? ['extended_l2=on', 'cluster_size=128k'] : [];
 
-    my $json = PVE::Storage::Plugin::qemu_img_measure($size, $fmt, 5, $options);
+    my $json = PVE::Storage::Common::qemu_img_measure($size, $fmt, 5, $options);
     die "failed to query file information with qemu-img measure\n" if !$json;
     my $info = eval { decode_json($json) };
     if ($@) {
@@ -871,8 +871,8 @@ sub volume_resize {
     );
 
     if (!$running && $format eq 'qcow2') {
-        my $preallocation = PVE::Storage::Plugin::preallocation_cmd_opt($scfg, $fmt);
-        PVE::Storage::Plugin::qemu_img_resize($path, $format, $size, $preallocation, 10);
+        my $preallocation = PVE::Storage::Plugin::preallocation_cmd_opt($scfg, $format);
+        PVE::Storage::Common::qemu_img_resize($path, $format, $size, $preallocation, 10);
     }
 
     return 1;
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 6eb91b7..aee145f 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -51,10 +51,6 @@ our $RAW_PREALLOCATION = {
     full => 1,
 };
 
-my $QCOW2_CLUSTERS = {
-    backed => ['extended_l2=on', 'cluster_size=128k'],
-};
-
 our $MAX_VOLUMES_PER_GUEST = 1024;
 
 cfs_register_file(
@@ -640,137 +636,6 @@ sub preallocation_cmd_opt {
     return;
 }
 
-=pod
-
-=head3 qemu_img_create
-
-    qemu_img_create($fmt, $size, $path, $options)
-
-Create a new qemu image with a specific format C<$format> and size C<$size> for a target C<$path>.
-
-C<$options> currently allows setting the C<preallocation> value
-
-=cut
-
-sub qemu_img_create {
-    my ($fmt, $size, $path, $options) = @_;
-
-    my $cmd = ['/usr/bin/qemu-img', 'create'];
-
-    push @$cmd, '-o', "preallocation=$options->{preallocation}"
-        if defined($options->{preallocation});
-
-    push @$cmd, '-f', $fmt, $path, "${size}K";
-
-    run_command($cmd, errmsg => "unable to create image");
-}
-
-=pod
-
-=head3 qemu_img_create_qcow2_backed
-
-    qemu_img_create_qcow2_backed($path, $backing_path, $backing_format, $options)
-
-Create a new qemu qcow2 image C<$path> using an existing backing image C<$backing_path> with backing_format C<$backing_format>.
-
-C<$options> currently allows setting the C<preallocation> value.
-
-=cut
-
-sub qemu_img_create_qcow2_backed {
-    my ($path, $backing_path, $backing_format, $options) = @_;
-
-    my $cmd = [
-        '/usr/bin/qemu-img',
-        'create',
-        '-F',
-        $backing_format,
-        '-b',
-        $backing_path,
-        '-f',
-        'qcow2',
-        $path,
-    ];
-
-    my $opts = $QCOW2_CLUSTERS->{backed};
-
-    push @$opts, $options->{preallocation} if defined($options->{preallocation});
-    push @$cmd, '-o', join(',', @$opts) if @$opts > 0;
-
-    run_command($cmd, errmsg => "unable to create image");
-}
-
-=pod
-
-=head3 qemu_img_info
-
-    qemu_img_info($filename, $file_format, $timeout, $follow_backing_files)
-
-Returns a json with qemu image C<$filename> informations with format <$file_format>.
-If C<$follow_backing_files> option is defined, return a json with the whole chain
-of backing files images.
-
-=cut
-
-sub qemu_img_info {
-    my ($filename, $file_format, $timeout, $follow_backing_files) = @_;
-
-    my $cmd = ['/usr/bin/qemu-img', 'info', '--output=json', $filename];
-    push $cmd->@*, '-f', $file_format if $file_format;
-    push $cmd->@*, '--backing-chain' if $follow_backing_files;
-
-    return PVE::Storage::Common::run_qemu_img_json($cmd, $timeout);
-}
-
-=pod
-
-=head3 qemu_img_measure
-
-    qemu_img_measure($size, $fmt, $timeout, $options)
-
-Returns a json with the maximum size including all metadatas overhead for an image with format C<$fmt> and original size C<$size>Kb.
-
-C<$options> allows specifying qemu-img options that might affect the sizing calculation, such as cluster size.
-
-=cut
-
-sub qemu_img_measure {
-    my ($size, $fmt, $timeout, $options) = @_;
-
-    die "format is missing" if !$fmt;
-
-    my $cmd = ['/usr/bin/qemu-img', 'measure', '--output=json', '--size', "${size}K", '-O', $fmt];
-    if ($options) {
-        push $cmd->@*, '-o', join(',', @$options) if @$options > 0;
-    }
-    return PVE::Storage::Common::run_qemu_img_json($cmd, $timeout);
-}
-
-=pod
-
-=head3 qemu_img_resize
-
-    qemu_img_resize($scfg, $path, $format, $size, $preallocation, $timeout)
-
-Resize a qemu image C<$path> with format C<$format> to a target Kb size C<$size>.
-Default timeout C<$timeout> is 10s if not specified.
-C<$preallocation> allows to specify the preallocation option for the resize operation.
-
-=cut
-
-sub qemu_img_resize {
-    my ($scfg, $path, $format, $size, $preallocation, $timeout) = @_;
-
-    die "format is missing" if !$format;
-
-    my $cmd = ['/usr/bin/qemu-img', 'resize'];
-    push $cmd->@*, "--preallocation=$preallocation" if $preallocation;
-    push $cmd->@*, '-f', $format, $path, $size;
-
-    $timeout = 10 if !$timeout;
-    run_command($cmd, timeout => $timeout);
-}
-
 # Storage implementation
 
 # called during addition of storage (before the new storage config got written)
@@ -1076,7 +941,9 @@ sub clone_image {
         my $options = {
             preallocation => preallocation_cmd_opt($scfg, $format),
         };
-        qemu_img_create_qcow2_backed($path, "../$basevmid/$basename", $format, $options);
+        PVE::Storage::Common::qemu_img_create_qcow2_backed(
+            $path, "../$basevmid/$basename", $format, $options,
+        );
     };
     my $err = $@;
 
@@ -1117,7 +984,7 @@ sub alloc_image {
         my $preallocation = preallocation_cmd_opt($scfg, $fmt);
         my $options = {};
         $options->{preallocation} = $preallocation if $preallocation;
-        eval { qemu_img_create($fmt, $size, $path, $options) };
+        eval { PVE::Storage::Common::qemu_img_create($fmt, $size, $path, $options) };
         if ($@) {
             unlink $path;
             rmdir $imagedir;
@@ -1139,7 +1006,11 @@ my sub alloc_backed_image {
     $options->{preallocation} = $preallocation if $preallocation;
     my $backing_volname = get_snap_name($class, $volname, $backing_snap);
     #qemu_img use relative path from base image for the backing_volname by default
-    eval { qemu_img_create_qcow2_backed($path, $backing_volname, $backing_format, $options) };
+    eval {
+        PVE::Storage::Common::qemu_img_create_qcow2_backed(
+            $path, $backing_volname, $backing_format, $options,
+        );
+    };
     if ($@) {
         unlink $path;
         die "$@";
@@ -1271,7 +1142,7 @@ sub file_size_info {
             "file_size_info: '$filename': falling back to 'raw' from unknown format '$file_format'\n";
         $file_format = 'raw';
     }
-    my $json = qemu_img_info($filename, $file_format, $timeout);
+    my $json = PVE::Storage::Common::qemu_img_info($filename, $file_format, $timeout);
     if (!$json) {
         die "failed to query file information with qemu-img\n" if $untrusted;
         # skip decoding if there was no output, e.g. if there was a timeout.
@@ -1387,7 +1258,7 @@ sub volume_resize {
     my $format = ($class->parse_volname($volname))[6];
 
     my $preallocation = preallocation_cmd_opt($scfg, $format);
-    qemu_img_resize($path, $format, $size, $preallocation, 10);
+    PVE::Storage::Common::qemu_img_resize($path, $format, $size, $preallocation, 10);
 
     return undef;
 }
@@ -1879,7 +1750,7 @@ sub volume_snapshot_info {
     my ($vtype, $name, $vmid, $basename, $basevmid, $isBase, $format) =
         $class->parse_volname($volname);
 
-    my $json = qemu_img_info($path, undef, 10, 1);
+    my $json = PVE::Storage::Common::qemu_img_info($path, undef, 10, 1);
     die "failed to query file information with qemu-img\n" if !$json;
     my $json_decode = eval { decode_json($json) };
     if ($@) {
-- 
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

[pve-devel] [PATCH FOLLOW-UP qemu-server 3/4] generate_backing_blockdev: use current_sub for private recursive

[Alexandre Derumier via pve-devel]

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

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH FOLLOW-UP qemu-server 3/4] generate_backing_blockdev: use current_sub for private recursive
Date: Wed, 16 Jul 2025 08:31:40 +0200
Message-ID: <20250716063153.1647681-6-alexandre.derumier@groupe-cyllene.com>

Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 src/PVE/QemuServer/Blockdev.pm | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/PVE/QemuServer/Blockdev.pm b/src/PVE/QemuServer/Blockdev.pm
index dd975e3c..73eb7c1e 100644
--- a/src/PVE/QemuServer/Blockdev.pm
+++ b/src/PVE/QemuServer/Blockdev.pm
@@ -367,9 +367,8 @@ my sub generate_format_blockdev {
     return $blockdev;
 }
 
-my sub generate_backing_blockdev;
-
-sub generate_backing_blockdev {
+my sub generate_backing_blockdev {
+    use feature 'current_sub';
     my ($storecfg, $snapshots, $deviceid, $drive, $machine_version, $options) = @_;
 
     my $snap_id = $options->{'snapshot-name'};
@@ -386,7 +385,7 @@ sub generate_backing_blockdev {
 
     if ($parentid) {
         my $options = { 'snapshot-name' => $parentid };
-        $snap_fmt_blockdev->{backing} = generate_backing_blockdev(
+        $snap_fmt_blockdev->{backing} = __SUB__->(
             $storecfg, $snapshots, $deviceid, $drive, $machine_version, $options,
         );
     }
-- 
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

[pve-devel] [PATCH FOLLOW-UP storage 03/14] rename_snapshot: fix parameter checks

[Alexandre Derumier via pve-devel]

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

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH FOLLOW-UP storage 03/14] rename_snapshot: fix parameter checks
Date: Wed, 16 Jul 2025 08:31:41 +0200
Message-ID: <20250716063153.1647681-7-alexandre.derumier@groupe-cyllene.com>

From: Fabian Grünbichler <f.gruenbichler@proxmox.com>

both source and target snapshot need to be provided when renaming.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 src/PVE/Storage.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index 53965ee..b3ca094 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -2348,7 +2348,8 @@ sub rename_snapshot {
     my ($cfg, $volid, $source_snap, $target_snap) = @_;
 
     die "no volid provided\n" if !$volid;
-    die "no source or target snap provided\n" if !$source_snap && !$target_snap;
+    die "no source snapshot provided\n" if !$source_snap;
+    die "no target snapshot provided\n" if !$target_snap;
 
     my ($storeid, $volname) = parse_volume_id($volid);
 
-- 
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

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

[Alexandre Derumier via pve-devel]

[-- 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

[pve-devel] [PATCH FOLLOW-UP storage 04/14] lvm snapshot: activate volume

[Alexandre Derumier via pve-devel]

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

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH FOLLOW-UP storage 04/14] lvm snapshot: activate volume
Date: Wed, 16 Jul 2025 08:31:43 +0200
Message-ID: <20250716063153.1647681-9-alexandre.derumier@groupe-cyllene.com>

Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 src/PVE/Storage/LVMPlugin.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 4b60e32..54070d1 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -924,6 +924,8 @@ sub volume_snapshot {
         return;
     }
 
+    $class->activate_volume($storeid, $scfg, $volname);
+
     #rename current volume to snap volume
     eval { $class->rename_snapshot($scfg, $storeid, $volname, 'current', $snap) };
     die "error rename $volname to $snap\n" if $@;
-- 
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

[pve-devel] [PATCH FOLLOW-UP storage 05/14] common: fix qemu_img_resize

[Alexandre Derumier via pve-devel]

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

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH FOLLOW-UP storage 05/14] common: fix qemu_img_resize
Date: Wed, 16 Jul 2025 08:31:44 +0200
Message-ID: <20250716063153.1647681-10-alexandre.derumier@groupe-cyllene.com>

Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 src/PVE/Storage/Common.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/PVE/Storage/Common.pm b/src/PVE/Storage/Common.pm
index 71d123a..746a262 100644
--- a/src/PVE/Storage/Common.pm
+++ b/src/PVE/Storage/Common.pm
@@ -245,7 +245,7 @@ sub qemu_img_measure {
 
 =head3 qemu_img_resize
 
-    qemu_img_resize($scfg, $path, $format, $size, $preallocation, $timeout)
+    qemu_img_resize($path, $format, $size, $preallocation, $timeout)
 
 Resize a qemu image C<$path> with format C<$format> to a target Kb size C<$size>.
 Default timeout C<$timeout> is 10s if not specified.
@@ -254,7 +254,7 @@ C<$preallocation> allows to specify the preallocation option for the resize oper
 =cut
 
 sub qemu_img_resize {
-    my ($scfg, $path, $format, $size, $preallocation, $timeout) = @_;
+    my ($path, $format, $size, $preallocation, $timeout) = @_;
 
     die "format is missing" if !$format;
 
-- 
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

[pve-devel] [PATCH FOLLOW-UP storage 06/14] plugin: volume_export: don't allow export of external snapshots

[Alexandre Derumier via pve-devel]

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

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH FOLLOW-UP storage 06/14] plugin: volume_export: don't allow export of external snapshots
Date: Wed, 16 Jul 2025 08:31:45 +0200
Message-ID: <20250716063153.1647681-11-alexandre.derumier@groupe-cyllene.com>

not yet implemented

Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 src/PVE/Storage/Plugin.pm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index aee145f..06d258e 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -2010,6 +2010,9 @@ sub volume_export {
     my ($class, $scfg, $storeid, $fh, $volname, $format, $snapshot, $base_snapshot, $with_snapshots)
         = @_;
 
+    die "cannot export volumes together with their snapshots in $class\n"
+        if $with_snapshots && $scfg->{'external-snapshots'};
+
     my $err_msg = "volume export format $format not available for $class\n";
     if ($scfg->{path} && !defined($snapshot) && !defined($base_snapshot)) {
         my ($file) = $class->path($scfg, $volname, $storeid) or die $err_msg;
-- 
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

[pve-devel] [PATCH FOLLOW-UP storage 07/14] lvmplugin: alloc_snap_image: die if file_size_info return empty size

[Alexandre Derumier via pve-devel]

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

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH FOLLOW-UP storage 07/14] lvmplugin: alloc_snap_image: die if file_size_info return empty size
Date: Wed, 16 Jul 2025 08:31:46 +0200
Message-ID: <20250716063153.1647681-12-alexandre.derumier@groupe-cyllene.com>

Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 src/PVE/Storage/LVMPlugin.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 54070d1..cb5fd37 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -668,6 +668,8 @@ my sub alloc_snap_image {
 
     #we need to use same size than the backing image qcow2 virtual-size
     my $size = PVE::Storage::Plugin::file_size_info($path, 5, $format);
+    die "file_size_info on '$volname' failed\n" if !defined($size);
+
     $size = $size / 1024; #we use kb in lvcreate
 
     alloc_lvm_image($class, $storeid, $scfg, $vmid, $format, $volname, $size, $backing_snap);
-- 
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

[pve-devel] [PATCH FOLLOW-UP storage 08/14] lvmplugin: snapshot: use relative path for backing image

[Alexandre Derumier via pve-devel]

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

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH FOLLOW-UP storage 08/14] lvmplugin: snapshot: use relative path for backing image
Date: Wed, 16 Jul 2025 08:31:47 +0200
Message-ID: <20250716063153.1647681-13-alexandre.derumier@groupe-cyllene.com>

Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 src/PVE/Storage/LVMPlugin.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index cb5fd37..9aadbc2 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -583,8 +583,8 @@ my sub lvm_qcow2_format {
         preallocation => PVE::Storage::Plugin::preallocation_cmd_opt($scfg, $fmt),
     };
     if ($backing_snap) {
-        my $backing_path = $class->path($scfg, $name, $storeid, $backing_snap);
-        PVE::Storage::Common::qemu_img_create_qcow2_backed($path, $backing_path, $fmt, $options);
+        my $backing_volname = get_snap_name($class, $name, $backing_snap);
+        PVE::Storage::Common::qemu_img_create_qcow2_backed($path, $backing_volname, $fmt, $options);
     } else {
         PVE::Storage::Common::qemu_img_create($fmt, $size, $path, $options);
     }
-- 
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

[pve-devel] [PATCH FOLLOW-UP storage 09/14] plugin|lvmplugin: don't allow volume rename if external snapshots exist.

[Alexandre Derumier via pve-devel]

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

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH FOLLOW-UP storage 09/14] plugin|lvmplugin: don't allow volume rename if external snapshots exist.
Date: Wed, 16 Jul 2025 08:31:48 +0200
Message-ID: <20250716063153.1647681-14-alexandre.derumier@groupe-cyllene.com>

Just to safe, as this is already check higher in the stack.

Technically, it's possible to implement snapshot file renaming,
and update backing_file info with "qemu-img rebase -u".

Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 src/PVE/Storage/LVMPlugin.pm | 6 ++++++
 src/PVE/Storage/Plugin.pm    | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 9aadbc2..8a61bc5 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -1233,6 +1233,12 @@ sub rename_volume {
     my (
         undef, $source_image, $source_vmid, $base_name, $base_vmid, undef, $format,
     ) = $class->parse_volname($source_volname);
+
+    if ($format eq 'qcow2') {
+        my $snapshots = $class->volume_snapshot_info($scfg, $storeid, $source_volname);
+        die "we can't rename volume if external snapshot exists" if $snapshots->{current}->{parent};
+    }
+
     $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid, $format)
         if !$target_volname;
 
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 06d258e..8ea62e4 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -2175,6 +2175,11 @@ sub rename_volume {
     die "not implemented in storage plugin '$class'\n" if $class->can('api') && $class->api() < 10;
     die "no path found\n" if !$scfg->{path};
 
+    if ($scfg->{'external-snapshots'}) {
+        my $snapshots = $class->volume_snapshot_info($scfg, $storeid, $source_volname);
+        die "we can't rename volume if external snapshot exists" if $snapshots->{current}->{parent};
+    }
+
     my (
         undef, $source_image, $source_vmid, $base_name, $base_vmid, undef, $format,
     ) = $class->parse_volname($source_volname);
-- 
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

[pve-devel] [PATCH FOLLOW-UP storage 10/14] lvmplugin: add volume_snapshot_info

[Alexandre Derumier via pve-devel]

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

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH FOLLOW-UP storage 10/14] lvmplugin: add volume_snapshot_info
Date: Wed, 16 Jul 2025 08:31:49 +0200
Message-ID: <20250716063153.1647681-15-alexandre.derumier@groupe-cyllene.com>

and remove public methods:
get_snapname_from_path
get_snap_volname

Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 src/PVE/Storage/LVMPlugin.pm | 76 ++++++++++++++++++++++++++----------
 src/PVE/Storage/Plugin.pm    | 41 ++++++++-----------
 2 files changed, 73 insertions(+), 44 deletions(-)

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 8a61bc5..0e2c38c 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -443,20 +443,6 @@ sub parse_volname {
     die "unable to parse lvm volume name '$volname'\n";
 }
 
-#return snapshot name from a file path
-sub get_snapname_from_path {
-    my ($class, $volname, $path) = @_;
-
-    my $basepath = basename($path);
-    my $name = ($volname =~ s/\.[^.]+$//r);
-    if ($basepath =~ m/^snap_${name}_(.*)\.qcow2$/) {
-        return $1;
-    } elsif ($basepath eq $volname) {
-        return 'current';
-    }
-    return undef;
-}
-
 my sub get_snap_name {
     my ($class, $volname, $snapname) = @_;
 
@@ -472,12 +458,6 @@ my sub get_snap_name {
     }
 }
 
-sub get_snap_volname {
-    my ($class, $volname, $snapname) = @_;
-
-    return get_snap_name($class, $volname, $snapname);
-}
-
 sub filesystem_path {
     my ($class, $scfg, $volname, $snapname) = @_;
 
@@ -787,6 +767,62 @@ sub status {
     return undef;
 }
 
+sub volume_snapshot_info {
+    my ($class, $scfg, $storeid, $volname) = @_;
+
+    my $get_snapname_from_path = sub {
+        my ($volname, $path) = @_;
+
+        my $basepath = basename($path);
+        my $name = ($volname =~ s/\.[^.]+$//r);
+        if ($basepath =~ m/^snap_${name}_(.*)\.qcow2$/) {
+            return $1;
+        } elsif ($basepath eq $volname) {
+            return 'current';
+        }
+        return undef;
+    };
+
+    my $path = $class->filesystem_path($scfg, $volname);
+    my ($vtype, $name, $vmid, $basename, $basevmid, $isBase, $format) =
+        $class->parse_volname($volname);
+
+    my $json = PVE::Storage::Common::qemu_img_info($path, undef, 10, 1);
+    die "failed to query file information with qemu-img\n" if !$json;
+    my $json_decode = eval { decode_json($json) };
+    if ($@) {
+        die "Can't decode qemu snapshot list. Invalid JSON: $@\n";
+    }
+    my $info = {};
+    my $order = 0;
+    return $info if ref($json_decode) ne 'ARRAY';
+
+    #no snapshot or external  snapshots is an arrayref
+    my $snapshots = $json_decode;
+    for my $snap (@$snapshots) {
+        my $snapfile = $snap->{filename};
+        my $snapname = $get_snapname_from_path->($volname, $snapfile);
+        #not a proxmox snapshot
+        next if !$snapname;
+
+        my $snapvolname = get_snap_name($class, $volname, $snapname);
+
+        $info->{$snapname}->{order} = $order;
+        $info->{$snapname}->{file} = $snapfile;
+        $info->{$snapname}->{volname} = "$snapvolname";
+        $info->{$snapname}->{volid} = "$storeid:$snapvolname";
+
+        my $parentfile = $snap->{'backing-filename'};
+        if ($parentfile) {
+            my $parentname = $get_snapname_from_path->($volname, $parentfile);
+            $info->{$snapname}->{parent} = $parentname;
+            $info->{$parentname}->{child} = $snapname;
+        }
+        $order++;
+    }
+    return $info;
+}
+
 sub activate_storage {
     my ($class, $storeid, $scfg, $cache) = @_;
 
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 8ea62e4..aafb13c 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1718,27 +1718,6 @@ sub status {
     return ($res->{total}, $res->{avail}, $res->{used}, 1);
 }
 
-sub get_snap_volname {
-    my ($class, $volname, $snapname) = @_;
-
-    my $vmid = ($class->parse_volname($volname))[2];
-    my $name = get_snap_name($class, $volname, $snapname);
-    return "$vmid/$name";
-}
-
-#return snapshot name from a file path
-sub get_snapname_from_path {
-    my ($class, $volname, $path) = @_;
-
-    my $basepath = basename($path);
-    if ($basepath =~ m/^snap-(.*)-vm(.*)$/) {
-        return $1;
-    } elsif ($basepath eq basename($volname)) {
-        return 'current';
-    }
-    return undef;
-}
-
 # Returns a hash with the snapshot names as keys and the following data:
 # id        - Unique id to distinguish different snapshots even if the have the same name.
 # timestamp - Creation time of the snapshot (seconds since epoch).
@@ -1746,6 +1725,18 @@ sub get_snapname_from_path {
 sub volume_snapshot_info {
     my ($class, $scfg, $storeid, $volname) = @_;
 
+    my $get_snapname_from_path = sub {
+        my ($volname, $path) = @_;
+
+        my $basepath = basename($path);
+        if ($basepath =~ m/^snap-(.*)-vm(.*)$/) {
+            return $1;
+        } elsif ($basepath eq basename($volname)) {
+            return 'current';
+        }
+        return undef;
+    };
+
     my $path = $class->filesystem_path($scfg, $volname);
     my ($vtype, $name, $vmid, $basename, $basevmid, $isBase, $format) =
         $class->parse_volname($volname);
@@ -1772,11 +1763,13 @@ sub volume_snapshot_info {
         my $snapshots = $json_decode;
         for my $snap (@$snapshots) {
             my $snapfile = $snap->{filename};
-            my $snapname = $class->get_snapname_from_path($volname, $snapfile);
+            my $snapname = $get_snapname_from_path->($volname, $snapfile);
             #not a proxmox snapshot
             next if !$snapname;
 
-            my $snapvolname = $class->get_snap_volname($volname, $snapname);
+            my $snapvolname = get_snap_name($class, $volname, $snapname);
+            $snapvolname = "$vmid/$snapvolname";
+
             $info->{$snapname}->{order} = $order;
             $info->{$snapname}->{file} = $snapfile;
             $info->{$snapname}->{volname} = "$snapvolname";
@@ -1785,7 +1778,7 @@ sub volume_snapshot_info {
 
             my $parentfile = $snap->{'backing-filename'};
             if ($parentfile) {
-                my $parentname = $class->get_snapname_from_path($volname, $parentfile);
+                my $parentname = $get_snapname_from_path->($volname, $parentfile);
                 $info->{$snapname}->{parent} = $parentname;
                 $info->{$parentname}->{child} = $snapname;
             }
-- 
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

[pve-devel] [PATCH FOLLOW-UP storage 11/14] plugin: lvmplugin: add parse_snap_name

[Alexandre Derumier via pve-devel]

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

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH FOLLOW-UP storage 11/14] plugin: lvmplugin: add parse_snap_name
Date: Wed, 16 Jul 2025 08:31:50 +0200
Message-ID: <20250716063153.1647681-16-alexandre.derumier@groupe-cyllene.com>

Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 src/PVE/Storage/LVMPlugin.pm | 17 ++++++++++++-----
 src/PVE/Storage/Plugin.pm    | 17 +++++++++++++----
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 0e2c38c..6e77eb3 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -458,6 +458,14 @@ my sub get_snap_name {
     }
 }
 
+my sub parse_snap_name {
+    my ($name) = @_;
+
+    if ($name =~ m/^snap_\S+_(.*)\.qcow2$/) {
+        return $1;
+    }
+}
+
 sub filesystem_path {
     my ($class, $scfg, $volname, $snapname) = @_;
 
@@ -773,11 +781,10 @@ sub volume_snapshot_info {
     my $get_snapname_from_path = sub {
         my ($volname, $path) = @_;
 
-        my $basepath = basename($path);
-        my $name = ($volname =~ s/\.[^.]+$//r);
-        if ($basepath =~ m/^snap_${name}_(.*)\.qcow2$/) {
-            return $1;
-        } elsif ($basepath eq $volname) {
+        my $name = basename($path);
+        if (my $snapname = parse_snap_name($name)) {
+            return $snapname;
+        } elsif ($name eq $volname) {
             return 'current';
         }
         return undef;
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index aafb13c..59ffa5e 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -779,6 +779,14 @@ my sub get_snap_name {
     return $name;
 }
 
+my sub parse_snap_name {
+    my ($name) = @_;
+
+    if ($name =~ m/^snap-(.*)-vm(.*)$/) {
+        return $1;
+    }
+}
+
 sub filesystem_path {
     my ($class, $scfg, $volname, $snapname) = @_;
 
@@ -1728,10 +1736,11 @@ sub volume_snapshot_info {
     my $get_snapname_from_path = sub {
         my ($volname, $path) = @_;
 
-        my $basepath = basename($path);
-        if ($basepath =~ m/^snap-(.*)-vm(.*)$/) {
-            return $1;
-        } elsif ($basepath eq basename($volname)) {
+        my $name = basename($path);
+
+        if (my $snapname = parse_snap_name($name)) {
+            return $snapname;
+        } elsif ($name eq basename($volname)) {
             return 'current';
         }
         return undef;
-- 
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

[pve-devel] [PATCH FOLLOW-UP storage 12/14] plugin : improve parse_namedir warning

[Alexandre Derumier via pve-devel]

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

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH FOLLOW-UP storage 12/14] plugin : improve parse_namedir warning
Date: Wed, 16 Jul 2025 08:31:51 +0200
Message-ID: <20250716063153.1647681-17-alexandre.derumier@groupe-cyllene.com>

display the volname
skip warning for external snapshot name

Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 src/PVE/Storage/Plugin.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 59ffa5e..439e4c3 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -694,7 +694,7 @@ sub parse_name_dir {
         my $isbase = $2 eq 'base-' ? $2 : undef;
         return ($1, $4, $isbase); # (name, format, isBase)
     } elsif ($name =~ m!^((base-)?[^/\s]+\.(raw|qcow2|vmdk|subvol))$!) {
-        warn "this volume filename is not supported anymore\n";
+        warn "this volume name `$name` is not supported anymore\n" if !parse_snap_name($name);
     }
 
     die "unable to parse volume filename '$name'\n";
-- 
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

[pve-devel] [PATCH FOLLOW-UP storage 13/14] lvmplugin: add external-snapshots option && forbid creation of qcow2 volumes without it

[Alexandre Derumier via pve-devel]

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

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH FOLLOW-UP storage 13/14] lvmplugin: add external-snapshots option && forbid creation of qcow2 volumes without it
Date: Wed, 16 Jul 2025 08:31:52 +0200
Message-ID: <20250716063153.1647681-18-alexandre.derumier@groupe-cyllene.com>

Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 src/PVE/Storage/LVMPlugin.pm | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 6e77eb3..1f5a5f1 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -399,6 +399,7 @@ sub options {
         base => { fixed => 1, optional => 1 },
         tagged_only => { optional => 1 },
         bwlimit => { optional => 1 },
+        'external-snapshots' => { optional => 1 },
     };
 }
 
@@ -603,6 +604,10 @@ my sub alloc_lvm_image {
 
     die "unsupported format '$fmt'" if $fmt ne 'raw' && $fmt ne 'qcow2';
 
+    die "external-snapshots option need to be enabled to use qcow2 format"
+        if $fmt eq 'qcow2'
+        && !$scfg->{'external-snapshots'};
+
     $class->parse_volname($name);
 
     my $vgs = lvm_vgs();
-- 
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

[pve-devel] [PATCH FOLLOW-UP storage 14/14] storage: remove $running param from volume_snapshot

[Alexandre Derumier via pve-devel]

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

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH FOLLOW-UP storage 14/14] storage: remove $running param from volume_snapshot
Date: Wed, 16 Jul 2025 08:31:53 +0200
Message-ID: <20250716063153.1647681-19-alexandre.derumier@groupe-cyllene.com>

not needed anymore after change in qemu-server

Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 ApiChangeLog                         |  4 ----
 src/PVE/Storage.pm                   |  4 ++--
 src/PVE/Storage/ESXiPlugin.pm        |  2 +-
 src/PVE/Storage/ISCSIDirectPlugin.pm |  2 +-
 src/PVE/Storage/LVMPlugin.pm         | 11 +----------
 src/PVE/Storage/LvmThinPlugin.pm     |  2 +-
 src/PVE/Storage/PBSPlugin.pm         |  2 +-
 src/PVE/Storage/Plugin.pm            | 16 ++++++----------
 src/PVE/Storage/RBDPlugin.pm         |  2 +-
 src/PVE/Storage/ZFSPoolPlugin.pm     |  2 +-
 10 files changed, 15 insertions(+), 32 deletions(-)

diff --git a/ApiChangeLog b/ApiChangeLog
index 68e94fd..58996a9 100644
--- a/ApiChangeLog
+++ b/ApiChangeLog
@@ -22,10 +22,6 @@ Future changes should be documented in here.
   Feel free to request allowing more drivers or options on the pve-devel mailing list based on your
   needs.
 
-* Add `running` parameter to `volume_snapshot()`
-    The parameter *can* be used if some extra actions need to be done at the storage
-    layer when the snapshot has already be done at qemu level when the vm is running.
-
 * Introduce rename_snapshot() plugin method
     This method allow to rename a vm disk snapshot name to a different snapshot name.
 
diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index b3ca094..893b611 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -449,13 +449,13 @@ sub volume_rollback_is_possible {
 }
 
 sub volume_snapshot {
-    my ($cfg, $volid, $snap, $running) = @_;
+    my ($cfg, $volid, $snap) = @_;
 
     my ($storeid, $volname) = parse_volume_id($volid, 1);
     if ($storeid) {
         my $scfg = storage_config($cfg, $storeid);
         my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
-        return $plugin->volume_snapshot($scfg, $storeid, $volname, $snap, $running);
+        return $plugin->volume_snapshot($scfg, $storeid, $volname, $snap);
     } elsif ($volid =~ m|^(/.+)$| && -e $volid) {
         die "snapshot file/device '$volid' is not possible\n";
     } else {
diff --git a/src/PVE/Storage/ESXiPlugin.pm b/src/PVE/Storage/ESXiPlugin.pm
index 66ef289..eeb6a48 100644
--- a/src/PVE/Storage/ESXiPlugin.pm
+++ b/src/PVE/Storage/ESXiPlugin.pm
@@ -561,7 +561,7 @@ sub volume_size_info {
 }
 
 sub volume_snapshot {
-    my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
+    my ($class, $scfg, $storeid, $volname, $snap) = @_;
 
     die "creating snapshots is not supported for $class\n";
 }
diff --git a/src/PVE/Storage/ISCSIDirectPlugin.pm b/src/PVE/Storage/ISCSIDirectPlugin.pm
index 93cfd3c..62e9026 100644
--- a/src/PVE/Storage/ISCSIDirectPlugin.pm
+++ b/src/PVE/Storage/ISCSIDirectPlugin.pm
@@ -232,7 +232,7 @@ sub volume_resize {
 }
 
 sub volume_snapshot {
-    my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
+    my ($class, $scfg, $storeid, $volname, $snap) = @_;
     die "volume snapshot is not possible on iscsi device\n";
 }
 
diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 1f5a5f1..35e1732 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -959,21 +959,12 @@ sub volume_size_info {
 }
 
 sub volume_snapshot {
-    my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
+    my ($class, $scfg, $storeid, $volname, $snap) = @_;
 
     my ($vmid, $format) = ($class->parse_volname($volname))[2, 6];
 
     die "can't snapshot '$format' volume\n" if $format ne 'qcow2';
 
-    if ($running) {
-        #rename with blockdev-reopen is done at qemu level when running
-        eval { alloc_snap_image($class, $storeid, $scfg, $volname, $snap) };
-        if ($@) {
-            die "can't allocate new volume $volname: $@\n";
-        }
-        return;
-    }
-
     $class->activate_volume($storeid, $scfg, $volname);
 
     #rename current volume to snap volume
diff --git a/src/PVE/Storage/LvmThinPlugin.pm b/src/PVE/Storage/LvmThinPlugin.pm
index 13c0275..6f8a92e 100644
--- a/src/PVE/Storage/LvmThinPlugin.pm
+++ b/src/PVE/Storage/LvmThinPlugin.pm
@@ -353,7 +353,7 @@ sub create_base {
 # sub volume_resize {} reuse code from parent class
 
 sub volume_snapshot {
-    my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
+    my ($class, $scfg, $storeid, $volname, $snap) = @_;
 
     my $vg = $scfg->{vgname};
     my $snapvol = "snap_${volname}_$snap";
diff --git a/src/PVE/Storage/PBSPlugin.pm b/src/PVE/Storage/PBSPlugin.pm
index 45edc46..00170f5 100644
--- a/src/PVE/Storage/PBSPlugin.pm
+++ b/src/PVE/Storage/PBSPlugin.pm
@@ -966,7 +966,7 @@ sub volume_resize {
 }
 
 sub volume_snapshot {
-    my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
+    my ($class, $scfg, $storeid, $volname, $snap) = @_;
     die "volume snapshot is not possible on pbs device";
 }
 
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 439e4c3..2e16c59 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1272,7 +1272,7 @@ sub volume_resize {
 }
 
 sub volume_snapshot {
-    my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
+    my ($class, $scfg, $storeid, $volname, $snap) = @_;
 
     if ($scfg->{'external-snapshots'}) {
 
@@ -1280,18 +1280,14 @@ sub volume_snapshot {
 
         my $vmid = ($class->parse_volname($volname))[2];
 
-        if (!$running) {
-            #rename volume unless qemu has already done it for us
-            $class->rename_snapshot($scfg, $storeid, $volname, 'current', $snap);
-        }
+        #rename volume unless qemu has already done it for us
+        $class->rename_snapshot($scfg, $storeid, $volname, 'current', $snap);
+
         eval { alloc_backed_image($class, $storeid, $scfg, $volname, $snap) };
         if ($@) {
             warn "$@ \n";
-            #if running, the revert is done by qemu with blockdev-reopen
-            if (!$running) {
-                eval { $class->rename_snapshot($scfg, $storeid, $volname, $snap, 'current'); };
-                warn $@ if $@;
-            }
+            eval { $class->rename_snapshot($scfg, $storeid, $volname, $snap, 'current'); };
+            warn $@ if $@;
             die "can't allocate new volume $volname with $snap backing image\n";
         }
 
diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index 45f8a7f..541752e 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -866,7 +866,7 @@ sub volume_resize {
 }
 
 sub volume_snapshot {
-    my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
+    my ($class, $scfg, $storeid, $volname, $snap) = @_;
 
     my ($vtype, $name, $vmid) = $class->parse_volname($volname);
 
diff --git a/src/PVE/Storage/ZFSPoolPlugin.pm b/src/PVE/Storage/ZFSPoolPlugin.pm
index 28d4795..cdf5868 100644
--- a/src/PVE/Storage/ZFSPoolPlugin.pm
+++ b/src/PVE/Storage/ZFSPoolPlugin.pm
@@ -480,7 +480,7 @@ sub volume_size_info {
 }
 
 sub volume_snapshot {
-    my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
+    my ($class, $scfg, $storeid, $volname, $snap) = @_;
 
     my $vname = ($class->parse_volname($volname))[1];
 
-- 
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

[pve-devel] applied-series: [PATCH FOLLOW-UP qemu-server 1/4] api2: move_disk: use parse_volname to find old volume format

[Wolfgang Bumiller]

Applied the current state with 2 minor fixups and one major change:

I renamed the storage sub to `volume_qemu_snapshot_method()` and its
values to:
┌────────────┬───────────┐
│ previous   │ new       │
├────────────┼───────────┤
│ "internal" │ "qemu"    │
│ "external" │ "mixed"   │
│ undef      │ "storage" │
└────────────┴───────────┘

Updated the `ApiChangeLog` with descriptions for all of them.

On Wed, Jul 16, 2025 at 08:31:36AM +0200, Alexandre Derumier via pve-devel wrote:
> From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
> To: pve-devel@lists.proxmox.com
> Subject: [PATCH FOLLOW-UP qemu-server 1/4] api2: move_disk: use
>  parse_volname to find old volume format
> Date: Wed, 16 Jul 2025 08:31:36 +0200
> Message-Id: <20250716063153.1647681-2-alexandre.derumier@groupe-cyllene.com>
> X-Mailer: git-send-email 2.39.5
> 
> Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
> ---
>  src/PVE/API2/Qemu.pm | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
> index 2e6358e4..dbc08737 100644
> --- a/src/PVE/API2/Qemu.pm
> +++ b/src/PVE/API2/Qemu.pm
> @@ -4677,11 +4677,8 @@ __PACKAGE__->register_method({
>              die "you can't move a cdrom\n" if PVE::QemuServer::drive_is_cdrom($drive, 1);
>  
>              my $old_volid = $drive->{file};
> -            my $oldfmt;
>              my ($oldstoreid, $oldvolname) = PVE::Storage::parse_volume_id($old_volid);
> -            if ($oldvolname =~ m/\.(raw|qcow2|vmdk)$/) {
> -                $oldfmt = $1;
> -            }
> +            my $oldfmt = (PVE::Storage::parse_volname($storecfg, $old_volid))[6];
>  
>              die "you can't move to the same storage with same format\n"
>                  if $oldstoreid eq $storeid && (!$format || !$oldfmt || $oldfmt eq $format);
> -- 
> 2.39.5


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