cleanup do_import, s/optional/params/ and move skiplock into params

mixed with indentation changes a whole lot of other changes which
should normally not mixed to much together, but this is all a bit
tangled and I'm not sure if splitting it into two or three parts
would help anybody.. just use "-w" (ignore whitespace changes) when
looking at the diff..

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>

diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
index f5169e2333e9d8e2bc17427c1598cc6c028ee72b..9eadedd2be3eef5a9d67e576f6c8d93e0c0eaf28 100755 (executable)
--- a/PVE/CLI/qm.pm
+++ b/PVE/CLI/qm.pm
@@ -488,7 +488,7 @@ __PACKAGE__->register_method ({
        die "storage $storeid does not support vm images\n"
            if !$target_storage_config->{content}->{images};
 
-       PVE::QemuServer::ImportDisk::do_import($source, $vmid, $storeid, 0, { format => $format });
+       PVE::QemuServer::ImportDisk::do_import($source, $vmid, $storeid, { format => $format });
 
        return undef;
     }});
@@ -640,8 +640,11 @@ __PACKAGE__->register_method ({
 
            foreach my $disk (@{ $parsed->{disks} }) {
                my ($file, $drive) = ($disk->{backing_file}, $disk->{disk_address});
-               PVE::QemuServer::ImportDisk::do_import($file, $vmid, $storeid,
-                   1, { drive_name => $drive, format => $format });
+               PVE::QemuServer::ImportDisk::do_import($file, $vmid, $storeid, {
+                   drive_name => $drive,
+                   format => $format,
+                   skiplock => 1,
+               });
            }
 
            # reload after disks entries have been created

diff --git a/PVE/QemuServer/ImportDisk.pm b/PVE/QemuServer/ImportDisk.pm
index 9cae4614903a764d0995c386af083b055d4b4a88..bae380c4eb93e2b2f496878d27a3b3e251597529 100755 (executable)
--- a/PVE/QemuServer/ImportDisk.pm
+++ b/PVE/QemuServer/ImportDisk.pm
@@ -9,14 +9,13 @@ use PVE::Tools qw(run_command extract_param);
 
 # imports an external disk image to an existing VM
 # and creates by default a drive entry unused[n] pointing to the created volume
-# $optional->{drive_name} may be used to specify ide0, scsi1, etc ...
-# $optional->{format} may be used to specify qcow2, raw, etc ...
+# $params->{drive_name} may be used to specify ide0, scsi1, etc ...
+# $params->{format} may be used to specify qcow2, raw, etc ...
 sub do_import {
-    my ($src_path, $vmid, $storage_id, $skiplock, $optional) = @_;
+    my ($src_path, $vmid, $storage_id, $params) = @_;
 
-    my $drive_name = extract_param($optional, 'drive_name');
-    my $format = extract_param($optional, 'format');
-    my $debug = extract_param($optional, 'debug');
+    my $drive_name = extract_param($params, 'drive_name');
+    my $format = extract_param($params, 'format');
     if ($drive_name && !(PVE::QemuServer::is_valid_drivename($drive_name))) {
        die "invalid drive name: $drive_name\n";
     }
@@ -26,53 +25,39 @@ sub do_import {
 
     # get target format, target image's path, and whether it's possible to sparseinit
     my $storecfg = PVE::Storage::config();
-    my $dst_format = PVE::QemuServer::resolve_dst_disk_format($storecfg,
-       $storage_id, undef, $format);
-    warn "format : $dst_format\n" if $debug;
+    my $dst_format = PVE::QemuServer::resolve_dst_disk_format($storecfg, $storage_id, undef, $format);
 
-    my $dst_volid = PVE::Storage::vdisk_alloc($storecfg, $storage_id, $vmid,
-       $dst_format, undef, $src_size / 1024);
-    my $dst_path = PVE::Storage::path($storecfg, $dst_volid);
-
-    warn "args:  $src_path, $vmid, $storage_id, $optional\n",
-       "\$dst_volid: $dst_volid\n", if $debug;
+    my $dst_volid = PVE::Storage::vdisk_alloc($storecfg, $storage_id, $vmid, $dst_format, undef, $src_size / 1024);
 
     my $zeroinit = PVE::Storage::volume_has_feature($storecfg, 'sparseinit', $dst_volid);
 
     my $create_drive = sub {
        my $vm_conf = PVE::QemuConfig->load_config($vmid);
-       if (!$skiplock) {
+       if (!$params->{skiplock}) {
            PVE::QemuConfig->check_lock($vm_conf);
        }
 
        if ($drive_name) {
-               # should never happen as setting $drive_name is not exposed to public interface
-               die "cowardly refusing to overwrite existing entry: $drive_name\n" if $vm_conf->{$drive_name};
-
-               my $modified = {}; # record what $option we modify
-               $modified->{$drive_name} = 1;
-               $vm_conf->{pending}->{$drive_name} = $dst_volid;
-               PVE::QemuConfig->write_config($vmid, $vm_conf);
+           # should never happen as setting $drive_name is not exposed to public interface
+           die "cowardly refusing to overwrite existing entry: $drive_name\n" if $vm_conf->{$drive_name};
 
-               my $running = PVE::QemuServer::check_running($vmid);
-               if ($running) {
-                   my $errors = {};
-                   PVE::QemuServer::vmconfig_hotplug_pending($vmid, $vm_conf, $storecfg, $modified, $errors);
-                   if (scalar(keys %$errors)) {
-                       foreach my $k (keys %$errors) {
-                           warn "$k: $errors->{$k}\n" if $debug;
-                           warn "hotplugging imported disk failed\n";
-                       }
-                   }
-               } else {
-                   PVE::QemuServer::vmconfig_apply_pending($vmid, $vm_conf, $storecfg);
-               }
+           my $modified = {}; # record what $option we modify
+           $modified->{$drive_name} = 1;
+           $vm_conf->{pending}->{$drive_name} = $dst_volid;
+           PVE::QemuConfig->write_config($vmid, $vm_conf);
 
+           my $running = PVE::QemuServer::check_running($vmid);
+           if ($running) {
+               my $errors = {};
+               PVE::QemuServer::vmconfig_hotplug_pending($vmid, $vm_conf, $storecfg, $modified, $errors);
+               warn "hotplugging imported disk '$_' failed: $errors->{$_}\n" for keys %$errors;
+           } else {
+               PVE::QemuServer::vmconfig_apply_pending($vmid, $vm_conf, $storecfg);
+           }
        } else {
            PVE::QemuConfig->add_unused_volume($vm_conf, $dst_volid);
            PVE::QemuConfig->write_config($vmid, $vm_conf);
        }
-
     };
 
     eval {
@@ -81,18 +66,16 @@ sub do_import {
            local $SIG{TERM} =
            local $SIG{QUIT} =
            local $SIG{HUP} =
-           local $SIG{PIPE} = sub { die "interrupted by signal\n"; };
+           local $SIG{PIPE} = sub { die "interrupted by signal $!\n"; };
+
        PVE::Storage::activate_volumes($storecfg, [$dst_volid]);
        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);
     };
-
-    my $err = $@;
-    if ($err) {
-       eval { # do not die before we returned $err
-           PVE::Storage::vdisk_free($storecfg, $dst_volid);
-       };
+    if (my $err = $@) {
+       eval { PVE::Storage::vdisk_free($storecfg, $dst_volid) };
+       warn "cleanup of $dst_volid failed: $@\n" if $@;
        die $err;
     }
 }