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

[Fabian Ebner]

On 16.03.20 10:10, Fabian Grünbichler wrote:
> 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.
> 

Ok, I'll check the call sites and do a follow-up for this.

> 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
>>
>>
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>