Skip to content

Commit 729c5de

Browse files
guitsjerrypu1000
andcommitted
ceph-volume: do not convert LVs's symlink to real path
This commit: - Adds a new function `get_lvm_mappers` in `ceph_volume/util/disk.py` to retrieve a list of LVM device mappers. - Updates the `is_lv` property in `ceph_volume/util/device.py` to use the new `get_lvm_mappers` function for better accuracy. - Modifies the symlink handling in `Device` class to properly identify LVM logical volumes. - Adds a new test `test_reject_lv_symlink_to_device` to ensure LVM symlinks are correctly identified and handled. - Updates relevant tests to cover the changes in LVM device detection. These changes improve the reliability and accuracy of LVM device detection and handling, ensuring that symlinks to LVM logical volumes are correctly processed. Fixes: https://tracker.ceph.com/issues/61597 Signed-off-by: Guillaume Abrioux <[email protected]> Co-Authored-by: Jerry Pu <[email protected]>
1 parent d95de4f commit 729c5de

File tree

4 files changed

+88
-10
lines changed

4 files changed

+88
-10
lines changed

src/ceph-volume/ceph_volume/tests/devices/lvm/test_migrate.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ def mock_process(self, *args, **kwargs):
172172
return ('', '', 0)
173173

174174
def test_init(self, monkeypatch):
175+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
175176
source_tags = 'ceph.osd_id=0,ceph.journal_uuid=x,ceph.type=data,ceph.osd_fsid=1234'
176177
source_db_tags = 'ceph.osd_id=0,journal_uuid=x,ceph.type=db, osd_fsid=1234'
177178
source_wal_tags = 'ceph.osd_id=0,ceph.journal_uuid=x,ceph.type=wal'
@@ -219,6 +220,7 @@ def test_init(self, monkeypatch):
219220
assert 'wal' == t.old_wal_tags['ceph.type']
220221

221222
def test_update_tags_when_lv_create(self, monkeypatch):
223+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
222224
source_tags = \
223225
'ceph.osd_id=0,ceph.journal_uuid=x,' \
224226
'ceph.type=data,ceph.osd_fsid=1234'
@@ -277,6 +279,7 @@ def test_update_tags_when_lv_create(self, monkeypatch):
277279
'/dev/VolGroup/lv2'] == self.mock_process_input[2]
278280

279281
def test_remove_lvs(self, monkeypatch):
282+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
280283
source_tags = \
281284
'ceph.osd_id=0,ceph.journal_uuid=x,' \
282285
'ceph.type=data,ceph.osd_fsid=1234,ceph.wal_uuid=aaaaa'
@@ -336,6 +339,7 @@ def test_remove_lvs(self, monkeypatch):
336339
'/dev/VolGroup/lv2'] == self.mock_process_input[2]
337340

338341
def test_replace_lvs(self, monkeypatch):
342+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
339343
source_tags = \
340344
'ceph.osd_id=0,ceph.type=data,ceph.osd_fsid=1234,'\
341345
'ceph.wal_uuid=wal_uuid,ceph.db_device=/dbdevice'
@@ -412,6 +416,7 @@ def test_replace_lvs(self, monkeypatch):
412416
'/dev/VolGroup/lv_target'].sort()
413417

414418
def test_undo(self, monkeypatch):
419+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
415420
source_tags = 'ceph.osd_id=0,ceph.journal_uuid=x,ceph.type=data,ceph.osd_fsid=1234'
416421
source_db_tags = 'ceph.osd_id=0,journal_uuid=x,ceph.type=db, osd_fsid=1234'
417422
source_wal_tags = 'ceph.osd_id=0,ceph.journal_uuid=x,ceph.type=wal'
@@ -569,6 +574,7 @@ def test_newdb_already_in_use(self, is_root, monkeypatch, capsys):
569574
assert expected in stderr
570575

571576
def test_newdb(self, is_root, monkeypatch, capsys):
577+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
572578
source_tags = \
573579
'ceph.osd_id=0,ceph.type=data,ceph.osd_fsid=1234,'\
574580
'ceph.wal_uuid=wal_uuid,ceph.db_device=/dbdevice'
@@ -725,6 +731,7 @@ def test_newdb_active_systemd(self, is_root, monkeypatch, capsys):
725731
assert not stdout
726732

727733
def test_newdb_no_systemd(self, is_root, monkeypatch):
734+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
728735
source_tags = \
729736
'ceph.osd_id=0,ceph.type=data,ceph.osd_fsid=1234,'\
730737
'ceph.wal_uuid=wal_uuid,ceph.db_device=/dbdevice'
@@ -814,6 +821,7 @@ def test_newdb_no_systemd(self, is_root, monkeypatch):
814821
'--command', 'bluefs-bdev-new-db']
815822

