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 <d.csa...@proxmox.com>
---
changes from v1:
* changed error messages to something more sensible and include the volid
  (also adapt the test for it)

 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 8dda594..3a9aa1e 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 db7db63..5d391e6 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 eb38bf6..8a57108 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 0000000..e69de29
-- 
2.20.1


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

Reply via email to