[f21-branch][PATCH] Search for a valid stage1 device on disks with stage1 mount points (#1168118)

Adam Williamson awilliam at redhat.com
Fri Nov 28 17:11:21 UTC 2014


On Fri, 2014-11-28 at 12:55 +0100, Vratislav Podzimek wrote:

> > Looks good to me, ACK!
> Thanks. Pushed to f21-branch, open for discussion for master and
> rhel7-branch.

I was actually expecting you folks to be stricter on this! I really
intended it as a prototype / PoC patch. It is slightly ugly, and I did
sort of think that sorting the disks list might possibly be safer
against some odd case where there's one disk with an apparently-valid
mount point on it but it gets thrown out by the next check or
something. 

Would you find something like this better?

        if platform.bootStage1ConstraintDict["mountpoints"]:
            # sort the list of disks with those that contain possibly-valid
            # stage1 mountpoints first, so we'll wind up picking a disk with
            # one on it as bootDrive: RHBZ #1168118
            s1disks = [part.disk for part in storage.devices if
                       getattr(part.format, "mountpoint", None) in
                       platform.bootStage1ConstraintDict["mountpoints"]]
            disks = sorted(storage.disks, key=s1disks.count, reverse=True)
        else:
            disks = storage.disks

That's untested (except with a little test script to check at least the
concept of reverse-sorting by count in another list works), I'm about to
go and test it now.

I also am a bit worried about the platforms that use non-mounted
partitions for stage1, I think it's reasonable to assume that they have
the same issue, though as I said I haven't managed to test yet. We might
want to do something like:

        if platform.bootStage1ConstraintDict["format_types"]:
            # filter the list of disks with those that contain possibly-valid
            # stage1 partitions first, so we'll wind up picking a disk with
            # one on it as bootDrive: 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 filter 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]
            disks = sorted(storage.disks, key=lambda x: s1disks.count(x) > 0, reverse=True)
        else:
            disks = storage.disks

to cover both cases. The use of a lambda function is to cover a case
where, say, the first disk has one possibly-valid format_type and the
second disk has two, I think it's probably more conservative to not sort
based on the total amount of such partitions on the disk so we still
default to the first disk there, though looking through the list of
platforms and considering possible scenarios, I think it'd be pretty
rare for this to matter. omapARM uses a fairly common partition type -
vfat - but it also requires it to be mounted at /boot/uboot , so in
practice we're always going to reduce the set to 'the disk with a
vfat /boot/uboot on it' if there is one. The platforms which need a
format_type but no mountpoint all use fairly odd format_types that
wouldn't typically show up more than once per disk at most.
-- 
Adam Williamson
Fedora QA Community Monkey
IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT happyassassin . net
http://www.happyassassin.net



More information about the anaconda-patches mailing list