Skip to content

Commit

Permalink
storage: add --fs ignore to lvreduce on lvm > 2.03.17
Browse files Browse the repository at this point in the history
Since LVM v2.03.17 (lvmteam/lvm2@f6f2737), 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>
  • Loading branch information
BrooklynDewolf authored and sandrobonazzola committed May 27, 2024
1 parent ade627e commit 98d7f32
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 0 deletions.
2 changes: 2 additions & 0 deletions lib/vdsm/osinfo.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -281,6 +282,7 @@ def package_versions():
'qemu-kvm': 'qemu-kvm',
'spice-server': 'libspice-server1',
'vdsm': 'vdsmd',
'lvm2': 'lvm2',
}

if glusterEnabled:
Expand Down
13 changes: 13 additions & 0 deletions lib/vdsm/storage/lvm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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('')
Expand Down Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions tests/lib/osinfo_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
22 changes: 22 additions & 0 deletions tests/storage/lvm_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import uuid

import pytest
from unittest import mock

from vdsm.common import commands
from vdsm.common import concurrent
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit 98d7f32

Please sign in to comment.