[pve-devel] [RFC v10 qemu-server 6/7] api: support VM disk import

[Fabian Ebner <f.ebner@proxmox.com>]

From: Dominic Jäger <d.jaeger@proxmox.com>

Extend qm importdisk functionality to the API.

Co-authored-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Co-authored-by: Dominic Jäger <d.jaeger@proxmox.com>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v9:

* Instead of adding an import-sources parameter to the API, use a new
  import-from property for the disk, that's only available with
  import/alloc-enabled API endpoints via its own version of the schema

    Avoids the split across regular drive key parameters and
    'import_soruces', which avoids quite a bit of cross-checking
    between the two and parsing/passing around the latter.

    The big downsides are:
    * Schema handling is a bit messy.
    * Need to special case print_drive, because we do intermediate
      parse/print to cleanup drive paths. Seems not too easy to avoid
      without complicating things elsewehere.
    * Using the import-aware parse_drive in parse_volume, because that
      is used via the foreach_volume iterators handling the parameters
      of the import-enabled endpoints. Could be avoided by using for
      loops instead.

    Counter-arguments for using a single schema (citing Fabian G.):
    * docs/schema dump/api docs: shouldn't look like you can put that
      everywhere where we use the config schema
    * shouldn't have nasty side-effects if someone puts it into the
      config

* Don't iterate over unused disks in create_disks()

    Would need to be its own patch and need to make sure everything
    also works with respect to usual (i.e. non-import) new disk
    creation, etc.

* Re-use do_import function

    Rather than duplicating most of it. The down side is the need to
    add a new parameter for skipping configuration update. But I
    suppose the plan is to have qm import switch to the new API at
    some point, and then do_import can be simplified.

* Drop format supported check

    Instead rely on resolve_dst_disk_format (via do_import) to pick
    the most appropriate format.

 PVE/API2/Qemu.pm             | 86 +++++++++++++++++++++++++-----------
 PVE/QemuConfig.pm            |  2 +-
 PVE/QemuServer/Drive.pm      | 32 +++++++++++---
 PVE/QemuServer/ImportDisk.pm |  2 +-
 4 files changed, 87 insertions(+), 35 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index e6a6cdc..8c74ecc 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -21,8 +21,9 @@ use PVE::ReplicationConfig;
 use PVE::GuestHelpers;
 use PVE::QemuConfig;
 use PVE::QemuServer;
-use PVE::QemuServer::Drive;
 use PVE::QemuServer::CPUConfig;
+use PVE::QemuServer::Drive;
+use PVE::QemuServer::ImportDisk;
 use PVE::QemuServer::Monitor qw(mon_cmd);
 use PVE::QemuServer::Machine;
 use PVE::QemuMigrate;
@@ -89,6 +90,10 @@ my $check_storage_access = sub {
 	} else {
 	    PVE::Storage::check_volume_access($rpcenv, $authuser, $storecfg, $vmid, $volid);
 	}
+
+	if (my $source_image = $drive->{'import-from'}) {
+	    PVE::Storage::check_volume_access($rpcenv, $authuser, $storecfg, $vmid, $source_image);
+	}
     });
 
    $rpcenv->check($authuser, "/storage/$settings->{vmstatestorage}", ['Datastore.AllocateSpace'])
