[node-patches] Change in ovirt-node[node-3.0]: installer: Fix EFI boot entry removal

fabiand at fedoraproject.org fabiand at fedoraproject.org
Thu Sep 19 10:36:52 UTC 2013


Fabian Deutsch has uploaded a new change for review.

Change subject: installer: Fix EFI boot entry removal
......................................................................

installer: Fix EFI boot entry removal

Previously removing EFI entries during a reinstall failed due to an
incorrect call to efibootmgr.
This patch adds a wrapper around efibootmgr and provides an API to add
an remove EFI boot entries.

This shall prevent incorrect function calls to efibootmgr.
The new API is now also used in the legacy (installer) code.

Change-Id: I761ac056f4a168eefbc49319c0ff66239a9ded38
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=950553
Signed-off-by: Fabian Deutsch <fabiand at fedoraproject.org>
---
M src/ovirt/node/utils/system.py
M src/ovirtnode/install.py
M src/ovirtnode/ovirtfunctions.py
3 files changed, 142 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-node refs/changes/84/19384/1

diff --git a/src/ovirt/node/utils/system.py b/src/ovirt/node/utils/system.py
index 08dce48..8454c38 100644
--- a/src/ovirt/node/utils/system.py
+++ b/src/ovirt/node/utils/system.py
@@ -23,6 +23,7 @@
 from ovirt.node.utils.fs import File
 import logging
 import os
+import re
 import rpm
 import subprocess
 import sys
@@ -394,3 +395,115 @@
                 )
             )
             self.logger.info("Reboot Scheduled")
+
+
+class EFI(base.Base):
+    """A simple wrapper around efibootmgr to modify the EFI boot entries
+    """
+    class BootEntry(base.Base):
+        bootnum = None
+        label = None
+        value = None
+
+        def to_tuple(self):
+            return self.bootnum, self.label, self.value
+
+        def __cmp__(self, other):
+            return self.to_tuple() == other.to_tuple()
+
+        def __repr__(self):
+            return str(self)
+
+        def __str__(self):
+            """String representation of a boot entry
+
+            >>> e = EFI.BootEntry()
+            >>> e.bootnum, e.label, e.value = (42, "Foo", "Bar")
+            >>> str(e) # doctest: +ELLIPSIS
+            "<BootEntry bootnum='42' label='Foo' value='Bar' at ...>"
+            """
+            return self.build_str(["bootnum", "label", "value"])
+
+    def _efibootmgr(self, cmdargs):
+        """Run efibootmgr with cmdargs
+
+        >>> e = EFI()
+        >>> e._call = lambda c: c
+        >>> e._efibootmgr([("verbose", None),
+        ...                ("label", "Foo")])
+        ['efibootmgr', '--verbose', '--label', 'Foo']
+        """
+        cmd = ["efibootmgr"]
+
+        for k, v in cmdargs:
+            cmd.append("--%s" % k)
+            if v is not None:
+                cmd.append(str(v))
+
+        self.logger.debug("About to run: %s" % cmd)
+        return self._call(cmd)
+
+    def _call(self, cmd):
+        return process.check_output(cmd)
+
+    def add_entry(self, label, loader_filename, disk):
+        """Add a new EFI boot entry
+
+        Args:
+            label: Label to be shown in the EFI boot menu
+            loader_filename: Filename of the bootloader (e.g. grub2) to use
+            disk: Disk where the bootloader resides on
+        """
+        self.logger.debug(("Adding EFI boot entry: " +
+                           "label=%s, loader=%s, disk=%s") %
+                          (label, loader_filename, disk))
+        cmdargs = [("verbose", None),
+                   ("create", None),
+                   ("label", label),
+                   ("loader", loader_filename),
+                   ("disk", disk)]
+        self._efibootmgr(cmdargs)
+
+        return True
+
+    def list_entries(self):
+        pat = re.compile("^Boot([0-9a-zA-Z]{4})[\* ] ([^\t]+)\t(.*)$")
+        entries = []
+
+        lines = self._efibootmgr([("verbose", None)])
+
+        self.logger.debug("Parsing EFI boot entries from: %s" % lines)
+
+        # Parse the lines
+        for line in lines.split("\n"):
+            match = pat.search(line)
+            if match:
+                entry = EFI.BootEntry()
+                entry.bootnum, entry.label, entry.value = match.groups()
+                entries.append(entry)
+
+        return entries
+
+    def remove_entry(self, entry):
+        """Remove an EFI boot entry
+
+        Args:
+            entry: An EFI.BootEntry object, can be retrieved with
+                   efi.list_entries()
+        """
+        entry_exists = False
+
+        for other_entry in self.list_entries():
+            if other_entry == entry:
+                entry_exists = True
+
+        if not entry_exists:
+            raise RuntimeError("Tried to remove non-existent " +
+                               "EFI boot entry: %s" % entry)
+
+        self.logger.debug("Removing EFI boot entry: %s" % entry)
+        self._efibootmgr([("verbose", None),
+                          ("bootnum", entry.bootnum),
+                          ("delete-bootnum", None)])
+
+        return True
diff --git a/src/ovirtnode/install.py b/src/ovirtnode/install.py
index c0ad30c..3cd0354 100755
--- a/src/ovirtnode/install.py
+++ b/src/ovirtnode/install.py
@@ -271,7 +271,7 @@
                 efi_grub_conf.write(GRUB2_BACKUP_TEMPLATE % self.grub_dict)
                 efi_grub_conf.close()
             _functions.system("umount /liveos")
