From: "David Lehman" <dlehman(a)redhat.com>
To: anaconda-patches(a)lists.fedorahosted.org
Sent: Friday, June 20, 2014 10:10:46 AM
Subject: Re: [blivet:master 10/11] If btrfs creation fails, try with fewer specified
options (#1109195)
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(a)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
_______________________________________________
anaconda-patches mailing list
anaconda-patches(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
We really do use None to mean "none specified", so, if it fails with
the given options, it fails with the options specified somehow, through
anaconda or directly through blivet.
I am withdrawing this patch entirely.
- mulhern