ceph-volume: has_bluestore_label checks all bluestore label replica offsets

BlueStore replicates the block device label at fixed offsets (0 and
multiples of 1Gb up to 1000gb). has_bluestore_label() only read the
first 22 bytes, so disks with a wiped primary label but intact
replicas are missed.

with this commit, has_bluestore_label() scans each known offset with
seek/read and compares the ASCII prefix as bytes.

Fixes: https://tracker.ceph.com/issues/76060
Signed-off-by: Guillaume Abrioux <gabrioux@ibm.com>

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 5fc8a52272a279067e45205888940d9304cbaa13..76e2b3458d92b80eaea05a5d51419f6931c591b0 100644 (file)
--- a/src/ceph-volume/ceph_volume/tests/util/test_disk.py
+++ b/src/ceph-volume/ceph_volume/tests/util/test_disk.py
@@ -1,3 +1,4 @@
+import errno
 import pytest
 import stat
 from typing import Any, Callable, ClassVar, Optional
@@ -679,6 +680,91 @@ class TestHasBlueStoreLabel(object):
         fake_filesystem.create_dir(device_path)
         assert not disk.has_bluestore_label(device_path)
 
+    def test_bluestore_label_at_replica_offset(self):
+        class _FakeBlockDev:
+            def __init__(self, label_offset):
+                self._label_offset = label_offset
+                self._pos = 0
+
+            def seek(self, offset, whence=0):
+                if whence != 0:
+                    raise NotImplementedError
+                self._pos = offset
+                return offset
+
+            def read(self, n):
+                buf_start = self._pos
+                self._pos += n
+                sig = disk.BLUESTORE_BDEV_LABEL_SIGNATURE
+                if buf_start == self._label_offset:
+                    return sig[:n] + (b'\x00' * max(0, n - len(sig)))
+                return b'\x00' * n
+
+            def __enter__(self):
+                return self
+
+            def __exit__(self, *args):
+                return None
+
+        for off in disk.BLUESTORE_BDEV_LABEL_OFFSETS[1:3]:
+            with patch('builtins.open', lambda *a, **k: _FakeBlockDev(off)):
+                assert disk.has_bluestore_label('/dev/fake'), off
+
+    def test_bluestore_label_non_seekable_returns_false(self):
+        class _NonSeekable:
+            def seek(self, offset, whence=0):
+                raise OSError(errno.EINVAL, 'Invalid argument')
+
+            def read(self, n):
+                return b'\x00' * n
+
+            def __enter__(self):
+                return self
+
+            def __exit__(self, *args):
+                return None
+
+        with patch('builtins.open', lambda *a, **k: _NonSeekable()):
+            assert not disk.has_bluestore_label('/dev/fake')
+
+    def test_bluestore_label_non_seekable_fallback_read_matches(self):
+        class _NonSeekableWithLabelAtStart:
+            def seek(self, offset, whence=0):
+                raise OSError(errno.EINVAL, 'Invalid argument')
+
+            def read(self, n):
+                return disk.BLUESTORE_BDEV_LABEL_SIGNATURE[:n] + (
+                    b'\x00' * max(0, n - len(disk.BLUESTORE_BDEV_LABEL_SIGNATURE))
+                )
+
+            def __enter__(self):
+                return self
+
+            def __exit__(self, *args):
+                return None
+
+        with patch('builtins.open', lambda *a, **k: _NonSeekableWithLabelAtStart()):
+            assert disk.has_bluestore_label('/dev/fake')
+
+    def test_bluestore_label_io_error_propagates(self):
+        class _EIOOnSeek:
+            def seek(self, offset, whence=0):
+                raise OSError(errno.EIO, 'Input/output error')
+
+            def read(self, n):
+                return b'\x00' * n
+
+            def __enter__(self):
+                return self
+
+            def __exit__(self, *args):
+                return None
+
+        with patch('builtins.open', lambda *a, **k: _EIOOnSeek()):
+            with pytest.raises(OSError) as excinfo:
+                disk.has_bluestore_label('/dev/fake')
+            assert excinfo.value.errno == errno.EIO
+
 
 class TestBlockSysFs(TestCase):
     def setUp(self) -> None:

diff --git a/src/ceph-volume/ceph_volume/util/disk.py b/src/ceph-volume/ceph_volume/util/disk.py
index 643fa92a68c82bf4d852e812310610a794d6e840..3efbeae0ae5c21ff546962a12af9ba14ab50fe07 100644 (file)
--- a/src/ceph-volume/ceph_volume/util/disk.py
+++ b/src/ceph-volume/ceph_volume/util/disk.py
@@ -1,3 +1,4 @@
+import errno
 import logging
 import os
 import re
@@ -12,6 +13,16 @@ from typing import Dict, List, Any, Union, Optional
 
 logger = logging.getLogger(__name__)
 
+_ONE_GIB = 1024 * 1024 * 1024
+BLUESTORE_BDEV_LABEL_OFFSETS = (
+    0,
+    _ONE_GIB,
+    10 * _ONE_GIB,
+    100 * _ONE_GIB,
+    1000 * _ONE_GIB,
+)
+BLUESTORE_BDEV_LABEL_SIGNATURE = b'bluestore block device'
+
 
 # The blkid CLI tool has some oddities which prevents having one common call
 # to extract the information instead of having separate utilities. The `udev`
@@ -884,21 +895,30 @@ def get_devices(_sys_block_path='/sys/block', device=''):
     return device_facts
 
 def has_bluestore_label(device_path: str) -> bool:
-    isBluestore = False
-    bluestoreDiskSignature = 'bluestore block device' # 22 bytes long
-
-    # throws OSError on failure
     logger.info("opening device {} to check for BlueStore label".format(device_path))
+    sig_len = len(BLUESTORE_BDEV_LABEL_SIGNATURE)
     try:
         with open(device_path, "rb") as fd:
-            # read first 22 bytes looking for bluestore disk signature
-            signature = fd.read(22)
-            if signature.decode('ascii', 'replace') == bluestoreDiskSignature:
-                isBluestore = True
+            for position in BLUESTORE_BDEV_LABEL_OFFSETS:
+                try:
+                    fd.seek(position)
+                except OSError as exc:
+                    err = exc.errno
+                    if err is None or err not in (
+                        errno.EINVAL,
+                        errno.ESPIPE,
+                        errno.ENOTTY,
+                    ):
+                        raise
+                    if position != 0:
+                        continue
+                signature = fd.read(sig_len)
+                if signature == BLUESTORE_BDEV_LABEL_SIGNATURE:
+                    return True
     except IsADirectoryError:
         logger.info(f'{device_path} is a directory, skipping.')
 
-    return isBluestore
+    return False
 
 def has_seastore_label(device_path: str) -> bool:
     is_seastore = False