-            _functions.remove_efi_entry(self.efi_dir_name)
+            _functions.remove_efi_entry(_functions.PRODUCT_SHORT)
             logger.info("Grub2 Install Completed")
             return True
         return True
@@ -456,18 +456,16 @@
                 _functions.system("cp /boot/efi/EFI/redhat/grub.efi " +
                       "/liveos/efi/EFI/redhat/grub.efi")
                 if not "/dev/mapper/" in self.disk:
-                   efi_disk = self.disk[:-1]
+                    efi_disk = self.disk[:-1]
                 else:
-                   efi_disk = re.sub("p[1,2,3]$", "", self.disk)
+                    efi_disk = re.sub("p[1,2,3]$", "", self.disk)
                 # generate grub legacy config for efi partition
                 #remove existing efi entries
-                _functions.remove_efi_entry(self.efi_dir_name)
-                efi_mgr_cmd = ("efibootmgr -c -l '\\EFI\\%s\\grubx64.efi' " +
-                              "-L '%s' -d %s -v") % (self.efi_dir_name,  \
-                                               _functions.PRODUCT_SHORT, \
-                                               efi_disk)
-                logger.info(efi_mgr_cmd)
-                _functions.system(efi_mgr_cmd)
+                _functions.remove_efi_entry(_functions.PRODUCT_SHORT)
+                _functions.add_efi_entry(_functions.PRODUCT_SHORT,
+                                         ("\\EFI\\%s\\grub.efi" %
+                                          self.efi_dir_name),
+                                         efi_disk)
         self.kernel_image_copy()
 
         # reorder tty0 to allow both serial and phys console after installation
diff --git a/src/ovirtnode/ovirtfunctions.py b/src/ovirtnode/ovirtfunctions.py
index 7b0d41e..972c8c3 100644
--- a/src/ovirtnode/ovirtfunctions.py
+++ b/src/ovirtnode/ovirtfunctions.py
@@ -1769,22 +1769,29 @@
                 args_dict[opt] = opt
     return args_dict
 
-def remove_efi_entry(entry):
-    efi_mgr_cmd = "efibootmgr|grep '%s'" % entry
-    efi_mgr = subprocess_closefds(efi_mgr_cmd, \
-                                      shell=True, \
-                                      stdout=subprocess.PIPE, \
-                                      stderr=subprocess.STDOUT)
-    efi_out = efi_mgr.stdout.read().strip()
-    logger.debug(efi_mgr_cmd)
-    logger.debug(efi_out)
-    for line in efi_out.splitlines():
-        if not "Warning" in line:
-            num = line[4:8]  # grabs 4 digit hex id
-            cmd = "efibootmgr -B -b %s" % num
-            system(cmd)
+def remove_efi_entry(dir_name):
+    from ovirt.node.utils.system import EFI
+    efi = EFI()
+    removed_entries = []
+    for entry in efi.list_entries():
+        # The following condition is just a good guess
+        # Normally an entry.value contains sth like:
+        # HD(...)File(...)
+        # Where <p> in File(<p>) is the path to the bootloader.
+        # So basically we are looking if the dir_name appears in the <p> part
+        if dir_name in entry.label:
+            efi.remove_entry(entry)
+            removed_entries.append(entry)
+    if len(removed_entries) > 1:
+        logger.warning("Removed more that one EFI boot entry!")
+    logger.info("Removed EFI boot entry: %s" % removed_entries)
     return
 
+def add_efi_entry(label, loader_filename, disk):
+    from ovirt.node.utils.system import EFI
+    efi = EFI()
+    return efi.add_entry(label, loader_filename, disk)
+
 def grub2_available():
     if os.path.exists("/sbin/grub2-install"):
         return True


-- 
To view, visit http://gerrit.ovirt.org/19384
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I761ac056f4a168eefbc49319c0ff66239a9ded38
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-node
Gerrit-Branch: node-3.0
Gerrit-Owner: Fabian Deutsch <fabiand at fedoraproject.org>



More information about the node-patches mailing list