[PATCH] The boot menu entry should contain the RHEL major release number (#1001960)

Vratislav Podzimek vpodzime at redhat.com
Mon Jun 16 15:03:43 UTC 2014


On Mon, 2014-06-16 at 07:35 -0400, Martin Kolman wrote:
> 
> ----- Original Message -----
> > From: "Vratislav Podzimek" <vpodzime at redhat.com>
> > To: "anaconda patch review" <anaconda-patches at lists.fedorahosted.org>
> > Sent: Friday, June 13, 2014 10:02:53 AM
> > Subject: Re: [PATCH] The boot menu entry should contain the RHEL major	release number (#1001960)
> > 
> > On Thu, 2014-06-12 at 14:59 +0200, Martin Kolman wrote:
> > > Make Grub and Efi write the menu entry with the major version
> > > suffix and modify the related code to handle entries that contain
> > > the suffix as well as the current entries without the suffix.
> > > 
> > > Signed-off-by: Martin Kolman <mkolman at redhat.com>
> > > ---
> > >  booty/bootloaderInfo.py | 18 +++++++++++++-----
> > >  booty/x86.py            |  5 ++---
> > >  product.py              |  4 ++++
> > >  3 files changed, 19 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/booty/bootloaderInfo.py b/booty/bootloaderInfo.py
> > > index 5d76733..ff687e5 100644
> > > --- a/booty/bootloaderInfo.py
> > > +++ b/booty/bootloaderInfo.py
> > > @@ -287,7 +287,11 @@ class BootImages:
> > >              self.default = storage.rootDevice.name
> > >              (label, longlabel, type) = self.images[self.default]
> > >              if not label:
> > > -                self.images[self.default] = ("linux", productName, type)
> > > +                # use the productNameMajorVersion as distribution name
> > > +                # so that users can discern which major version of RHEL
> > > +                # the given entry is (for example if there is RHEL5 or 7
> > > +                # on the machine and in the boot menu)
> > > +                self.images[self.default] = ("linux",
> > > productNameMajorVersion, type)
> > >  
> > >      # Return a list of (storage.Device, string) tuples that are bootable
> > >      # devices.  The string is the type of the device, which is just a
> > >      string
> > > @@ -661,7 +665,8 @@ class efiBootloaderInfo(bootloaderInfo):
> > >              fields = string.split(line)
> > >              if len(fields) < 2:
> > >                  continue
> > > -            if string.join(fields[1:], " ") == productName:
> > > +            joined_string = string.join(fields[1:], " ")
> > While changing this you can also change it to the new " ".join() syntax.
> OK.
> > And the name joined_string tells basically nothing about the variable's
> > contents.
> Well, I'm not exactly sure what that string should be, but indeed something like
> potential_efi_product_name will be better.
> 
> > 
> > > +            if joined_string == productName or joined_string ==
> > > productNameMajorVersion:
> > This could be 'joined_string in (productName, productNameMajorVersion)'.
> Oh, right - shorter and more readable. :)
> > 
> > >                  entry = fields[0][4:8]
> > >                  rc = iutil.execWithRedirect('efibootmgr',
> > >                                              ["-b", entry, "-B"],
> > > @@ -705,7 +710,7 @@ class efiBootloaderInfo(bootloaderInfo):
> > >  
> > >          for d in bootdevlist:
> > >              argv = [ "efibootmgr", "-c" , "-w", "-L",
> > > -                     productName, "-d", "%s" % (d.path,),
> > > +                     productNameMajorVersion, "-d", "%s" % (d.path,),
> > >                       "-p", "%s" % (bootpart,),
> > >                       "-l", "\\EFI\\redhat\\" + self.bootloader ]
> > >              rc = iutil.execWithRedirect(argv[0], argv[1:], root =
> > >              instRoot,
> > > @@ -715,7 +720,7 @@ class efiBootloaderInfo(bootloaderInfo):
> > >          # return last rc, the API doesn't provide anything better than
> > >          this
> > >          return rc
> > >  
> > > -    def getEfiProductPath(self, productName, force=False):
> > > +    def getEfiProductPath(self, force=False):
> > What is removing the productName parameter good for?
> Well, the only caller is directly passing productName anyway and I am also changing the value used
> from productName to the new productNameMajorVersion variable. 
> So I would have to rename the parameter and change the import to the new variable anyway.
> Like this the productNameMajorVersion variable is already available in the module that defines
> getEfiProductPath() and doesn't have to be imported and we get rid of an effectively useless function parameter.
Makes sense, thanks for the additional explanation.

-- 
Vratislav Podzimek

Anaconda Rider | RHCE | Red Hat, Inc. | Brno - Czech Republic




More information about the anaconda-patches mailing list