[blivet] Add support for FBA EDEV/EAV installation. (#885957)

Samantha N. Bueno sbueno+anaconda at redhat.com
Wed Jun 5 16:59:19 UTC 2013


On Wed, Jun 05, 2013 at 11:22:19AM -0500, David Lehman wrote:
> On Wed, 2013-06-05 at 08:56 -0400, Samantha N. Bueno wrote:
> > Note: these patches have been tested by IBM. Two minor issues arose
> > from testing (noted in comment 15 of the bug if interested), but they
> > are non-fatal and not directly related to the patches below.
> > 
> > ====
> > 
> > Fixed Block Access (FBA) DASDs are mainframe-specific disk devices
> > which are layed out as a sequence of 512-byte sectors. In contrast
> > to ECKD DASDs, these disks do not require formatting and resemble
> > the LBA layout of non-mainframe disks. Despite this resemblance,
> > the Linux kernel applies special handling during partition detection
> > for FBA DASDs, resulting in a single, immutable partition being
> > reported.
> > 
> > While actual FBA DASD hardware is no longer available, the z/VM
> > hypervisor can simulate FBA DASD disks, backed by either ECKD or
> > SCSI devices.
> 
> This should really not require _any_ special code in blivet unless to
> add support for a new disklabel type. I have several comments, below.

Thanks for the feedback--since IBM originally wrote these patches, I'm
going to point them to this thread so they can use the feedback to
reshape them.

Samantha
 
> > 
> > This patch adds support for successful installation on FBA DASD
> > devies (EDEV)
> > 
> > Patches originally submitted by Nageswara R Sastry
> > <rnsastry at linux.vnet.ibm.com>; I only refactored them as needed to mesh
> > with current code since they were written last year.
> > ---
> >  blivet/__init__.py          |  7 +++++--
> >  blivet/devices.py           | 17 +++++++++--------
> >  blivet/formats/disklabel.py | 23 ++++++++++++++++++++++-
> >  blivet/partitioning.py      |  2 ++
> >  4 files changed, 38 insertions(+), 11 deletions(-)
> > 
> > diff --git a/blivet/__init__.py b/blivet/__init__.py
> > index 2d69eb1..95d77e0 100644
> > --- a/blivet/__init__.py
> > +++ b/blivet/__init__.py
> > @@ -802,8 +802,11 @@ class Blivet(object):
> >                          # disk. Still, we need it out of the devicetree.
> >                          self.devicetree._removeDevice(part, moddisk=False)
> >  
> > -        if disk.partitioned and disk.format.partitions:
> > -            raise ValueError("cannot initialize a disk that has partitions")
> > +        log.debug("Checking to see if device is LDL DASD")
> > +        if not disk.format.isLDL():
> > +            log.debug("Disk is not LDL DASD")
> > +            if disk.partitioned and disk.format.partitions:
> > +                raise ValueError("cannot initialize a disk that has partitions")
> 
> I don't want the logs littered with strings about LDL DASD when 99.8% of
> the user-base doesn't know or care what that is. If there must be
> anything in the logs about these it should be only when we see one.
> 
> >  
> >          # remove existing formatting from the disk
> >          destroy_action = ActionDestroyFormat(disk)
> > diff --git a/blivet/devices.py b/blivet/devices.py
> > index 2bb7289..e8b3e0b 100644
> > --- a/blivet/devices.py
> > +++ b/blivet/devices.py
> > @@ -1484,15 +1484,16 @@ class PartitionDevice(StorageDevice):
> >      def _create(self):
> >          """ Create the device. """
> >          log_method_call(self, self.name, status=self.status)
> > -        self.disk.format.addPartition(self.partedPartition)
> > +        # Do not create partition on LDL formatted
> > +        if not self.disk.format.isLDL():
> > +            self.disk.format.addPartition(self.partedPartition)
> 
> So, what -- we're just going to silently not add a partition we have
> already planned to add? This isn't right.
> 
> >  
> > -        self._wipe()
> > -        try:
> > -            self.disk.format.commit()
> > -        except DiskLabelCommitError:
> > -            part = self.disk.format.partedDisk.getPartitionByPath(self.path)
> > -            self.disk.format.removePartition(part)
> > -            raise
> > +            try:
> > +                self.disk.format.commit()
> > +            except DiskLabelCommitError:
> > +                part = self.disk.format.partedDisk.getPartitionByPath(self.path)
> > +                self.disk.format.removePartition(part)
> > +                raise
> >  
> >      def _postCreate(self):
> >          if self.isExtended:
> > diff --git a/blivet/formats/disklabel.py b/blivet/formats/disklabel.py
> > index 90b73f3..7cb41f5 100644
> > --- a/blivet/formats/disklabel.py
> > +++ b/blivet/formats/disklabel.py
> > @@ -144,6 +144,19 @@ class DiskLabel(DeviceFormat):
> >          log_method_call(self, device=self.device, labelType=self._labelType)
> >          return parted.freshDisk(device=self.partedDevice, ty=self._labelType)
> >  
> > +    def newPartedDisk(self):
> > +        """ Return a new, empty parted.Disk instance for this device. """
> > +        log_method_call(self, device=self.device)
> > +        return parted.newDisk(device=self.partedDevice)
> > +
> > +    def isLDL(self):
> > +        """ Return True for LDL formatted disk. """
> > +        log_method_call(self, device=self.device)
> > +        if (self._labelType == "dasd" and self.partedDisk.maxPrimaryPartitionCount == 1):
> > +            return True
> > +        else:
> > +            return False
> > +
> >      @property
> >      def partedDisk(self):
> >          if not self._partedDisk:
> > @@ -165,7 +178,13 @@ class DiskLabel(DeviceFormat):
> >                  # preexisting disklabels if the passed type was wrong
> >                  self._labelType = self._partedDisk.type
> >              else:
> > -                self._partedDisk = self.freshPartedDisk()
> > +                if (self._labelType == "dasd"):
> > +                    self._partedDisk = self.newPartedDisk()
> > +                    # Don't want to break for the normal DASD
> > +                    if self._partedDisk.maxPrimaryPartitionCount != 1:
> > +                        self._partedDisk = self.freshPartedDisk()
> > +                else:
> > +                    self.partedDisk = self.freshPartedDisk()
> 
> Why does this special type of DASD require the use of newDisk instead of
> freshDisk, which works for every other disk type?
> 
> >  
> >              # turn off cylinder alignment
> >              if self._partedDisk.isFlagAvailable(parted.DISK_CYLINDER_ALIGNMENT):
> > @@ -442,6 +461,8 @@ class DiskLabel(DeviceFormat):
> >              return 1
> >          elif self.labelType == "sun":
> >              return 3
> > +        elif self.isLDL():
> > +            return 1
> >          else:
> >              return 0
> >  
> > diff --git a/blivet/partitioning.py b/blivet/partitioning.py
> > index 8660969..1bd0d08 100644
> > --- a/blivet/partitioning.py
> > +++ b/blivet/partitioning.py
> > @@ -425,6 +425,8 @@ def getNextPartitionType(disk, no_primary=None):
> >                  part_type = parted.PARTITION_LOGICAL
> >      elif extended and logical_count < max_logicals:
> >          part_type = parted.PARTITION_LOGICAL
> > +    elif (disk.type == "dasd" and disk.maxPrimaryPartitionCount == 1):
> > +        part_type = parted.PARTITION_NORMAL
> 
> Is this really necessary? If these disks have special requirements
> and/or limitations they should have code in parted (if not a new
> disklabel type) to convey these instead of junking up blivet with all
> this special-case code.
> 
> >  
> >      return part_type
> >  
> 
> 
> _______________________________________________
> 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