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

Re: [PATCH] Catch errors from bootloader installation and tell the user (#502210).



Looks fine.

On 06/05/2009 05:00 AM, Chris Lumens wrote:
---
  bootloader.py           |   11 ++++++--
  booty/alpha.py          |   26 ++++++++++++-------
  booty/bootloaderInfo.py |   37 ++++++++++++++++++----------
  booty/ia64.py           |   18 +++++++------
  booty/ppc.py            |   28 +++++++++++++--------
  booty/s390.py           |   25 +++++++++++-------
  booty/sparc.py          |   18 +++++++------
  booty/x86.py            |   61 +++++++++++++++++++++++++++-------------------
  8 files changed, 136 insertions(+), 88 deletions(-)

diff --git a/bootloader.py b/bootloader.py
index 0c28c8a..eb28e0a 100644
--- a/bootloader.py
+++ b/bootloader.py
@@ -199,11 +199,16 @@ def writeBootloader(anaconda):

      dosync()
      try:
-        anaconda.id.bootloader.write(anaconda.rootPath, anaconda.id.bootloader,
-                                     kernelList, otherList, defaultDev,
-                                     justConfigFile)
+        rc = anaconda.id.bootloader.write(anaconda.rootPath, anaconda.id.bootloader,
+                                          kernelList, otherList, defaultDev,
+                                          justConfigFile)
  	if not justConfigFile:
  	    w.pop()
+
+        if rc and anaconda.intf:
+            anaconda.intf.messageWindow(_("Warning"),
+                               _("There was an error installing the bootloader.  "
+                                 "Your system may not be bootable."))
      except booty.BootyNoKernelWarning:
  	if not justConfigFile:
  	    w.pop()
diff --git a/booty/alpha.py b/booty/alpha.py
index efe4446..33cef82 100644
--- a/booty/alpha.py
+++ b/booty/alpha.py
@@ -111,10 +111,12 @@ class alphaBootloaderInfo(bootloaderInfo):
              # to and the second argument is a path to the bootstrap loader
              # file.
              args = [("/dev/%s" % wbd), "/boot/bootlx"]
-            iutil.execWithRedirect ('/sbin/swriteboot', args,
-                                    root = instRoot,
-                                    stdout = "/dev/tty5",
-                                    stderr = "/dev/tty5")
+            rc = iutil.execWithRedirect ('/sbin/swriteboot', args,
+                                         root = instRoot,
+                                         stdout = "/dev/tty5",
+                                         stderr = "/dev/tty5")
+            if rc:
+                return rc

              # Calling abootconf to configure the installed aboot. The
              # first argument is the disk to use, the second argument is
@@ -122,10 +124,14 @@ class alphaBootloaderInfo(bootloaderInfo):
              # It's always the boot partition whether it's / or /boot (with
              # the mount point being omitted.)
              args = [("/dev/%s" % wbd), str (bdpn)]
-            iutil.execWithRedirect ('/sbin/abootconf', args,
-                                    root = instRoot,
-                                    stdout = "/dev/tty5",
-                                    stderr = "/dev/tty5")
+            rc = iutil.execWithRedirect ('/sbin/abootconf', args,
+                                         root = instRoot,
+                                         stdout = "/dev/tty5",
+                                         stderr = "/dev/tty5")
+            if rc:
+                return rc
+
+        return 0


      def write(self, instRoot, bl, kernelList, chainList,
@@ -133,8 +139,8 @@ class alphaBootloaderInfo(bootloaderInfo):
          if len(kernelList)<  1:
              raise BootyNoKernelWarning

-        self.writeAboot(instRoot, bl, kernelList,
-                        chainList, defaultDev, justConfig)
+        return self.writeAboot(instRoot, bl, kernelList,
+                               chainList, defaultDev, justConfig)

      def __init__(self, storage):
          bootloaderInfo.__init__(self, storage)
diff --git a/booty/bootloaderInfo.py b/booty/bootloaderInfo.py
index a1a27b7..8e09c50 100644
--- a/booty/bootloaderInfo.py
+++ b/booty/bootloaderInfo.py
@@ -405,15 +405,17 @@ class bootloaderInfo:

      def write(self, instRoot, bl, kernelList, chainList,
              defaultDev, justConfig):
+        rc = 0
+
          if len(kernelList)>= 1:
              config = self.getBootloaderConfig(instRoot, bl,
                                                kernelList, chainList,
                                                defaultDev)
-            config.write(instRoot + self.configfile, perms = self.perms)
+            rc = config.write(instRoot + self.configfile, perms = self.perms)
          else:
              raise booty.BootyNoKernelWarning

-        return ""
+        return rc

      def getArgList(self):
          args = []
@@ -533,9 +535,11 @@ class efiBootloaderInfo(bootloaderInfo):
      # XXX wouldn't it be nice to have a real interface to use efibootmgr from?
      def removeOldEfiEntries(self, instRoot):
          p = os.pipe()
-        iutil.execWithRedirect('/usr/sbin/efibootmgr', [],
-                               root = instRoot, stdout = p[1])
+        rc = iutil.execWithRedirect('/usr/sbin/efibootmgr', [],
+                                    root = instRoot, stdout = p[1])
          os.close(p[1])
+        if rc:
+            return rc

          c = os.read(p[0], 1)
          buf = c
@@ -550,10 +554,14 @@ class efiBootloaderInfo(bootloaderInfo):
                  continue
              if string.join(fields[1:], " ") == productName:
                  entry = fields[0][4:8]
-                iutil.execWithRedirect('/usr/sbin/efibootmgr',
-                                       ["-b", entry, "-B"],
-                                       root = instRoot,
-                                       stdout="/dev/tty5", stderr="/dev/tty5")
+                rc = iutil.execWithRedirect('/usr/sbin/efibootmgr',
+                                            ["-b", entry, "-B"],
+                                            root = instRoot,
+                                            stdout="/dev/tty5", stderr="/dev/tty5")
+                if rc:
+                    return rc
+
+        return 0

      def addNewEfiEntry(self, instRoot):
          try:
@@ -581,16 +589,19 @@ class efiBootloaderInfo(bootloaderInfo):
          argv = [ "/usr/sbin/efibootmgr", "-c" , "-w", "-L",
                   productName, "-d", "/dev/%s" % bootdisk,
                   "-p", bootpart, "-l", "\\EFI\\redhat\\" + self.bootloader ]
-        iutil.execWithRedirect(argv[0], argv[1:], root = instRoot,
-                               stdout = "/dev/tty5",
-                               stderr = "/dev/tty5")
+        rc = iutil.execWithRedirect(argv[0], argv[1:], root = instRoot,
+                                    stdout = "/dev/tty5",
+                                    stderr = "/dev/tty5")
+        return rc

      def installGrub(self, instRoot, bootDevs, grubTarget, grubPath,
                      target, cfPath):
          if not iutil.isEfi():
              raise EnvironmentError
-        self.removeOldEfiEntries(instRoot)
-        self.addNewEfiEntry(instRoot)
+        rc = self.removeOldEfiEntries(instRoot)
+        if rc:
+            return rc
+        return self.addNewEfiEntry(instRoot)

      def __init__(self, storage, initialize = True):
          if initialize:
diff --git a/booty/ia64.py b/booty/ia64.py
index b9646d3..6c9cdc2 100644
--- a/booty/ia64.py
+++ b/booty/ia64.py
@@ -11,25 +11,27 @@ class ia64BootloaderInfo(efiBootloaderInfo):
          config.addEntry("relocatable")

          return config
-
+
      def writeLilo(self, instRoot, bl, kernelList,
                    chainList, defaultDev, justConfig):
          config = self.getBootloaderConfig(instRoot, bl,
                                            kernelList, chainList, defaultDev)
-        config.write(instRoot + self.configfile, perms = 0755)
+        return config.write(instRoot + self.configfile, perms = 0755)

-        return ""
-
      def write(self, instRoot, bl, kernelList, chainList,
              defaultDev, justConfig):
          if len(kernelList)>= 1:
-            out = self.writeLilo(instRoot, bl, kernelList,
-                                 chainList, defaultDev, justConfig)
+            rc = self.writeLilo(instRoot, bl, kernelList,
+                                chainList, defaultDev, justConfig)
+            if rc:
+                return rc
          else:
              raise BootyNoKernelWarning

-        self.removeOldEfiEntries(instRoot)
-        self.addNewEfiEntry(instRoot)
+        rc = self.removeOldEfiEntries(instRoot)
+        if rc:
+            return rc
+        return self.addNewEfiEntry(instRoot)

      def makeInitrd(self, kernelTag):
          return "/boot/efi/EFI/redhat/initrd%s.img" % kernelTag
diff --git a/booty/ppc.py b/booty/ppc.py
index 4633f07..8cd4275 100644
--- a/booty/ppc.py
+++ b/booty/ppc.py
@@ -141,34 +141,40 @@ class ppcBootloaderInfo(bootloaderInfo):
          isys.sync()

          ybinargs = [ yabootProg, "-f", "-C", cf ]
-
+
          if not flags.test:
-            iutil.execWithRedirect(ybinargs[0],
-                                   ybinargs[1:],
-                                   stdout = "/dev/tty5",
-                                   stderr = "/dev/tty5",
-                                   root = instRoot)
+            rc = iutil.execWithRedirect(ybinargs[0],
+                                        ybinargs[1:],
+                                        stdout = "/dev/tty5",
+                                        stderr = "/dev/tty5",
+                                        root = instRoot)
+            if rc:
+                return rc

          if (not os.access(instRoot + "/etc/yaboot.conf", os.R_OK) and
              os.access(instRoot + "/boot/etc/yaboot.conf", os.R_OK)):
              os.symlink("../boot/etc/yaboot.conf",
                         instRoot + "/etc/yaboot.conf")
-
-        return ""
+
+        return 0

      def setPassword(self, val, isCrypted = 1):
          # yaboot just handles the password and doesn't care if its crypted
          # or not
          self.password = val
-
+
      def write(self, instRoot, bl, kernelList, chainList,
              defaultDev, justConfig):
          if len(kernelList)>= 1:
-            out = self.writeYaboot(instRoot, bl, kernelList,
-                                 chainList, defaultDev, justConfig)
+            rc = self.writeYaboot(instRoot, bl, kernelList,
+                                  chainList, defaultDev, justConfig)
+            if rc:
+                return rc
          else:
              raise BootyNoKernelWarning

+        return 0
+
      def __init__(self, storage):
          bootloaderInfo.__init__(self, storage)
          self.useYabootVal = 1
diff --git a/booty/s390.py b/booty/s390.py
index 717f50f..9b62600 100644
--- a/booty/s390.py
+++ b/booty/s390.py
@@ -153,19 +153,24 @@ class s390BootloaderInfo(bootloaderInfo):
          f.close()

          if not justConfigFile:
-            iutil.execWithRedirect("/sbin/zipl", [], root = instRoot,
-                                   stdout = "/dev/stdout",
-                                   stderr = "/dev/stderr")
-
-        return ""
+            rc = iutil.execWithRedirect("/sbin/zipl", [], root = instRoot,
+                                        stdout = "/dev/stdout",
+                                        stderr = "/dev/stderr")
+            if rc:
+                return rc
+
+        return 0

      def write(self, instRoot, bl, kernelList, chainList,
              defaultDev, justConfig):
-        out = self.writeZipl(instRoot, bl, kernelList,
-                             chainList, defaultDev,
-                             justConfig | (not self.useZiplVal))
-        out = self.writeChandevConf(bl, instRoot)
-
+        rc = self.writeZipl(instRoot, bl, kernelList,
+                            chainList, defaultDev,
+                            justConfig | (not self.useZiplVal))
+        if rc:
+            return rc
+
+        return self.writeChandevConf(bl, instRoot)
+
      def __init__(self, storage):
          bootloaderInfo.__init__(self, storage)
          self.useZiplVal = 1      # only used on s390
diff --git a/booty/sparc.py b/booty/sparc.py
index 39b84f1..b12b207 100644
--- a/booty/sparc.py
+++ b/booty/sparc.py
@@ -94,18 +94,20 @@ class sparcBootloaderInfo(bootloaderInfo):
              sbinargs += ["-U"]

          if not flags.test:
-            iutil.execWithRedirect(sbinargs[0],
-                                   sbinargs[1:],
-                                   stdout = "/dev/tty5",
-                                   stderr = "/dev/tty5",
-                                   root = instRoot)
+            rc = iutil.execWithRedirect(sbinargs[0],
+                                        sbinargs[1:],
+                                        stdout = "/dev/tty5",
+                                        stderr = "/dev/tty5",
+                                        root = instRoot)
+            if rc:
+                return rc

          if (not os.access(instRoot + "/etc/silo.conf", os.R_OK) and
              os.access(instRoot + "/boot/etc/silo.conf", os.R_OK)):
              os.symlink("../boot/etc/silo.conf",
                         instRoot + "/etc/silo.conf")

-        return ""
+        return 0

      def setPassword(self, val, isCrypted = 1):
          # silo just handles the password unencrypted
@@ -114,8 +116,8 @@ class sparcBootloaderInfo(bootloaderInfo):
      def write(self, instRoot, bl, kernelList, chainList,
              defaultDev, justConfig):
          if len(kernelList)>= 1:
-            self.writeSilo(instRoot, bl, kernelList, chainList,
-                        defaultDev, justConfig)
+            return self.writeSilo(instRoot, bl, kernelList, chainList,
+                                  defaultDev, justConfig)
          else:
              raise BootyNoKernelWarning

diff --git a/booty/x86.py b/booty/x86.py
index 5d52c31..8ba5d23 100644
--- a/booty/x86.py
+++ b/booty/x86.py
@@ -67,10 +67,12 @@ class x86BootloaderInfo(efiBootloaderInfo):
              syncDataToDisk(bootDev, "/", instRoot)

          # copy the stage files over into /boot
-        iutil.execWithRedirect("/sbin/grub-install",
-                               ["--just-copy"],
-                               stdout = "/dev/tty5", stderr = "/dev/tty5",
-                               root = instRoot)
+        rc = iutil.execWithRedirect("/sbin/grub-install",
+                                    ["--just-copy"],
+                                    stdout = "/dev/tty5", stderr = "/dev/tty5",
+                                    root = instRoot)
+        if rc:
+            return rc

          # really install the bootloader
          for cmd in cmds:
@@ -85,20 +87,22 @@ class x86BootloaderInfo(efiBootloaderInfo):
              else:
                  syncDataToDisk(bootDev, "/", instRoot)

-            iutil.execWithRedirect('/sbin/grub' ,
-                                   [ "--batch", "--no-floppy",
-                                     "--device-map=/boot/grub/device.map" ],
-                                   stdin = p[0],
-                                   stdout = "/dev/tty5", stderr = "/dev/tty5",
-                                   root = instRoot)
+            rc = iutil.execWithRedirect('/sbin/grub' ,
+                                        [ "--batch", "--no-floppy",
+                                          "--device-map=/boot/grub/device.map" ],
+                                        stdin = p[0],
+                                        stdout = "/dev/tty5", stderr = "/dev/tty5",
+                                        root = instRoot)
              os.close(p[0])

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

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

@@ -117,7 +121,11 @@ class x86BootloaderInfo(efiBootloaderInfo):
                  (args, grubPath, stage1Target, grubPath, bPart, grubPath)
              cmds.append(cmd)

-            self.runGrubInstall(instRoot, bootDev, cmds, cfPath)
+            rc = self.runGrubInstall(instRoot, bootDev, cmds, cfPath)
+            if rc:
+                return rc
+
+        return 0

      def writeGrub(self, instRoot, bl, kernelList, chainList,
              defaultDev, justConfigFile):
@@ -344,10 +352,10 @@ class x86BootloaderInfo(efiBootloaderInfo):
          f.close()

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

-        return ""
+        return 0

      def getMatchingPart(self, bootDev, target):
          bootName, bootPartNum = getDiskPart(bootDev, self.storage)
@@ -488,9 +496,9 @@ class x86BootloaderInfo(efiBootloaderInfo):
              cmds.append(cmd)

              if not justConfigFile:
-                self.runGrubInstall(instRoot, bootDev, cmds, cfPath)
+                return self.runGrubInstall(instRoot, bootDev, cmds, cfPath)

-        return ""
+        return 0

      def writeSysconfig(self, instRoot, installDev):
          sysconf = '/etc/sysconfig/grub'
@@ -508,16 +516,18 @@ class x86BootloaderInfo(efiBootloaderInfo):
          # XXX HACK ALERT - see declaration above
          if self.doUpgradeOnly:
              if self.useGrubVal:
-                self.upgradeGrub(instRoot, bl, kernelList,
-                                 chainList, defaultDev, justConfig)
-            return
+                return self.upgradeGrub(instRoot, bl, kernelList,
+                                        chainList, defaultDev, justConfig)
+            return 0

          if len(kernelList)<  1:
              raise BootyNoKernelWarning

-        out = self.writeGrub(instRoot, bl, kernelList,
-                             chainList, defaultDev,
-                             justConfig | (not self.useGrubVal))
+        rc = self.writeGrub(instRoot, bl, kernelList,
+                            chainList, defaultDev,
+                            justConfig | (not self.useGrubVal))
+        if rc:
+            return rc

          # XXX move the lilo.conf out of the way if they're using GRUB
          # so that /sbin/installkernel does a more correct thing
@@ -525,6 +535,7 @@ class x86BootloaderInfo(efiBootloaderInfo):
              os.rename(instRoot + "/etc/lilo.conf",
                        instRoot + "/etc/lilo.conf.anaconda")

+        return 0

      def getArgList(self):
          args = bootloaderInfo.getArgList(self)


--
David Cantrell <dcantrell redhat com>
Red Hat / Honolulu, HI


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