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