Re: [pve-devel] [RFC qemu-server 4/9] api: clone_vm: add check if storage supports vm images

[Daniel Kral <d.kral@proxmox.com>]

On 11/29/24 15:23, Fiona Ebner wrote:
> Am 16.09.24 um 18:38 schrieb Daniel Kral:
>> @@ -197,6 +198,25 @@ sub check_volume_alloc : prototype($$;$) {
>>       return 1;
>>   }
>>   
>> +=head3 alloc_volume_disk($storecfg, $storeid, $vmid, $format, $name, $size_kb)
>> +
>> +Allocates a volume disk image on C<$storeid>, that is defined in C<$storecfg> (which is typically
>> +retrieved with L<PVE::Storage::config>), with the VM id C<$vmid>, the format C<$format> (e.g.
>> +C<"raw">), the name C<$name> and the image size in kilobytes C<$size_kb>.
>> +
>> +This subroutine will check whether the storage, where the volume disk image should be allocated,
>> +supports the allocation beforehand with L<check_volume_alloc>.
>> +
>> +=cut
>> +
>> +sub alloc_volume_disk : prototype($$$$$$) {
> 
> I think the volume+disk is redundant. Maybe simply allocate_image or
> allocate_volume?

ACK

> 
> Thinking about this, are there any cases where we do not want to have
> the checks done first? I.e. can we simply add the checks as part of
> vdisk_alloc itself (would require passing along the content type for the
> checks but would avoid the need for this helper)?

AFAICS from a quick grep over all usages of vdisk_alloc (in 
pve-container and qemu-server), we could move them there. I'll do that 
if it doesn't break any existing API from the user-side.

> 
>> +    my ($storecfg, $storeid, $vmid, $format, $name, $size_kb) = @_;
>> +
>> +    check_volume_alloc($storecfg, $storeid);
>> +
>> +    return PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, $format, $name, $size_kb);
>> +}
>> +
>>   sub min_version {
>>       my ($verstr, $major, $minor, $pve) = @_;
>>   
> 



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel