[blivet:rhel7/master 2/2] Check if device has enough members when setting RAID level (#1019685)

Vratislav Podzimek vpodzime at redhat.com
Fri Sep 12 06:30:26 UTC 2014


On Thu, 2014-09-11 at 13:34 -0400, mulhern wrote:
> Related: rhbz#1019685
> 
> RAID levels have minimum member requirements. Level setters now raise a
> BTRFSValueError if device does not have enough members to meet the minimum
> requirement. However, if the device exists its members may not be known
> yet, so the check is omitted.
> 
> Also, RaidError is rethrown as BTRFSValueError in level setter if the level
> is not valid for its use. Previously the RaidError was just propagated.
> 
> Signed-off-by: mulhern <amulhern at redhat.com>
> ---
>  blivet/devices.py | 62 ++++++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 52 insertions(+), 10 deletions(-)
> 
> diff --git a/blivet/devices.py b/blivet/devices.py
> index fe87226..0e02396 100644
> --- a/blivet/devices.py
> +++ b/blivet/devices.py
> @@ -5127,6 +5127,50 @@ class BTRFSVolumeDevice(BTRFSDevice, ContainerDevice):
>  
>          self._defaultSubVolumeID = None
>  
> +    def _validateRaidLevel(self, level):
> +        """ Returns an error message if the RAID level is invalid for this
> +            device, otherwise None.
> +
> +            :param level: a RAID level
> +            :type level: :class:`~.devicelibs.raid.RAIDLevel`
> +            :returns: an error message if the RAID level is invalid, else None
> +            :rtype: str or NoneType
> +        """
> +        if not self.exists and len(self.parents) < level.min_members:
> +            return "RAID level %s requires that device have at least %d members, but device has only %d members." % (level, level.min_members, len(self.parents))
> +        return None
> +
> +    def _setLevel(self, value, data):
> +        """ Sets a valid level for this device and level type.
> +
> +            :param object value: value for this RAID level
> +            :param bool data: True if for data, False if for metadata
> +
> +            :returns: a valid level for value, if any, else None
> +            :rtype: :class:`~.devicelibs.raid.RAIDLevel` or NoneType
> +
> +            :raises: :class:`~.errors.BTRFSValueError` if value represents
> +            an invalid level.
> +        """
> +        level = None
> +        if value:
> +            try:
> +                levels = btrfs.RAID_levels if data else btrfs.metadata_levels
> +                level = levels.raidLevel(value)
> +            except errors.RaidError:
> +                data_type_str = "data" if data else "metadata"
> +                raise errors.BTRFSValueError("%s is an invalid value for %s RAID level." % (value, data_type_str))
> +
> +            error_msg = self._validateRaidLevel(level)
> +            if error_msg:
> +                raise errors.BTRFSValueError(error_msg)
> +
> +        # pylint: disable=attribute-defined-outside-init
Why not to define these in __init__?

> +        if data:
> +            self._dataLevel = level
> +        else:
> +            self._metaDataLevel = level
> +
>      @property
>      def dataLevel(self):
>          """ Return the RAID level for data.
> @@ -5140,12 +5184,11 @@ class BTRFSVolumeDevice(BTRFSDevice, ContainerDevice):
>      def dataLevel(self, value):
>          """ Set the RAID level for data.
>  
> -            :param value: new raid level
> -            :param type:  a valid raid level descriptor
> -            :returns:     None
> +            :param object value: new raid level
> +            :returns: None
> +            :raises: :class:`~.errors.BTRFSValueError`
>          """
> -        # pylint: disable=attribute-defined-outside-init
> -        self._dataLevel = btrfs.RAID_levels.raidLevel(value) if value else None
> +        self._setLevel(value, True)
>  
>      @property
>      def metaDataLevel(self):
> @@ -5160,12 +5203,11 @@ class BTRFSVolumeDevice(BTRFSDevice, ContainerDevice):
>      def metaDataLevel(self, value):
>          """ Set the RAID level for metadata.
>  
> -            :param value: new raid level
> -            :param type:  a valid raid level descriptor
> -            :returns:     None
> +            :param object value: new raid level
> +            :returns: None
> +            :raises: :class:`~.errors.BTRFSValueError`
>          """
> -        # pylint: disable=attribute-defined-outside-init
> -        self._metaDataLevel = btrfs.metadata_levels.raidLevel(value) if value else None
> +        self._setLevel(value, False)
>  
>      @property
>      def formatImmutable(self):

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list