From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id BF4E51FF0E1 for ; Mon, 13 Jul 2026 18:00:54 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 81146213E5; Mon, 13 Jul 2026 18:00:54 +0200 (CEST) From: Lukas Sichert To: pve-devel@lists.proxmox.com Subject: [PATCH docs/manager/storage v9 0/5] fix #7339: lvmthick: add option to free storage for deleted VMs Date: Mon, 13 Jul 2026 18:00:01 +0200 Message-ID: <20260713160008.121125-1-l.sichert@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: 1783958403085 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.350 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) KAM_SHORT 0.001 Use of a URL Shortener for very short URL RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low 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: KM64Q2EYL26QBDSOWVBBQPLGKXH5OWUI X-Message-ID-Hash: KM64Q2EYL26QBDSOWVBBQPLGKXH5OWUI X-MailFrom: l.sichert@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 CC: Lukas Sichert X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Logical volumes (LV) in an LVM (thick) volume group (VG) are thick-provisioned, but the underlying backing storage can be thin-provisioned. In particular, this can be the case if the VG resides on a LUN provided by a SAN via iSCSI/FC/SAS [1], where the LUN may be thin-provisioned on the SAN side. In such setups, one usually wants that deleting an LV (e.g. VM disk) frees up space on the SAN side, especially when using snapshots-as-volume-chains, because snapshot LVs are thick-provisioned LVs from the LVM point of view, so users may want to over-provision the LUN on the SAN side. One option to free up space when deleting an LV is to set `issue_discards = 1` in the LVM config. With this setting, `lvremove` will send discards for the regions previously used by the LV, which will (if the SAN supports it) inform the SAN that the space is not in use anymore and can be freed up. Since 'lvremove' modifies LVM metadata, it has to be issued while holding a cluster-wide lock on the storage. Unfortunately, depending on the setup, 'issue_discards = 1' can make `lvremove` take very long for big disks (due to the large number of discards being issued), so that it eventually hits the 60s timeout of the cluster lock. The 60s are a hard-coded limit and cannot be easily changed [2]. A better option is to issue discard before the final `lvremove`. This informs the backing storage that the LV's blocks are no longer in use without tying the potentially long-running discard operation to the metadata update done by `lvremove`. There is already a setting for `saferemove`, which zeroes out to-be-deleted LVs before removing them. This series reworks that worker to process the LV range by range instead of zeroing the whole LV in one separate pass. This allows zero-out and discard to be combined: if both actions are enabled, the worker zeroes one range, discards that same range, and only then continues with the next range. This avoids forcing a thin-provisioned SAN to allocate the whole LV with zeroes before the space can be reclaimed again. This series adds a new `on-volume-remove` property string with an initial `discard` action. Following Fabian's feedback [4], the frontend serializes the selected option into that property string, which is then passed to the backend and parsed there. If only discard is enabled, the renamed LV is discarded before the final remove. If `saferemove` is enabled too, the worker performs the range-by-range zero-out and discard described above. Changes from v8 to v9 (thanks @Michael, Fiona): -factor failed cleanup LV renaming into a private helper -start failed LV numbering at `failed-0-del-*` -keep the existing cstream ENOSPC behavior in the intermediate patch -retry short syswrites instead of warning -use `undef` instead of a magic throughput value and make the `saferemove_throughput` parameter an integer -improve error messages for size probing, lsblk probing/parsing and blkdiscard failures -clean up lsblk invocation and discard support parsing -clean up style issues in imports, conditionals, comments and FIXME format -restore the pve-manager commit message Changes from v7 to v8 (thanks @Fiona, Friedrich and Michael): -keep renamed LVs as `failed--del-*` if zeroout or discard fails -check if the SAN supports discard before allowing the option to be set in the frontend -restructure module import order to adhere to the style guidelines -use 10 MiB/s default, if "write zeroes" is not supported -print in the frontend, if the throughput limit is applied -import the SEEK_SET constant instead of 0 for the WHENCE value -don't die, just warn if a short syswrite happens -only log first and total blkdiscard failures -add and rewrite some comments Changes from v6 to v7 (thanks @Friedrich and Michael): -rework the cleanup-worker to zero-out range by range -rework the log and frontend messages -document new 'saferemove' and 'discard' behavior -make 'discard' opt-in and not default for newly created storages -only discard bytes that were actually zeroed out before Changes from v5 to v6: -replace old storage config usage in the 'if'/'else' branches with the new API variables Changes from v4 to v5 (thanks @Friedrich and Fabian): -rework the API layout to use a property string for volume-removal options -use 'if'/'else' instead of nested '?:' expressions for task log output -revert renaming 'cleanup worker' to `discard worker` Changes from v3 to v4 (thanks @Friedrich and Thomas): -rework the worker-starting logic to avoid breaking abstraction layers -rewrite the 'imgdel' task description in the UI to better match the worker's behavior -extend the code comment to also describe discard handling -add additional syslog logging Changes from v2 to v3 (thanks @Michael): -correct issue_blkdiscard -> 'issue-blkdiscard' in the commit message for the pve-manager -replace 'previous commit' with a more obvious reference to first commit of this series in the commit message for pve-manager Changes from v1 to v2 (thanks @Michael, Maximiliano, Fabian): -add more explicit descriptions in front- and backend, specifically mentioning discard (TRIM) -add a verbose description in the backend explaining the mechanism and why it should be used for thin-provisioned storage -add a forked fallback worker execution to allow other plugins to issue workers without these config options -rename variable issue_blkdiscard -> 'issue-blkdiscard' to conform to newer style [1] https://pve.proxmox.com/wiki/Migrate_to_Proxmox_VE#Storage_boxes_(SAN/NAS) [2] https://forum.proxmox.com/threads/175849/post-820043 [3] https://man7.org/linux/man-pages/man8/blkdiscard.8.html [4] https://lore.proxmox.com/all/177885528916.1932366.10236780530533306479@yuna.proxmox.com/ Link: bugzilla.proxmox.com/show_bug.cgi?id=7339 storage: Lukas Sichert (3): lvm: saferemove: keep LVs where zero-out failed for manual zero-out lvm: saferemove: zero out volumes range by range fix #7339: lvm: add discard action for removed volumes src/PVE/Storage/LVMPlugin.pm | 425 ++++++++++++++++++++++++++++++----- 1 file changed, 366 insertions(+), 59 deletions(-) manager: Lukas Sichert (1): fix #7339: lvm: add discard-on-remove option to UI www/manager6/Utils.js | 2 +- www/manager6/storage/LVMEdit.js | 45 +++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) docs: Lukas Sichert (1): fix #7339: lvm: document discard option pve-storage-lvm.adoc | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) Summary over all repositories: 4 files changed, 439 insertions(+), 67 deletions(-) -- Generated by murpp 0.12.0