Re: [pve-devel] [RFC v10 qemu-server 6/7] api: support VM disk import

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On February 22, 2022 1:11 pm, Fabian Ebner wrote:
> Am 13.01.22 um 11:08 schrieb Fabian Ebner:
>> @@ -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);
>> +	}
>>      });
>>  
> 
> AFAICT, if $vmid doesn't match the one from the volume, the check
> requires Datastore.Allocate privileges on the storage, which might be a
> bit much for many scenarios. Should the check rather be something like
> 
> if ($ownerid) {
>     # check VM.Clone for owner VM
>     # Note that v11 will use clone_disk() for such disks

sounds sensible to me - if it's possible to copy the volume (and other 
stuff) already with that privilege, then re-using that check for the 
similar functionality here is fine.

check_volume_access is like that because it assumes that the caller 
already checked that changes/access to the guest itself is okay - so if 
a 'foreign' volume is queried, it requires more permissions since it 
assumes that assumption doesn't hold.

since most queries here will be for 'foreign' volumes, checking that 
cloning from that owner is okay and then passing in that vmid instead of 
the target one would probably be best (as that still handles the 
question of whether accessing the source storage is okay in general), 
and of course we'd need that fallback anyway in case there is no 
owner/we don't have clone permissions?

> } else {
>     # PVE::Storage::check_volume_access
> }
> 
> ?
>