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 18F8D1FF0E5 for ; Wed, 29 Jul 2026 17:12:26 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 8616920818; Wed, 29 Jul 2026 17:12:25 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Wed, 29 Jul 2026 17:12:00 +0200 Message-Id: From: =?utf-8?q?Michael_K=C3=B6ppl?= To: "Lukas Sichert" , Subject: Re: [PATCH storage v11 4/6] fix #7339: lvm: add discard action for removed volumes X-Mailer: aerc 0.21.0 References: <20260723135656.102972-1-l.sichert@proxmox.com> <20260723135656.102972-5-l.sichert@proxmox.com> In-Reply-To: <20260723135656.102972-5-l.sichert@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1785337913486 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.317 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_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: R7ARTNJ4G2IELLRF4XB5QNXR4FHEPXT6 X-Message-ID-Hash: R7ARTNJ4G2IELLRF4XB5QNXR4FHEPXT6 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: On Thu Jul 23, 2026 at 3:56 PM CEST, Lukas Sichert wrote: [snip] > for (my $offset =3D 0; $offset < $size; $offset +=3D $writte= n) { > =20 > if ($offset + $stepsize > $size) { > @@ -441,6 +452,18 @@ my sub free_lvm_volumes_locked { > } > =20 > } > + 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=3D...`, with an otherwise identical config (saferemove=3D1, discard=3D1, saferemove-stepsize=3D1). 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=3D0, 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 =3D $@) { > + if ($discard_failures =3D=3D 0) { > + log_warn( > + "blkdiscard for $written bytes at offset= $offset failed: $err"); > + } > + $discard_failures +=3D 1; > + } [snip]