[pve-devel] [PATCH qemu-server 3/3] Move importdisk from qm to API

["Dominic Jäger" <d.jaeger@proxmox.com>]

Additionally, add parameters that enable directly (avoiding the intermediate
step as unused disk) attaching the disk to a bus/device with all known disk
options.

Required to create a GUI for importdisk.

Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
---
 PVE/API2/Qemu.pm             | 113 ++++++++++++++++++++++++++++++++++-
 PVE/CLI/qm.pm                |  57 +-----------------
 PVE/QemuServer/Drive.pm      |  14 +++++
 PVE/QemuServer/ImportDisk.pm |   2 +-
 4 files changed, 127 insertions(+), 59 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index b33359d..6efbbe3 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -24,6 +24,7 @@ use PVE::QemuServer;
 use PVE::QemuServer::Drive;
 use PVE::QemuServer::CPUConfig;
 use PVE::QemuServer::Monitor qw(mon_cmd);
+use PVE::QemuServer::ImportDisk qw(do_import);
 use PVE::QemuMigrate;
 use PVE::RPCEnvironment;
 use PVE::AccessControl;
@@ -45,8 +46,6 @@ BEGIN {
     }
 }
 
-use Data::Dumper; # fixme: remove
-
 use base qw(PVE::RESTHandler);
 
 my $opt_force_description = "Force physical removal. Without this, we simple remove the disk from the config file and create an additional configuration entry called 'unused[n]', which contains the volume ID. Unlink of unused[n] always cause physical removal.";
@@ -4252,4 +4251,114 @@ __PACKAGE__->register_method({
 	return PVE::QemuServer::Cloudinit::dump_cloudinit_config($conf, $param->{vmid}, $param->{type});
     }});
 
+__PACKAGE__->register_method ({
+    name => 'importdisk',
+    path => '{vmid}/importdisk',
+    method => 'POST',
+    protected => 1, # for worker upid file
+    proxyto => 'node',
+    description => "Import an external disk image into a VM. "
+	."The image format has to be supported by qemu-img.",
+    permissions => {
+	check => [ 'and',
+	    [ 'perm', '/storage/{storage}', ['Datastore.AllocateTemplate']],
+	    [ 'perm', '/storage/{storage}', ['Datastore.AllocateSpace']],
+	    [ 'perm', '/vms/{vmid}', ['VM.Allocate']],
+	],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    vmid => get_standard_option('pve-vmid',
+		{completion => \&PVE::QemuServer::complete_vmid}),
+	    source => {
+		description => 'Path of the disk image that should be imported',
+		type => 'string',
+	    },
+	    device => {
+		type => 'string',
+		description => "What device the imported image should be "
+		    ."(e.g. 'ide0', 'scsi2'). Will add the image as unused disk "
+		    ."if omitted.",
+		enum => [PVE::QemuServer::Drive::valid_drive_names()],
+		optional => 1,
+	    },
+	    device_options => {
+		type => 'string',
+		format => 'drive_options',
+		description => "What options to set for the device "
+		    ."(e.g. 'discard=on,backup=0')",
+		optional => 1,
+	    },
+	    storage => get_standard_option('pve-storage-id', {
+		description => 'Which storage to use for the imported image.',
+		completion => \&PVE::QemuServer::complete_storage,
+	    }),
+	    format => {
+		type => 'string',
+		description => 'Target format.',
+		enum => [ 'raw', 'qcow2', 'vmdk' ],
+		optional => 1,
+	    },
+	    digest => get_standard_option('pve-config-digest'),
+	},
+    },
+    returns => { type => 'string'},
+    code => sub {
+	my ($params) = @_;
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+
+	my $vmid = extract_param($params, 'vmid');
+	my $source = extract_param($params, 'source');
+	my $digest = extract_param($params, 'digest');
+	my $device_options = extract_param($params, 'device_options');
+	my $device = extract_param($params, 'device');
+	my $storage = extract_param($params, 'storage');
+	my $vm_conf = PVE::QemuConfig->load_config($vmid);
+
+	die "Could not import because source parameter is an empty string\n"
+	    if ($source eq "");
+	die "Could not import because source '$source' is not an absolute path\n"
+	    if $source !~ m!/!;
+	die "Could not import because source '$source' does not exist"
+	    if !-e $source;
+	die "VM $vmid config checksum missmatch (file change by other user?)\n"
+	    if $digest && $digest ne $vm_conf->{digest};
+	die "Could not import because device $device is already in use in "
+	    ."VM $vmid. Choose a different device!\n"
+	    if $device && $vm_conf->{$device};
+
+	my $worker = sub {
+	    my $message = $device ? "to $device on" : 'as unused disk to';
+	    print "Importing disk '$source' $message VM $vmid ...\n";
+	    my ($unused_disk, $volid) = eval {
+		PVE::QemuServer::ImportDisk::do_import(
+		    $source, $vmid, $storage,
+		    {format => extract_param($params, 'format')},
+		)
+	    };
+	    die "Importing disk failed: $@\n" if $@;
+
+	    if ($device) {
+		eval {
+		    $update_vm_api->({
+			vmid => $vmid,
+			$device => "$volid,$device_options",
+		    }, 1);
+		};
+		# Importing large disks takes a lot of time
+		# => don't remove disk automatically on option update error
+		die "Copying disk to $unused_disk succeeded but attaching it "
+		    ."as $device and setting its options to $device_options "
+		    ."failed: $@.\n Adjust them manually.\n" if $@;
+	    }
+	    print "Successfully imported disk '$source' as "
+		.($device ? $device : $unused_disk) . ": $volid'\n";
+	};
+	my $upid = $rpcenv->fork_worker('importdisk', $vmid, $authuser, $worker);
+	return $upid;
+    }});
+
 1;
diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
index 282fa86..4a304ce 100755
--- a/PVE/CLI/qm.pm
+++ b/PVE/CLI/qm.pm
@@ -445,61 +445,6 @@ __PACKAGE__->register_method ({
 	return undef;
     }});
 
-__PACKAGE__->register_method ({
-    name => 'importdisk',
-    path => 'importdisk',
-    method => 'POST',
-    description => "Import an external disk image as an unused disk in a VM. The
- image format has to be supported by qemu-img(1).",
-    parameters => {
-	additionalProperties => 0,
-	properties => {
-	    vmid => get_standard_option('pve-vmid', {completion => \&PVE::QemuServer::complete_vmid}),
-	    source => {
-		description => 'Path to the disk image to import',
-		type => 'string',
-		optional => 0,
-	    },
-            storage => get_standard_option('pve-storage-id', {
-		description => 'Target storage ID',
-		completion => \&PVE::QemuServer::complete_storage,
-		optional => 0,
-            }),
-	    format => {
-		type => 'string',
-		description => 'Target format',
-		enum => [ 'raw', 'qcow2', 'vmdk' ],
-		optional => 1,
-	    },
-	},
-    },
-    returns => { type => 'null'},
-    code => sub {
-	my ($param) = @_;
-
-	my $vmid = extract_param($param, 'vmid');
-	my $source = extract_param($param, 'source');
-	my $storeid = extract_param($param, 'storage');
-	my $format = extract_param($param, 'format');
-
-	my $vm_conf = PVE::QemuConfig->load_config($vmid);
-	PVE::QemuConfig->check_lock($vm_conf);
-	die "$source: non-existent or non-regular file\n" if (! -f $source);
-
-	my $storecfg = PVE::Storage::config();
-	PVE::Storage::storage_check_enabled($storecfg, $storeid);
-
-	my $target_storage_config =  PVE::Storage::storage_config($storecfg, $storeid);
-	die "storage $storeid does not support vm images\n"
-	    if !$target_storage_config->{content}->{images};
-
-	print "importing disk '$source' to VM $vmid ...\n";
-	my ($drive_id, $volid) = PVE::QemuServer::ImportDisk::do_import($source, $vmid, $storeid, { format => $format });
-	print "Successfully imported disk as '$drive_id:$volid'\n";
-
-	return undef;
-    }});
-
 __PACKAGE__->register_method ({
     name => 'terminal',
     path => 'terminal',
@@ -984,7 +929,7 @@ our $cmddef = {
 
     terminal => [ __PACKAGE__, 'terminal', ['vmid']],
 
-    importdisk => [ __PACKAGE__, 'importdisk', ['vmid', 'source', 'storage']],
+    importdisk => [ "PVE::API2::Qemu", 'importdisk', ['vmid', 'source', 'storage'], { node => $nodename }],
 
     importovf => [ __PACKAGE__, 'importovf', ['vmid', 'manifest', 'storage']],
 
diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
index 91c33f8..87727ea 100644
--- a/PVE/QemuServer/Drive.pm
+++ b/PVE/QemuServer/Drive.pm
@@ -201,6 +201,7 @@ my %wwn_fmt = (
     },
 );
 
+
 my $add_throttle_desc = sub {
     my ($key, $type, $what, $unit, $longunit, $minimum) = @_;
     my $d = {
@@ -308,6 +309,19 @@ my $alldrive_fmt = {
     %wwn_fmt,
 };
 
+my %optional_file_drivdesc_base = %drivedesc_base;
+$optional_file_drivdesc_base{file}{optional} = 1; # TODO verify
+my $drive_options_fmt = {
+    %optional_file_drivdesc_base, # first bad thing here is the missing file stuff
+    %iothread_fmt,
+    %model_fmt,
+    %queues_fmt,
+    %scsiblock_fmt,
+    %ssd_fmt,
+    %wwn_fmt,
+};
+PVE::JSONSchema::register_format('drive_options', $drive_options_fmt);
+
 my $efidisk_fmt = {
     volume => { alias => 'file' },
     file => {
diff --git a/PVE/QemuServer/ImportDisk.pm b/PVE/QemuServer/ImportDisk.pm
index 51ad52e..d89cd4d 100755
--- a/PVE/QemuServer/ImportDisk.pm
+++ b/PVE/QemuServer/ImportDisk.pm
@@ -38,7 +38,7 @@ sub do_import {
 	}
 
 	if ($drive_name) {
-	    # should never happen as setting $drive_name is not exposed to public interface
+	    # Exposed in importdisk API only
 	    die "cowardly refusing to overwrite existing entry: $drive_name\n" if $vm_conf->{$drive_name};
 
 	    my $modified = {}; # record what $option we modify
-- 
2.20.1