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 f9e2b76b27d40eb7bb83b18d1b781fa5d2c2b6d5..f71aeb21a23f9b7237a93488fdc2cbc061d770d9 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,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 1706a21d6050b7cf205696237ba7fe64717bd9c9..4eaff6e67e73cc00cec9933a7f987b2c6114726c 100644 (file)
--- a/src/ceph-volume/ceph_volume/util/device.py
+++ b/src/ceph-volume/ceph_volume/util/device.py
@@ -584,7 +584,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]
@@ -620,6 +619,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 4152942d38b416cb70c3d617e5bba416c64cabc7..548de795cd6f858f190e1db30c8351a73787b501 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
@@ -914,7 +892,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