[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



Hi,

On 06/29/2009 12:44 PM, Radek Vykydal wrote:
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):


Thanks!



---
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.


Ah, good catch, looking further I cannot help but notice, that for the
other use of gtPart, one can just as well use bootDev, as getMatchingPart()
searches for the first physical device which is on the same disk
as bootDev, and then we use getDiskPart() to get only the diskname, so
if we just pass in gtPart directly, nothing changes.

I was looking at this, because if we do this, we can then get rid of
the gtPart variable and of the getMatchingPart() method (which
seems to be a bullshit method anyways).

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"


I'm afraid this check is not 100% bullet proof, when we have the
key iswmd, we can still be installing grub on a partition, or despite
the presence of the flag, the mdraid device node path, may actually point
to a regular mdraid set, not one using isw metadata.

Regards,

Hans


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]