816823
def test_newwal(self, is_root, monkeypatch, capsys):
824+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
817825
source_tags = \
818826
'ceph.osd_id=0,ceph.type=data,ceph.osd_fsid=1234'
819827

@@ -925,6 +933,7 @@ def test_newwal_active_systemd(self, is_root, monkeypatch, capsys):
925933
assert not stdout
926934

927935
def test_newwal_no_systemd(self, is_root, monkeypatch):
936+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
928937
source_tags = \
929938
'ceph.osd_id=0,ceph.type=data,ceph.osd_fsid=1234'
930939

@@ -988,6 +997,7 @@ def test_newwal_no_systemd(self, is_root, monkeypatch):
988997

989998
@patch('os.getuid')
990999
def test_newwal_encrypted(self, m_getuid, monkeypatch, capsys):
1000+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
9911001
m_getuid.return_value = 0
9921002

9931003
source_tags = \
@@ -1219,6 +1229,7 @@ def test_migrate_without_args(self, capsys):
12191229

12201230
@patch.object(Zap, 'main')
12211231
def test_migrate_data_db_to_new_db(self, m_zap, is_root, monkeypatch):
1232+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
12221233

12231234
source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
12241235
'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev'
@@ -1321,6 +1332,7 @@ def test_migrate_data_db_to_new_db(self, m_zap, is_root, monkeypatch):
13211332
@patch.object(Zap, 'main')
13221333
@patch('os.getuid')
13231334
def test_migrate_data_db_to_new_db_encrypted(self, m_getuid, m_zap, monkeypatch):
1335+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
13241336
m_getuid.return_value = 0
13251337

