[pve-devel] [PATCH-SERIES v11 qemu-server/manager] API for disk import and OVF

[pve-devel] [PATCH-SERIES v11 qemu-server/manager] API for disk import and OVF

[Fabian Ebner]

Extend qm importdisk/importovf functionality to the API.

Changes from v10:
    * Add fix for device unplug issue (patch #1).
    * Add fixes related to calling create_disks() (patches #2 #3).
    * Refactor clone_disk() in preparation to re-use it for import
      (patches #4 #5 #6).
    * Add patch to print the newly allocated drive (patch #14).
    * Switch to using clone_disk for PVE-managed volumes and check for
      VM.Clone in the permission check if there is an owner ID.
    * Require <storeid>:0 syntax when using import-from. Allowing
      other values than 0 for the size would be confusing, because
      with import-from that size is never used (the size of the source
      image is).
    * Avoid making all foreach_volume iterators parse with the
      extended schema. Instead, provide a custom iterator for the
      places where it's actually required.

Still missing GUI integration for import from ovf, but that will be it's
own series.

Previous discussion:
https://lists.proxmox.com/pipermail/pve-devel/2022-January/051379.html


qemu-server:

Dominic Jäger (1):
  api: support VM disk import

Fabian Ebner (13):
  device unplug: verify that unplugging scsi disk completed
  api: create disks: always activate/update size when attaching existing
    volume
  api: update: pass correct config when creating disks
  clone disk: remove check for min QEMU version 2.7
  clone disk: group source and target parameters
  clone disk: allow cloning from an unused or unreferenced disk
  schema: add pve-volume-id-or-absolute-path
  parse ovf: untaint path when calling file_size_info
  api: add endpoint for parsing .ovf files
  image convert: allow block device as source
  api: factor out check/cleanup for drive params
  schema: drive: use separate schema when disk allocation is possible
  api: update vm: print drive string for newly allocated/imported drives

 PVE/API2/Qemu.pm             | 365 ++++++++++++++++++++++++++---------
 PVE/API2/Qemu/Makefile       |   2 +-
 PVE/API2/Qemu/OVF.pm         |  55 ++++++
 PVE/QemuServer.pm            |  99 +++++++---
 PVE/QemuServer/Drive.pm      |  94 ++++++---
 PVE/QemuServer/ImportDisk.pm |   2 +-
 PVE/QemuServer/OVF.pm        |   9 +-
 7 files changed, 487 insertions(+), 139 deletions(-)
 create mode 100644 PVE/API2/Qemu/OVF.pm


manager:

Fabian Ebner (1):
  api: nodes: add readovf endpoint

 PVE/API2/Nodes.pm | 7 +++++++
 1 file changed, 7 insertions(+)

-- 
2.30.2

[pve-devel] [PATCH v11 qemu-server 01/14] device unplug: verify that unplugging scsi disk completed

[Fabian Ebner]

Avoids the error
  adding drive failed: Duplicate ID 'drive-scsi1' for drive
that could happen when switching over to a new disk (e.g. via qm set),
if unplugging wasn't fast enough.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

New in v11.

 PVE/QemuServer.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 42f0fbd..b7e6a8e 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4249,6 +4249,7 @@ sub vm_deviceunplug {
 	my $device = parse_drive($deviceid, $conf->{$deviceid});
 
 	qemu_devicedel($vmid, $deviceid);
+	qemu_devicedelverify($vmid, $deviceid);
 	qemu_drivedel($vmid, $deviceid);
 	qemu_deletescsihw($conf, $vmid, $deviceid);
 
-- 
2.30.2

[pve-devel] [PATCH v11 qemu-server 02/14] api: create disks: always activate/update size when attaching existing volume

[Fabian Ebner]

For creation, activation and size update never triggered, because the
passed in $conf is essentially the same as the creation $settings, so
the disk was always detected to be the same as the "existing" one. But
actually, all disks are new, so it makes sense to do it.

For update, activation and size update nearly always triggered,
because only the pending changes are passed in as $conf. The case
where it didn't trigger is when the same pending change was made twice
(there are cases where hotplug isn't done, but makes it even more
unlikely).

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

New in v11.

 PVE/API2/Qemu.pm | 21 ++++-----------------
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 9be1caf..02b26d2 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -213,26 +213,13 @@ my $create_disks = sub {
 	    delete $disk->{format}; # no longer needed
 	    $res->{$ds} = PVE::QemuServer::print_drive($disk);
 	} else {
-
 	    PVE::Storage::check_volume_access($rpcenv, $authuser, $storecfg, $vmid, $volid);
 
-	    my $volid_is_new = 1;
-
-	    if ($conf->{$ds}) {
-		my $olddrive = PVE::QemuServer::parse_drive($ds, $conf->{$ds});
-		$volid_is_new = undef if $olddrive->{file} && $olddrive->{file} eq $volid;
-	    }
-
-	    if ($volid_is_new) {
+	    PVE::Storage::activate_volumes($storecfg, [ $volid ]) if $storeid;
 
-		PVE::Storage::activate_volumes($storecfg, [ $volid ]) if $storeid;
-
-		my $size = PVE::Storage::volume_size_info($storecfg, $volid);
-
-		die "volume $volid does not exist\n" if !$size;
-
-		$disk->{size} = $size;
-	    }
+	    my $size = PVE::Storage::volume_size_info($storecfg, $volid);
+	    die "volume $volid does not exist\n" if !$size;
+	    $disk->{size} = $size;
 
 	    $res->{$ds} = PVE::QemuServer::print_drive($disk);
 	}
-- 
2.30.2

[pve-devel] [PATCH v11 qemu-server 03/14] api: update: pass correct config when creating disks

[Fabian Ebner]

While the new options should be written to the pending config, the
decisions (currently only one) in create_disks needs to be made for
the current config.

Seems to fix EFI disk creation, but actually, it's only
future-proofing, because, currently, the same OVMF_VARS file is
used independently of $smm.

The correct config is also needed to determine the correct size for
the EFI disk for the upcoming import-from feature.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

New in v11.

 PVE/API2/Qemu.pm | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 02b26d2..c6587ef 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -237,12 +237,7 @@ my $create_disks = sub {
 	die $err;
     }
 
-    # modify vm config if everything went well
-    foreach my $ds (keys %$res) {
-	$conf->{$ds} = $res->{$ds};
-    }
-
-    return $vollist;
+    return ($vollist, $res);
 };
 
 my $check_cpu_model_access = sub {
@@ -712,7 +707,18 @@ __PACKAGE__->register_method({
 
 		my $vollist = [];
 		eval {
-		    $vollist = &$create_disks($rpcenv, $authuser, $conf, $arch, $storecfg, $vmid, $pool, $param, $storage);
+		    ($vollist, my $created_opts) = $create_disks->(
+			$rpcenv,
+			$authuser,
+			$conf,
+			$arch,
+			$storecfg,
+			$vmid,
+			$pool,
+			$param,
+			$storage,
+		    );
+		    $conf->{$_} = $created_opts->{$_} for keys $created_opts->%*;
 
 		    if (!$conf->{boot}) {
 			my $devs = PVE::QemuServer::get_default_bootdevices($conf);
@@ -1364,7 +1370,17 @@ my $update_vm_api  = sub {
 		    PVE::QemuServer::vmconfig_register_unused_drive($storecfg, $vmid, $conf, PVE::QemuServer::parse_drive($opt, $conf->{pending}->{$opt}))
 			if defined($conf->{pending}->{$opt});
 
-		    &$create_disks($rpcenv, $authuser, $conf->{pending}, $arch, $storecfg, $vmid, undef, {$opt => $param->{$opt}});
+		    my (undef, $created_opts) = $create_disks->(
+			$rpcenv,
+			$authuser,
+			$conf,
+			$arch,
+			$storecfg,
+			$vmid,
+			undef,
+			{$opt => $param->{$opt}},
+		    );
+		    $conf->{pending}->{$_} = $created_opts->{$_} for keys $created_opts->%*;
 
 		    # default legacy boot order implies all cdroms anyway
 		    if (@bootorder) {
-- 
2.30.2

[pve-devel] [PATCH v11 qemu-server 04/14] clone disk: remove check for min QEMU version 2.7

[Fabian Ebner]

Upgrading a cluster node entails re-starting or migrating VMs and even
PVE 6.0 already had QEMU 4.0.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

New in v11.

 PVE/QemuServer.pm | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index b7e6a8e..c0fca49 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -7642,15 +7642,8 @@ sub clone_disk {
 		qemu_img_convert($drive->{file}, $newvolid, $size, $snapname, $sparseinit);
 	    }
 	} else {
-
 	    die "cannot move TPM state while VM is running\n" if $drivename eq 'tpmstate0';
 
-	    my $kvmver = get_running_qemu_version ($vmid);
-	    if (!min_version($kvmver, 2, 7)) {
-		die "drive-mirror with iothread requires qemu version 2.7 or higher\n"
-		    if $drive->{iothread};
-	    }
-
 	    qemu_drive_mirror($vmid, $drivename, $newvolid, $newvmid, $sparseinit, $jobs,
 	        $completion, $qga, $bwlimit);
 	}
-- 
2.30.2

[pve-devel] [PATCH v11 qemu-server 05/14] clone disk: group source and target parameters

[Fabian Ebner]

to make the interface more digestible.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

New in v11.

 PVE/API2/Qemu.pm  | 52 +++++++++++++++++++++++++++++++----------------
 PVE/QemuServer.pm |  9 ++++++--
 2 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index c6587ef..14cac5b 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -3218,23 +3218,31 @@ __PACKAGE__->register_method({
 		    push @$storage_list, $storage if defined($storage);
 		    my $clonelimit = PVE::Storage::get_bandwidth_limit('clone', $storage_list, $bwlimit);
 
+		    my $source_info = {
+			vmid => $vmid,
+			running => $running,
+			drivename => $opt,
+			drive => $drive,
+			snapname => $snapname,
+		    };
+
+		    my $dest_info = {
+			vmid => $newid,
+			conf => $oldconf, # because it's a clone
+			storage => $storage,
+			format => $format,
+		    };
+
 		    my $newdrive = PVE::QemuServer::clone_disk(
 			$storecfg,
-			$vmid,
-			$running,
-			$opt,
-			$drive,
-			$snapname,
-			$newid,
-			$storage,
-			$format,
+			$source_info,
+			$dest_info,
 			$fullclone->{$opt},
 			$newvollist,
 			$jobs,
 			$completion,
 			$oldconf->{agent},
 			$clonelimit,
-			$oldconf
 		    );
 
 		    $newconf->{$opt} = PVE::QemuServer::print_drive($newdrive);
@@ -3469,23 +3477,31 @@ __PACKAGE__->register_method({
 		    $bwlimit
 		);
 
+		my $source_info = {
+		    vmid => $vmid,
+		    running => $running,
+		    drivename => $disk,
+		    drive => $drive,
+		    snapname => undef,
+		};
+
+		my $dest_info = {
+		    vmid => $vmid,
+		    conf => $conf,
+		    storage => $storeid,
+		    format => $format,
+		};
+
 		my $newdrive = PVE::QemuServer::clone_disk(
 		    $storecfg,
-		    $vmid,
-		    $running,
-		    $disk,
-		    $drive,
-		    undef,
-		    $vmid,
-		    $storeid,
-		    $format,
+		    $source_info,
+		    $dest_info,
 		    1,
 		    $newvollist,
 		    undef,
 		    undef,
 		    undef,
 		    $movelimit,
-		    $conf,
 		);
 		$conf->{$disk} = PVE::QemuServer::print_drive($newdrive);
 
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index c0fca49..56437c5 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -7571,8 +7571,13 @@ sub qemu_blockjobs_cancel {
 }
 
 sub clone_disk {
-    my ($storecfg, $vmid, $running, $drivename, $drive, $snapname,
-	$newvmid, $storage, $format, $full, $newvollist, $jobs, $completion, $qga, $bwlimit, $conf) = @_;
+    my ($storecfg, $source, $dest, $full, $newvollist, $jobs, $completion, $qga, $bwlimit) = @_;
+
+    my ($vmid, $running) = $source->@{qw(vmid running)};
+    my ($drivename, $drive, $snapname) = $source->@{qw(drivename drive snapname)};
+
+    my ($newvmid, $conf) = $dest->@{qw(vmid conf)};
+    my ($storage, $format) = $dest->@{qw(storage format)};
 
     my $newvolid;
 
-- 
2.30.2

[pve-devel] [PATCH v11 qemu-server 06/14] clone disk: allow cloning from an unused or unreferenced disk

[Fabian Ebner]

and also when source and target drivename are different. In those
cases, it is done via qemu-img convert/dd.

In preparation to allow import from existing PVE-managed disks.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

New in v11.

 PVE/API2/Qemu.pm  |  2 ++
 PVE/QemuServer.pm | 29 +++++++++++++++++++----------
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 14cac5b..01321c8 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -3229,6 +3229,7 @@ __PACKAGE__->register_method({
 		    my $dest_info = {
 			vmid => $newid,
 			conf => $oldconf, # because it's a clone
+			drivename => $opt,
 			storage => $storage,
 			format => $format,
 		    };
@@ -3488,6 +3489,7 @@ __PACKAGE__->register_method({
 		my $dest_info = {
 		    vmid => $vmid,
 		    conf => $conf,
+		    drivename => $disk,
 		    storage => $storeid,
 		    format => $format,
 		};
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 56437c5..0217d16 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -7574,15 +7574,25 @@ sub clone_disk {
     my ($storecfg, $source, $dest, $full, $newvollist, $jobs, $completion, $qga, $bwlimit) = @_;
 
     my ($vmid, $running) = $source->@{qw(vmid running)};
-    my ($drivename, $drive, $snapname) = $source->@{qw(drivename drive snapname)};
+    my ($src_drivename, $drive, $snapname) = $source->@{qw(drivename drive snapname)};
 
-    my ($newvmid, $conf) = $dest->@{qw(vmid conf)};
+    my ($newvmid, $conf, $dst_drivename) = $dest->@{qw(vmid conf drivename)};
     my ($storage, $format) = $dest->@{qw(storage format)};
 
+    if ($src_drivename && $dst_drivename && $src_drivename ne $dst_drivename) {
+	die "cloning from/to EFI disk requires EFI disk\n"
+	    if $src_drivename eq 'efidisk0' || $dst_drivename eq 'efidisk0';
+	die "cloning from/to TPM state requires TPM state\n"
+	    if $src_drivename eq 'tpmstate0' || $dst_drivename eq 'tpmstate0';
+    }
+
     my $newvolid;
 
+    print "create " . ($full ? 'full' : 'linked') . " clone of drive ";
+    print "$src_drivename " if $src_drivename;
+    print "($drive->{file})\n";
+
     if (!$full) {
-	print "create linked clone of drive $drivename ($drive->{file})\n";
 	$newvolid = PVE::Storage::vdisk_clone($storecfg,  $drive->{file}, $newvmid, $snapname);
 	push @$newvollist, $newvolid;
     } else {
@@ -7592,7 +7602,6 @@ sub clone_disk {
 
 	my $dst_format = resolve_dst_disk_format($storecfg, $storeid, $volname, $format);
 
-	print "create full clone of drive $drivename ($drive->{file})\n";
 	my $name = undef;
 	my $size = undef;
 	if (drive_is_cloudinit($drive)) {
@@ -7603,9 +7612,9 @@ sub clone_disk {
 	    }
 	    $snapname = undef;
 	    $size = PVE::QemuServer::Cloudinit::CLOUDINIT_DISK_SIZE;
-	} elsif ($drivename eq 'efidisk0') {
+	} elsif ($dst_drivename eq 'efidisk0') {
 	    $size = get_efivars_size($conf);
-	} elsif ($drivename eq 'tpmstate0') {
+	} elsif ($dst_drivename eq 'tpmstate0') {
 	    $dst_format = 'raw';
 	    $size = PVE::QemuServer::Drive::TPMSTATE_DISK_SIZE;
 	} else {
@@ -7629,9 +7638,9 @@ sub clone_disk {
 	}
 
 	my $sparseinit = PVE::Storage::volume_has_feature($storecfg, 'sparseinit', $newvolid);
-	if (!$running || $snapname) {
+	if (!$running || !$src_drivename || $snapname) {
 	    # TODO: handle bwlimits
-	    if ($drivename eq 'efidisk0') {
+	    if ($dst_drivename eq 'efidisk0') {
 		# the relevant data on the efidisk may be smaller than the source
 		# e.g. on RBD/ZFS, so we use dd to copy only the amount
 		# that is given by the OVMF_VARS.fd
@@ -7647,9 +7656,9 @@ sub clone_disk {
 		qemu_img_convert($drive->{file}, $newvolid, $size, $snapname, $sparseinit);
 	    }
 	} else {
-	    die "cannot move TPM state while VM is running\n" if $drivename eq 'tpmstate0';
+	    die "cannot move TPM state while VM is running\n" if $src_drivename eq 'tpmstate0';
 
-	    qemu_drive_mirror($vmid, $drivename, $newvolid, $newvmid, $sparseinit, $jobs,
+	    qemu_drive_mirror($vmid, $src_drivename, $newvolid, $newvmid, $sparseinit, $jobs,
 	        $completion, $qga, $bwlimit);
 	}
     }
-- 
2.30.2

[pve-devel] [PATCH v11 qemu-server 07/14] schema: add pve-volume-id-or-absolute-path

[Fabian Ebner]

Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
[split into its own patch + style fixes]
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v10.

 PVE/QemuServer.pm | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 0217d16..b5fb457 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -1036,11 +1036,17 @@ PVE::JSONSchema::register_format('pve-volume-id-or-qm-path', \&verify_volume_id_
 sub verify_volume_id_or_qm_path {
     my ($volid, $noerr) = @_;
 
-    if ($volid eq 'none' || $volid eq 'cdrom' || $volid =~ m|^/|) {
-	return $volid;
-    }
+    return $volid if $volid eq 'none' || $volid eq 'cdrom';
+
+    return verify_volume_id_or_absolute_path($volid, $noerr);
+}
+
+PVE::JSONSchema::register_format('pve-volume-id-or-absolute-path', \&verify_volume_id_or_absolute_path);
+sub verify_volume_id_or_absolute_path {
+    my ($volid, $noerr) = @_;
+
+    return $volid if $volid =~ m|^/|;
 
-    # if its neither 'none' nor 'cdrom' nor a path, check if its a volume-id
     $volid = eval { PVE::JSONSchema::check_format('pve-volume-id', $volid, '') };
     if ($@) {
 	return if $noerr;
-- 
2.30.2

[pve-devel] [PATCH v11 qemu-server 08/14] parse ovf: untaint path when calling file_size_info

[Fabian Ebner]

Prepare for calling parse_ovf via API, where the -T switch is used.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v10:
    * Move untaint to outside of the function call.

 PVE/QemuServer/OVF.pm | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/PVE/QemuServer/OVF.pm b/PVE/QemuServer/OVF.pm
index 0376cbf..b97b052 100644
--- a/PVE/QemuServer/OVF.pm
+++ b/PVE/QemuServer/OVF.pm
@@ -221,10 +221,11 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id);
 	    die "error parsing $filepath, file seems not to exist at $backing_file_path\n";
 	}
 
-	my $virtual_size;
-	if ( !($virtual_size = PVE::Storage::file_size_info($backing_file_path)) ) {
-	    die "error parsing $backing_file_path, size seems to be $virtual_size\n";
-	}
+	($backing_file_path) = $backing_file_path =~ m|^(/.*)|; # untaint
+
+	my $virtual_size = PVE::Storage::file_size_info($backing_file_path);
+	die "error parsing $backing_file_path, cannot determine file size\n"
+	    if !$virtual_size;
 
 	$pve_disk = {
 	    disk_address => $pve_disk_address,
-- 
2.30.2

[pve-devel] [PATCH v11 qemu-server 09/14] api: add endpoint for parsing .ovf files

[Fabian Ebner]

Co-developed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
[split into its own patch + minor improvements/style fixes]
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v10:
    * Add "Path to" to 'manifest' parameter description.

 PVE/API2/Qemu/Makefile |  2 +-
 PVE/API2/Qemu/OVF.pm   | 55 ++++++++++++++++++++++++++++++++++++++++++
 PVE/QemuServer.pm      | 32 ++++++++++++++++++++++++
 3 files changed, 88 insertions(+), 1 deletion(-)
 create mode 100644 PVE/API2/Qemu/OVF.pm

diff --git a/PVE/API2/Qemu/Makefile b/PVE/API2/Qemu/Makefile
index 5d4abda..bdd4762 100644
--- a/PVE/API2/Qemu/Makefile
+++ b/PVE/API2/Qemu/Makefile
@@ -1,4 +1,4 @@
-SOURCES=Agent.pm CPU.pm Machine.pm
+SOURCES=Agent.pm CPU.pm Machine.pm OVF.pm
 
 .PHONY: install
 install:
diff --git a/PVE/API2/Qemu/OVF.pm b/PVE/API2/Qemu/OVF.pm
new file mode 100644
index 0000000..5fa0ef0
--- /dev/null
+++ b/PVE/API2/Qemu/OVF.pm
@@ -0,0 +1,55 @@
+package PVE::API2::Qemu::OVF;
+
+use strict;
+use warnings;
+
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::QemuServer::OVF;
+use PVE::RESTHandler;
+
+use base qw(PVE::RESTHandler);
+
+__PACKAGE__->register_method ({
+    name => 'index',
+    path => '',
+    method => 'GET',
+    proxyto => 'node',
+    description => "Read an .ovf manifest.",
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    manifest => {
+		description => "Path to .ovf manifest.",
+		type => 'string',
+	    },
+	},
+    },
+    returns => {
+	description => "VM config according to .ovf manifest.",
+	type => "object",
+    },
+    returns => {
+	type => 'object',
+	additionalProperties => 1,
+	properties => PVE::QemuServer::json_ovf_properties({}),
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $manifest = $param->{manifest};
+	die "check for file $manifest failed - $!\n" if !-f $manifest;
+
+	my $parsed = PVE::QemuServer::OVF::parse_ovf($manifest);
+	my $result;
+	$result->{cores} = $parsed->{qm}->{cores};
+	$result->{name} =  $parsed->{qm}->{name};
+	$result->{memory} = $parsed->{qm}->{memory};
+	my $disks = $parsed->{disks};
+	for my $disk (@$disks) {
+	    $result->{$disk->{disk_address}} = $disk->{backing_file};
+	}
+	return $result;
+    }});
+
+1;
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index b5fb457..f1b1aa3 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2203,6 +2203,38 @@ sub json_config_properties {
     return $prop;
 }
 
+# Properties that we can read from an OVF file
+sub json_ovf_properties {
+    my $prop = shift;
+
+    for my $device (PVE::QemuServer::Drive::valid_drive_names()) {
+	$prop->{$device} = {
+	    type => 'string',
+	    format => 'pve-volume-id-or-absolute-path',
+	    description => "Disk image that gets imported to $device",
+	    optional => 1,
+	};
+    }
+
+    $prop->{cores} = {
+	type => 'integer',
+	description => "The number of CPU cores.",
+	optional => 1,
+    };
+    $prop->{memory} = {
+	type => 'integer',
+	description => "Amount of RAM for the VM in MB.",
+	optional => 1,
+    };
+    $prop->{name} = {
+	type => 'string',
+	description => "Name of the VM.",
+	optional => 1,
+    };
+
+    return $prop;
+}
+
 # return copy of $confdesc_cloudinit to generate documentation
 sub cloudinit_config_properties {
 
-- 
2.30.2

[pve-devel] [PATCH v11 qemu-server 10/14] image convert: allow block device as source

[Fabian Ebner]

Necessary to import from an existing storage using block-device
volumes like ZFS.

Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
[split into its own patch]
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v10.

 PVE/QemuServer.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index f1b1aa3..339536a 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -7312,7 +7312,7 @@ sub qemu_img_convert {
 	$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) {
+    } elsif (-f $src_volid || -b $src_volid) {
 	$src_path = $src_volid;
 	if ($src_path =~ m/\.($PVE::QemuServer::Drive::QEMU_FORMAT_RE)$/) {
 	    $src_format = $1;
-- 
2.30.2

[pve-devel] [PATCH v11 qemu-server 11/14] api: factor out check/cleanup for drive params

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

New in v11.

 PVE/API2/Qemu.pm | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 01321c8..791a23f 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -63,6 +63,23 @@ my $resolve_cdrom_alias = sub {
     }
 };
 
+my $check_drive_param = sub {
+    my ($param, $storecfg, $extra_checks) = @_;
+
+    for my $opt (sort keys $param->%*) {
+	next if !PVE::QemuServer::is_valid_drivename($opt);
+
+	my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
+	raise_param_exc({ $opt => "unable to parse drive options" }) if !$drive;
+
+	PVE::QemuServer::cleanup_drive_path($opt, $storecfg, $drive);
+
+	$extra_checks->($drive) if $extra_checks;
+
+	$param->{$opt} = PVE::QemuServer::print_drive($drive);
+    }
+};
+
 my $NEW_DISK_RE = qr!^(([^/:\s]+):)?(\d+(\.\d+)?)$!;
 my $check_storage_access = sub {
    my ($rpcenv, $authuser, $storecfg, $vmid, $settings, $default_storage) = @_;
@@ -617,15 +634,7 @@ __PACKAGE__->register_method({
 
 	    &$check_cpu_model_access($rpcenv, $authuser, $param);
 
-	    foreach my $opt (keys %$param) {
-		if (PVE::QemuServer::is_valid_drivename($opt)) {
-		    my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
-		    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);
-		}
-	    }
+	    $check_drive_param->($param, $storecfg);
 
 	    PVE::QemuServer::add_random_macs($param);
 	} else {
@@ -1195,15 +1204,10 @@ my $update_vm_api  = sub {
 	die "cannot add non-replicatable volume to a replicated VM\n";
     };
 
+    $check_drive_param->($param, $storecfg, $check_replication);
+
     foreach my $opt (keys %$param) {
-	if (PVE::QemuServer::is_valid_drivename($opt)) {
-	    # cleanup drive path
-	    my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
-	    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);
-	} elsif ($opt =~ m/^net(\d+)$/) {
+	if ($opt =~ m/^net(\d+)$/) {
 	    # add macaddr
 	    my $net = PVE::QemuServer::parse_net($param->{$opt});
 	    $param->{$opt} = PVE::QemuServer::print_net($net);
-- 
2.30.2

[pve-devel] [PATCH v11 qemu-server 12/14] schema: drive: use separate schema when disk allocation is possible

[Fabian Ebner]

via the special syntax <storeid>:<size>.

Not worth it by itself, but this is anticipating a new 'import-from'
parameter which is only used upon import/allocation, but shouldn't be
part of the schema for the config or other API enpoints.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v10:
    * Add initial space when appending parameter description.

 PVE/API2/Qemu.pm        | 12 ++++++--
 PVE/QemuServer.pm       |  9 ++++--
 PVE/QemuServer/Drive.pm | 62 +++++++++++++++++++++++++++++------------
 3 files changed, 60 insertions(+), 23 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 791a23f..f30c56f 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -569,7 +569,9 @@ __PACKAGE__->register_method({
 		    default => 0,
 		    description => "Start VM after it was created successfully.",
 		},
-	    }),
+	    },
+	    1, # with_disk_alloc
+	),
     },
     returns => {
 	type => 'string',
@@ -1552,7 +1554,9 @@ __PACKAGE__->register_method({
 		    maximum => 30,
 		    optional => 1,
 		},
-	    }),
+	    },
+	    1, # with_disk_alloc
+	),
     },
     returns => {
 	type => 'string',
@@ -1600,7 +1604,9 @@ __PACKAGE__->register_method({
 		    maxLength => 40,
 		    optional => 1,
 		},
-	    }),
+	    },
+	    1, # with_disk_alloc
+	),
     },
     returns => { type => 'null' },
     code => sub {
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 339536a..2c0a9e2 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2184,7 +2184,7 @@ sub verify_usb_device {
 
 # add JSON properties for create and set function
 sub json_config_properties {
-    my $prop = shift;
+    my ($prop, $with_disk_alloc) = @_;
 
     my $skip_json_config_opts = {
 	parent => 1,
@@ -2197,7 +2197,12 @@ sub json_config_properties {
 
     foreach my $opt (keys %$confdesc) {
 	next if $skip_json_config_opts->{$opt};
-	$prop->{$opt} = $confdesc->{$opt};
+
+	if ($with_disk_alloc && is_valid_drivename($opt)) {
+	    $prop->{$opt} = $PVE::QemuServer::Drive::drivedesc_hash_with_alloc->{$opt};
+	} else {
+	    $prop->{$opt} = $confdesc->{$opt};
+	}
     }
 
     return $prop;
diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
index 7b82fb2..d5d4723 100644
--- a/PVE/QemuServer/Drive.pm
+++ b/PVE/QemuServer/Drive.pm
@@ -3,6 +3,8 @@ package PVE::QemuServer::Drive;
 use strict;
 use warnings;
 
+use Storable qw(dclone);
+
 use PVE::Storage;
 use PVE::JSONSchema qw(get_standard_option);
 
@@ -33,6 +35,8 @@ our $MAX_SATA_DISKS = 6;
 our $MAX_UNUSED_DISKS = 256;
 
 our $drivedesc_hash;
+# Schema when disk allocation is possible.
+our $drivedesc_hash_with_alloc = {};
 
 my %drivedesc_base = (
     volume => { alias => 'file' },
@@ -262,14 +266,10 @@ my $ide_fmt = {
 };
 PVE::JSONSchema::register_format("pve-qm-ide", $ide_fmt);
 
-my $ALLOCATION_SYNTAX_DESC =
-    "Use the special syntax STORAGE_ID:SIZE_IN_GiB to allocate a new volume.";
-
 my $idedesc = {
     optional => 1,
     type => 'string', format => $ide_fmt,
-    description => "Use volume as IDE hard disk or CD-ROM (n is 0 to " .($MAX_IDE_DISKS -1) . "). " .
-	$ALLOCATION_SYNTAX_DESC,
+    description => "Use volume as IDE hard disk or CD-ROM (n is 0 to " .($MAX_IDE_DISKS - 1) . ").",
 };
 PVE::JSONSchema::register_standard_option("pve-qm-ide", $idedesc);
 
@@ -285,8 +285,7 @@ my $scsi_fmt = {
 my $scsidesc = {
     optional => 1,
     type => 'string', format => $scsi_fmt,
-    description => "Use volume as SCSI hard disk or CD-ROM (n is 0 to " . ($MAX_SCSI_DISKS - 1) . "). " .
-	$ALLOCATION_SYNTAX_DESC,
+    description => "Use volume as SCSI hard disk or CD-ROM (n is 0 to " . ($MAX_SCSI_DISKS - 1) . ").",
 };
 PVE::JSONSchema::register_standard_option("pve-qm-scsi", $scsidesc);
 
@@ -298,8 +297,7 @@ my $sata_fmt = {
 my $satadesc = {
     optional => 1,
     type => 'string', format => $sata_fmt,
-    description => "Use volume as SATA hard disk or CD-ROM (n is 0 to " . ($MAX_SATA_DISKS - 1). "). " .
-	$ALLOCATION_SYNTAX_DESC,
+    description => "Use volume as SATA hard disk or CD-ROM (n is 0 to " . ($MAX_SATA_DISKS - 1). ").",
 };
 PVE::JSONSchema::register_standard_option("pve-qm-sata", $satadesc);
 
@@ -311,8 +309,7 @@ my $virtio_fmt = {
 my $virtiodesc = {
     optional => 1,
     type => 'string', format => $virtio_fmt,
-    description => "Use volume as VIRTIO hard disk (n is 0 to " . ($MAX_VIRTIO_DISKS - 1) . "). " .
-	$ALLOCATION_SYNTAX_DESC,
+    description => "Use volume as VIRTIO hard disk (n is 0 to " . ($MAX_VIRTIO_DISKS - 1) . ").",
 };
 PVE::JSONSchema::register_standard_option("pve-qm-virtio", $virtiodesc);
 
@@ -359,9 +356,7 @@ my $efidisk_fmt = {
 my $efidisk_desc = {
     optional => 1,
     type => 'string', format => $efidisk_fmt,
-    description => "Configure a Disk for storing EFI vars. " .
-	$ALLOCATION_SYNTAX_DESC . " Note that SIZE_IN_GiB is ignored here " .
-	"and that the default EFI vars are copied to the volume instead.",
+    description => "Configure a Disk for storing EFI vars.",
 };
 
 PVE::JSONSchema::register_standard_option("pve-qm-efidisk", $efidisk_desc);
@@ -397,10 +392,7 @@ my $tpmstate_fmt = {
 my $tpmstate_desc = {
     optional => 1,
     type => 'string', format => $tpmstate_fmt,
-    description => "Configure a Disk for storing TPM state. " .
-	$ALLOCATION_SYNTAX_DESC . " Note that SIZE_IN_GiB is ignored here " .
-	"and that the default size of 4 MiB will always be used instead. The " .
-	"format is also fixed to 'raw'.",
+    description => "Configure a Disk for storing TPM state. The format is fixed to 'raw'.",
 };
 use constant TPMSTATE_DISK_SIZE => 4 * 1024 * 1024;
 
@@ -434,27 +426,61 @@ my $unuseddesc = {
     description => "Reference to unused volumes. This is used internally, and should not be modified manually.",
 };
 
+my $with_alloc_desc_cache = {
+    unused => $unuseddesc, # Allocation for unused is not supported currently.
+};
+my $desc_with_alloc = sub {
+    my ($type, $desc) = @_;
+
+    return $with_alloc_desc_cache->{$type} if $with_alloc_desc_cache->{$type};
+
+    my $new_desc = dclone($desc);
+
+    my $extra_note = '';
+    if ($type eq 'efidisk') {
+	$extra_note = " Note that SIZE_IN_GiB is ignored here and that the default EFI vars are ".
+	    "copied to the volume instead.";
+    } elsif ($type eq 'tpmstate') {
+	$extra_note = " Note that SIZE_IN_GiB is ignored here and 4 MiB will be used instead.";
+    }
+
+    $new_desc->{description} .= " Use the special syntax STORAGE_ID:SIZE_IN_GiB to allocate a new ".
+	"volume.${extra_note}";
+
+    $with_alloc_desc_cache->{$type} = $new_desc;
+
+    return $new_desc;
+};
+
 for (my $i = 0; $i < $MAX_IDE_DISKS; $i++)  {
     $drivedesc_hash->{"ide$i"} = $idedesc;
+    $drivedesc_hash_with_alloc->{"ide$i"} = $desc_with_alloc->('ide', $idedesc);
 }
 
 for (my $i = 0; $i < $MAX_SATA_DISKS; $i++)  {
     $drivedesc_hash->{"sata$i"} = $satadesc;
+    $drivedesc_hash_with_alloc->{"sata$i"} = $desc_with_alloc->('sata', $satadesc);
 }
 
 for (my $i = 0; $i < $MAX_SCSI_DISKS; $i++)  {
     $drivedesc_hash->{"scsi$i"} = $scsidesc;
+    $drivedesc_hash_with_alloc->{"scsi$i"} = $desc_with_alloc->('scsi', $scsidesc);
 }
 
 for (my $i = 0; $i < $MAX_VIRTIO_DISKS; $i++)  {
     $drivedesc_hash->{"virtio$i"} = $virtiodesc;
+    $drivedesc_hash_with_alloc->{"virtio$i"} = $desc_with_alloc->('virtio', $virtiodesc);
 }
 
 $drivedesc_hash->{efidisk0} = $efidisk_desc;
+$drivedesc_hash_with_alloc->{efidisk0} = $desc_with_alloc->('efidisk', $efidisk_desc);
+
 $drivedesc_hash->{tpmstate0} = $tpmstate_desc;
+$drivedesc_hash_with_alloc->{tpmstate0} = $desc_with_alloc->('tpmstate', $tpmstate_desc);
 
 for (my $i = 0; $i < $MAX_UNUSED_DISKS; $i++) {
     $drivedesc_hash->{"unused$i"} = $unuseddesc;
+    $drivedesc_hash_with_alloc->{"unused$i"} = $desc_with_alloc->('unused', $unuseddesc);
 }
 
 sub valid_drive_names_for_boot {
-- 
2.30.2

[pve-devel] [PATCH v11 qemu-server 13/14] api: support VM disk import

[Fabian Ebner]

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 v10:
    * Switch to using clone_disk for PVE-managed volumes and check for
      VM.Clone in the permission check if there is an owner ID.
    * Require <storeid>:0 syntax when using import-from. Allowing
      other values than 0 for the size would be confusing, because
      with import-from that size is never used (the size of the source
      image is). The check moved to check_drive_param, as that seemed
      to be the more fitting place.
    * Avoid making all foreach_volume iterators parse with the
      extended schema. Instead, provide a custom iterator for the
      places where it's actually required.
    * Mention that source volume should not be actively used in
      import-from description.
    * Add missing newline to error for size check for source image,
      and also die when size is zero.

 PVE/API2/Qemu.pm             | 215 ++++++++++++++++++++++++++++++-----
 PVE/QemuServer/Drive.pm      |  34 +++++-
 PVE/QemuServer/ImportDisk.pm |   2 +-
 3 files changed, 216 insertions(+), 35 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index f30c56f..c6d57e2 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;
@@ -63,28 +64,46 @@ my $resolve_cdrom_alias = sub {
     }
 };
 
+# Used in import-enabled API endpoints. Parses drives using the extended '_with_alloc' schema.
+my $foreach_volume_with_alloc = sub {
+    my ($param, $func) = @_;
+
+    for my $opt (sort keys $param->%*) {
+	next if !PVE::QemuServer::is_valid_drivename($opt);
+
+	my $drive = PVE::QemuServer::Drive::parse_drive($opt, $param->{$opt}, 1);
+	next if !$drive;
+
+	$func->($opt, $drive);
+    }
+};
+
+my $NEW_DISK_RE = qr!^(([^/:\s]+):)?(\d+(\.\d+)?)$!;
+
 my $check_drive_param = sub {
     my ($param, $storecfg, $extra_checks) = @_;
 
     for my $opt (sort keys $param->%*) {
 	next 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;
 
+	die "'import-from' requires special syntax - use <storage ID>:0,import-from=<source>\n"
+	    if $drive->{'import-from'} && ($drive->{file} !~ $NEW_DISK_RE || $3 != 0);
+
 	PVE::QemuServer::cleanup_drive_path($opt, $storecfg, $drive);
 
 	$extra_checks->($drive) if $extra_checks;
 
-	$param->{$opt} = PVE::QemuServer::print_drive($drive);
+	$param->{$opt} = PVE::QemuServer::print_drive($drive, 1);
     }
 };
 
-my $NEW_DISK_RE = qr!^(([^/:\s]+):)?(\d+(\.\d+)?)$!;
 my $check_storage_access = sub {
    my ($rpcenv, $authuser, $storecfg, $vmid, $settings, $default_storage) = @_;
 
-   PVE::QemuConfig->foreach_volume($settings, sub {
+   $foreach_volume_with_alloc->($settings, sub {
 	my ($ds, $drive) = @_;
 
 	my $isCDROM = PVE::QemuServer::drive_is_cdrom($drive);
@@ -106,6 +125,20 @@ my $check_storage_access = sub {
 	} else {
 	    PVE::Storage::check_volume_access($rpcenv, $authuser, $storecfg, $vmid, $volid);
 	}
+
+	if (my $src_image = $drive->{'import-from'}) {
+	    my $src_vmid;
+	    my ($src_storeid) = PVE::Storage::parse_volume_id($src_image, 1);
+	    if ($src_storeid) { # PVE-managed volume
+		$src_vmid = (PVE::Storage::parse_volname($storecfg, $src_image))[2]
+	    }
+
+	    if ($src_vmid) { # might be actively used by VM and will be copied via clone_disk()
+		$rpcenv->check($authuser, "/vms/${src_vmid}", ['VM.Clone']);
+	    } else {
+		PVE::Storage::check_volume_access($rpcenv, $authuser, $storecfg, $vmid, $src_image);
+	    }
+	}
     });
 
    $rpcenv->check($authuser, "/storage/$settings->{vmstatestorage}", ['Datastore.AllocateSpace'])
@@ -164,6 +197,87 @@ my $check_storage_access_migrate = sub {
 	if !$scfg->{content}->{images};
 };
 
+my $import_from_volid = sub {
+    my ($storecfg, $src_volid, $dest_info, $vollist) = @_;
+
+    die "cannot import from cloudinit disk\n"
+	if PVE::QemuServer::Drive::drive_is_cloudinit({ file => $src_volid });
+
+    my ($src_storeid, $src_volname) = PVE::Storage::parse_volume_id($src_volid);
+    my $src_vmid = (PVE::Storage::parse_volname($storecfg, $src_volid))[2];
+
+    my $src_vm_state = sub {
+	my $exists = $src_vmid && PVE::Cluster::get_vmlist()->{ids}->{$src_vmid} ? 1 : 0;
+
+	my $runs = 0;
+	if ($exists) {
+	    eval { PVE::QemuConfig::assert_config_exists_on_node($src_vmid); };
+	    die "owner VM $src_vmid not on local node\n" if $@;
+	    $runs = PVE::QemuServer::Helpers::vm_running_locally($src_vmid) || 0;
+	}
+
+	return ($exists, $runs);
+    };
+
+    my ($src_vm_exists, $running) = $src_vm_state->();
+
+    die "cannot import from '$src_volid' - full clone feature is not supported\n"
+	if !PVE::Storage::volume_has_feature($storecfg, 'copy', $src_volid, undef, $running);
+
+    my $clonefn = sub {
+	my ($src_vm_exists_now, $running_now) = $src_vm_state->();
+
+	die "owner VM $src_vmid changed state unexpectedly\n"
+	    if $src_vm_exists_now != $src_vm_exists || $running_now != $running;
+
+	my $src_conf = $src_vm_exists_now ? PVE::QemuConfig->load_config($src_vmid) : {};
+
+	my $src_drive = { file => $src_volid };
+	my $src_drivename;
+	PVE::QemuConfig->foreach_volume($src_conf, sub {
+	    my ($ds, $drive) = @_;
+
+	    return if $src_drivename;
+
+	    if ($drive->{file} eq $src_volid) {
+		$src_drive = $drive;
+		$src_drivename = $ds;
+	    }
+	});
+
+	my $source_info = {
+	    vmid => $src_vmid,
+	    running => $running_now,
+	    drivename => $src_drivename,
+	    drive => $src_drive,
+	    snapname => undef,
+	};
+
+	return PVE::QemuServer::clone_disk(
+	    $storecfg,
+	    $source_info,
+	    $dest_info,
+	    1,
+	    $vollist,
+	    undef,
+	    undef,
+	    $src_conf->{agent},
+	    PVE::Storage::get_bandwidth_limit('clone', [$src_storeid, $dest_info->{storage}]),
+	);
+    };
+
+    my $cloned;
+    if ($running) {
+	$cloned = PVE::QemuConfig->lock_config_full($src_vmid, 30, $clonefn);
+    } elsif ($src_vmid) {
+	$cloned = PVE::QemuConfig->lock_config_shared($src_vmid, 30, $clonefn);
+    } else {
+	$cloned = $clonefn->();
+    }
+
+    return $cloned->@{qw(file size)};
+};
+
 # Note: $pool is only needed when creating a VM, because pool permissions
 # are automatically inherited if VM already exists inside a pool.
 my $create_disks = sub {
@@ -207,28 +321,73 @@ 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'}) {
+		my $dst_volid;
+		my ($src_storeid) = PVE::Storage::parse_volume_id($source, 1);
+
+		if ($src_storeid) { # PVE-managed volume
+		    die "could not get size of $source\n"
+			if !PVE::Storage::volume_size_info($storecfg, $source, 10);
+
+		    my $dest_info = {
+			vmid => $vmid,
+			conf => $conf,
+			drivename => $ds,
+			storage => $storeid,
+			format => $disk->{format},
+		    };
+
+		    ($dst_volid, $size) = eval {
+			$import_from_volid->($storecfg, $source, $dest_info, $vollist);
+		    };
+		    die "cannot import from '$source' - $@" if $@;
+		} else {
+		    $source = PVE::Storage::abs_filesystem_path($storecfg, $source, 1);
+		    $size = PVE::Storage::file_size_info($source);
+		    die "could not get file size of $source\n" if !$size;
+
+		    (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} = $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);
 
@@ -242,7 +401,7 @@ my $create_disks = sub {
 	}
     };
 
-    eval { PVE::QemuConfig->foreach_volume($settings, $code); };
+    eval { $foreach_volume_with_alloc->($settings, $code); };
 
     # free allocated images on error
     if (my $err = $@) {
@@ -1285,7 +1444,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']);
@@ -1391,7 +1550,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/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
index d5d4723..88f013a 100644
--- a/PVE/QemuServer/Drive.pm
+++ b/PVE/QemuServer/Drive.pm
@@ -409,6 +409,22 @@ 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 => "Create a new disk, importing from this source. If the volume is not ".
+	    "managed by Proxmox VE, it's up to you to ensure that it's not actively used by ".
+	    "another process during the import!",
+	optional => 1,
+    },
+);
+my $alldrive_fmt_with_alloc = {
+    %$alldrive_fmt,
+    %import_from_fmt,
+};
+
 my $unused_fmt = {
     volume => { alias => 'file' },
     file => {
@@ -436,6 +452,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 +463,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 STORAGE_ID:0 and the 'import-from' parameter to import from an ".
+	"existing volume.";
 
     $with_alloc_desc_cache->{$type} = $new_desc;
 
@@ -547,7 +566,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 +577,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 +644,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

[pve-devel] [PATCH v11 qemu-server 14/14] api: update vm: print drive string for newly allocated/imported drives

[Fabian Ebner]

In the spirit of c75bf16 ("qm importdisk: tell user to what VM disk we
actually imported"), and so that the information is not lost once qm
importdisk switches to re-using the API call.

Added for cloudinit too, because a new disk is allocated.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

New in v11.

The name for cloudinit is rather predictable, so not too sure if it's
worth it there.

 PVE/API2/Qemu.pm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index c6d57e2..946fad0 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -318,6 +318,7 @@ my $create_disks = sub {
 	    push @$vollist, $volid;
 	    delete $disk->{format}; # no longer needed
 	    $res->{$ds} = PVE::QemuServer::print_drive($disk);
+	    print "$ds: successfully created disk '$res->{$ds}'\n";
 	} elsif ($volid =~ $NEW_DISK_RE) {
 	    my ($storeid, $size) = ($2 || $default_storage, $3);
 	    die "no storage ID specified (and no default storage)\n" if !$storeid;
@@ -388,6 +389,8 @@ my $create_disks = sub {
 		delete $disk->{format}; # no longer needed
 		$res->{$ds} = PVE::QemuServer::print_drive($disk);
 	    }
+
+	    print "$ds: successfully created disk '$res->{$ds}'\n";
 	} else {
 	    PVE::Storage::check_volume_access($rpcenv, $authuser, $storecfg, $vmid, $volid);
 
-- 
2.30.2

[pve-devel] [PATCH v11 manager 1/1] api: nodes: add readovf endpoint

[Fabian Ebner]

Because the paths under /nodes/{node}/qemu/ are already occupied by
a {vmid} regex, it's not possible to use /nodes/{node}/qemu/readovf
for the new call. As the call does not depend upon a particular vmid,
it's placed under /nodes/{node} instead.

Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
[split into its own patch + add to index]
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Needs dependency bump for qemu-server.

Changes from v10:
    * Add reason for placing it directly under /nodes/{node} to commit
      message.

 PVE/API2/Nodes.pm | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 655493a3..f595808a 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -49,6 +49,7 @@ use PVE::API2::LXC;
 use PVE::API2::Network;
 use PVE::API2::NodeConfig;
 use PVE::API2::Qemu::CPU;
+use PVE::API2::Qemu::OVF;
 use PVE::API2::Qemu;
 use PVE::API2::Replication;
 use PVE::API2::Services;
@@ -71,6 +72,11 @@ __PACKAGE__->register_method ({
     path => 'qemu',
 });
 
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::Qemu::OVF",
+    path => 'readovf',
+});
+
 __PACKAGE__->register_method ({
     subclass => "PVE::API2::LXC",
     path => 'lxc',
@@ -233,6 +239,7 @@ __PACKAGE__->register_method ({
 	    { name => 'network' },
 	    { name => 'qemu' },
 	    { name => 'query-url-metadata' },
+	    { name => 'readovf' },
 	    { name => 'replication' },
 	    { name => 'report' },
 	    { name => 'rrd' }, # fixme: remove?
-- 
2.30.2