@@ -162,6 +167,9 @@ my $create_disks = sub {
 	my $volid = $disk->{file};
 	my ($storeid, $volname) = PVE::Storage::parse_volume_id($volid, 1);
 
+	die "'import-from' requires special volume ID - use <storage ID>:0,import-from=<source>\n"
+	    if $disk->{'import-from'} && $volid !~ $NEW_DISK_RE;
+
 	if (!$volid || $volid eq 'none' || $volid eq 'cdrom') {
 	    delete $disk->{size};
 	    $res->{$ds} = PVE::QemuServer::print_drive($disk);
@@ -190,28 +198,52 @@ my $create_disks = sub {
 	} elsif ($volid =~ $NEW_DISK_RE) {
 	    my ($storeid, $size) = ($2 || $default_storage, $3);
 	    die "no storage ID specified (and no default storage)\n" if !$storeid;
-	    my $defformat = PVE::Storage::storage_default_format($storecfg, $storeid);
-	    my $fmt = $disk->{format} || $defformat;
-
-	    $size = PVE::Tools::convert_size($size, 'gb' => 'kb'); # vdisk_alloc uses kb
-
-	    my $volid;
-	    if ($ds eq 'efidisk0') {
-		my $smm = PVE::QemuServer::Machine::machine_type_is_q35($conf);
-		($volid, $size) = PVE::QemuServer::create_efidisk(
-		    $storecfg, $storeid, $vmid, $fmt, $arch, $disk, $smm);
-	    } elsif ($ds eq 'tpmstate0') {
-		# swtpm can only use raw volumes, and uses a fixed size
-		$size = PVE::Tools::convert_size(PVE::QemuServer::Drive::TPMSTATE_DISK_SIZE, 'b' => 'kb');
-		$volid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, "raw", undef, $size);
+
+	    if (my $source = delete $disk->{'import-from'}) {
+		$source = PVE::Storage::abs_filesystem_path($storecfg, $source, 1);
+		my $src_size = PVE::Storage::file_size_info($source);
+		die "Could not get file size of $source" if !defined($src_size);
+
+		my (undef, $dst_volid) = PVE::QemuServer::ImportDisk::do_import(
+		    $source,
+		    $vmid,
+		    $storeid,
+		    {
+			drive_name => $ds,
+			format => $disk->{format},
+			'skip-config-update' => 1,
+		    },
+		);
+
+		push @$vollist, $dst_volid;
+		$disk->{file} = $dst_volid;
+		$disk->{size} = $src_size;
+		delete $disk->{format}; # no longer needed
+		$res->{$ds} = PVE::QemuServer::print_drive($disk);
 	    } else {
-		$volid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, $fmt, undef, $size);
+		my $defformat = PVE::Storage::storage_default_format($storecfg, $storeid);
+		my $fmt = $disk->{format} || $defformat;
+
+		$size = PVE::Tools::convert_size($size, 'gb' => 'kb'); # vdisk_alloc uses kb
+
+		my $volid;
+		if ($ds eq 'efidisk0') {
+		    my $smm = PVE::QemuServer::Machine::machine_type_is_q35($conf);
+		    ($volid, $size) = PVE::QemuServer::create_efidisk(
+			$storecfg, $storeid, $vmid, $fmt, $arch, $disk, $smm);
+		} elsif ($ds eq 'tpmstate0') {
+		    # swtpm can only use raw volumes, and uses a fixed size
+		    $size = PVE::Tools::convert_size(PVE::QemuServer::Drive::TPMSTATE_DISK_SIZE, 'b' => 'kb');
+		    $volid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, "raw", undef, $size);
+		} else {
+		    $volid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, $fmt, undef, $size);
+		}
+		push @$vollist, $volid;
+		$disk->{file} = $volid;
+		$disk->{size} = PVE::Tools::convert_size($size, 'kb' => 'b');
+		delete $disk->{format}; # no longer needed
+		$res->{$ds} = PVE::QemuServer::print_drive($disk);
 	    }
-	    push @$vollist, $volid;
-	    $disk->{file} = $volid;
-	    $disk->{size} = PVE::Tools::convert_size($size, 'kb' => 'b');
-	    delete $disk->{format}; # no longer needed
-	    $res->{$ds} = PVE::QemuServer::print_drive($disk);
 	} else {
 
 	    PVE::Storage::check_volume_access($rpcenv, $authuser, $storecfg, $vmid, $volid);
@@ -639,11 +671,11 @@ __PACKAGE__->register_method({
 
 	    foreach my $opt (keys %$param) {
 		if (PVE::QemuServer::is_valid_drivename($opt)) {
-		    my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
+		    my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt}, 1);
 		    raise_param_exc({ $opt => "unable to parse drive options" }) if !$drive;
 
 		    PVE::QemuServer::cleanup_drive_path($opt, $storecfg, $drive);
-		    $param->{$opt} = PVE::QemuServer::print_drive($drive);
+		    $param->{$opt} = PVE::QemuServer::print_drive($drive, 1);
 		}
 	    }
 
@@ -1207,11 +1239,11 @@ my $update_vm_api  = sub {
     foreach my $opt (keys %$param) {
 	if (PVE::QemuServer::is_valid_drivename($opt)) {
 	    # cleanup drive path
-	    my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
+	    my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt}, 1);
 	    raise_param_exc({ $opt => "unable to parse drive options" }) if !$drive;
 	    PVE::QemuServer::cleanup_drive_path($opt, $storecfg, $drive);
 	    $check_replication->($drive);
-	    $param->{$opt} = PVE::QemuServer::print_drive($drive);
+	    $param->{$opt} = PVE::QemuServer::print_drive($drive, 1);
 	} elsif ($opt =~ m/^net(\d+)$/) {
 	    # add macaddr
 	    my $net = PVE::QemuServer::parse_net($param->{$opt});
@@ -1288,7 +1320,7 @@ my $update_vm_api  = sub {
 
 	    my $check_drive_perms = sub {
 		my ($opt, $val) = @_;
-		my $drive = PVE::QemuServer::parse_drive($opt, $val);
+		my $drive = PVE::QemuServer::parse_drive($opt, $val, 1);
 		# FIXME: cloudinit: CDROM or Disk?
 		if (PVE::QemuServer::drive_is_cdrom($drive)) { # CDROM
 		    $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.CDROM']);
@@ -1384,7 +1416,7 @@ my $update_vm_api  = sub {
 		    # default legacy boot order implies all cdroms anyway
 		    if (@bootorder) {
 			# append new CD drives to bootorder to mark them bootable
-			my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
+			my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt}, 1);
 			if (PVE::QemuServer::drive_is_cdrom($drive, 1) && !grep(/^$opt$/, @bootorder)) {
 			    push @bootorder, $opt;
 			    $conf->{pending}->{boot} = PVE::QemuServer::print_bootorder(\@bootorder);
diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index b993378..76b4314 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -101,7 +101,7 @@ sub parse_volume {
 	}
 	$volume = { 'file' => $volume_string };
     } else {
-	$volume = PVE::QemuServer::Drive::parse_drive($key, $volume_string);
+	$volume = PVE::QemuServer::Drive::parse_drive($key, $volume_string, 1);
     }
 
     die "unable to parse volume\n" if !defined($volume) && !$noerr;
diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
index f024269..828076d 100644
--- a/PVE/QemuServer/Drive.pm
+++ b/PVE/QemuServer/Drive.pm
@@ -409,6 +409,20 @@ my $alldrive_fmt = {
     %efitype_fmt,
 };
 
+my %import_from_fmt = (
+    'import-from' => {
+	type => 'string',
+	format => 'pve-volume-id-or-absolute-path',
+	format_description => 'source volume',
+	description => "When creating a new disk, import from this source.",
+	optional => 1,
+    },
+);
+my $alldrive_fmt_with_alloc = {
+    %$alldrive_fmt,
+    %import_from_fmt,
+};
+
 my $unused_fmt = {
     volume => { alias => 'file' },
     file => {
@@ -436,6 +450,8 @@ my $desc_with_alloc = sub {
 
     my $new_desc = dclone($desc);
 
+    $new_desc->{format}->{'import-from'} = $import_from_fmt{'import-from'};
+
     my $extra_note = '';
     if ($type eq 'efidisk') {
 	$extra_note = " Note that SIZE_IN_GiB is ignored here and that the default EFI vars are ".
@@ -445,7 +461,8 @@ my $desc_with_alloc = sub {
     }
 
     $new_desc->{description} .= "Use the special syntax STORAGE_ID:SIZE_IN_GiB to allocate a new ".
-	"volume.${extra_note}";
+	"volume.${extra_note} Use the 'import-from' parameter to import from an existing volume ".
+	"instead (SIZE_IN_GiB is ignored then).";
 
     $with_alloc_desc_cache->{$type} = $new_desc;
 
@@ -547,7 +564,7 @@ sub drive_is_read_only {
 #        [,iothread=on][,serial=serial][,model=model]
 
 sub parse_drive {
-    my ($key, $data) = @_;
+    my ($key, $data, $with_alloc) = @_;
 
     my ($interface, $index);
 
@@ -558,12 +575,14 @@ sub parse_drive {
 	return;
     }
 
-    if (!defined($drivedesc_hash->{$key})) {
+    my $desc_hash = $with_alloc ? $drivedesc_hash_with_alloc : $drivedesc_hash;
+
+    if (!defined($desc_hash->{$key})) {
 	warn "invalid drive key: $key\n";
 	return;
     }
 
-    my $desc = $drivedesc_hash->{$key}->{format};
+    my $desc = $desc_hash->{$key}->{format};
     my $res = eval { PVE::JSONSchema::parse_property_string($desc, $data) };
     return if !$res;
     $res->{interface} = $interface;
@@ -623,9 +642,10 @@ sub parse_drive {
 }
 
 sub print_drive {
-    my ($drive) = @_;
+    my ($drive, $with_alloc) = @_;
     my $skip = [ 'index', 'interface' ];
-    return PVE::JSONSchema::print_property_string($drive, $alldrive_fmt, $skip);
+    my $fmt = $with_alloc ? $alldrive_fmt_with_alloc : $alldrive_fmt;
+    return PVE::JSONSchema::print_property_string($drive, $fmt, $skip);
 }
 
 sub get_bootdisks {
diff --git a/PVE/QemuServer/ImportDisk.pm b/PVE/QemuServer/ImportDisk.pm
index 51ad52e..7557cac 100755
--- a/PVE/QemuServer/ImportDisk.pm
+++ b/PVE/QemuServer/ImportDisk.pm
@@ -71,7 +71,7 @@ sub do_import {
 	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);
+	PVE::QemuConfig->lock_config($vmid, $create_drive) if !$params->{'skip-config-update'};
     };
     if (my $err = $@) {
 	eval { PVE::Storage::vdisk_free($storecfg, $dst_volid) };
-- 
2.30.2