From 98d7f32202ed6e72bd0cc1acf041f63d88c550ef Mon Sep 17 00:00:00 2001
From: Brooklyn Dewolf <contact@brooklyn.gent>
Date: Thu, 23 May 2024 09:56:11 +0200
Subject: [PATCH] storage: add --fs ignore to lvreduce on lvm > 2.03.17

Since LVM v2.03.17 (https://github.com/lvmteam/lvm2/commit/f6f2737015746b1b6c7fbd0d297a4596c584749b), reducing a logical volume (LV) requires the LV to be active due to the default 'checksize' option, which requires an active LV. This results in the following error when attempting to reduce a non-active LV:

----
err=[\'  The LV must be active to safely reduce (see --fs options.)\']'
----

To resolve this, we now check if the LVM version is newer than 2.03.17. If so, we bypass the 'checksize' by using the '--fs ignore' option. This approach is viable because the oVirt already handles checksize, making lvreduce redundant.

Signed-off-by: Brooklyn Dewolf <contact@brooklyn.gent>
---
 lib/vdsm/osinfo.py        |  2 ++
 lib/vdsm/storage/lvm.py   | 13 +++++++++++++
 tests/lib/osinfo_test.py  |  1 +
 tests/storage/lvm_test.py | 22 ++++++++++++++++++++++
 4 files changed, 38 insertions(+)

diff --git a/lib/vdsm/osinfo.py b/lib/vdsm/osinfo.py
index be404f575e..fb158fb312 100644
--- a/lib/vdsm/osinfo.py
+++ b/lib/vdsm/osinfo.py
@@ -248,6 +248,7 @@ def package_versions():
             'qemu-kvm': ('qemu-kvm', 'qemu-kvm-rhev', 'qemu-kvm-ev'),
             'spice-server': ('spice-server',),
             'vdsm': ('vdsm',),
+            'lvm2': ('lvm2', 'lvm2-libs'),
         }
 
         if glusterEnabled:
@@ -281,6 +282,7 @@ def package_versions():
             'qemu-kvm': 'qemu-kvm',
             'spice-server': 'libspice-server1',
             'vdsm': 'vdsmd',
+            'lvm2': 'lvm2',
         }
 
         if glusterEnabled:
diff --git a/lib/vdsm/storage/lvm.py b/lib/vdsm/storage/lvm.py
index 38d212c58d..53e1000222 100644
--- a/lib/vdsm/storage/lvm.py
+++ b/lib/vdsm/storage/lvm.py
@@ -22,6 +22,7 @@
 from itertools import chain
 
 from vdsm import constants
+from vdsm import osinfo
 from vdsm import utils
 from vdsm.common import commands
 from vdsm.common import errors
@@ -240,6 +241,15 @@ def __getattr__(self, attrName):
 USE_DEVICES = config.get("lvm", "config_method").lower() == "devices"
 
 
+def _get_lvm_version():
+    packages = osinfo.package_versions()
+    lvm_version = tuple(
+        int(v)
+        for v in packages['lvm2']['version'].split('.')
+    )
+    return lvm_version
+
+
 def _prepare_device_set(devs):
     devices = set(d.strip() for d in chain(devs, USER_DEV_LIST))
     devices.discard('')
@@ -1781,11 +1791,14 @@ def extendLV(vgName, lvName, size_mb, refresh=True):
 
 
 def reduceLV(vgName, lvName, size_mb, force=False):
+    lvm_version = _get_lvm_version()
     log.info("Reducing LV %s/%s to %s megabytes (force=%s)",
              vgName, lvName, size_mb, force)
     cmd = ("lvreduce",) + LVM_NOBACKUP
     if force:
         cmd += ("--force",)
+    if lvm_version >= (2, 3, 17):
+        cmd += ("--fs", "ignore")
     cmd += ("--size", "%sm" % (size_mb,), "%s/%s" % (vgName, lvName))
 
     try:
diff --git a/tests/lib/osinfo_test.py b/tests/lib/osinfo_test.py
index 583ad29fdf..98751dd87b 100644
--- a/tests/lib/osinfo_test.py
+++ b/tests/lib/osinfo_test.py
@@ -36,6 +36,7 @@ def test_kernel_args(test_input, expected_result):
 def test_package_versions():
     pkgs = osinfo.package_versions()
     assert 'kernel' in pkgs
+    assert 'lvm2' in pkgs
 
 
 def test_get_boot_uuid(fake_findmnt):
diff --git a/tests/storage/lvm_test.py b/tests/storage/lvm_test.py
index fb25ab00f5..d580d190d0 100644
--- a/tests/storage/lvm_test.py
+++ b/tests/storage/lvm_test.py
@@ -9,6 +9,7 @@
 import uuid
 
 import pytest
+from unittest import mock
 
 from vdsm.common import commands
 from vdsm.common import concurrent
@@ -1776,6 +1777,27 @@ def test_lv_extend_reduce(tmp_storage):
     assert int(lv.size) == 1 * GiB
 
 
+@pytest.mark.parametrize("lvm_version, expected_cmd", [
+    ((2, 3, 16), ("lvreduce", "--autobackup", "n", "--size",
+                  "100m", "vg/lv")),
+    ((2, 3, 17), ("lvreduce", "--autobackup", "n", "--fs", "ignore", "--size",
+                  "100m", "vg/lv"))
+])
+def test_reducelv_with_different_lvm_versions(lvm_version, expected_cmd):
+    with mock.patch.object(lvm, "_get_lvm_version",
+                           return_value=lvm_version), \
+            mock.patch.object(lvm._lvminfo,
+                              "run_command") as mock_run_command, \
+            mock.patch.object(lvm._lvminfo, "_invalidatevgs"), \
+            mock.patch.object(lvm._lvminfo, "_invalidatelvs"):
+
+        lvm.reduceLV("vg", "lv", 100)
+
+        mock_run_command.assert_called_once_with(
+            expected_cmd, devices=mock.ANY
+        )
+
+
 @requires_root
 @pytest.mark.root
 def test_lv_extend_with_refresh(tmp_storage):