* [pve-devel] [PATCH qemu-server 2/3] image convert: re-use generate_drive_blockdev()
2025-07-30 15:03 [pve-devel] [PATCH-SERIES qemu-server 0/3] image convert: fix handling qcow2 on snapshot-as-volume-chain dir target Fiona Ebner
2025-07-30 15:03 ` [pve-devel] [PATCH qemu-server 1/3] image convert: avoid combining target image options and zeroinit filter Fiona Ebner
@ 2025-07-30 15:03 ` Fiona Ebner
2025-07-30 15:03 ` [pve-devel] [PATCH qemu-server 3/3] image convert: make using zeroinit with target-image-opts work Fiona Ebner
2025-07-30 22:23 ` [pve-devel] applied: [PATCH-SERIES qemu-server 0/3] image convert: fix handling qcow2 on snapshot-as-volume-chain dir target Thomas Lamprecht
3 siblings, 0 replies; 5+ messages in thread
From: Fiona Ebner @ 2025-07-30 15:03 UTC (permalink / raw)
To: pve-devel
This avoids having the handling for 'discard-no-unref' in two places.
In the tests, rename the relevant target images with a '-target'
suffix to test for them in the mocked volume_snapshot_info() helper.
Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Changes since last iteration:
* rebase + make tidy
src/PVE/QemuServer/QemuImage.pm | 65 +++++++++++++++++++++++---
src/test/run_qemu_img_convert_tests.pl | 24 +++++++---
2 files changed, 76 insertions(+), 13 deletions(-)
diff --git a/src/PVE/QemuServer/QemuImage.pm b/src/PVE/QemuServer/QemuImage.pm
index 8fe75e92..f2cadd69 100644
--- a/src/PVE/QemuServer/QemuImage.pm
+++ b/src/PVE/QemuServer/QemuImage.pm
@@ -5,11 +5,13 @@ use warnings;
use Fcntl qw(S_ISBLK);
use File::stat;
+use JSON;
use PVE::Format qw(render_bytes);
use PVE::Storage;
use PVE::Tools;
+use PVE::QemuServer::Blockdev;
use PVE::QemuServer::Drive qw(checked_volume_format);
use PVE::QemuServer::Helpers;
@@ -31,16 +33,62 @@ sub convert_iscsi_path {
}
my sub qcow2_target_image_opts {
- my ($path, @qcow2_opts) = @_;
+ my ($storecfg, $drive, @qcow2_opts) = @_;
- # FIXME this duplicates logic from qemu_blockdev_options
- my $st = File::stat::stat($path) or die "stat for '$path' failed - $!\n";
+ # There is no machine version, the qemu-img binary version is what's important.
+ my $version = PVE::QemuServer::Helpers::kvm_user_version();
- my $driver = S_ISBLK($st->mode) ? 'host_device' : 'file';
+ my $blockdev = PVE::QemuServer::Blockdev::generate_drive_blockdev(
+ $storecfg,
+ $drive,
+ $version,
+ { 'no-throttle' => 1 },
+ );
- my $qcow2_opts_str = ',' . join(',', @qcow2_opts);
+ my $opts = [];
+ my $opt_prefix = '';
+ my $next_child = $blockdev;
+ while ($next_child) {
+ my $current = $next_child;
+ $next_child = delete($current->{file});
- return "driver=qcow2$qcow2_opts_str,file.driver=$driver,file.filename=$path";
+ # TODO should cache settings be configured here (via appropriate drive configuration) rather
+ # than via dedicated qemu-img options?
+ delete($current->{cache});
+ # TODO e.g. can't use aio 'native' without cache.direct, just use QEMU default like for
+ # other targets for now
+ delete($current->{aio});
+
+ # no need for node names
+ delete($current->{'node-name'});
+
+ # it's the write target, while the flag should be 'false' anyways, remove to be sure
+ delete($current->{'read-only'});
+
+ # TODO should those be set (via appropriate drive configuration)?
+ delete($current->{'detect-zeroes'});
+ delete($current->{'discard'});
+
+ for my $key (sort keys $current->%*) {
+ my $value;
+ if (ref($current->{$key})) {
+ if ($current->{$key} eq JSON::false) {
+ $value = 'false';
+ } elsif ($current->{$key} eq JSON::true) {
+ $value = 'true';
+ } else {
+ die "target image options: unhandled structured key: $key\n";
+ }
+ } else {
+ $value = $current->{$key};
+ }
+ push $opts->@*, "$opt_prefix$key=$value";
+ }
+
+ $opt_prefix .= 'file.';
+ }
+
+ return join(',', $opts->@*);
}
# The possible options are:
@@ -115,7 +163,10 @@ sub convert {
if ($dst_is_iscsi) {
$dst_path = convert_iscsi_path($dst_path);
} elsif ($dst_needs_discard_no_unref) {
- $dst_path = qcow2_target_image_opts($dst_path, 'discard-no-unref=true');
+ # don't use any other drive options, those are intended for use with a running VM and just
+ # use scsi0 as a dummy interface+index for now
+ my $dst_drive = { file => $dst_volid, interface => 'scsi', index => 0 };
+ $dst_path = qcow2_target_image_opts($storecfg, $dst_drive, 'discard-no-unref=true');
} else {
push @$cmd, '-O', $dst_format;
}
diff --git a/src/test/run_qemu_img_convert_tests.pl b/src/test/run_qemu_img_convert_tests.pl
index 3c8f09f0..393fc4a8 100755
--- a/src/test/run_qemu_img_convert_tests.pl
+++ b/src/test/run_qemu_img_convert_tests.pl
@@ -532,7 +532,7 @@ my $tests = [
name => "qcow2_external_snapshot_target",
parameters => [
"local:$vmid/vm-$vmid-disk-0.raw",
- "localsnapext:$vmid/vm-$vmid-disk-0.qcow2",
+ "localsnapext:$vmid/vm-$vmid-disk-target.qcow2",
1024 * 10,
],
expected => [
@@ -544,14 +544,15 @@ my $tests = [
"raw",
"--target-image-opts",
"/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw",
- "driver=qcow2,discard-no-unref=true,file.driver=file,"
- . "file.filename=/var/lib/vzsnapext/images/$vmid/vm-$vmid-disk-0.qcow2",
+ "discard-no-unref=true,driver=qcow2,file.driver=file"
+ . ",file.filename=/var/lib/vzsnapext/images/$vmid/vm-$vmid-disk-target.qcow2",
],
},
{
name => "lvmqcow2_external_snapshot_target",
parameters => [
- "local:$vmid/vm-$vmid-disk-0.raw", "lvm-store:vm-$vmid-disk-0.qcow2", 1024 * 10,
+ "local:$vmid/vm-$vmid-disk-0.raw", "lvm-store:vm-$vmid-disk-target.qcow2",
+ 1024 * 10,
],
expected => [
"/usr/bin/qemu-img",
@@ -562,8 +563,8 @@ my $tests = [
"raw",
"--target-image-opts",
"/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw",
- "driver=qcow2,discard-no-unref=true,file.driver=host_device,"
- . "file.filename=/dev/pve/vm-$vmid-disk-0.qcow2",
+ "discard-no-unref=true,driver=qcow2,file.driver=host_device"
+ . ",file.filename=/dev/pve/vm-$vmid-disk-target.qcow2",
],
},
];
@@ -588,6 +589,17 @@ $storage_module->mock(
activate_volumes => sub {
return 1;
},
+ volume_snapshot_info => sub {
+ my ($cfg, $volid) = @_;
+ if (
+ $volid eq "lvm-store:vm-$vmid-disk-target.qcow2"
+ || $volid eq "localsnapext:$vmid/vm-$vmid-disk-target.qcow2"
+ ) {
+ # target volumes don't have snapshots
+ return {};
+ }
+ die "mocked volume_snapshot_info called with unexpected volid $volid\n";
+ },
);
my $lio_module = Test::MockModule->new("PVE::Storage::LunCmd::LIO");
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* [pve-devel] [PATCH qemu-server 3/3] image convert: make using zeroinit with target-image-opts work
2025-07-30 15:03 [pve-devel] [PATCH-SERIES qemu-server 0/3] image convert: fix handling qcow2 on snapshot-as-volume-chain dir target Fiona Ebner
2025-07-30 15:03 ` [pve-devel] [PATCH qemu-server 1/3] image convert: avoid combining target image options and zeroinit filter Fiona Ebner
2025-07-30 15:03 ` [pve-devel] [PATCH qemu-server 2/3] image convert: re-use generate_drive_blockdev() Fiona Ebner
@ 2025-07-30 15:03 ` Fiona Ebner
2025-07-30 22:23 ` [pve-devel] applied: [PATCH-SERIES qemu-server 0/3] image convert: fix handling qcow2 on snapshot-as-volume-chain dir target Thomas Lamprecht
3 siblings, 0 replies; 5+ messages in thread
From: Fiona Ebner @ 2025-07-30 15:03 UTC (permalink / raw)
To: pve-devel
Also add a test to witness this combination.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
src/PVE/QemuServer/QemuImage.pm | 17 +++++++++++------
src/test/run_qemu_img_convert_tests.pl | 21 +++++++++++++++++++++
2 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/src/PVE/QemuServer/QemuImage.pm b/src/PVE/QemuServer/QemuImage.pm
index f2cadd69..71be3abb 100644
--- a/src/PVE/QemuServer/QemuImage.pm
+++ b/src/PVE/QemuServer/QemuImage.pm
@@ -33,16 +33,16 @@ sub convert_iscsi_path {
}
my sub qcow2_target_image_opts {
- my ($storecfg, $drive, @qcow2_opts) = @_;
+ my ($storecfg, $drive, $qcow2_opts, $zeroinit) = @_;
# There is no machine version, the qemu-img binary version is what's important.
my $version = PVE::QemuServer::Helpers::kvm_user_version();
+ my $blockdev_opts = { 'no-throttle' => 1 };
+ $blockdev_opts->{'zero-initialized'} = 1 if $zeroinit;
+
my $blockdev = PVE::QemuServer::Blockdev::generate_drive_blockdev(
- $storecfg,
- $drive,
- $version,
- { 'no-throttle' => 1 },
+ $storecfg, $drive, $version, $blockdev_opts,
);
my $opts = [];
@@ -166,7 +166,12 @@ sub convert {
# don't use any other drive options, those are intended for use with a running VM and just
# use scsi0 as a dummy interface+index for now
my $dst_drive = { file => $dst_volid, interface => 'scsi', index => 0 };
- $dst_path = qcow2_target_image_opts($storecfg, $dst_drive, 'discard-no-unref=true');
+ $dst_path = qcow2_target_image_opts(
+ $storecfg,
+ $dst_drive,
+ ['discard-no-unref=true'],
+ $opts->{'is-zero-initialized'},
+ );
} else {
push @$cmd, '-O', $dst_format;
}
diff --git a/src/test/run_qemu_img_convert_tests.pl b/src/test/run_qemu_img_convert_tests.pl
index 393fc4a8..8a0ad283 100755
--- a/src/test/run_qemu_img_convert_tests.pl
+++ b/src/test/run_qemu_img_convert_tests.pl
@@ -548,6 +548,27 @@ my $tests = [
. ",file.filename=/var/lib/vzsnapext/images/$vmid/vm-$vmid-disk-target.qcow2",
],
},
+ {
+ name => "qcow2_external_snapshot_target_zeroinit",
+ parameters => [
+ "local:$vmid/vm-$vmid-disk-0.raw",
+ "localsnapext:$vmid/vm-$vmid-disk-target.qcow2",
+ 1024 * 10,
+ { 'is-zero-initialized' => 1 },
+ ],
+ expected => [
+ "/usr/bin/qemu-img",
+ "convert",
+ "-p",
+ "-n",
+ "-f",
+ "raw",
+ "--target-image-opts",
+ "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw",
+ "driver=zeroinit,file.discard-no-unref=true,file.driver=qcow2,file.file.driver=file"
+ . ",file.file.filename=/var/lib/vzsnapext/images/$vmid/vm-$vmid-disk-target.qcow2",
+ ],
+ },
{
name => "lvmqcow2_external_snapshot_target",
parameters => [
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 5+ messages in thread