Re: [PATCH] Enable support for bootloader on md RAID1 partitions.

[Radek Vykydal <rvykydal redhat com>]

Hans de Goede wrote:
> Hi,
>
> Can you explain in a bit more detail, what (and how) is exactly going 
> wrong
> at the moment, and how your patch fixes this ?
>
> IOW how did you come to the conclusion that even if you select 
> partition that
> grub will end up on the mbr ?  And if this was through testing, have you
> also been able to reconstruct the code path leading to this behaviour ?

For F11, I tested it and the code path is described below. For F10, I 
didn't look at the code, just tested it.

I noticed it first it when I was fixing upgrade of grub on /dev/md0 in 
F11 (bz #505966), which counts the fact that we install on mbr for /dev/md0.

> And can you explain what that code path looks like ?

Yes, see comments to the patch (in order):

> > ---
> >   booty/x86.py |    4 +++-
> >   1 files changed, 3 insertions(+), 1 deletions(-)
> >
> > diff --git a/booty/x86.py b/booty/x86.py
> > index d0972c8..09eb665 100644
> > --- a/booty/x86.py
> > +++ b/booty/x86.py
> > @@ -120,7 +120,7 @@ class x86BootloaderInfo(efiBootloaderInfo):
> >
> >               stage1Target = gtDisk

2) here, we are in a loop for boot devices (e.g. sda2, sda3 for 
/dev/md0), and for target mbr the variable to install stage1 on is set 
to mbr (e.g. would be installed twice on sda mbr for /dev/md0 on sda2 
and sda3)

> >               if target == "partition":
> > -                stage1Target = self.grubbyPartitionName(gtPart)
> > +                stage1Target = bPart

3) and here (the path code we didn't reach for /dev/md* as target was 
always set to mbr in 1)) we would install stage 1 to gtPart (obtained by 
getMatchingPart function) which is e.g. sda2 for sda2, and sda2 for sda3 
- so we would install twice on the frirst raid member sda2 without the 
patch.

> >               cmd += "install %s%s/stage1 d %s %s/stage2 p 
> > %s%s/grub.conf" % \
> >                   (args, grubPath, stage1Target, grubPath, bPart, 
> > grubPath)
> > @@ -158,6 +158,8 @@ class x86BootloaderInfo(efiBootloaderInfo):
> >                       target = "partition"
> >           elif grubTarget[-1].isdigit() and not path.startswith('md'):
> >               target = "partition"

1) here we kept target set to mbr for grub target /dev/md*

> > +        elif path.startswith('md') and not 
> > flags.cmdline.has_key("iswmd"):
> > +            target = "partition"
> >
> >           f = open(cf, "w+")

It is possible that I am simplifying too much and missing something 
because I was thinking about and testing only case when all raid boot 
part members are on one disk.

Radek