all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Thomas Ellmenreich" <t.ellmenreich@proxmox.com>
To: "Lukas Sichert" <l.sichert@proxmox.com>, <pve-devel@lists.proxmox.com>
Subject: Re: [PATCH docs/manager/storage v12 0/6] fix #7339: lvmthick: add option to free storage for deleted VMs
Date: Wed, 09 Sep 2026 10:27:22 +0200	[thread overview]
Message-ID: <DLAN64TIJJ8E.31HNMO0GHB8QZ@proxmox.com> (raw)
In-Reply-To: <20260811150534.137170-1-l.sichert@proxmox.com>

I have performed a considerable number of tests for this series, both with and
without the patches, and my conclusion is that they work well. Especially the
last test, which I have described below, confirmed this for me.

I mounted a 10GB disk to a nested PVE instance, on which I plan to run the
tests. The new disk then contains a VG with a thin LVM pool, which in turn
contains a separate VG. This VG is then added as a Storage to the nested PVE
instance so that a VM can be created on it. Important was to configure the
storage as `shared=1`, as well as applying some throttling to the 10GB Disk
from the "root" PVE instance.

"root" PVE instance -> nested PVE -> VG -> thin pool -> LV -> VG
        |                 ^                                   ^
        +---10 GB Disk----+                          added as Storage
                                                     to the nested PVE

Testing the Discards
--------------------

Without patches:
    Here I set `issue_discards=1` on the `lvm.conf` config and applied the
    throttling which led to a timeout:

    > root@node2:~# time qm destroy 102
    > WARN: Could not remove disk 'sanc:vm-102-disk-0', check manually: lvremove 'san_c/vm-102-disk-0' error: 'storage-sanc'-locked command timed out - aborting
    > Task finished with 1 warning(s)!
    >
    > real    1m0.834s
    > user    0m0.723s
    > sys     0m0.098s

With patches:
    Instead of setting `issue_discards` I set the new `on-volume-remove discard=1`
    property on the storage and also applied the same throttling. Compared to
    the previous example this one took longer but did not time out:

    > root@node2:~# time qm destroy 102
    >   Renamed "vm-102-disk-0" to "del-vm-102-disk-0" in volume group "san_c"
    > discard (TRIM) data on image vm-102-disk-0 (/dev/san_c/del-vm-102-disk-0)
    > blkdiscard: /dev/san_c/del-vm-102-disk-0 contains existing partition (dos).
    >   Logical volume "del-vm-102-disk-0" successfully removed.
    > successfully removed volume vm-102-disk-0 (san_c/del-vm-102-disk-0)
    >
    > real    2m24.354s
    > user    0m0.781s
    > sys     0m0.205s

Since the discarding continues even when the timeout is hit, in both cases all
data is successfully discarded. The version without the patches takes longer
but does so without hitting a timeout.

LV                           Data%
                      No Patches     Patches
issue_discards      =0        =1          =0
--------------+-------+---------+-----------
lun_c         |  87.52|     0.02|       0.02
pool_c        |  77.80|     0.02|       0.02

Testing the Zeroing
-------------------

For this I initially used the same setup as the one mentioned above which did
yield good results although it did use the `syswrite` variant for zeroing which
is very unperformant. Zeroing of a 7GB disk with this method took about 12
minutes. (For the zeroing, I'm only showing the output with patches, since
the comparison is not that interesting).

    > root@node2:~# time qm destroy 102
    >   Renamed "vm-102-disk-0" to "del-vm-102-disk-0" in volume group "san_c"
    > zero-out and discard (TRIM) data on image vm-102-disk-0 (/dev/san_c/del-vm-102-disk-0)
    > WRITE_ZEROES operation not supported, falling back to syswrite to zero-out '/dev/san_c/del-vm-102-disk-0'
    > reduce stepsize to 1 MiB for syswrite
    > using default syswrite-saferemove throughput limit: 10 MiB/s
    > zeroed out 1.00 MiB of 7.00 GiB (0.01%) using syswrite in 0s

    ... 233 more lines like these

    > zeroed out 6.99 GiB of 7.00 GiB (99.90%) using syswrite in 11m 56s
    >   Logical volume "del-vm-102-disk-0" successfully removed.
    > successfully removed volume vm-102-disk-0 (san_c/del-vm-102-disk-0)
    >
    > real    11m57.860s
    > user    0m1.117s
    > sys     0m8.758s

