From 83a42f3e232c7c4a02deb3539972c82b6dca284b Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Fri, 4 Oct 2019 12:30:03 +0200 Subject: [PATCH 1/2] Add a new "sector_size" property to storage devices. This represents the logical sector size of the device. Related: rhbz#1754446 --- blivet/devices/disk.py | 6 +++++- blivet/devices/md.py | 11 +++++++++++ blivet/devices/partition.py | 7 +++++++ blivet/devices/storage.py | 15 +++++++++++++++ 4 files changed, 38 insertions(+), 1 deletion(-) diff --git a/blivet/devices/disk.py b/blivet/devices/disk.py index bf2f7a4f..7dfeabf0 100644 --- a/blivet/devices/disk.py +++ b/blivet/devices/disk.py @@ -687,7 +687,7 @@ def __init__(self, device, **kwargs): """ self.mode = kwargs.pop("mode") self.devname = kwargs.pop("devname") - self.sector_size = kwargs.pop("sector_size") + self._sector_size = kwargs.pop("sector_size") DiskDevice.__init__(self, device, **kwargs) @@ -710,3 +710,7 @@ def description(self): % {'devname': self.devname, 'mode': self.mode, 'path': self.path} + + @property + def sector_size(self): + return self._sector_size diff --git a/blivet/devices/md.py b/blivet/devices/md.py index 6a837df0..0b6da980 100644 --- a/blivet/devices/md.py +++ b/blivet/devices/md.py @@ -19,10 +19,13 @@ # Red Hat Author(s): David Lehman # +import math import os import six import time +from six.moves import reduce + import gi gi.require_version("BlockDev", "2.0") @@ -195,6 +198,14 @@ def level(self, value): self._level = level + @property + def sector_size(self): + if not self.exists: + # Least common multiple of parents' sector sizes + return reduce(lambda a, b: a * b // math.gcd(a, b), (int(p.sector_size) for p in self.parents)) + + return super(MDRaidArrayDevice, self).sector_size + @property def chunk_size(self): if self.exists and self._chunk_size == Size(0): diff --git a/blivet/devices/partition.py b/blivet/devices/partition.py index 623e1c9d..73daa76f 100644 --- a/blivet/devices/partition.py +++ b/blivet/devices/partition.py @@ -729,6 +729,13 @@ def protected(self): def protected(self, value): self._protected = value + @property + def sector_size(self): + if self.disk: + return self.disk.sector_size + + return super(PartitionDevice, self).sector_size + def _pre_resize(self): if not self.exists: raise errors.DeviceError("device has not been created", self.name) diff --git a/blivet/devices/storage.py b/blivet/devices/storage.py index e087fa64..91c5e60e 100644 --- a/blivet/devices/storage.py +++ b/blivet/devices/storage.py @@ -190,6 +190,21 @@ def raw_device(self): """ The device itself, or when encrypted, the backing device. """ return self + @property + def sector_size(self): + """ Logical sector (block) size of this device """ + if not self.exists: + if self.parents: + return self.parents[0].sector_size + else: + return LINUX_SECTOR_SIZE + + block_size = util.get_sysfs_attr(self.sysfs_path, "queue/logical_block_size") + if block_size: + return int(block_size) + else: + return LINUX_SECTOR_SIZE + @property def controllable(self): return self._controllable and not flags.testing and not self.unavailable_type_dependencies() From 9f81bd1ffb877862760223ba88f2086deebd2d06 Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Fri, 4 Oct 2019 12:37:01 +0200 Subject: [PATCH 2/2] Do not allow creating VGs with PVs with different sector size New versions of LVM don't allow mixing PVs with different sector sizes in one VG. Resolves: rhbz#1754446 --- blivet/devices/lvm.py | 12 ++++++++++++ tests/devices_test/lvm_test.py | 13 ++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/blivet/devices/lvm.py b/blivet/devices/lvm.py index 4347f483..b9da286a 100644 --- a/blivet/devices/lvm.py +++ b/blivet/devices/lvm.py @@ -356,6 +356,18 @@ def _remove_log_vol(self, lv): def _add_parent(self, parent): super(LVMVolumeGroupDevice, self)._add_parent(parent) + # we are creating new VG or adding a new PV to an existing (complete) one + if not self.exists or (self.exists and self._complete): + parent_sectors = set([p.sector_size for p in self.pvs] + [parent.sector_size]) + if len(parent_sectors) != 1: + if not self.exists: + msg = "The volume group %s cannot be created. Selected disks have " \ + "inconsistent sector sizes (%s)." % (self.name, parent_sectors) + else: + msg = "Disk %s cannot be added to this volume group. LVM doesn't " \ + "allow using physical volumes with inconsistent (logical) sector sizes." % parent.name + raise ValueError(msg) + if (self.exists and parent.format.exists and len(self.parents) + 1 == self.pv_count): self._complete = True diff --git a/tests/devices_test/lvm_test.py b/tests/devices_test/lvm_test.py index 8ed577f4..a32c1d83 100644 --- a/tests/devices_test/lvm_test.py +++ b/tests/devices_test/lvm_test.py @@ -2,7 +2,7 @@ import test_compat # pylint: disable=unused-import import six -from six.moves.mock import patch # pylint: disable=no-name-in-module,import-error +from six.moves.mock import patch, PropertyMock # pylint: disable=no-name-in-module,import-error import unittest import blivet @@ -352,6 +352,17 @@ def test_target_size(self): self.assertEqual(lv.target_size, orig_size) self.assertEqual(lv.size, orig_size) + def test_lvm_inconsistent_sector_size(self): + pv = StorageDevice("pv1", fmt=blivet.formats.get_format("lvmpv"), + size=Size("1024 MiB")) + pv2 = StorageDevice("pv2", fmt=blivet.formats.get_format("lvmpv"), + size=Size("1024 MiB")) + + with patch("blivet.devices.StorageDevice.sector_size", new_callable=PropertyMock) as mock_property: + mock_property.__get__ = lambda _mock, pv, _class: 512 if pv.name == "pv1" else 4096 + with six.assertRaisesRegex(self, ValueError, "The volume group testvg cannot be created."): + LVMVolumeGroupDevice("testvg", parents=[pv, pv2]) + class TypeSpecificCallsTest(unittest.TestCase): def test_type_specific_calls(self):