[pve-devel] [PATCH storage 1/3] Lock storage when calling volume_import

[Fabian Grünbichler]

On January 13, 2020 12:56 pm, Fabian Ebner wrote:
> Could I get some feedback for this? The same locking is done for 
> 'vdisk_alloc' and 'vdisk_clone' already (among others), so I thought it 
> makes sense for 'volume_import' as well.

the main user of this is storage_migrate via 'pvesm import'. that means 
that replication also takes this path. we definitely don't want to hold 
a storage lock for each whole replication run, as those can take quite a 
while (initial sync).

maybe we could instead add a storage lock around the alloc operation in 
PVE::Storage::{Plugin,LVMPlugin}::volume_import ? for the ZFS case, 
allocation happens transparently for a full stream..

the cluster_lock_storage is meant to protect against races regarding 
volume IDs, for storage_migrate we normally don't even have a config yet 
so nothing in our tooling should cause a race on the importing node..

> 
> On 12/12/19 11:17 AM, Fabian Ebner wrote:
>> to avoid a potential race for two processes trying to allocate the same volume.
>> 
>> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
>> ---
>> 
>> This is conceptually independent from patches 2+3 (but patch 3 modfies the same
>> hunk as this one).
>> 
>>   PVE/Storage.pm | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
>> index bb3b874..ae2ea53 100755
>> --- a/PVE/Storage.pm
>> +++ b/PVE/Storage.pm
>> @@ -1402,8 +1402,10 @@ sub volume_import {
>>       die "cannot import into volume '$volid'\n" if !$storeid;
>>       my $scfg = storage_config($cfg, $storeid);
>>       my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
>> -    return $plugin->volume_import($scfg, $storeid, $fh, $volname, $format,
>> -                                  $base_snapshot, $with_snapshots);
>> +    return $plugin->cluster_lock_storage($storeid, $scfg->{shared}, undef, sub {
>> +	return $plugin->volume_import($scfg, $storeid, $fh, $volname, $format,
>> +				      $base_snapshot, $with_snapshots);
>> +    });
>>   }
>>   
>>   sub volume_export_formats {
>> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>