[f21-branch][PATCH] clean up stage1 volume check, check format as well as mountpoint

Adam Williamson awilliam at redhat.com
Thu Dec 4 00:54:49 UTC 2014


On Mon, 2014-12-01 at 15:29 -0500, Anne Mulhern wrote:
> Newest version of patch attached.
> 
> Basic changes from previous are:
> 
> * Don't bother to sorts disk names before passing to set().
> * Only calculate s1disks if needed for designating bootDrive.
> * Don't sort the disks since we only need a best.
> * Treat constraints as if they are orthogonal, in case sometime it turns out
> that they are.
> 
> Assumes that disks has at least one element; so did the previous code.

So the F21 ship has sailed now, just wanted to follow up on the status
of the changes that originated from me as regards F22.

I know dlehman had some concerns, but I do still believe this is the
right thing to do *at least* for the mount point constraint case, for
master. That one seems clear cut to me. The logic is: if we hit
Bootloader.execute without self.bootDrive explicitly set, pick a disk
that has a valid stage1 mountpoint on it as bootDrive if possible.

I just can't see anything wrong with that, regardless of what else we
change with bootloaders. We're already choosing a bootDrive ourselves in
this case, we're just doing it badly. I can't see any way in which this
logic for doing it is not an improvement. If the user has set a valid
stage1 target mountpoint for a partition, it seems inarguably the most
logical assumption that what they want us to do is use it as the stage1
target. This isn't the perfect fix to all bootloader issues ever, but I
really think it's a sensible improvement for a significant case.

The case where there's a format constraint but not a mountpoint
constraint - i.e. the stage1 target is required to be a partition of a
particular type, but not with a particular mount point - is more
arguable, I think. In some cases on that path we definitely don't have
any clear expectation on the user's part, and we're basically guessing,
and changing the logic of how we guess may be a surprise to someone.
Maybe they've got used to us creating a new boot partition on the first
selected disk and using it as the stage1 target, in that case (which is,
I think, what we do on the guided path, there). If we started preferring
a disk with an existing boot partition, that might be an unexpected
surprise.

For the record I did go and look at what the official documentation
claims we do, to see if it matches what we actually do, and I'm pretty
sure it's just wrong. It claims we install the bootloader on the same
disk as the root partition if the user doesn't specify a boot target
disk:

====

https://docs.fedoraproject.org/en-US/Fedora/20/html/Installation_Guide/s1-x86-bootloader.html

"The installation program installs GRUB in the master boot record, or
MBR, of the device for the root file system."

https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Installation_Guide/sect-disk-partitioning-setup-x86.html#sect-bootloader-x86

"The installation program installs GRUB2 either in the master boot
record (MBR) or the GUID partition table (GPT) of the device for the
root file system."

====

That's wrong, we don't do that (maybe we used to?) If you do an install
to two disks, and put all the Fedora partitions on the second, we'll
(try and) write the bootloader to the first disk if you don't explicitly
choose the second. So, with the patch we would not conform to the
documented behaviour, but then we already don't. =)
-- 
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