13261338
source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
@@ -1500,6 +1512,7 @@ def test_migrate_data_db_to_new_db_active_systemd(self, is_root, monkeypatch, ca
15001512

15011513
@patch.object(Zap, 'main')
15021514
def test_migrate_data_db_to_new_db_no_systemd(self, m_zap, is_root, monkeypatch):
1515+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
15031516
source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
15041517
'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev'
15051518
source_db_tags = 'ceph.osd_id=2,ceph.type=db,ceph.osd_fsid=1234,' \
@@ -1599,6 +1612,7 @@ def test_migrate_data_db_to_new_db_no_systemd(self, m_zap, is_root, monkeypatch)
15991612

16001613
@patch.object(Zap, 'main')
16011614
def test_migrate_data_db_to_new_db_skip_wal(self, m_zap, is_root, monkeypatch):
1615+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
16021616
source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
16031617
'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev'
16041618
source_db_tags = 'ceph.osd_id=2,ceph.type=db,ceph.osd_fsid=1234,' \
@@ -1721,6 +1735,7 @@ def test_migrate_data_db_to_new_db_skip_wal(self, m_zap, is_root, monkeypatch):
17211735

17221736
@patch.object(Zap, 'main')
17231737
def test_migrate_data_db_wal_to_new_db(self, m_zap, is_root, monkeypatch):
1738+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
17241739
source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
17251740
'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev,' \
17261741
'ceph.wal_uuid=waluuid,ceph.wal_device=wal_dev'
@@ -1849,6 +1864,7 @@ def test_migrate_data_db_wal_to_new_db(self, m_zap, is_root, monkeypatch):
18491864
@patch.object(Zap, 'main')
18501865
@patch('os.getuid')
18511866
def test_migrate_data_db_wal_to_new_db_encrypted(self, m_getuid, m_zap, monkeypatch):
1867+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
18521868
m_getuid.return_value = 0
18531869

18541870
source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
@@ -2134,6 +2150,7 @@ def test_migrate_data_db_to_db(self,
21342150
is_root,
21352151
monkeypatch,
21362152
capsys):
2153+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
21372154
source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
21382155
'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev,' \
21392156
'ceph.wal_uuid=waluuid,ceph.wal_device=wal_dev'
@@ -2281,6 +2298,7 @@ def test_migrate_data_db_to_db_active_systemd(self, is_root, monkeypatch, capsys
22812298
assert not stdout
22822299

22832300
def test_migrate_data_db_to_db_no_systemd(self, is_root, monkeypatch):
2301+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
22842302
source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
22852303
'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev,' \
22862304
'ceph.wal_uuid=waluuid,ceph.wal_device=wal_dev'
@@ -2360,6 +2378,7 @@ def test_migrate_data_wal_to_db(self,
23602378
is_root,
23612379
monkeypatch,
23622380
capsys):
2381+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
23632382
source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
23642383
'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev,' \
23652384
'ceph.wal_uuid=waluuid,ceph.wal_device=wal_dev'
@@ -2467,6 +2486,7 @@ def test_migrate_wal_to_db(self,
24672486
m_zap,
24682487
monkeypatch,
24692488
capsys):
2489+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
24702490
m_getuid.return_value = 0
24712491

24722492
source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
@@ -2558,6 +2578,7 @@ def test_migrate_data_wal_to_db_encrypted(self,
25582578
m_zap,
25592579
monkeypatch,
25602580
capsys):
2581+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
25612582
m_getuid.return_value = 0
25622583

25632584
source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
@@ -2743,6 +2764,7 @@ def test_migrate_data_wal_to_db_active_systemd(self, is_root, monkeypatch, capsy
27432764

27442765
@patch.object(Zap, 'main')
27452766
def test_migrate_data_wal_to_db_no_systemd(self, m_zap, is_root, monkeypatch):
2767+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
27462768
source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
27472769
'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev,' \
27482770
'ceph.wal_uuid=waluuid,ceph.wal_device=wal_dev'

src/ceph-volume/ceph_volume/tests/util/test_device.py

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ def test_lvm_size_rounds_down(self, fake_call, device_info):
4747
disk = device.Device("/dev/sda")
4848
assert disk.lvm_size.gb == 4
4949

50-
def test_is_lv(self, fake_call, device_info):
50+
def test_is_lv(self, fake_call, device_info, monkeypatch):
51+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
5152
data = {"lv_path": "vg/lv", "vg_name": "vg", "name": "lv"}
5253
lsblk = {"TYPE": "lvm", "NAME": "vg-lv"}
5354
device_info(lv=data,lsblk=lsblk)
@@ -310,6 +311,22 @@ def test_reject_readonly_device(self, fake_call, device_info):
310311
disk = device.Device("/dev/cdrom")
311312
assert not disk.available
312313

314+
@patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
315+
@patch('ceph_volume.util.device.os.path.realpath')
316+
@patch('ceph_volume.util.device.os.path.islink')
317+
def test_reject_lv_symlink_to_device(self,
318+
m_os_path_islink,
319+
m_os_path_realpath,
320+
device_info,
321+
fake_call):
322+
m_os_path_islink.return_value = True
323+
m_os_path_realpath.return_value = '/dev/mapper/vg-lv'
324+
lv = {"lv_path": "/dev/vg/lv", "vg_name": "vg", "name": "lv"}
325+
lsblk = {"TYPE": "lvm", "NAME": "vg-lv"}
326+
device_info(lv=lv,lsblk=lsblk)
327+
disk = device.Device("/dev/vg/lv")
328+
assert disk.path == '/dev/vg/lv'
329+
313330
@patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
314331
def test_reject_smaller_than_5gb(self, fake_call, device_info):
315332
data = {"/dev/sda": {"size": 5368709119}}
@@ -528,7 +545,8 @@ def test_mapper_is_not_encrypted_plain(self, fake_call, device_info, monkeypatch
528545
assert disk.is_encrypted is False
529546

530547
@patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
531-
def test_lv_is_encrypted_blkid(self, fake_call, device_info):
548+
def test_lv_is_encrypted_blkid(self, fake_call, device_info, monkeypatch):
549+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
532550
lsblk = {'TYPE': 'lvm', 'NAME': 'sda'}
533551
blkid = {'TYPE': 'crypto_LUKS'}
534552
device_info(lsblk=lsblk, blkid=blkid)
@@ -537,7 +555,8 @@ def test_lv_is_encrypted_blkid(self, fake_call, device_info):
537555
assert disk.is_encrypted is True
538556

539557
@patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
540-
def test_lv_is_not_encrypted_blkid(self, fake_call, factory, device_info):
558+
def test_lv_is_not_encrypted_blkid(self, fake_call, factory, device_info, monkeypatch):
559+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
541560
lsblk = {'TYPE': 'lvm', 'NAME': 'sda'}
542561
blkid = {'TYPE': 'xfs'}
543562
device_info(lsblk=lsblk, blkid=blkid)
@@ -546,7 +565,8 @@ def test_lv_is_not_encrypted_blkid(self, fake_call, factory, device_info):
546565
assert disk.is_encrypted is False
547566

548567
@patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
549-
def test_lv_is_encrypted_lsblk(self, fake_call, device_info):
568+
def test_lv_is_encrypted_lsblk(self, fake_call, device_info, monkeypatch):
569+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
550570
lsblk = {'FSTYPE': 'crypto_LUKS', 'NAME': 'sda', 'TYPE': 'lvm'}
551571
blkid = {'TYPE': 'mapper'}
552572
device_info(lsblk=lsblk, blkid=blkid)
@@ -555,7 +575,8 @@ def test_lv_is_encrypted_lsblk(self, fake_call, device_info):
555575
assert disk.is_encrypted is True
556576

557577
@patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
558-
def test_lv_is_not_encrypted_lsblk(self, fake_call, factory, device_info):
578+
def test_lv_is_not_encrypted_lsblk(self, fake_call, factory, device_info, monkeypatch):
579+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
559580
lsblk = {'FSTYPE': 'xfs', 'NAME': 'sda', 'TYPE': 'lvm'}
560581
blkid = {'TYPE': 'mapper'}
561582
device_info(lsblk=lsblk, blkid=blkid)
@@ -564,7 +585,8 @@ def test_lv_is_not_encrypted_lsblk(self, fake_call, factory, device_info):
564585
assert disk.is_encrypted is False
565586

566587
@patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
567-
def test_lv_is_encrypted_lvm_api(self, fake_call, factory, device_info):
588+
def test_lv_is_encrypted_lvm_api(self, fake_call, factory, device_info, monkeypatch):
589+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
568590
lsblk = {'FSTYPE': 'xfs', 'NAME': 'sda', 'TYPE': 'lvm'}
569591
blkid = {'TYPE': 'mapper'}
570592
device_info(lsblk=lsblk, blkid=blkid)
@@ -573,7 +595,8 @@ def test_lv_is_encrypted_lvm_api(self, fake_call, factory, device_info):
573595
assert disk.is_encrypted is True
574596

575597
@patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
576-
def test_lv_is_not_encrypted_lvm_api(self, fake_call, factory, device_info):
598+
def test_lv_is_not_encrypted_lvm_api(self, fake_call, factory, device_info, monkeypatch):
599+
monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
577600
lsblk = {'FSTYPE': 'xfs', 'NAME': 'sda', 'TYPE': 'lvm'}
578601
blkid = {'TYPE': 'mapper'}
579602
device_info(lsblk=lsblk, blkid=blkid)

src/ceph-volume/ceph_volume/util/device.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ def __init__(self, path, with_lsm=False, lvs=None, lsblk_all=None, all_devices_v
119119
self.symlink = self.path
120120
real_path = os.path.realpath(self.path)
121121
# check if we are not a device mapper
122-
if "dm-" not in real_path:
122+
if "dm-" not in real_path and not self.is_lv:
123123
self.path = real_path
124124
if not sys_info.devices.get(self.path):
125125
sys_info.devices = disk.get_devices()
@@ -468,8 +468,9 @@ def is_mpath(self):
468468
return self.device_type == 'mpath'
469469

470470
@property
471-
def is_lv(self):
472-
return self.lv_api is not None
471+
def is_lv(self) -> bool:
472+
path = os.path.realpath(self.path)
473+
return path in disk.get_lvm_mappers()
473474

474475
@property
475476
def is_partition(self):

src/ceph-volume/ceph_volume/util/disk.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -931,3 +931,35 @@ def has_bluestore_label(device_path):
931931
logger.info(f'{device_path} is a directory, skipping.')
932932

933933
return isBluestore
934+
935+
def get_lvm_mappers(sys_block_path: str = '/sys/block') -> List[str]:
936+
"""
937+
Retrieve a list of Logical Volume Manager (LVM) device mappers.
938+
939+
This function scans the given system block path for device mapper (dm) devices
940+
and identifies those that are managed by LVM. For each LVM device found, it adds
941+
the corresponding paths to the result list.
942+
943+
Args:
944+
sys_block_path (str, optional): The path to the system block directory. Defaults to '/sys/block'.
945+
946+
Returns:
947+
List[str]: A list of strings representing the paths of LVM device mappers.
948+
Each LVM device will have two entries: the /dev/mapper/ path and the /dev/ path.
949+
"""
950+
result: List[str] = []
951+
for device in os.listdir(sys_block_path):
952+
path: str = os.path.join(sys_block_path, device, 'dm')
953+
uuid_path: str = os.path.join(path, 'uuid')
954+
name_path: str = os.path.join(path, 'name')
955+
956+
if os.path.exists(uuid_path):
957+
with open(uuid_path, 'r') as f:
958+
mapper_type: str = f.read().split('-')[0]
959+
960+
if mapper_type == 'LVM':
961+
with open(name_path, 'r') as f:
962+
name: str = f.read()
963+
result.append(f'/dev/mapper/{name.strip()}')
964+
result.append(f'/dev/{device}')
965+
return result

0 commit comments

Comments
 (0)