grub (boot on mdraid) upgrade patches for master

[Radek Vykydal]

Folow-up patches for commit 358a9a34f496fdd2b032edf12274a146b606d1a4
(only in master).

When doing bootloader (grub) update during upgrade (especially with boot
on mdraid device):

- Install it by the same code path as when doing install (install was
  fixed beforementioned commit).
- Remove code duplication that was done when detecting where grub
  had been installed.
- Handle grub stage1 destination that had been written incorrectly
  in releases F11 and earlier (real destination for /dev/mdX were mbrs).
  I am not 100% sure about it - see patch [4/4] comment.

[PATCH 1/4] Remove target parameter from grub installation code - it is no more needed.

[Radek Vykydal]

This is one of follow-up commits for commit
358a9a34f496fdd2b032edf12274a146b606d1a4, dealing with upgrade.
---
 booty/bootloaderInfo.py |    3 +--
 booty/x86.py            |   14 +++-----------
 2 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/booty/bootloaderInfo.py b/booty/bootloaderInfo.py
index ba9d442..36cfb42 100644
--- a/booty/bootloaderInfo.py
+++ b/booty/bootloaderInfo.py
@@ -626,8 +626,7 @@ class efiBootloaderInfo(bootloaderInfo):
                                     stderr = "/dev/tty5")
         return rc

-    def installGrub(self, instRoot, bootDev, grubTarget, grubPath,
-                    target, cfPath):
+    def installGrub(self, instRoot, bootDev, grubTarget, grubPath, cfPath):
         if not iutil.isEfi():
             raise EnvironmentError
         rc = self.removeOldEfiEntries(instRoot)
diff --git a/booty/x86.py b/booty/x86.py
index 9519e82..407f7c9 100644
--- a/booty/x86.py
+++ b/booty/x86.py
@@ -115,11 +115,10 @@ class x86BootloaderInfo(efiBootloaderInfo):
                updatedMatches.append((getDiskPart(mdBootPart, self.storage)[0], mdBootPart))
         return updatedMatches

-    def installGrub(self, instRoot, bootDev, grubTarget, grubPath,
-                    target, cfPath):
+    def installGrub(self, instRoot, bootDev, grubTarget, grubPath, cfPath):
         if iutil.isEfi():
             return efiBootloaderInfo.installGrub(self, instRoot, bootDev, grubTarget,
-                                                 grubPath, target, cfPath)
+                                                 grubPath, cfPath)

args = "--stage2=/boot/grub/stage2 "

@@ -214,12 +213,6 @@ class x86BootloaderInfo(efiBootloaderInfo):
             os.rename(cf, cf + '.rpmsave')

grubTarget = bl.getDevice()
-        targetDevice = self.storage.devicetree.getDeviceByName(grubTarget)
-        path = targetDevice.path[5:]
-        if targetDevice.type == "partition" or targetDevice.type == "mdarray":
-            target = "partition"
-        else:
-            target = "mbr"

f = open(cf, "w+")

@@ -419,8 +412,7 @@ class x86BootloaderInfo(efiBootloaderInfo):
         f.close()

if not justConfigFile:
-            return self.installGrub(instRoot, bootDev, grubTarget, grubPath,
-                             target, cfPath)
+            return self.installGrub(instRoot, bootDev, grubTarget, grubPath, cfPath)

return 0

[PATCH 2/4] Remove code duplication, use fixed code from writeGrub.

[Radek Vykydal]

This is one of follow-up commits for commit
358a9a34f496fdd2b032edf12274a146b606d1a4, dealing with upgrade.
---
 booty/x86.py |   47 +++--------------------------------------------
 1 files changed, 3 insertions(+), 44 deletions(-)

diff --git a/booty/x86.py b/booty/x86.py
index 407f7c9..ce446d2 100644
--- a/booty/x86.py
+++ b/booty/x86.py
@@ -527,56 +527,15 @@ class x86BootloaderInfo(efiBootloaderInfo):
         # so we have to do shenanigans to get updated grub installed...
         # steal some more code above
         try:
-            bootDev = self.storage.mountpoints["/boot"].name
+            bootDev = self.storage.mountpoints["/boot"]
             grubPath = "/grub"
             cfPath = "/"
         except KeyError:
-            bootDev = self.storage.rootDevice.name
+            bootDev = self.storage.rootDevice
             grubPath = "/boot/grub"
             cfPath = "/boot/"

