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