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 7A9941FF13E for ; Wed, 01 Jul 2026 16:13:57 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id ECC5B21470; Wed, 01 Jul 2026 16:13:45 +0200 (CEST) From: =?UTF-8?q?Michael=20K=C3=B6ppl?= To: pve-devel@lists.proxmox.com Subject: [PATCH container/qemu-server v10 00/10] fix #3711 and adapt drive detach/remove behavior Date: Wed, 1 Jul 2026 16:13:29 +0200 Message-ID: <20260701141339.181276-1-m.koeppl@proxmox.com> X-Mailer: git-send-email 2.47.3 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: 1782915215324 X-SPAM-LEVEL: Spam detection results: 0 DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) 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: VA3KQRYEVGOU2VRILJ6RBHPW733YVKKM X-Message-ID-Hash: VA3KQRYEVGOU2VRILJ6RBHPW733YVKKM 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: This series aims to fix #3711 [0] and streamline the detach/remove behavior around volumes that are either mounted into a container or attached to a VM as a hard disk. It also adds warnings in case a volume's underlying storage does not exist anymore. It is a continuation of a series from 2022 [1], but makes the following changes: Changes v9 -> v10 (thanks @Fiona for the feedback!): - Remove pve-storage patch since it was not required for the rest of the series to work - Add patch for pve-container that adds a helper function for skip-and-warn if a volume's underlying storage doesn't exist. The helper is used in the following patch to avoid repeating the same if-else in 4 call sites - Adapt mountpoint-related patch for pve-container to use the with_checked_volid helper - Add missing PVE::RESTEnvironment import - Fix multi-line post-if style nit (although without directly importing log_warn() to keep it consistent with the remaining codebase) - Add comments explaining why printing a warning and continuing is fine for the linked clone checks in pve-container and qemu-server Changes v8 -> v9: - Drop the parse_volname() warning patch in pve-storage. The removed storage case should now be handled by the new pending-change checks and vdisk_free - Pass the noerr flag to storage_config() in vdisk_free() so the warning branch is actually reached - Add patch to allow pending mount point changes for LXC if the storage is gone, matching VM behavior. Thanks for pointing that out in v8, @Lukas! Changes v7 -> v8: - Rebased on master Changes v6 -> v7: - Added descriptions of changes made to original patches where applicable - Move check if storage exists to vdisk_free() and parse_volname() in pve-storage, print warnings - Remove explicit storage exist check in destroy_lxc_container, wrap in eval instead - Move parse_volume() calls to where input is known to be volume - Add patch that renames $volume to $volid in destroy_lxc_container() and delete_mountpoint_volume() - Rewrite commit message of pve-container 3/4 (2/4 in v6) such that it's clear this is about future-proofing - Add FIXME note regarding is_volume_in_use() check - Add warning to destroy_vm() if volume is still used by linked clone - Adapt commit message of qemu-server 2/4 - Add patch to use log_warn in destroy_vm() instead of warn - Rewrite patch in in qemu-server to print warnings of storage does not exist instead of marking volume without underlying storage as unused (vmconfig_register_unused_drive()) Thanks to Fiona for the comprehensive feedback on v6. Changes v5 -> v6: - Fix links in cover letter - Use Originally-by instead of Co-authored-by - Add documentation for the second patch regarding the use of $mp->{volume} instead of $conf->{$opt} - Note that no functional changes are intended for the the second patch Changes v4 -> v5: - Always ignore errors that originate from a removed storage and continue with destruction of a container or removal of a volume, instead of adding an option to ignore these errors. - Remove web UI checkbox - Remove formatting patch - Additionally allow removing a mount point with a removed storage from a running container (previously hotplug removal was not possible) - Fix style nits from v4 review - Print warnings for any errors that occur instead of ignoring them - Add explicit check if storage still exists when destroying a container to differentiate between that case and other error cases (which should still fail) Where at least some of the implementation was taken from the previous series, the patch was marked to be Originally-by by the original author. [0] https://bugzilla.proxmox.com/show_bug.cgi?id=3711 [1] https://lore.proxmox.com/pve-devel/20221125144008.2988072-1-s.hrdlicka@proxmox.com/ pve-container: Michael Köppl (6): fix #3711: warn about storage errors during mountpoint delete destroy_lxc, delete_mp_volume: rename $volume to $volid config: ensure valid volid through parse_volume() lxc config: add helper for parsing volid and checking if storage exists allow pending mount point changes if storage is gone destroy template: add early check for linked clones of volumes src/PVE/LXC.pm | 41 +++++++++++++++++++++++------- src/PVE/LXC/Config.pm | 58 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 82 insertions(+), 17 deletions(-) qemu-server: Michael Köppl (4): adapt linked clone check to not die if an error occurs during check fix #3711: make removal of VM possible if store does not exist anymore destroy_vm: use log_warn for vdisk_free errors for consistency display warnings for storage errors or if storage no longer exists src/PVE/QemuServer.pm | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) Summary over all repositories: 3 files changed, 99 insertions(+), 24 deletions(-) -- Generated by murpp 0.11.0