[PATCH storage] fix #6915: lvmthin: leave room for repair

[PATCH storage] fix #6915: lvmthin: leave room for repair

[Erik Fastermann]

The pool is sized to fill the volume group, leaving a flat 128M free
regardless of device size. That is not enough to repair damaged pool
metadata. As described in lvmthin(7), `lvconvert --repair` writes the
recovered metadata into the VGs pmspare LV, promotes it to be the pool
metadata LV and keeps the damaged one as a visible ThinPool_metaN
backup, so a replacement spare has to be allocated afterwards. Without
free extents the repair is not possible.

Subtract the metadata size a third time, so one metadata LV worth of
extents stays unallocated.

The reserve is deliberately limited to what a repair needs, as the
utility for a pool that ran full is questionable:

- a thin pool cannot be shrunk, so this is a one time reserve and does
  not help if the pool runs full again
- extending does not undo the damage, guest file systems that took I/O
  errors need an fsck and possibly a restore from backup
- pool metadata is limited to 16G, so on very large pools it cannot be
  extended at all and free extents do not help there
- moving disks to another storage temporarily or deleting snapshots
  is often the preferred solution if metadata is not exhausted

Creating a partition for the physical volume, as suggested in the
report, is left out. The installer partitions because the disk has to be
bootable, not for the sake of LVM, and a data disk added later has no
such requirement. All LVM based storages PVE creates for data use the
whole device, Ceph OSDs included, so this is the consistent behavior.
The standard tooling is not confused by it either and existing pools
keep their layout in any case.

It should be noted that metadata exhaustion, which was one of the
original motivations for this change, is hard to reach with the current
defaults. The installations that hit this in practice are older ones,
created before metadata was sized at 1% of the pool, and those pools are
unaffected by this change.

Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
---
 src/PVE/API2/Disks/LVMThin.pm | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/PVE/API2/Disks/LVMThin.pm b/src/PVE/API2/Disks/LVMThin.pm
index a0ce497..2b14ca1 100644
--- a/src/PVE/API2/Disks/LVMThin.pm
+++ b/src/PVE/API2/Disks/LVMThin.pm
@@ -153,8 +153,11 @@ __PACKAGE__->register_method({
                 $metadatasize = 1024 * 1024 if $metadatasize < 1024 * 1024;
                 # but at most 16G, which is the current lvm max
                 $metadatasize = 16 * 1024 * 1024 if $metadatasize > 16 * 1024 * 1024;
-                # shrink data by needed amount for metadata
-                $datasize -= 2 * $metadatasize;
+                # shrink data by twice the metadata size for the pool metadata LV
+                # and pmspare, and once more to leave the volume group room for
+                # `lvconvert --repair`
+                $datasize -= 3 * $metadatasize;
+                die "device too small for a thin pool\n" if $datasize <= 0;
 
                 run_command([
                     '/sbin/lvcreate',
-- 
2.47.3