[pve-devel] [PATCH v4 qemu-server] Change format for unused drives

[Fabian Grünbichler]

On March 12, 2020 11:19 am, Fabian Ebner wrote:
> and make it match with what parse_drive does. Even though the 'real' format
> was pve-volume-id, callers already expected that parse_drive returns a hash
> with a valid 'file' key (e.g. PVE/API2/Qemu.pm:1147ff).
> 
> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
> ---
> 
> This is the last patch left over from the series
> refactoring the drive code [0], although not directly related.
> 
> [0]: https://pve.proxmox.com/pipermail/pve-devel/2020-March/042007.html
> 
> Changes from v3:
>     * make sure the format describes a hash with a 'file' key
>       as that's what callers expect
> 
>  PVE/QemuServer/Drive.pm | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
> index e927abf..d412147 100644
> --- a/PVE/QemuServer/Drive.pm
> +++ b/PVE/QemuServer/Drive.pm
> @@ -355,9 +355,20 @@ for (my $i = 0; $i < $MAX_VIRTIO_DISKS; $i++)  {
>  
>  $drivedesc_hash->{efidisk0} = $efidisk_desc;
>  
> +my $unused_fmt = {
> +    volume => { alias => 'file' },
> +    file => {
> +	type => 'string',
> +	format => 'pve-volume-id',
> +	default_key => 1,
> +	format_description => 'volume',
> +	description => "The drive's backing volume.",
> +    },
> +};
> +
>  our $unuseddesc = {
>      optional => 1,
> -    type => 'string', format => 'pve-volume-id',
> +    type => 'string', format => $unused_fmt,
>      description => "Reference to unused volumes. This is used internally, and should not be modified manually.",
>  };
>  
> @@ -418,7 +429,7 @@ sub parse_drive {
>  	return undef;
>      }
>  
> -    my $desc = $key =~ /^unused\d+$/ ? $alldrive_fmt
> +    my $desc = $key =~ /^unused\d+$/ ? $unuseddesc->{format}
>                                       : $drivedesc_hash->{$key}->{format};

couldn't we just put the unused schema into $drivedesc_hash as well?

is_valid_drivename would need to skip them[1], but we'd have all the disk 
schema in one place.

that being said - the patch as is LGTM and the above can be done as 
follow-up just as well:

Reviewed-By: Fabian Grünbichler <f.gruenbichler at proxmox.com>

1: in general? or just by default? maybe there are call sites that would 
benefit/get simpler by including unused as well..


>      if (!$desc) {
>  	warn "invalid drive key: $key\n";
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>