To make sure `blkdiscard` was used I simplified the storage setup, removing the
inbetween thin pool. By doing so `write_zeroes_max_bytes` was correctly passed
through and elapsed time went down to about 1 second.

    > root@node2:~# time qm destroy 102
    >   renamed "vm-102-disk-0" to "del-vm-102-disk-0" in volume group "arrays"
    > discard (trim) data on image vm-102-disk-0 (/dev/arrays/del-vm-102-disk-0)
    > blkdiscard: /dev/arrays/del-vm-102-disk-0 contains existing partition (dos).
    >   logical volume "del-vm-102-disk-0" successfully removed.
    > successfully removed volume vm-102-disk-0 (arrays/del-vm-102-disk-0)
    >
    > real    0m1.057s
    > user    0m0.705s
    > sys     0m0.161s

In both cases a quick reallocation: `pvesm alloc sanc 102 vm-102-disk-0 7G` and
a simple hexdump show all data zeroed out:

    > root@node2:~# hexdump /dev/arrays/vm-102-disk-0
    > 0000000 0000 0000 0000 0000 0000 0000 0000 0000
    > *
    > 1c0000000

Additonal Notes
---------------

* For all tests I have performed I have kept the output logs as well as details
  about the exact configuration used, so if there are any questions about the
  details of my setup I'm happy to answer them.

* Although here I have only showed the conclusions, I did establish baselines to
  make sure my methods and my understanding of the system were correct.

* And finally, I did also cover all combinations of the configurations mentioned
  above. This means with and without the following: patches, zeroing, discarding
  discarding for lvm, throttling. For all of these the journal was checked,
  ensuring that no errors were overseen.

What was not Tested
-------------------

I did not perform any tests in regards to Snapshots as Volume-Chains, as well
as the two other settings added by the series: `saferemove-stepsize` and
`saferemove_throughput`

Conclusion
----------

The series does what it claims, without producing any errors or weird
behaviour, and also leads to considerable speedups, so:

Tested-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>

On Tue Aug 11, 2026 at 5:05 PM CEST, Lukas Sichert wrote:
> 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 ranges until reaching a boundary
> aligned with both the zero-out step size and the discard granularity,
> then discards the accumulated range before continuing. This ensures that
> complete discard chunks are not skipped. It also 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.

[snip]

> [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/
> Buglink: https://bugzilla.proxmox.com/show_bug.cgi?id=7339

[snip]




      parent reply	other threads:[~2026-09-09  8:27 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-11 15:05 [PATCH docs/manager/storage v12 0/6] fix #7339: lvmthick: add option to free storage for deleted VMs Lukas Sichert
2026-08-11 15:05 ` [PATCH storage v12 1/6] lvm: saferemove: keep LVs where zero-out failed for manual zero-out Lukas Sichert
2026-08-11 15:05 ` [PATCH storage v12 2/6] lvm: saferemove: zero out volumes range by range Lukas Sichert
2026-08-11 15:05 ` [PATCH storage v12 3/6] lvm: saferemove: make throughput an integer property Lukas Sichert
2026-08-11 15:05 ` [PATCH storage v12 4/6] fix #7339: lvm: add discard action for removed volumes Lukas Sichert
2026-08-11 15:05 ` [PATCH manager v12 5/6] fix #7339: lvm: add discard-on-remove option to UI Lukas Sichert
2026-08-11 15:05 ` [PATCH docs v12 6/6] fix #7339: lvm: document discard option Lukas Sichert
2026-09-09  8:27 ` Thomas Ellmenreich [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DLAN64TIJJ8E.31HNMO0GHB8QZ@proxmox.com \
    --to=t.ellmenreich@proxmox.com \
    --cc=l.sichert@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal