5
0
mirror of git://git.proxmox.com/git/qemu-server.git synced 2025-08-04 08:21:54 +03:00

image convert: re-use generate_drive_blockdev()

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>
Link: https://lore.proxmox.com/20250730150325.138087-3-f.ebner@proxmox.com
This commit is contained in:
Fiona Ebner
2025-07-30 17:03:16 +02:00
committed by Thomas Lamprecht
parent 4866264c59
commit 659b0716ce
2 changed files with 76 additions and 13 deletions

View File

@ -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;
}

View File

@ -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");