[f21-branch][PATCH] clean up stage1 volume check, check format as well as mountpoint
Anne Mulhern
amulhern at redhat.com
Mon Dec 1 14:51:54 UTC 2014
----- Original Message -----
> From: "Adam Williamson" <awilliam at redhat.com>
> To: anaconda-patches at lists.fedorahosted.org
> Sent: Friday, November 28, 2014 4:51:35 PM
> Subject: [f21-branch][PATCH] clean up stage1 volume check, check format as well as mountpoint
>
> This is a follow-on from e2d10a04. To reduce the change from
> the previous behaviour and minimize the chance of any strange
> corner-case breakage, this sorts the disk list, rather than
> filtering it.
>
> It also now checks the format_type constraint as well as the
> mountpoint constraint. There are platforms (PPC) which use a
> volume as the stage1 target, but don't require it to be mounted;
> this handles those as well as platforms which require the target
> volume to be mounted to a specific location.
>
> It's also slightly re-arranged to flow better with the existing
> check that filters out protected, hidden and unusable disks.
>
> The overall behaviour change can be described as: on platforms
> which require the stage1 target to be a volume, use the first
> enumerated disk which contains a potentially valid volume as
> the 'boot disk' if the user does not explicitly specify one,
> rather than just using the first enumerated disk and failing
> if it does not contain a valid stage1 target volume (#1168118).
>
> This also debug logs the result of the 'disks with potential
> stage1 targets' check.
> ---
> pyanaconda/kickstart.py | 36 ++++++++++++++++++++++--------------
> 1 file changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py
> index d44b349..4aa4fd8 100644
> --- a/pyanaconda/kickstart.py
> +++ b/pyanaconda/kickstart.py
> @@ -379,22 +379,30 @@ class Bootloader(commands.bootloader.F21_Bootloader):
> if self.timeout is not None:
> storage.bootloader.timeout = self.timeout
>
> - # try to reduce the set of possible 'boot disks' to the disks
> containing
> - # a valid stage1 mount point, if there are any, because those disks
> - # should be searched for a stage1_valid device
> - disks = storage.disks
> - if platform.bootStage1ConstraintDict["mountpoints"]:
> - disks = [p.disk for p in storage.devices if getattr(p.format,
> "mountpoint", None) in platform.bootStage1ConstraintDict["mountpoints"]]
> - if not disks:
> - # but if not, just go ahead and use the set of all disks or
> - # else we'll error out later
> - disks = storage.disks
> -
> # Throw out drives specified that don't exist or cannot be used
> (iSCSI
> # device on an s390 machine)
> - disk_names = [d.name for d in disks
> - if not d.format.hidden and not d.protected and
> - (not blivet.arch.isS390() or not isinstance(d,
> blivet.devices.iScsiDiskDevice))]
> + disks = [d for d in storage.disks
> + if not d.format.hidden and not d.protected and
> + (not blivet.arch.isS390() or not isinstance(d,
> blivet.devices.iScsiDiskDevice))]
> + if platform.bootStage1ConstraintDict["format_types"]:
> + # sort the list of disks with those that contain possibly-valid
> + # stage1 partitions first, so we'll pick a disk with one as
> + # bootDrive if it has not been explicitly set: RHBZ #1168118
> + s1vols = [vol for vol in storage.devices if
> + getattr(vol.format, "type", None) in
> + platform.bootStage1ConstraintDict["format_types"]]
> + if platform.bootStage1ConstraintDict["mountpoints"]:
> + # further sort by volumes with valid mount points, if
> + # the platform requires this
> + s1disks = [vol.disk for vol in s1vols if
> + getattr(vol.format, "mountpoint", None) in
> + platform.bootStage1ConstraintDict["mountpoints"]]
> + else:
> + s1disks = [vol.disk for vol in s1vols]
> + log.debug("Disks with potentially valid stage1 volumes: %s",
> + ' '.join([d.name for d in s1disks]))
> + disks.sort(key=lambda x: x in s1disks, reverse=True)
AFAICT the whole chunk of code above, other than the assignment to disks is a no-op.
The code goes to a lot of work to find this sldisks value, and then sorts disks using
that value. But then, the sortedness of the disks matters not a whit, since the sorted
names are just passed to set() which is unsorted by definition.
Do you mean filter, instead of sort, or is there another patch still un-applied
which makes use of the sorted disks value in the execute() method?
> + disk_names = [d.name for d in disks]
> diskSet = set(disk_names)
>
> for drive in self.driveorder[:]:
> --
> 2.1.0
>
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
>
- mulhern
More information about the anaconda-patches
mailing list