Re: [PATCH storage v11 4/6] fix #7339: lvm: add discard action for removed volumes

["Michael Köppl" <m.koeppl@proxmox.com>]

On Thu Jul 23, 2026 at 3:56 PM CEST, Lukas Sichert wrote:

[snip]

>              for (my $offset = 0; $offset < $size; $offset += $written) {
>  
>                  if ($offset + $stepsize > $size) {
> @@ -441,6 +452,18 @@ my sub free_lvm_volumes_locked {
>                      }
>  
>                  }
> +                if ($on_remove_opts->{discard}) {
> +                    $discard_attempts++;
> +
> +                    eval { blockdev_ioctl_range($fh, BLKDISCARD, $offset, $written); };


This uses the same offset and stepsize as for BLKZEROOUT, but $stepsize
is based on saferemove-stepsize and write_zeroes_max_bytes. The
separate setting of discard_granularity is never read. It seems that
this causes problems because AFAICT the discard action only frees whole
allocation units. So how much space is reclaimed seems to depend on
whether the step size happens to be a multiple of the granularity of
allocation units, which is never checked.

I tried this by creating 3 thin pools differing in chunk size only
using `lvcreate --chunksize=...`, with an otherwise identical config
(saferemove=1, discard=1, saferemove-stepsize=1). On each, I created a
thin LV, then allocated a VM disk on it, wrote some data to it, and
afterwards freed it using `pvesm free` and checked how much space was
reclaimed:

  granularity   reclaimed
  512K          100%
  768K          50%
  2M            0%

The same 2M pool reclaims fully with saferemove=0, where the discard is
a single whole-device /sbin/blkdiscard (AFAIU).

I'm not entirely sure if this is something that often occurs in the
wild. Perhaps someone else can chime in here, but if varying
granularities occur from time to time in production setups, it's  a
problem because it happens silently. Logs always end successfully in
"successfully removed volume", even though the space might not have
been reclaimed.

> +                    if (my $err = $@) {
> +                        if ($discard_failures == 0) {
> +                            log_warn(
> +                                "blkdiscard for $written bytes at offset $offset failed: $err");
> +                        }
> +                        $discard_failures += 1;
> +                    }

[snip]