From af1f1ec03807cbdb4c4a3001bec06d0f70705db5 Mon Sep 17 00:00:00 2001 From: Dominik Csapak Date: Thu, 17 Oct 2019 13:32:34 +0200 Subject: [PATCH] fix #2395: refactor qemu_img_convert to accept files as source and use it also for efidisk creation and importdisk this way we correctly handle zfs-over-iscsi options for those cases also write tests for it Signed-off-by: Dominik Csapak --- PVE/QemuServer.pm | 112 +++++++++++++++-------------- PVE/QemuServer/ImportDisk.pm | 11 +-- test/run_qemu_img_convert_tests.pl | 32 +++++++++ test/test.vmdk | 0 4 files changed, 94 insertions(+), 61 deletions(-) create mode 100644 test/test.vmdk diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index 8dda594b..3a9aa1ed 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -6884,66 +6884,77 @@ sub qemu_img_convert { my ($src_storeid, $src_volname) = PVE::Storage::parse_volume_id($src_volid, 1); my ($dst_storeid, $dst_volname) = PVE::Storage::parse_volume_id($dst_volid, 1); - if ($src_storeid && $dst_storeid) { + die "destination '$dst_volid' is not a valid volid form qemu-img convert\n" if !$dst_storeid; - PVE::Storage::activate_volumes($storecfg, [$src_volid], $snapname); + my $cachemode; + my $src_path; + my $src_is_iscsi = 0; + my $src_format = 'raw'; + if ($src_storeid) { + PVE::Storage::activate_volumes($storecfg, [$src_volid], $snapname); my $src_scfg = PVE::Storage::storage_config($storecfg, $src_storeid); - my $dst_scfg = PVE::Storage::storage_config($storecfg, $dst_storeid); + $src_format = qemu_img_format($src_scfg, $src_volname); + $src_path = PVE::Storage::path($storecfg, $src_volid, $snapname); + $src_is_iscsi = ($src_path =~ m|^iscsi://|); + $cachemode = 'none' if $src_scfg->{type} eq 'zfspool'; + } elsif (-f $src_volid) { + $src_path = $src_volid; + if ($src_path =~ m/\.($QEMU_FORMAT_RE)$/) { + $src_format = $1; + } + } - my $src_format = qemu_img_format($src_scfg, $src_volname); - my $dst_format = qemu_img_format($dst_scfg, $dst_volname); + die "source '$src_volid' is not a valid volid nor path for qemu-img convert\n" if !$src_path; - my $src_path = PVE::Storage::path($storecfg, $src_volid, $snapname); - my $dst_path = PVE::Storage::path($storecfg, $dst_volid); + my $dst_scfg = PVE::Storage::storage_config($storecfg, $dst_storeid); + my $dst_format = qemu_img_format($dst_scfg, $dst_volname); + my $dst_path = PVE::Storage::path($storecfg, $dst_volid); + my $dst_is_iscsi = ($dst_path =~ m|^iscsi://|); - my $src_is_iscsi = ($src_path =~ m|^iscsi://|); - my $dst_is_iscsi = ($dst_path =~ m|^iscsi://|); + my $cmd = []; + push @$cmd, '/usr/bin/qemu-img', 'convert', '-p', '-n'; + push @$cmd, '-l', "snapshot.name=$snapname" if($snapname && $src_format eq "qcow2"); + push @$cmd, '-t', 'none' if $dst_scfg->{type} eq 'zfspool'; + push @$cmd, '-T', $cachemode if defined($cachemode); + + if ($src_is_iscsi) { + push @$cmd, '--image-opts'; + $src_path = convert_iscsi_path($src_path); + } else { + push @$cmd, '-f', $src_format; + } - my $cmd = []; - push @$cmd, '/usr/bin/qemu-img', 'convert', '-p', '-n'; - push @$cmd, '-l', "snapshot.name=$snapname" if($snapname && $src_format eq "qcow2"); - push @$cmd, '-t', 'none' if $dst_scfg->{type} eq 'zfspool'; - push @$cmd, '-T', 'none' if $src_scfg->{type} eq 'zfspool'; + if ($dst_is_iscsi) { + push @$cmd, '--target-image-opts'; + $dst_path = convert_iscsi_path($dst_path); + } else { + push @$cmd, '-O', $dst_format; + } - if ($src_is_iscsi) { - push @$cmd, '--image-opts'; - $src_path = convert_iscsi_path($src_path); - } else { - push @$cmd, '-f', $src_format; - } + push @$cmd, $src_path; - if ($dst_is_iscsi) { - push @$cmd, '--target-image-opts'; - $dst_path = convert_iscsi_path($dst_path); - } else { - push @$cmd, '-O', $dst_format; - } + if (!$dst_is_iscsi && $is_zero_initialized) { + push @$cmd, "zeroinit:$dst_path"; + } else { + push @$cmd, $dst_path; + } - push @$cmd, $src_path; + my $parser = sub { + my $line = shift; + if($line =~ m/\((\S+)\/100\%\)/){ + my $percent = $1; + my $transferred = int($size * $percent / 100); + my $remaining = $size - $transferred; - if (!$dst_is_iscsi && $is_zero_initialized) { - push @$cmd, "zeroinit:$dst_path"; - } else { - push @$cmd, $dst_path; + print "transferred: $transferred bytes remaining: $remaining bytes total: $size bytes progression: $percent %\n"; } - my $parser = sub { - my $line = shift; - if($line =~ m/\((\S+)\/100\%\)/){ - my $percent = $1; - my $transferred = int($size * $percent / 100); - my $remaining = $size - $transferred; - - print "transferred: $transferred bytes remaining: $remaining bytes total: $size bytes progression: $percent %\n"; - } - - }; + }; - eval { run_command($cmd, timeout => undef, outfunc => $parser); }; - my $err = $@; - die "copy failed: $err" if $err; - } + eval { run_command($cmd, timeout => undef, outfunc => $parser); }; + my $err = $@; + die "copy failed: $err" if $err; } sub qemu_img_format { @@ -7269,15 +7280,12 @@ sub create_efidisk($$$$$) { my (undef, $ovmf_vars) = get_ovmf_files($arch); die "EFI vars default image not found\n" if ! -f $ovmf_vars; - my $vars_size = PVE::Tools::convert_size(-s $ovmf_vars, 'b' => 'kb'); + my $vars_size_b = -s $ovmf_vars; + my $vars_size = PVE::Tools::convert_size($vars_size_b, 'b' => 'kb'); my $volid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, $fmt, undef, $vars_size); PVE::Storage::activate_volumes($storecfg, [$volid]); - my $path = PVE::Storage::path($storecfg, $volid); - eval { - run_command(['/usr/bin/qemu-img', 'convert', '-n', '-f', 'raw', '-O', $fmt, $ovmf_vars, $path]); - }; - die "Copying EFI vars image failed: $@" if $@; + qemu_img_convert($ovmf_vars, $volid, $vars_size_b, undef, 0); return ($volid, $vars_size); } diff --git a/PVE/QemuServer/ImportDisk.pm b/PVE/QemuServer/ImportDisk.pm index db7db634..5d391e69 100755 --- a/PVE/QemuServer/ImportDisk.pm +++ b/PVE/QemuServer/ImportDisk.pm @@ -37,14 +37,7 @@ sub do_import { warn "args: $src_path, $vmid, $storage_id, $optional\n", "\$dst_volid: $dst_volid\n", if $debug; - # qemu-img convert does the hard job - # we don't attempt to guess filetypes ourselves - my $convert_command = ['qemu-img', 'convert', $src_path, '-p', '-n', '-O', $dst_format]; - if (PVE::Storage::volume_has_feature($storecfg, 'sparseinit', $dst_volid)) { - push @$convert_command, "zeroinit:$dst_path"; - } else { - push @$convert_command, $dst_path; - } + my $zeroinit = PVE::Storage::volume_has_feature($storecfg, 'sparseinit', $dst_volid); my $create_drive = sub { my $vm_conf = PVE::QemuConfig->load_config($vmid); @@ -88,7 +81,7 @@ sub do_import { local $SIG{HUP} = local $SIG{PIPE} = sub { die "interrupted by signal\n"; }; PVE::Storage::activate_volumes($storecfg, [$dst_volid]); - run_command($convert_command); + PVE::QemuServer::qemu_img_convert($src_path, $dst_volid, $src_size, undef, $zeroinit); PVE::Storage::deactivate_volumes($storecfg, [$dst_volid]); PVE::QemuConfig->lock_config($vmid, $create_drive); }; diff --git a/test/run_qemu_img_convert_tests.pl b/test/run_qemu_img_convert_tests.pl index eb38bf6b..8a57108d 100755 --- a/test/run_qemu_img_convert_tests.pl +++ b/test/run_qemu_img_convert_tests.pl @@ -152,6 +152,38 @@ my $tests = [ parameters => [ "local-lvm:vm-$vmid-disk-0", "not-existing:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 1 ], expected => "storage 'not-existing' does not exists\n", }, + { + name => "vmdkfile", + parameters => [ "./test.vmdk", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0 ], + expected => [ + "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "vmdk", "-O", "raw", + "./test.vmdk", + "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw", + ] + }, + { + name => "notexistingfile", + parameters => [ "/foo/bar", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0 ], + expected => "source '/foo/bar' is not a valid volid nor path for qemu-img convert\n", + }, + { + name => "efidisk", + parameters => [ "/usr/share/kvm/OVMF_VARS-pure-efi.fd", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0 ], + expected => [ + "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "raw", "-O", "raw", + "/usr/share/kvm/OVMF_VARS-pure-efi.fd", + "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw", + ] + }, + { + name => "efi2zos", + parameters => [ "/usr/share/kvm/OVMF_VARS-pure-efi.fd", "zfs-over-iscsi:vm-$vmid-disk-0", 1024*10, undef, 0 ], + expected => [ + "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "raw", "--target-image-opts", + "/usr/share/kvm/OVMF_VARS-pure-efi.fd", + "file.driver=iscsi,file.transport=tcp,file.initiator-name=foobar,file.portal=127.0.0.1,file.target=iqn.2019-10.org.test:foobar,file.lun=1,driver=raw", + ] + } ]; my $command; diff --git a/test/test.vmdk b/test/test.vmdk new file mode 100644 index 00000000..e69de29b -- 2.39.5