From: Guillaume Abrioux Date: Thu, 16 Apr 2026 07:29:23 +0000 (+0200) Subject: ceph-volume: has_bluestore_label checks all bluestore label replica offsets X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=981dfa4acef2d87938a92c1e69715e95ad7d845b;p=ceph.git 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 --- 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 5fc8a52272a2..76e2b3458d92 100644 --- 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 643fa92a68c8..3efbeae0ae5c 100644 --- 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