[blivet:master 10/11] If btrfs creation fails, try with fewer specified options (#1109195)

David Lehman dlehman at redhat.com
Fri Jun 20 14:10:46 UTC 2014


On 06/19/2014 11:52 AM, mulhern wrote:
> Resolves: fed#1109195
>
> At the point where the filesystem is being created failure because btrfs
> is not comfortable with the options specified is worse than success
> w/out those particular options.
>
> Signed-off-by: mulhern <amulhern at redhat.com>
> ---
>   blivet/devices.py | 47 +++++++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 43 insertions(+), 4 deletions(-)
>
> diff --git a/blivet/devices.py b/blivet/devices.py
> index 4a485e8..4b41f3e 100644
> --- a/blivet/devices.py
> +++ b/blivet/devices.py
> @@ -27,6 +27,7 @@ import tempfile
>   import abc
>   from decimal import Decimal
>   import re
> +import itertools
>
>   # device backend modules
>   from .devicelibs import mdraid
> @@ -5220,10 +5221,48 @@ class BTRFSVolumeDevice(BTRFSDevice, ContainerDevice):
>
>       def _create(self):
>           log_method_call(self, self.name, status=self.status)
> -        btrfs.create_volume(devices=[d.path for d in self.parents],
> -                            label=self.format.label,
> -                            data=self.dataLevel,
> -                            metadata=self.metaDataLevel)
> +
> +        options = [
> +           ("label", self.format.label),
> +           ("data", self.dataLevel),
> +           ("metadata", self.metaDataLevel)
> +        ]
> +
> +        log.info(
> +           "attempting to create device %s with specified options %s",
> +           self,
> +           dict(options))
> +
> +        # Initially, try to create the volume with all non-None options.
> +        # If that fails, try all possible combinations of non-None options.
> +        # This is a workable approach because:
> +        # - The options are independent
> +        # - btrfs.create_volume treats None as none specified
> +        # This strategy maximizes the number of specified options that are
> +        # actually used in the creation of the volume.
> +        # It does not prefer one combination of the same length to another;
> +        # that choice is more or less random.
> +        options = dict((n,v) for n, v in options if v is not None)
> +        option_names = options.keys()
> +        option_choices = itertools.chain(*(itertools.combinations(option_names, n) for n in range(len(option_names), -1, -1)))
> +        kwargs_choices = (dict(map(lambda n: (n, options[n]), names)) for names in option_choices)
> +
> +        for kwargs in kwargs_choices:
> +            log.info(
> +               "attempting to create device %s with options %s",
> +               self,
> +               kwargs)
> +            try:
> +                btrfs.create_volume(
> +                   devices=[d.path for d in self.parents],
> +                   **kwargs)
> +            except errors.BTRFSError:
> +                log.info("failed to create device")
> +            else:
> +                log.info("created device")
> +                return
> +
> +        raise errors.DeviceCreateError("failed to create device %s" % self)
>
>       def _postCreate(self):
>           super(BTRFSVolumeDevice, self)._postCreate()
>

I am not comfortable with dropping/changing any user-specified option. 
If the problem is our own default, then I am fine with dropping that. I 
think it would make more sense to somehow differentiate between internal 
defaults and user-supplied levels, but that could mean values for 
raidLevel and metaDataLevel that are not (real) raid levels 
(DefaultRAIDLevel?). Another option is to encode this special case and 
any others that come along. My preference is for the former option 
(default -v- user-specified).

Even if you did use this game of roulette (as vpodzime called it IIRC) I 
would not include label in the game.

David


More information about the anaconda-patches mailing list