-        masterBootDev = bootDev
-        if masterBootDev[0:2] == 'md':
-            rootDevs = checkbootloader.getRaidDisks(masterBootDev,
-                            self.storage, raidLevel=1, stripPart=0)
-        else:
-            rootDevs = [masterBootDev]
-
-        if theDev[5:7] == 'md':
-            stage1Devs = checkbootloader.getRaidDisks(theDev[5:], self.storage,
-                              raidLevel=1)
-        else:
-            stage1Devs = [theDev[5:]]
-
-        cmds = []
-        for stage1Dev in stage1Devs:
-            # cross fingers; if we can't find a root device on the same
-            # hardware as this boot device, we just blindly hope the first
-            # thing in the list works.
-
-            grubbyStage1Dev = self.grubbyPartitionName(stage1Dev)
-
-            grubbyRootPart = self.grubbyPartitionName(rootDevs[0])
-
-            for rootDev in rootDevs:
-                testGrubbyRootDev = getDiskPart(rootDev, self.storage)[0]
-                testGrubbyRootDev = self.grubbyPartitionName(testGrubbyRootDev)
-
-                if grubbyStage1Dev == testGrubbyRootDev:
-                    grubbyRootPart = self.grubbyPartitionName(rootDev)
-                    break
-                    
-            args = "--stage2=/boot/grub/stage2 "
-            cmd ="root %s\n" % (grubbyRootPart,)
-            cmd += "install %s%s/stage1 d %s %s/stage2 p %s%s/grub.conf" \
-                % (args, grubPath, grubbyStage1Dev, grubPath, grubbyRootPart,
-                   grubPath)
-            cmds.append(cmd)
-        
-        if not justConfigFile:
-            return self.runGrubInstall(instRoot, bootDev, cmds, cfPath)
- 
-        return 0
+        return self.installGrub(instRoot, bootDev, theDev, grubPath, cfPath)

def writeSysconfig(self, instRoot, installDev):
         sysconf = '/etc/sysconfig/grub'

[PATCH 3/4] Remove another code duplication in grub upgrade code.

[Radek Vykydal]

This is one of follow-up commits for commit
358a9a34f496fdd2b032edf12274a146b606d1a4, dealing with upgrade.
---
 bootloader.py |    3 +--
 booty/x86.py  |   25 ++++---------------------
 2 files changed, 5 insertions(+), 23 deletions(-)

diff --git a/bootloader.py b/bootloader.py
index 3960851..54a37b5 100644
--- a/bootloader.py
+++ b/bootloader.py
@@ -126,8 +126,7 @@ def writeBootloader(anaconda):
     if anaconda.id.bootloader.defaultDevice == -1:
         return

-    # now make the upgrade stuff work for kickstart too. ick.
-    if anaconda.isKickstart and anaconda.id.bootloader.doUpgradeOnly:
+    if anaconda.id.bootloader.doUpgradeOnly:
         (bootType, theDev) = checkbootloader.getBootloaderTypeAndBoot(anaconda.rootPath, storage=anaconda.id.storage)

anaconda.id.bootloader.doUpgradeonly = 1
diff --git a/booty/x86.py b/booty/x86.py
index ce446d2..9106bb0 100644
--- a/booty/x86.py
+++ b/booty/x86.py
@@ -495,30 +495,13 @@ class x86BootloaderInfo(efiBootloaderInfo):
         if justConfigFile:
             return ""

-        theDev = None
-        for (fn, stanza) in [ ("/etc/sysconfig/grub", "boot="),
-                              ("/boot/grub/grub.conf", "#boot=") ]:
-            try:
-                f = open(instRoot + fn, "r")
-            except:
-                continue
-        
-            # the following bits of code are straight from checkbootloader.py
-            lines = f.readlines()
-            f.close()
-            for line in lines:
-                if line.startswith(stanza):
-                    theDev = checkbootloader.getBootDevString(line)
-                    break
-            if theDev is not None:
-                break
+        grubTarget = bl.getDevice()

-        if theDev is None:
-            # we could find the dev before, but can't now...  cry about it
+        if grubTarget is None:
             return ""

# migrate info to /etc/sysconfig/grub
-        self.writeSysconfig(instRoot, theDev)
+        self.writeSysconfig(instRoot, grubTarget)

# update device.map
         self.updateDeviceMap(instRoot)
@@ -535,7 +518,7 @@ class x86BootloaderInfo(efiBootloaderInfo):
             grubPath = "/boot/grub"
             cfPath = "/boot/"

-        return self.installGrub(instRoot, bootDev, theDev, grubPath, cfPath)
+        return self.installGrub(instRoot, bootDev, grubTarget, grubPath, cfPath)

