On 10/9/19 8:47 AM, Fabian Ebner wrote: > On 10/8/19 11:21 AM, Thomas Lamprecht wrote: >> On 10/8/19 10:48 AM, Fabian Ebner wrote: >>> Previously 'free_image' would be executed right away, which is not >>> the intended behaviour. >>> >>> Signed-off-by: Fabian Ebner <f.eb...@proxmox.com> >>> --- >>> >>> This is a followup to [0] but it has nothing to do with the original patch >>> so I didn't put a v2. >>> >>> [0]: https://pve.proxmox.com/pipermail/pve-devel/2019-October/039281.html >>> >>> PVE/Storage.pm | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/PVE/Storage.pm b/PVE/Storage.pm >>> index 298976f..64b79c9 100755 >>> --- a/PVE/Storage.pm >>> +++ b/PVE/Storage.pm >>> @@ -760,7 +760,9 @@ sub vdisk_free { >>> my (undef, undef, undef, undef, undef, $isBase, $format) = >>> $plugin->parse_volname($volname); >>> - $cleanup_worker = $plugin->free_image($storeid, $scfg, $volname, >>> $isBase, $format); >>> + $cleanup_worker = sub { >>> + $plugin->free_image($storeid, $scfg, $volname, $isBase, $format); >>> + }; >>> }); >>> return if !$cleanup_worker; >> but now above never evaluates to false? >> And makes the lock then sense at all? I mean it's used for the >> $volume_is_base_and_used__no_lock check only.. >> >> This code is like this since the beginning of the storage plugin system >> commit 1dc01b9f30f4cb201f68a344ff43539623164945 from 2012, and there's >> no explicit info about what was inteded, could also be that the >> $cleanup_worker >> was left over by mistake instead.. >> >> I mean deletion /could/ work without locking on some/most storages.. >> > As Fabian pointed out to me offline, my approach is wrong. I didn't > interpret the code here correctly. > For example with the LVMPlugin free_image can return a subroutine > to be spawned as a worker, which is why the code here is as it is. > And we don't always want to spawn a worker (which also results in > a task being created) when we do vdisk_free. > >
Yeah, actually only the LVM(thick) plugin uses this mechanism as opt-in _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
