[PATCH rhel6-branch] PPC: strengthen PReP-picking algorithm (#877852)

[Will Woods]

pSeries-style systems can have multiple PReP partitions, as long as
there's only one per disk. It's kind of like the x86 MBR that way.

The current code in booty and platform.py *always* picks the first PReP
partition, which will fail if if you have (e.g.) an OS already installed
on /dev/sda and you're doing a new install onto /dev/sdb.

This patch adds ppcBootloaderInfo.pickPReP(), which tries to find the
best PReP partition to use according to the following rules:

1) If we're creating a new PReP partition, use that;
2) Otherwise, if there's a PReP partition already on the disk that
   contains /boot, use that;
3) Failing that, use the first available PReP partition (as before).

This should make mkofboot actually install yaboot into the correct PReP
partition.

IPSeriesPPC.bootDevice() has been changed to use this same algorithm, so
the two of them will agree about what the boot device is.
---
 booty/ppc.py | 37 ++++++++++++++++++++++++++++++++-----
 platform.py  | 11 ++---------
 2 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/booty/ppc.py b/booty/ppc.py
index 6f62071..401f505 100644
--- a/booty/ppc.py
+++ b/booty/ppc.py
@@ -7,6 +7,35 @@ from bootloaderInfo import *
 import iutil

class ppcBootloaderInfo(bootloaderInfo):
+    def pickPReP(self):
+        # if we created a new PReP partition, pick that
+        for a in self.storage.findActions(type="create", obj="format"):
+            if a.format.type == "prepboot":
+                return a.device
+
+        # otherwise, look at the existing PReP partitions.
+        prepdevs = []
+        for dev in self.storage.fsset.devices:
+            if (dev.format.type == "prepboot" and
+                    dev.partedPartition.getFlag(parted.PARTITION_BOOT)):
+                prepdevs.append(dev)
+
+        # we'd prefer a PReP partition that's on the same disk as /boot.
+        try:
+            bootdisk = self.storage.mountpoints["/boot"].disk
+        except KeyError:
+            bootdisk = self.storage.rootDevice.disk
+        for dev in prepdevs:
+            if dev.disk == bootdisk:
+                return dev
+
+        # failing that, return the first PReP partition we found
+        if len(prepdevs) != 0:
+            return prepdevs[0]
+
+        # Alas, no PReP to be found..
+        return None
+
     def getBootDevs(self, bl):
         import parted

@@ -14,10 +43,9 @@ class ppcBootloaderInfo(bootloaderInfo):
         machine = iutil.getPPCMachine()

if machine == 'pSeries':
-            for dev in self.storage.fsset.devices:
-                if (dev.format.type == "prepboot" and
-                        dev.partedPartition.getFlag(parted.PARTITION_BOOT)):
-                    retval.append(dev.path)
+            prep = self.pickPReP()
+            if prep:
+                retval.append(prep.path)
         elif machine == 'PMac':
             for dev in self.storage.fsset.devices:
                 if dev.format.type == "hfs" and dev.format.bootable:
@@ -31,7 +59,6 @@ class ppcBootloaderInfo(bootloaderInfo):
                 except KeyError:
                     # Try / if we don't have this we're not going to work
                     device = self.storage.rootDevice
-
                 retval.append(device.path)
             else:
                 if bl.getDevice():
diff --git a/platform.py b/platform.py
index 7bc75ec..e866133 100644
--- a/platform.py
+++ b/platform.py
@@ -392,15 +392,8 @@ class IPSeriesPPC(PPC):
     _maxBootPartSize = 10

def bootDevice(self):
-        bootDev = None
-
-        # We want the first PReP partition.
-        for device in self.anaconda.id.storage.partitions:
-            if device.format.type == "prepboot":
-                bootDev = device
-                break
-
-        return bootDev
+        # use booty's PReP-picking algorithm
+        return self.anaconda.id.bootloader.pickPReP()

def bootloaderChoices(self, bl):
         ret = {}

[David Lehman]

On Wed, 2013-08-14 at 14:12 -0400, Will Woods wrote:
> pSeries-style systems can have multiple PReP partitions, as long as
> there's only one per disk. It's kind of like the x86 MBR that way.
>
> The current code in booty and platform.py *always* picks the first PReP
> partition, which will fail if if you have (e.g.) an OS already installed
> on /dev/sda and you're doing a new install onto /dev/sdb.
>
> This patch adds ppcBootloaderInfo.pickPReP(), which tries to find the
> best PReP partition to use according to the following rules:
> If we're creating a new PReP partition, use that;
> Otherwise, if there's a PReP partition already on the disk that
> contains /boot, use that;
> Failing that, use the first available PReP partition (as before).
>
> This should make mkofboot actually install yaboot into the correct PReP
> partition.
>
> IPSeriesPPC.bootDevice() has been changed to use this same algorithm, so
> the two of them will agree about what the boot device is.
>
> booty/ppc.py | 37 ++++++++++++++++++++++++++++++++-----
>  platform.py  | 11 ++---------
>  2 files changed, 34 insertions(+), 14 deletions(-)
>
> diff --git a/booty/ppc.py b/booty/ppc.py
> index 6f62071..401f505 100644
> --- a/booty/ppc.py
> +++ b/booty/ppc.py
> @@ -7,6 +7,35 @@ from bootloaderInfo import *
>  import iutil
>
> class ppcBootloaderInfo(bootloaderInfo):
> def pickPReP(self):
>    # if we created a new PReP partition, pick that
>    for a in self.storage.findActions(type="create", obj="format"):
>        if a.format.type == "prepboot":
>            return a.device
>    # otherwise, look at the existing PReP partitions.
>    prepdevs = []
>    for dev in self.storage.fsset.devices:
>        if (dev.format.type == "prepboot" and
>                dev.partedPartition.getFlag(parted.PARTITION_BOOT)):
>            prepdevs.append(dev)
>    # we'd prefer a PReP partition that's on the same disk as /boot.
>    try:
>        bootdisk = self.storage.mountpoints["/boot"].disk
>    except KeyError:
>        bootdisk = self.storage.rootDevice.disk
>    for dev in prepdevs:
>        if dev.disk == bootdisk:
>            return dev

