From: Guillaume Abrioux Date: Thu, 10 Aug 2023 08:01:12 +0000 (+0000) Subject: ceph-volume: drop is_locked_raw_device() X-Git-Tag: v18.2.1~314^2~1 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=d9ca3bd992a24ba94553bc550b2bfe0b56aa84f4;p=ceph.git ceph-volume: drop is_locked_raw_device() This functions works for what it is supposed to do: check if a device is busy. That being said, this induces a race condition in `get_devices()` Indeed, it does: 1/ `os.open()` with `(os.O_RDWR | os.O_EXCL)` 2/ `os.close()` The second call has an effect: it triggers a udev event which causes systemd-udevd to re-process the device. This seems to be a question of millisecond but because of this, /sys (sysfs) isn't fully populated as expected. Given that get_devices() collects a lot of details from sysfs in a loop, some of these details can be missed. ceph-volume overall doesn't make decisions based on `is_locked_raw_device()` This detail is used only for reporting (inventory). For this reason, dropping this function seems reasonnable. As a compromise, we can check if the device has partitions and/or a FileSystem on it. Signed-off-by: Guillaume Abrioux (cherry picked from commit 2422ad867dff9d526d7e8be543178c897991097f) --- 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 f9e2b76b27d40..f71aeb21a23f9 100644 --- a/src/ceph-volume/ceph_volume/tests/util/test_disk.py +++ b/src/ceph-volume/ceph_volume/tests/util/test_disk.py @@ -225,7 +225,6 @@ class TestGetDevices(object): result = disk.get_devices(_sys_block_path=str(tmpdir)) assert result == {} - @patch('ceph_volume.util.disk.is_locked_raw_device', lambda x: False) def test_sda_block_is_found(self, patched_get_block_devs_sysfs, fake_filesystem): sda_path = '/dev/sda' patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk']] @@ -235,7 +234,6 @@ class TestGetDevices(object): assert result[sda_path]['model'] == '' assert result[sda_path]['partitions'] == {} - @patch('ceph_volume.util.disk.is_locked_raw_device', lambda x: False) def test_sda_size(self, patched_get_block_devs_sysfs, fake_filesystem): sda_path = '/dev/sda' patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk']] @@ -244,7 +242,6 @@ class TestGetDevices(object): assert list(result.keys()) == [sda_path] assert result[sda_path]['human_readable_size'] == '512.00 KB' - @patch('ceph_volume.util.disk.is_locked_raw_device', lambda x: False) def test_sda_sectorsize_fallsback(self, patched_get_block_devs_sysfs, fake_filesystem): # if no sectorsize, it will use queue/hw_sector_size sda_path = '/dev/sda' @@ -254,7 +251,6 @@ class TestGetDevices(object): assert list(result.keys()) == [sda_path] assert result[sda_path]['sectorsize'] == '1024' - @patch('ceph_volume.util.disk.is_locked_raw_device', lambda x: False) def test_sda_sectorsize_from_logical_block(self, patched_get_block_devs_sysfs, fake_filesystem): sda_path = '/dev/sda' patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk']] @@ -262,7 +258,6 @@ class TestGetDevices(object): result = disk.get_devices() assert result[sda_path]['sectorsize'] == '99' - @patch('ceph_volume.util.disk.is_locked_raw_device', lambda x: False) def test_sda_sectorsize_does_not_fallback(self, patched_get_block_devs_sysfs, fake_filesystem): sda_path = '/dev/sda' patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk']] @@ -271,7 +266,6 @@ class TestGetDevices(object): result = disk.get_devices() assert result[sda_path]['sectorsize'] == '99' - @patch('ceph_volume.util.disk.is_locked_raw_device', lambda x: False) def test_is_rotational(self, patched_get_block_devs_sysfs, fake_filesystem): sda_path = '/dev/sda' patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk']] @@ -279,14 +273,12 @@ class TestGetDevices(object): result = disk.get_devices() assert result[sda_path]['rotational'] == '1' - @patch('ceph_volume.util.disk.is_locked_raw_device', lambda x: False) def test_is_ceph_rbd(self, patched_get_block_devs_sysfs, fake_filesystem): rbd_path = '/dev/rbd0' patched_get_block_devs_sysfs.return_value = [[rbd_path, rbd_path, 'disk']] result = disk.get_devices() assert rbd_path not in result - @patch('ceph_volume.util.disk.is_locked_raw_device', lambda x: False) def test_actuator_device(self, patched_get_block_devs_sysfs, fake_filesystem): sda_path = '/dev/sda' fake_actuator_nb = 2 diff --git a/src/ceph-volume/ceph_volume/util/device.py b/src/ceph-volume/ceph_volume/util/device.py index 7b6364fbc8c0d..7b88f74f8f4ca 100644 --- a/src/ceph-volume/ceph_volume/util/device.py +++ b/src/ceph-volume/ceph_volume/util/device.py @@ -594,7 +594,6 @@ class Device(object): reasons = [ ('removable', 1, 'removable'), ('ro', 1, 'read-only'), - ('locked', 1, 'locked'), ] rejected = [reason for (k, v, reason) in reasons if self.sys_api.get(k, '') == v] @@ -630,6 +629,8 @@ class Device(object): rejected.append('Has GPT headers') if self.has_partitions: rejected.append('Has partitions') + if self.has_fs: + rejected.append('Has a FileSystem') return rejected def _check_lvm_reject_reasons(self): diff --git a/src/ceph-volume/ceph_volume/util/disk.py b/src/ceph-volume/ceph_volume/util/disk.py index 5714ed70b7fe4..469096b776501 100644 --- a/src/ceph-volume/ceph_volume/util/disk.py +++ b/src/ceph-volume/ceph_volume/util/disk.py @@ -734,28 +734,6 @@ def is_mapper_device(device_name): return device_name.startswith(('/dev/mapper', '/dev/dm-')) -def is_locked_raw_device(disk_path): - """ - A device can be locked by a third party software like a database. - To detect that case, the device is opened in Read/Write and exclusive mode - """ - open_flags = (os.O_RDWR | os.O_EXCL) - open_mode = 0 - fd = None - - try: - fd = os.open(disk_path, open_flags, open_mode) - except OSError: - return 1 - - try: - os.close(fd) - except OSError: - return 1 - - return 0 - - class AllowLoopDevices(object): allow = False warned = False @@ -916,7 +894,6 @@ def get_devices(_sys_block_path='/sys/block', device=''): metadata['size'] = float(size) * 512 metadata['human_readable_size'] = human_readable_size(metadata['size']) metadata['path'] = diskname - metadata['locked'] = is_locked_raw_device(metadata['path']) metadata['type'] = block[2] device_facts[diskname] = metadata