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