[pve-devel] [RFC v10 qemu-server 6/7] api: support VM disk import
Fabian Ebner
f.ebner at proxmox.com
Thu Jan 13 11:08:30 CET 2022
From: Dominic Jäger <d.jaeger at proxmox.com>
Extend qm importdisk functionality to the API.
Co-authored-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
Co-authored-by: Dominic Jäger <d.jaeger at proxmox.com>
Signed-off-by: Fabian Ebner <f.ebner at 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
More information about the pve-devel
mailing list