From b9021fde8ccdd14cbe192b6597f7ca350b4bb585 Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Wed, 26 May 2021 12:15:54 +0200 Subject: [PATCH] Revert "More consistent lvm errors (API break)" This reverts commit 49ec071c6d0673224a0774d613904387c52c7381. --- blivet/devices/lvm.py | 72 +++++++++++------------ tests/unit_tests/devices_test/lvm_test.py | 14 ++--- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/blivet/devices/lvm.py b/blivet/devices/lvm.py index 38e49e18..b8595d63 100644 --- a/blivet/devices/lvm.py +++ b/blivet/devices/lvm.py @@ -304,7 +304,7 @@ class LVMVolumeGroupDevice(ContainerDevice): def _add_log_vol(self, lv): """ Add an LV to this VG. """ if lv in self._lvs: - raise errors.DeviceError("lv is already part of this vg") + raise ValueError("lv is already part of this vg") # verify we have the space, then add it # do not verify for growing vg (because of ks) @@ -337,7 +337,7 @@ class LVMVolumeGroupDevice(ContainerDevice): def _remove_log_vol(self, lv): """ Remove an LV from this VG. """ if lv not in self.lvs: - raise errors.DeviceError("specified lv is not part of this vg") + raise ValueError("specified lv is not part of this vg") self._lvs.remove(lv) @@ -430,7 +430,7 @@ class LVMVolumeGroupDevice(ContainerDevice): @thpool_reserve.setter def thpool_reserve(self, value): if value is not None and not isinstance(value, ThPoolReserveSpec): - raise AttributeError("Invalid thpool_reserve given, must be of type ThPoolReserveSpec") + raise ValueError("Invalid thpool_reserve given, must be of type ThPoolReserveSpec") self._thpool_reserve = value @property @@ -665,14 +665,14 @@ class LVMLogicalVolumeBase(DMDevice, RaidDevice): if seg_type not in [None, "linear", "thin", "thin-pool", "cache", "vdo-pool", "vdo", "cache-pool"] + lvm.raid_seg_types: raise ValueError("Invalid or unsupported segment type: %s" % seg_type) if seg_type and seg_type in lvm.raid_seg_types and not pvs: - raise errors.DeviceError("List of PVs has to be given for every non-linear LV") + raise ValueError("List of PVs has to be given for every non-linear LV") elif (not seg_type or seg_type == "linear") and pvs: if not all(isinstance(pv, LVPVSpec) for pv in pvs): - raise errors.DeviceError("Invalid specification of PVs for a linear LV: either no or complete " - "specification (with all space split into PVs has to be given") + raise ValueError("Invalid specification of PVs for a linear LV: either no or complete " + "specification (with all space split into PVs has to be given") elif sum(spec.size for spec in pvs) != size: - raise errors.DeviceError("Invalid specification of PVs for a linear LV: the sum of space " - "assigned to PVs is not equal to the size of the LV") + raise ValueError("Invalid specification of PVs for a linear LV: the sum of space " + "assigned to PVs is not equal to the size of the LV") # When this device's format is set in the superclass constructor it will # try to access self.snapshots. @@ -721,13 +721,13 @@ class LVMLogicalVolumeBase(DMDevice, RaidDevice): self._from_lvs = from_lvs if self._from_lvs: if exists: - raise errors.DeviceError("Only new LVs can be created from other LVs") + raise ValueError("Only new LVs can be created from other LVs") if size or maxsize or percent: - raise errors.DeviceError("Cannot specify size for a converted LV") + raise ValueError("Cannot specify size for a converted LV") if fmt: - raise errors.DeviceError("Cannot specify format for a converted LV") + raise ValueError("Cannot specify format for a converted LV") if any(lv.vg != self.vg for lv in self._from_lvs): - raise errors.DeviceError("Conversion of LVs only possible inside a VG") + raise ValueError("Conversion of LVs only possible inside a VG") self._cache = None if cache_request and not self.exists: @@ -746,13 +746,13 @@ class LVMLogicalVolumeBase(DMDevice, RaidDevice): elif isinstance(pv_spec, StorageDevice): self._pv_specs.append(LVPVSpec(pv_spec, Size(0))) else: - raise AttributeError("Invalid PV spec '%s' for the '%s' LV" % (pv_spec, self.name)) + raise ValueError("Invalid PV spec '%s' for the '%s' LV" % (pv_spec, self.name)) # Make sure any destination PVs are actually PVs in this VG if not set(spec.pv for spec in self._pv_specs).issubset(set(self.vg.parents)): missing = [r.name for r in set(spec.pv for spec in self._pv_specs).difference(set(self.vg.parents))] msg = "invalid destination PV(s) %s for LV %s" % (missing, self.name) - raise errors.DeviceError(msg) + raise ValueError(msg) if self._pv_specs: self._assign_pv_space() @@ -1130,7 +1130,7 @@ class LVMLogicalVolumeBase(DMDevice, RaidDevice): else: msg = "the specified internal LV '%s' doesn't belong to this LV ('%s')" % (int_lv.lv_name, self.name) - raise errors.DeviceError(msg) + raise ValueError(msg) def populate_ksdata(self, data): super(LVMLogicalVolumeBase, self).populate_ksdata(data) @@ -1229,7 +1229,7 @@ class LVMInternalLogicalVolumeMixin(object): def _init_check(self): # an internal LV should have no parents if self._parent_lv and self._parents: - raise errors.DeviceError("an internal LV should have no parents") + raise ValueError("an internal LV should have no parents") @property def is_internal_lv(self): @@ -1289,7 +1289,7 @@ class LVMInternalLogicalVolumeMixin(object): @readonly.setter def readonly(self, value): # pylint: disable=unused-argument - raise errors.DeviceError("Cannot make an internal LV read-write") + raise ValueError("Cannot make an internal LV read-write") @property def type(self): @@ -1325,7 +1325,7 @@ class LVMInternalLogicalVolumeMixin(object): def _check_parents(self): # an internal LV should have no parents if self._parents: - raise errors.DeviceError("an internal LV should have no parents") + raise ValueError("an internal LV should have no parents") def _add_to_parents(self): # nothing to do here, an internal LV has no parents (in the DeviceTree's @@ -1335,13 +1335,13 @@ class LVMInternalLogicalVolumeMixin(object): # internal LVs follow different rules limitting size def _set_size(self, newsize): if not isinstance(newsize, Size): - raise AttributeError("new size must of type Size") + raise ValueError("new size must of type Size") if not self.takes_extra_space: if newsize <= self.parent_lv.size: # pylint: disable=no-member self._size = newsize # pylint: disable=attribute-defined-outside-init else: - raise errors.DeviceError("Internal LV cannot be bigger than its parent LV") + raise ValueError("Internal LV cannot be bigger than its parent LV") else: # same rules apply as for any other LV raise NotTypeSpecific() @@ -1419,18 +1419,18 @@ class LVMSnapshotMixin(object): return if self.origin and not isinstance(self.origin, LVMLogicalVolumeDevice): - raise errors.DeviceError("lvm snapshot origin must be a logical volume") + raise ValueError("lvm snapshot origin must be a logical volume") if self.vorigin and not self.exists: - raise errors.DeviceError("only existing vorigin snapshots are supported") + raise ValueError("only existing vorigin snapshots are supported") if isinstance(self.origin, LVMLogicalVolumeDevice) and \ isinstance(self.parents[0], LVMVolumeGroupDevice) and \ self.origin.vg != self.parents[0]: - raise errors.DeviceError("lvm snapshot and origin must be in the same vg") + raise ValueError("lvm snapshot and origin must be in the same vg") if self.is_thin_lv: if self.origin and self.size and not self.exists: - raise errors.DeviceError("thin snapshot size is determined automatically") + raise ValueError("thin snapshot size is determined automatically") @property def is_snapshot_lv(self): @@ -1606,7 +1606,7 @@ class LVMThinPoolMixin(object): def _check_from_lvs(self): if self._from_lvs: if len(self._from_lvs) != 2: - raise errors.DeviceError("two LVs required to create a thin pool") + raise ValueError("two LVs required to create a thin pool") def _convert_from_lvs(self): data_lv, metadata_lv = self._from_lvs @@ -1652,7 +1652,7 @@ class LVMThinPoolMixin(object): def _add_log_vol(self, lv): """ Add an LV to this pool. """ if lv in self._lvs: - raise errors.DeviceError("lv is already part of this vg") + raise ValueError("lv is already part of this vg") # TODO: add some checking to prevent overcommit for preexisting self.vg._add_log_vol(lv) @@ -1663,7 +1663,7 @@ class LVMThinPoolMixin(object): def _remove_log_vol(self, lv): """ Remove an LV from this pool. """ if lv not in self._lvs: - raise errors.DeviceError("specified lv is not part of this vg") + raise ValueError("specified lv is not part of this vg") self._lvs.remove(lv) self.vg._remove_log_vol(lv) @@ -1772,14 +1772,14 @@ class LVMThinLogicalVolumeMixin(object): """Check that this device has parents as expected""" if isinstance(self.parents, (list, ParentList)): if len(self.parents) != 1: - raise errors.DeviceError("constructor requires a single thin-pool LV") + raise ValueError("constructor requires a single thin-pool LV") container = self.parents[0] else: container = self.parents if not container or not isinstance(container, LVMLogicalVolumeDevice) or not container.is_thin_pool: - raise errors.DeviceError("constructor requires a thin-pool LV") + raise ValueError("constructor requires a thin-pool LV") @property def is_thin_lv(self): @@ -1816,7 +1816,7 @@ class LVMThinLogicalVolumeMixin(object): def _set_size(self, newsize): if not isinstance(newsize, Size): - raise AttributeError("new size must of type Size") + raise ValueError("new size must of type Size") newsize = self.vg.align(newsize) newsize = self.vg.align(util.numeric_type(newsize)) @@ -2499,7 +2499,7 @@ class LVMLogicalVolumeDevice(LVMLogicalVolumeBase, LVMInternalLogicalVolumeMixin container = self.parents if not isinstance(container, LVMVolumeGroupDevice): - raise AttributeError("constructor requires a LVMVolumeGroupDevice") + raise ValueError("constructor requires a LVMVolumeGroupDevice") @type_specific def _add_to_parents(self): @@ -2510,12 +2510,12 @@ class LVMLogicalVolumeDevice(LVMLogicalVolumeBase, LVMInternalLogicalVolumeMixin @type_specific def _check_from_lvs(self): """Check the LVs to create this LV from""" - raise errors.DeviceError("Cannot create a new LV of type '%s' from other LVs" % self.seg_type) + raise ValueError("Cannot create a new LV of type '%s' from other LVs" % self.seg_type) @type_specific def _convert_from_lvs(self): """Convert the LVs to create this LV from into its internal LVs""" - raise errors.DeviceError("Cannot create a new LV of type '%s' from other LVs" % self.seg_type) + raise ValueError("Cannot create a new LV of type '%s' from other LVs" % self.seg_type) @property def external_dependencies(self): @@ -2535,7 +2535,7 @@ class LVMLogicalVolumeDevice(LVMLogicalVolumeBase, LVMInternalLogicalVolumeMixin @type_specific def _set_size(self, newsize): if not isinstance(newsize, Size): - raise AttributeError("new size must be of type Size") + raise ValueError("new size must be of type Size") newsize = self.vg.align(newsize) log.debug("trying to set lv %s size to %s", self.name, newsize) @@ -2544,7 +2544,7 @@ class LVMLogicalVolumeDevice(LVMLogicalVolumeBase, LVMInternalLogicalVolumeMixin # space for it. A similar reasoning applies to shrinking the LV. if not self.exists and newsize > self.size and newsize > self.vg.free_space + self.vg_space_used: log.error("failed to set size: %s short", newsize - (self.vg.free_space + self.vg_space_used)) - raise errors.DeviceError("not enough free space in volume group") + raise ValueError("not enough free space in volume group") LVMLogicalVolumeBase._set_size(self, newsize) @@ -2910,7 +2910,7 @@ class LVMCache(Cache): spec.size = spec.pv.format.free space_to_assign -= spec.pv.format.free if space_to_assign > 0: - raise errors.DeviceError("Not enough free space in the PVs for this cache: %s short" % space_to_assign) + raise ValueError("Not enough free space in the PVs for this cache: %s short" % space_to_assign) @property def size(self): diff --git a/tests/unit_tests/devices_test/lvm_test.py b/tests/unit_tests/devices_test/lvm_test.py index 47613fdc..995c2da4 100644 --- a/tests/unit_tests/devices_test/lvm_test.py +++ b/tests/unit_tests/devices_test/lvm_test.py @@ -32,10 +32,10 @@ class LVMDeviceTest(unittest.TestCase): lv = LVMLogicalVolumeDevice("testlv", parents=[vg], fmt=blivet.formats.get_format("xfs")) - with six.assertRaisesRegex(self, errors.DeviceError, "lvm snapshot origin must be a logical volume"): + with six.assertRaisesRegex(self, ValueError, "lvm snapshot origin must be a logical volume"): LVMLogicalVolumeDevice("snap1", parents=[vg], origin=pv) - with six.assertRaisesRegex(self, errors.DeviceError, "only existing vorigin snapshots are supported"): + with six.assertRaisesRegex(self, ValueError, "only existing vorigin snapshots are supported"): LVMLogicalVolumeDevice("snap1", parents=[vg], vorigin=True) lv.exists = True @@ -60,7 +60,7 @@ class LVMDeviceTest(unittest.TestCase): pool = LVMLogicalVolumeDevice("pool1", parents=[vg], size=Size("500 MiB"), seg_type="thin-pool") thinlv = LVMLogicalVolumeDevice("thinlv", parents=[pool], size=Size("200 MiB"), seg_type="thin") - with six.assertRaisesRegex(self, errors.DeviceError, "lvm snapshot origin must be a logical volume"): + with six.assertRaisesRegex(self, ValueError, "lvm snapshot origin must be a logical volume"): LVMLogicalVolumeDevice("snap1", parents=[pool], origin=pv, seg_type="thin") # now make the constructor succeed so we can test some properties @@ -310,21 +310,21 @@ class LVMDeviceTest(unittest.TestCase): vg = LVMVolumeGroupDevice("testvg", parents=[pv, pv2]) # pvs have to be specified for non-linear LVs - with self.assertRaises(errors.DeviceError): + with self.assertRaises(ValueError): lv = LVMLogicalVolumeDevice("testlv", parents=[vg], size=Size("512 MiB"), fmt=blivet.formats.get_format("xfs"), exists=False, seg_type="raid1") - with self.assertRaises(errors.DeviceError): + with self.assertRaises(ValueError): lv = LVMLogicalVolumeDevice("testlv", parents=[vg], size=Size("512 MiB"), fmt=blivet.formats.get_format("xfs"), exists=False, seg_type="striped") # no or complete specification has to be given for linear LVs - with self.assertRaises(errors.DeviceError): + with self.assertRaises(ValueError): lv = LVMLogicalVolumeDevice("testlv", parents=[vg], size=Size("512 MiB"), fmt=blivet.formats.get_format("xfs"), exists=False, pvs=[pv]) - with self.assertRaises(errors.DeviceError): + with self.assertRaises(ValueError): pv_spec = LVPVSpec(pv, Size("256 MiB")) pv_spec2 = LVPVSpec(pv2, Size("250 MiB")) lv = LVMLogicalVolumeDevice("testlv", parents=[vg], size=Size("512 MiB"), -- 2.37.3