From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id A2B1260813 for ; Thu, 13 Jan 2022 11:08:41 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 975CE1C2E8 for ; Thu, 13 Jan 2022 11:08:41 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id D0CB01C23A for ; Thu, 13 Jan 2022 11:08:36 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id AB2B9457B7 for ; Thu, 13 Jan 2022 11:08:36 +0100 (CET) From: Fabian Ebner To: pve-devel@lists.proxmox.com Date: Thu, 13 Jan 2022 11:08:30 +0100 Message-Id: <20220113100831.34113-8-f.ebner@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220113100831.34113-1-f.ebner@proxmox.com> References: <20220113100831.34113-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.138 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [drive.pm, qemuconfig.pm, qemu.pm, importdisk.pm] Subject: [pve-devel] [RFC v10 qemu-server 6/7] api: support VM disk import X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Jan 2022 10:08:41 -0000 From: Dominic Jäger Extend qm importdisk functionality to the API. Co-authored-by: Fabian Grünbichler Co-authored-by: Dominic Jäger Signed-off-by: Fabian Ebner --- 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 :0,import-from=\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