From 5600c5b22f61c1a545891f0e1ded57e13e1dc35d Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Tue, 29 Oct 2019 19:04:01 +0100 Subject: [PATCH] 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 --- PVE/CLI/qm.pm | 9 +++-- PVE/QemuServer/ImportDisk.pm | 71 ++++++++++++++---------------------- 2 files changed, 33 insertions(+), 47 deletions(-) diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm index f5169e23..9eadedd2 100755 --- 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 9cae4614..bae380c4 100755 --- 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}; + # 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); - - 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; } }