def writeSysconfig(self, instRoot, installDev):
         sysconf = '/etc/sysconfig/grub'

[PATCH 4/4] Fix upgrade of GRUB with md raid boot for versions F11 and earlier.

[Radek Vykydal]

Up to F11 /dev/mdX in /etc/sysconfig/grub means that grub was installed into
mbrs of all /boot md device members' disks. When upgrading grub of such a
system, reinstall grub in the mbrs, and update values sysconfig file
accordingly (to hold e.g. /dev/sda). Here I'm not completly sure. What if after
installation the user reinstalled grub manually - restored mbrs and installed
it in /dev/mdX?  He wouldn't be happy with our grub update then...

This is one of follow-up commits for commit
358a9a34f496fdd2b032edf12274a146b606d1a4, dealing with upgrade.
---
 bootloader.py |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/bootloader.py b/bootloader.py
index 54a37b5..729fe2c 100644
--- a/bootloader.py
+++ b/bootloader.py
@@ -29,6 +29,8 @@ import string
 from flags import flags
 from constants import *
 from storage.devices import devicePathToName
+from storage import getReleaseString
+from booty.util import getDiskPart

import gettext
 _ = lambda x: gettext.ldgettext("anaconda", x)
@@ -115,6 +117,22 @@ def bootloaderSetupChoices(anaconda):
     elif choices and choices.has_key("boot"):
         anaconda.id.bootloader.setDevice(choices["boot"][0])

+def fixedMdraidGrubTarget(anaconda, grubTarget):
+    # handle change made in F12 - before F12 mdX used to mean installation
+    # into mbrs of mdX members' disks
+    fixedGrubTarget = grubTarget
+    (product, version) = getReleaseString(anaconda.rootPath)
+    try:
+        if float(version) < 12:
+            stage1Devs = anaconda.id.bootloader.getPhysicalDevices(grubTarget)
+            fixedGrubTarget = getDiskPart(stage1Devs[0], anaconda.id.storage)[0]
+            log.info("Mdraid grub upgrade: %s -> %s" % (grubTarget,
+                                                       fixedGrubTarget))
+    except ValueError:
+        log.warning("Can't decide mdraid grub upgrade fix, product: %s, version: %s" % (product, version))
+
+    return fixedGrubTarget
+
 def writeBootloader(anaconda):
     def dosync():
         isys.sync()
@@ -131,6 +149,9 @@ def writeBootloader(anaconda):

anaconda.id.bootloader.doUpgradeonly = 1
         if bootType == "GRUB":
+            if theDev.startswith('/dev/md'):
+                theDev = fixedMdraidGrubTarget(anaconda,
+                                               devicePathToName(theDev))
             anaconda.id.bootloader.useGrubVal = 1
             anaconda.id.bootloader.setDevice(devicePathToName(theDev))
         else:

[PATCH 4/4] Fix upgrade of GRUB with md raid boot for versions F11 and earlier.

[Peter Jones]

On 10/19/2009 11:54 AM, Radek Vykydal wrote:
> Up to F11 /dev/mdX in /etc/sysconfig/grub means that grub was installed into
> mbrs of all /boot md device members' disks. When upgrading grub of such a
> system, reinstall grub in the mbrs, and update values sysconfig file
> accordingly (to hold e.g. /dev/sda). Here I'm not completly sure. What if after
> installation the user reinstalled grub manually - restored mbrs and installed
> it in /dev/mdX?  He wouldn't be happy with our grub update then...

