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 <gabrioux@ibm.com>
(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 fcd644a861b53be67147a9751e6b0d2942a1d726..7dc0b32224d404cea6655077b77fc2c3a7f4e6e6 100644 (file)
--- 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,7 +273,6 @@ 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']]
@@ -521,4 +514,4 @@ class TestHasBlueStoreLabel(object):
     def test_device_path_is_a_path(self, fake_filesystem):
         device_path = '/var/lib/ceph/osd/ceph-0'
         fake_filesystem.create_dir(device_path)
-        assert not disk.has_bluestore_label(device_path)
\ No newline at end of file
+        assert not disk.has_bluestore_label(device_path)

diff --git a/src/ceph-volume/ceph_volume/util/device.py b/src/ceph-volume/ceph_volume/util/device.py
index 349d0bd4de89b2ca82078e6980caf14cf8dc4f2e..04f56097a621e3fd5d1827aefe417dcdffc7db7a 100644 (file)
--- a/src/ceph-volume/ceph_volume/util/device.py
+++ b/src/ceph-volume/ceph_volume/util/device.py
@@ -590,7 +590,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]
@@ -626,6 +625,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 4238001ce3ac64efed05eaba49ce287555457ef7..caeea047ce051e60cffc0e7f053f8936f92d2aa9 100644 (file)
--- 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
@@ -906,7 +884,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