I think this needs to account for either /boot or / being on md, so
something like this:

bootDevice = self.storage.mountpoints.get("/boot", 
                                          self.storage.rootDevice)
if hasattr(bootDevice, "disk"):
  bootdisks = [bootDevice.disk]
else:
  bootdisks = list(set([p.disk for p in bootDevice.parents]))

for dev in prepdevs:
  if dev.disk in bootdisks:
    return dev

The rest looks okay to me. Is there a corresponding blivet patch for
master?

[Vratislav Podzimek]

On Wed, 2013-08-14 at 14:15 -0500, David Lehman wrote:
> On Wed, 2013-08-14 at 14:12 -0400, Will Woods wrote:
> > pSeries-style systems can have multiple PReP partitions, as long as
> > there's only one per disk. It's kind of like the x86 MBR that way.
> >
> > The current code in booty and platform.py *always* picks the first PReP
> > partition, which will fail if if you have (e.g.) an OS already installed
> > on /dev/sda and you're doing a new install onto /dev/sdb.
> >
> > This patch adds ppcBootloaderInfo.pickPReP(), which tries to find the
> > best PReP partition to use according to the following rules:
> > If we're creating a new PReP partition, use that;
> > Otherwise, if there's a PReP partition already on the disk that
> > contains /boot, use that;
> > Failing that, use the first available PReP partition (as before).
> >
> > This should make mkofboot actually install yaboot into the correct PReP
> > partition.
> >
> > IPSeriesPPC.bootDevice() has been changed to use this same algorithm, so
> > the two of them will agree about what the boot device is.
> >
> > booty/ppc.py | 37 ++++++++++++++++++++++++++++++++-----
> >  platform.py  | 11 ++---------
> >  2 files changed, 34 insertions(+), 14 deletions(-)
> >
> > diff --git a/booty/ppc.py b/booty/ppc.py
> > index 6f62071..401f505 100644
> > --- a/booty/ppc.py
> > +++ b/booty/ppc.py
> > @@ -7,6 +7,35 @@ from bootloaderInfo import *
> >  import iutil
> >
> > class ppcBootloaderInfo(bootloaderInfo):
> > def pickPReP(self):
> >    # if we created a new PReP partition, pick that
> >    for a in self.storage.findActions(type="create", obj="format"):
> >        if a.format.type == "prepboot":
> >            return a.device
> >    # otherwise, look at the existing PReP partitions.
> >    prepdevs = []
> >    for dev in self.storage.fsset.devices:
> >        if (dev.format.type == "prepboot" and
> >                dev.partedPartition.getFlag(parted.PARTITION_BOOT)):
> >            prepdevs.append(dev)
> >    # we'd prefer a PReP partition that's on the same disk as /boot.
> >    try:
> >        bootdisk = self.storage.mountpoints["/boot"].disk
> >    except KeyError:
> >        bootdisk = self.storage.rootDevice.disk
> >    for dev in prepdevs:
> >        if dev.disk == bootdisk:
> >            return dev
>
> I think this needs to account for either /boot or / being on md, so
> something like this:
>
> bootDevice = self.storage.mountpoints.get("/boot", 
>                                           self.storage.rootDevice)
> if hasattr(bootDevice, "disk"):
>   bootdisks = [bootDevice.disk]
> else:
>   bootdisks = list(set([p.disk for p in bootDevice.parents]))

No square brackets needed here ^^^.

[Will Woods]

On Wed, 2013-08-14 at 14:15 -0500, David Lehman wrote:
> I think this needs to account for either /boot or / being on md, so
> something like this:
>
> bootDevice = self.storage.mountpoints.get("/boot", 
>                                           self.storage.rootDevice)
> if hasattr(bootDevice, "disk"):
>   bootdisks = [bootDevice.disk]
> else:
>   bootdisks = list(set([p.disk for p in bootDevice.parents]))
>
> for dev in prepdevs:
>   if dev.disk in bootdisks:
>     return dev

I've incorporated this into v2 of the patch, which I'll send in a
second.
> The rest looks okay to me. Is there a corresponding blivet patch for
> master?

We talked about this on IRC (last month probably (!)) but AFAICT
blivet/master already Does The Right Thing with PReP partitions.

-w