This seems okay to me. If the user is smart enough to be fixing things manually,
they should be fixing them right ;). Also, Why would a user do this? It's not like
there are BIOSes that understand linux's raid format and boot it.
> This is one of follow-up commits for commit
> 358a9a34f496fdd2b032edf12274a146b606d1a4, dealing with upgrade.
>
> bootloader.py |   21 +++++++++++++++++++++
>  1 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/bootloader.py b/bootloader.py
> index 54a37b5..729fe2c 100644
> --- a/bootloader.py
> +++ b/bootloader.py
> @@ -29,6 +29,8 @@ import string
>  from flags import flags
>  from constants import *
>  from storage.devices import devicePathToName
> +from storage import getReleaseString
> +from booty.util import getDiskPart
>
> import gettext
>  _ = lambda x: gettext.ldgettext("anaconda", x)
> @@ -115,6 +117,22 @@ def bootloaderSetupChoices(anaconda):
>      elif choices and choices.has_key("boot"):
>          anaconda.id.bootloader.setDevice(choices["boot"][0])
>
> +def fixedMdraidGrubTarget(anaconda, grubTarget):
> # handle change made in F12 - before F12 mdX used to mean installation
> # into mbrs of mdX members' disks
> fixedGrubTarget = grubTarget
> (product, version) = getReleaseString(anaconda.rootPath)
> try:
>    if float(version) < 12:
>        stage1Devs = anaconda.id.bootloader.getPhysicalDevices(grubTarget)
>        fixedGrubTarget = getDiskPart(stage1Devs[0], anaconda.id.storage)[0]
>        log.info("Mdraid grub upgrade: %s -> %s" % (grubTarget,
>                                                   fixedGrubTarget))
> except ValueError:
>    log.warning("Can't decide mdraid grub upgrade fix, product: %s, version: %s" % (product, version))
> return fixedGrubTarget
>
> def writeBootloader(anaconda):
>      def dosync():
>          isys.sync()
> @@ -131,6 +149,9 @@ def writeBootloader(anaconda):
>      anaconda.id.bootloader.doUpgradeonly = 1
>      if bootType == "GRUB":
>        if theDev.startswith('/dev/md'):
>            theDev = fixedMdraidGrubTarget(anaconda,
>                                           devicePathToName(theDev))
>        anaconda.id.bootloader.useGrubVal = 1
>        anaconda.id.bootloader.setDevice(devicePathToName(theDev))
>    else:

[PATCH 4/4] Fix upgrade of GRUB with md raid boot for versions F11 and earlier.

[Bruno Wolff III]

On Mon, Oct 19, 2009 at 17:05:47 -0400,
  Peter Jones  wrote:
> On 10/19/2009 11:54 AM, Radek Vykydal wrote:
> > Up to F11 /dev/mdX in /etc/sysconfig/grub means that grub was installed into
> > mbrs of all /boot md device members' disks. When upgrading grub of such a
> > system, reinstall grub in the mbrs, and update values sysconfig file
> > accordingly (to hold e.g. /dev/sda). Here I'm not completly sure. What if after
> > installation the user reinstalled grub manually - restored mbrs and installed
> > it in /dev/mdX?  He wouldn't be happy with our grub update then...
>
> This seems okay to me. If the user is smart enough to be fixing things manually,
> they should be fixing them right ;). Also, Why would a user do this? It's not like
> there are BIOSes that understand linux's raid format and boot it.

mdraid 1 partitions are bootable (with the default location of the metadata).
A mirrored /boot partition is something I had for a long time. In that
situation you want grub installed on each of the array disks, so that if
you lose one you can still boot without having to track down a rescue disk
to run grub off of.

[PATCH 4/4] Fix upgrade of GRUB with md raid boot for versions F11 and earlier.

[Radek Vykydal]

Peter Jones wrote:
> On 10/19/2009 11:54 AM, Radek Vykydal wrote:
> > Up to F11 /dev/mdX in /etc/sysconfig/grub means that grub was installed into
> > mbrs of all /boot md device members' disks. When upgrading grub of such a
> > system, reinstall grub in the mbrs, and update values sysconfig file
> > accordingly (to hold e.g. /dev/sda). Here I'm not completly sure. What if after
> > installation the user reinstalled grub manually - restored mbrs and installed
> > it in /dev/mdX?  He wouldn't be happy with our grub update then...
>
> This seems okay to me. If the user is smart enough to be fixing things manually,
> they should be fixing them right ;). Also, Why would a user do this? It's not like
> there are BIOSes that understand linux's raid format and boot it.

I was rather thinking about having another bootloader in mbr and doing
chain boot from an array member (or array if supported by the bootloader),
though I don't know if it would work or if we support it, or if people 
are using it.
Anyway, we offer installation of grub into /dev/mdX boot partition.

[Hans de Goede]

Hi,

The entire set looks ok to me,

Regards,

Hans

On 10/19/2009 05:54 PM, Radek Vykydal wrote:
> Folow-up patches for commit 358a9a34f496fdd2b032edf12274a146b606d1a4
> (only in master).
>
> When doing bootloader (grub) update during upgrade (especially with boot
> on mdraid device):
> Install it by the same code path as when doing install (install was
>  fixed beforementioned commit).
> Remove code duplication that was done when detecting where grub
>  had been installed.
> Handle grub stage1 destination that had been written incorrectly
>  in releases F11 and earlier (real destination for /dev/mdX were mbrs).
>  I am not 100% sure about it - see patch [4/4] comment.
>
> Anaconda-devel-list mailing list
> [e-mail]
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list