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

Martin Kolman mkolman at redhat.com
Mon Jun 16 11:35:23 UTC 2014



----- 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.

> 
> >          """ Return the full EFI path of the installed product.
> >              eg. HD(4,2c8800,64000,902c1655-2677-4455-b2a5-29d0ce835610)
> >  
> > @@ -733,7 +738,10 @@ class efiBootloaderInfo(bootloaderInfo):
> >              line = line.strip()
> >              if not line:
> >                  continue
> > -            if productName in line:
> > +            if productNameMajorVersion in line:
> > +                efiProductPath =
> > line[line.rfind(productNameMajorVersion)+len(productNameMajorVersion):].strip()
> Can you please split this into multiple lines and assignments to
> properly-named variables?
Sure. :)
> 
> > +                break
> > +            elif productName in line:
> >                  efiProductPath =
> >                  line[line.rfind(productName)+len(productName):].strip()
> >                  break
> >  
> > diff --git a/booty/x86.py b/booty/x86.py
> > index a753201..90540c5 100644
> > --- a/booty/x86.py
> > +++ b/booty/x86.py
> > @@ -279,11 +279,10 @@ class x86BootloaderInfo(efiBootloaderInfo):
> >          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):
> > +            if self.getEfiProductPath():
> >                  (n, pn) = getDiskPart(bootDevs[0], self.storage)
> > -                f.write("device (%s) %s\n" % (self.grubbyDiskName(n),
> > self.getEfiProductPath(productName)))
> > +                f.write("device (%s) %s\n" % (self.grubbyDiskName(n),
> > self.getEfiProductPath()))
> >  
> >          # get the default image to boot... we have to walk and find it
> >          # since grub indexes by where it is in the config file
> > diff --git a/product.py b/product.py
> > index 3031f96..bf2acc1 100644
> > --- a/product.py
> > +++ b/product.py
> > @@ -32,6 +32,7 @@ else:
> >  productStamp = ""
> >  productName = "anaconda"
> >  productVersion = "bluesky"
> > +productNameMajorVersion = "anaconda blue"
> This should probably be "anaconda bluesky".
I went by the logic of how the product module with the RHEL 6.6
buildstamp contains "6.6" in the productVersion variable (major.minor).
For productVersion == "bluesky" the "major version" would then be "blue".
But of course I can change it.

> 
> >  productPath = "Packages"
> >  productArch = None
> >  bugUrl = "your distribution provided bug reporting tool."
> > @@ -46,6 +47,9 @@ if path is not None:
> >          productArch = productStamp[productStamp.index(".")+1:]
> >          productName = lines[1][:-1]
> >          productVersion = lines[2][:-1]
> > +        versionSplit = productVersion.split(".")
> > +        if len(versionSplit):
> > +            productNameMajorVersion = "%s %s" % (productName,
> > versionSplit[0])
> >  
> >          # set productIsFinal
> >          isfinal = lines[3].strip().lower()
> 
> --
> Vratislav Podzimek
> 
> Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
> 
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> 


More information about the anaconda-patches mailing list