From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id BA6AB1FF0DF for ; Thu, 05 Nov 2026 17:36:54 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 31A7121628; Thu, 05 Nov 2026 17:36:44 +0100 (CET) From: =?UTF-8?q?Michael=20K=C3=B6ppl?= To: pve-devel@lists.proxmox.com Subject: [PATCH storage 2/3] fix #8286: vdisk free: do not hold inherited locks in cleanup worker Date: Thu, 5 Nov 2026 17:36:36 +0100 Message-ID: <20261105163637.1318072-3-m.koeppl@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261105163637.1318072-1-m.koeppl@proxmox.com> References: <20261105163637.1318072-1-m.koeppl@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793896598557 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.212 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: ONU6HEYI377KJRMJIRDKGVSGYME2N7EI X-Message-ID-Hash: ONU6HEYI377KJRMJIRDKGVSGYME2N7EI X-MailFrom: m.koeppl@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: vdisk_free() forks the cleanup worker of a volume (e.g. for wiping LVs on LVM storages with saferemove), while the caller often still holds the guest config lock. The worker inherits the locked handle and keeps the lock held until the cleanup is done. For example, after destroying a VM through the API, re-creating a VM with the same VMID fails with a lock timeout until the wipe has finished. The cleanup workers in the LVM plugin only operate on volumes that are not referenced anymore. The plugin's implementation takes the lock on its own where needed, so close the inherited lock handles when the worker starts. This applies to the cleanup worker of any plugin, which must not rely on the cleanup worker holding the caller's lock. Signed-off-by: Michael Köppl --- Notes: I'm not quite sure about this one. I understand that it breaks this behavior for existing external plugins, but I'm not sure it's something they should rely on in the first place. An alternative to this would be to put the cleanup call into the LVM plugin's cleanup worker specifically, which would also require every plugin to implement this in its cleanup worker. Could perhaps also be a sensible approach for now to have the call in the LVM plugin specifically and move it to vdisk_free for PVE 10. I mostly decided on this version for now because of Fabian's comment on the Bugzilla ticket. src/PVE/Storage.pm | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm index d660cfc4..4cec7c31 100755 --- a/src/PVE/Storage.pm +++ b/src/PVE/Storage.pm @@ -1222,7 +1222,15 @@ sub vdisk_free { my $rpcenv = PVE::RPCEnvironment::get(); my $authuser = $rpcenv->get_user(); - $rpcenv->fork_worker('imgdel', undef, $authuser, $cleanup_worker); + $rpcenv->fork_worker( + 'imgdel', + undef, + $authuser, + sub { + PVE::Tools::close_inherited_lock_handles(); + $cleanup_worker->(@_); + }, + ); } sub vdisk_list { -- 2.47.3