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

Re: [PATCH rhel6-branch master f14-branch] Use full EFI path to map drives for grub (#598572)



On 08/05/2010 12:50 PM, David Cantrell wrote:
> Ack with one comment.
> 
> Without being an EFI expert, the code itself looks fine.  The only comment I
> have is below.
> 
> On Wed, 4 Aug 2010, Brian C. Lane wrote:
> 
>> NOTE: This requires grub-0.97-66 to work correctly.
>>
>> On EFI we map the boot drive so that there is no question as to where
>> /boot is located. This requires a change in grub to parse the EFI
>> device path from the map command.
>>
>> Related: rhbz#598572
>> ---
>> booty/bootloaderInfo.py |   36 ++++++++++++++++++++++++++++++++++++
>> booty/x86.py            |   10 ++++++++--
>> 2 files changed, 44 insertions(+), 2 deletions(-)
>>
>> diff --git a/booty/bootloaderInfo.py b/booty/bootloaderInfo.py
>> index 7647fb8..9022063 100644
>> --- a/booty/bootloaderInfo.py
>> +++ b/booty/bootloaderInfo.py
>> @@ -658,6 +658,40 @@ class efiBootloaderInfo(bootloaderInfo):
>>                                     stderr = "/dev/tty5")
>>         return rc
>>
>> +    def getEfiProductPath(self, productName, force=False):
>> +        """ Return the full EFI path of the installed product.
>> +            eg. HD(4,2c8800,64000,902c1655-2677-4455-b2a5-29d0ce835610)
>> +
>> +            pass force=True to skip the cache and rerun efibootmgr
>> +        """
>> +        if not force and self._efiProductPath:
>> +            return self._efiProductPath
>> +
>> +        argv = [ "/usr/sbin/efibootmgr", "-v" ]
> 
> Would rather this just be 'efibootmgr' since explicit paths in
> execWithCapture() calls have bitten us before.

Yeah, it should be.

> 
>> +        buf = iutil.execWithCapture(argv[0], argv[1:],
>> +                                    stderr="/dev/tty5")
>> +
>> +        efiProductPath = None
>> +        for line in buf.splitlines():
>> +            line = line.strip()
>> +            if not line:
>> +                continue
>> +            if productName in line:
>> +                efiProductPath = line[line.rfind(productName)+len(productName):].strip()
>> +                break
>> +
>> +        if efiProductPath:
>> +            # Grab just the drive path
>> +            import re
>> +            m = re.match("(.*?\(.*?\)).*", efiProductPath)
>> +            if m:
>> +                efiProductPath = m.group(1)
>> +            else:
>> +                efiProductPath = None
>> +
>> +        self._efiProductPath = efiProductPath
>> +        return self._efiProductPath
>> +
>>     def installGrub(self, instRoot, bootDev, grubTarget, grubPath, cfPath):
>>         if not iutil.isEfi():
>>             raise EnvironmentError
>> @@ -672,6 +706,8 @@ class efiBootloaderInfo(bootloaderInfo):
>>         else:
>>             self.storage = instData.storage
>>
>> +        self._efiProductPath = None
>> +
>>         if iutil.isEfi():
>>             self._configdir = "/boot/efi/EFI/redhat"
>>             self._configname = "grub.conf"
>> diff --git a/booty/x86.py b/booty/x86.py
>> index 39c9c88..1ef67dd 100644
>> --- a/booty/x86.py
>> +++ b/booty/x86.py
>> @@ -264,13 +264,19 @@ class x86BootloaderInfo(efiBootloaderInfo):
>>             f.write("# NOTICE:  You do not have a /boot partition.  "
>>                     "This means that\n")
>>             f.write("#          all kernel and initrd paths are relative "
>> -                    "to /, eg.\n")
>> -
>> +                    "to /, eg.\n")
>> +

Not sure what's up here, but it shouldn't matter.

>>         f.write('#          root %s\n' % self.grubbyPartitionName(bootDevs[0]))
>>         f.write("#          kernel %svmlinuz-version ro root=%s\n" % (cfPath, rootDev.path))
>>         f.write("#          initrd %sinitrd-[generic-]version.img\n" % (cfPath))
>>         f.write("#boot=/dev/%s\n" % (grubTarget))
>>
>> +        if iutil.isEfi():
>> +            from product import productName
>> +            # Map the target device to the full EFI path
>> +            if self.getEfiProductPath(productName):
>> +                f.write("map %s %s\n" % (self.getEfiProductPath(productName), grubTarget))
>> +

This should either be "device" or "efimap", not just "map".

>>         # get the default image to boot... we have to walk and find it
>>         # since grub indexes by where it is in the config file
>>         if defaultDev.name == rootDev.name:
>>
> 


-- 
        Peter

I hope you know that this will go down on your permanent record.

01234567890123456789012345678901234567890123456789012345678901234567890123456789


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