From: Guillaume Abrioux Date: Thu, 19 Mar 2026 11:30:42 +0000 (+0100) Subject: ceph-volume: fix inventory without /dev/vg/lv (slashed paths) X-Git-Tag: v21.0.1~693^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=0e96c625f69179126f3472761d276f2c36aa7b29;p=ceph.git ceph-volume: fix inventory without /dev/vg/lv (slashed paths) Ths makes ceph-volume use UdevData.preferred_block_path() in get_devices() so it keeps /dev/vg/lv (slashed path form) when it exists, else /dev/mapper/ (dashed path form). This is needed for thin-pool LVs and environments where udev does not create slashed paths. Fixes: https://tracker.ceph.com/issues/75615 Signed-off-by: Guillaume Abrioux --- diff --git a/src/ceph-volume/ceph_volume/tests/util/test_disk.py b/src/ceph-volume/ceph_volume/tests/util/test_disk.py index 68eb944a34c..7164088a367 100644 --- a/src/ceph-volume/ceph_volume/tests/util/test_disk.py +++ b/src/ceph-volume/ceph_volume/tests/util/test_disk.py @@ -1,5 +1,7 @@ import pytest import stat +from typing import Any, Callable, ClassVar, Optional + from ceph_volume.util import disk from unittest.mock import patch, Mock, MagicMock, mock_open from pyfakefs.fake_filesystem_unittest import TestCase @@ -358,7 +360,7 @@ class TestGetDevices(object): fake_filesystem.create_file('/sys/block/dm-0/queue/hw_sector_size', contents='512') with patch("ceph_volume.util.disk.UdevData") as MockUdevData: mock_instance = MagicMock() - mock_instance.slashed_path = lv_path + mock_instance.preferred_block_path = lv_path mock_instance.environment = {} MockUdevData.return_value = mock_instance result = disk.get_devices() @@ -676,7 +678,29 @@ class TestBlockSysFs(TestCase): assert b.active_mappers()['dm-1']['uuid'] == 'abcdef' +class _StatWithStRdev: + __slots__ = ('_st', 'st_rdev') + + def __init__(self, st: Any, rdev: int = 0) -> None: + object.__setattr__(self, '_st', st) + object.__setattr__(self, 'st_rdev', rdev) + + def __getattr__(self, name: str) -> Any: + return getattr(self._st, name) + + +def _udev_data_patched_os_stat(path: str, *args: Any, **kwargs: Any) -> Any: + if TestUdevData._pyfakefs_os_stat is None: + raise RuntimeError('TestUdevData.setUp must assign _pyfakefs_os_stat') + st = TestUdevData._pyfakefs_os_stat(path, *args, **kwargs) + if not hasattr(st, 'st_rdev'): + return _StatWithStRdev(st, 0) + return st + + class TestUdevData(TestCase): + _pyfakefs_os_stat: ClassVar[Optional[Callable[..., Any]]] = None + def setUp(self) -> None: udev_data_lv_device: str = """ S:disk/by-id/dm-uuid-LVM-1f1RaxWlzQ61Sbc7oCIHRMdh0M8zRTSnU03ekuStqWuiA6eEDmwoGg3cWfFtE2li @@ -719,13 +743,20 @@ V:1""" self.fs.create_file('/run/udev/data/b999:0', create_missing_dirs=True, contents=udev_data_bare_device) self.fs.create_file('/run/udev/data/b998:1', create_missing_dirs=True, contents=udev_data_lv_device) self.fs.create_file('/run/udev/data/b997:2', create_missing_dirs=True, contents="") + TestUdevData._pyfakefs_os_stat = disk.os.stat + + def tearDown(self) -> None: + try: + super().tearDown() + finally: + TestUdevData._pyfakefs_os_stat = None def test_device_not_found(self) -> None: self.fs.remove(self.fake_device) with pytest.raises(RuntimeError): disk.UdevData(self.fake_device) - @patch('ceph_volume.util.disk.os.stat', MagicMock()) + @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat) @patch('ceph_volume.util.disk.os.minor', Mock(return_value=0)) @patch('ceph_volume.util.disk.os.major', Mock(return_value=999)) def test_no_data(self) -> None: @@ -733,56 +764,110 @@ V:1""" with pytest.raises(RuntimeError): disk.UdevData(self.fake_device) - @patch('ceph_volume.util.disk.os.stat', MagicMock()) + @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat) @patch('ceph_volume.util.disk.os.minor', Mock(return_value=2)) @patch('ceph_volume.util.disk.os.major', Mock(return_value=997)) def test_empty_data(self) -> None: # no exception should be raised when a /run/udev/data/* file is empty _ = disk.UdevData(self.fake_device) - @patch('ceph_volume.util.disk.os.stat', MagicMock()) + @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat) @patch('ceph_volume.util.disk.os.minor', Mock(return_value=0)) @patch('ceph_volume.util.disk.os.major', Mock(return_value=999)) def test_is_dm_false(self) -> None: assert not disk.UdevData(self.fake_device).is_dm - @patch('ceph_volume.util.disk.os.stat', MagicMock()) + @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat) @patch('ceph_volume.util.disk.os.minor', Mock(return_value=1)) @patch('ceph_volume.util.disk.os.major', Mock(return_value=998)) def test_is_dm_true(self) -> None: assert disk.UdevData(self.fake_device).is_dm - @patch('ceph_volume.util.disk.os.stat', MagicMock()) + @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat) @patch('ceph_volume.util.disk.os.minor', Mock(return_value=1)) @patch('ceph_volume.util.disk.os.major', Mock(return_value=998)) def test_is_lvm_true(self) -> None: - assert disk.UdevData(self.fake_device).is_dm + assert disk.UdevData(self.fake_device).is_lvm - @patch('ceph_volume.util.disk.os.stat', MagicMock()) + @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat) @patch('ceph_volume.util.disk.os.minor', Mock(return_value=0)) @patch('ceph_volume.util.disk.os.major', Mock(return_value=999)) def test_is_lvm_false(self) -> None: - assert not disk.UdevData(self.fake_device).is_dm + assert not disk.UdevData(self.fake_device).is_lvm - @patch('ceph_volume.util.disk.os.stat', MagicMock()) + @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat) @patch('ceph_volume.util.disk.os.minor', Mock(return_value=1)) @patch('ceph_volume.util.disk.os.major', Mock(return_value=998)) def test_slashed_path_with_lvm(self) -> None: assert disk.UdevData(self.fake_device).slashed_path == '/dev/fake_vg1/fake-lv1' - @patch('ceph_volume.util.disk.os.stat', MagicMock()) + @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat) @patch('ceph_volume.util.disk.os.minor', Mock(return_value=1)) @patch('ceph_volume.util.disk.os.major', Mock(return_value=998)) def test_dashed_path_with_lvm(self) -> None: assert disk.UdevData(self.fake_device).dashed_path == '/dev/mapper/fake_vg1-fake-lv1' - @patch('ceph_volume.util.disk.os.stat', MagicMock()) + @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat) + @patch('ceph_volume.util.disk.os.minor', Mock(return_value=1)) + @patch('ceph_volume.util.disk.os.major', Mock(return_value=998)) + def test_preferred_block_path_lvm_falls_back_to_mapper(self) -> None: + """When /dev/vg/lv is missing, use /dev/mapper/vg-lv (container-style LVM).""" + mapper: str = '/dev/mapper/fake_vg1-fake-lv1' + self.fs.create_file(mapper, st_mode=(stat.S_IFBLK | 0o600)) + assert disk.UdevData(self.fake_device).preferred_block_path == mapper + + @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat) + @patch('ceph_volume.util.disk.os.minor', Mock(return_value=1)) + @patch('ceph_volume.util.disk.os.major', Mock(return_value=998)) + def test_preferred_block_path_lvm_skips_slashed_when_not_block_device(self) -> None: + """If /dev/vg/lv exists but is not a block device (for example, a directory), try mapper.""" + slashed: str = '/dev/fake_vg1/fake-lv1' + mapper: str = '/dev/mapper/fake_vg1-fake-lv1' + self.fs.create_dir(slashed) + self.fs.create_file(mapper, st_mode=(stat.S_IFBLK | 0o600)) + assert disk.UdevData(self.fake_device).preferred_block_path == mapper + + @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat) + @patch('ceph_volume.util.disk.os.minor', Mock(return_value=1)) + @patch('ceph_volume.util.disk.os.major', Mock(return_value=998)) + def test_preferred_block_path_lvm_falls_back_when_candidates_are_not_block_devices( + self, + ) -> None: + """Neither candidate may be used if they are only directories (for example, empty udev fields).""" + slashed: str = '/dev/fake_vg1/fake-lv1' + mapper: str = '/dev/mapper/fake_vg1-fake-lv1' + self.fs.create_dir(slashed) + self.fs.create_dir(mapper) + assert disk.UdevData(self.fake_device).preferred_block_path == self.fake_device + + @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat) + @patch('ceph_volume.util.disk.os.minor', Mock(return_value=1)) + @patch('ceph_volume.util.disk.os.major', Mock(return_value=998)) + def test_preferred_block_path_lvm_prefers_slashed_when_present(self) -> None: + slashed: str = '/dev/fake_vg1/fake-lv1' + self.fs.create_file(slashed, st_mode=(stat.S_IFBLK | 0o600)) + assert disk.UdevData(self.fake_device).preferred_block_path == slashed + + @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat) + @patch('ceph_volume.util.disk.os.minor', Mock(return_value=1)) + @patch('ceph_volume.util.disk.os.major', Mock(return_value=998)) + def test_preferred_block_path_lvm_falls_back_to_opening_path_when_no_symlinks(self) -> None: + """Neither /dev/vg/lv nor /dev/mapper/name exist; keep the original device node.""" + assert disk.UdevData(self.fake_device).preferred_block_path == self.fake_device + + @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat) + @patch('ceph_volume.util.disk.os.minor', Mock(return_value=0)) + @patch('ceph_volume.util.disk.os.major', Mock(return_value=999)) + def test_preferred_block_path_bare_device(self) -> None: + assert disk.UdevData(self.fake_device).preferred_block_path == '/dev/cephtest' + + @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat) @patch('ceph_volume.util.disk.os.minor', Mock(return_value=0)) @patch('ceph_volume.util.disk.os.major', Mock(return_value=999)) def test_slashed_path_with_bare_device(self) -> None: assert disk.UdevData(self.fake_device).slashed_path == '/dev/cephtest' - @patch('ceph_volume.util.disk.os.stat', MagicMock()) + @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat) @patch('ceph_volume.util.disk.os.minor', Mock(return_value=0)) @patch('ceph_volume.util.disk.os.major', Mock(return_value=999)) def test_dashed_path_with_bare_device(self) -> None: diff --git a/src/ceph-volume/ceph_volume/util/disk.py b/src/ceph-volume/ceph_volume/util/disk.py index d8871a17437..3a735007023 100644 --- a/src/ceph-volume/ceph_volume/util/disk.py +++ b/src/ceph-volume/ceph_volume/util/disk.py @@ -787,7 +787,7 @@ def get_devices(_sys_block_path='/sys/block', device=''): for block in block_devs: metadata: Dict[str, Any] = {} if block[2] == 'lvm': - block[1] = UdevData(block[1]).slashed_path + block[1] = UdevData(block[1]).preferred_block_path devname = os.path.basename(block[0]) diskname = block[1] if block[2] not in block_types: @@ -1430,3 +1430,33 @@ class UdevData: name: str = self.environment.get('DM_NAME', '') result = f'/dev/mapper/{name}' return result + + @staticmethod + def _path_is_block_device(path: str) -> bool: + """True if ``path`` exists and is a block device (follows symlinks).""" + try: + return _stat_is_device(os.stat(path).st_mode) + except OSError: + return False + + @property + def preferred_block_path(self) -> str: + """Return a device path that exists for typical open(2) / blkid usage. + + `slashed_path` (/dev/vg/lv) is only present when udev/LVM created those + nodes; many environments (e.g. containers) only provide + `dashed_path` (/dev/mapper/name). + + Returns: + str: For non-LVM, `path`. For LVM, `slashed_path` if it is a block + device, else `dashed_path` if it is a block device, else `path`. + """ + if not self.is_lvm: + return self.path + slashed: str = self.slashed_path + if self._path_is_block_device(slashed): + return slashed + dashed: str = self.dashed_path + if self._path_is_block_device(dashed): + return dashed + return self.path