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 8219F6AE16 for ; Thu, 17 Mar 2022 12:31:28 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id DE0122EBC for ; Thu, 17 Mar 2022 12:31:27 +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 69EB02CCD for ; Thu, 17 Mar 2022 12:31:16 +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 0D91C46EAB for ; Thu, 17 Mar 2022 12:31:16 +0100 (CET) From: Fabian Ebner To: pve-devel@lists.proxmox.com Date: Thu, 17 Mar 2022 12:31:04 +0100 Message-Id: <20220317113107.60466-7-f.ebner@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220317113107.60466-1-f.ebner@proxmox.com> References: <20220317113107.60466-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.120 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 T_SCC_BODY_TEXT_LINE -0.01 - 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, qemuserver.pm, qemu.pm] Subject: [pve-devel] [PATCH v13 qemu-server 6/8] schema: drive: use separate schema when disk allocation is possible 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, 17 Mar 2022 11:31:28 -0000 via the special syntax :. 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 --- Changes from v12: * Include adaptation of {parse,print}_drive and callers already in this patch rather than the next. PVE/API2/Qemu.pm | 41 +++++++++++++++------ PVE/QemuServer.pm | 9 +++-- PVE/QemuServer/Drive.pm | 79 +++++++++++++++++++++++++++++------------ 3 files changed, 94 insertions(+), 35 deletions(-) diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm index 15592d7a..1082d5e3 100644 --- a/PVE/API2/Qemu.pm +++ b/PVE/API2/Qemu.pm @@ -63,28 +63,43 @@ 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; 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); @@ -242,7 +257,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 = $@) { @@ -569,7 +584,9 @@ __PACKAGE__->register_method({ default => 0, description => "Start VM after it was created successfully.", }, - }), + }, + 1, # with_disk_alloc + ), }, returns => { type => 'string', @@ -1283,7 +1300,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']); @@ -1389,7 +1406,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); @@ -1552,7 +1569,9 @@ __PACKAGE__->register_method({ maximum => 30, optional => 1, }, - }), + }, + 1, # with_disk_alloc + ), }, returns => { type => 'string', @@ -1600,7 +1619,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 3bdc55a6..7b335cc1 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 7b82fb22..cebf1730 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; @@ -417,6 +409,10 @@ my $alldrive_fmt = { %efitype_fmt, }; +my $alldrive_fmt_with_alloc = { + %$alldrive_fmt, +}; + my $unused_fmt = { volume => { alias => 'file' }, file => { @@ -434,27 +430,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 { @@ -521,7 +551,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); @@ -532,12 +562,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; @@ -597,9 +629,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 { -- 2.30.2