<html>
  <head>

    <meta http-equiv="content-type" content="text/html; charset=ISO-8859-1">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <font size="3">In my view the following changes are required to
      support installation on
      FBA<br>
      disks. For each comment explained why that particular approach has
      been<br>
      chosen and request that please suggest if there is any better
      approach.<br>
      <br>
      &gt;&gt; Fixed Block Access (FBA) DASDs are mainframe-specific
      disk devices<br>
      &gt;&gt; which are layed out as a sequence of 512-byte sectors. In
      contrast<br>
      &gt;&gt; to ECKD DASDs, these disks do not require formatting and
      resemble<br>
      &gt;&gt; the LBA layout of non-mainframe disks. Despite this
      resemblance,<br>
      &gt;&gt; the Linux kernel applies special handling during
      partition detection<br>
      &gt;&gt; for FBA DASDs, resulting in a single, immutable partition
      being<br>
      &gt;&gt; reported.<br>
      &gt;&gt;<br>
      &gt;&gt; While actual FBA DASD hardware is no longer available,
      the z/VM<br>
      &gt;&gt; hypervisor can simulate FBA DASD disks, backed by either
      ECKD
      or<br>
      &gt;&gt; SCSI devices.<br>
      &gt;<br>
      &gt;This should really not require _any_ special code in blivet
      unless
      to<br>
      &gt;add support for a new disklabel type. I have several comments,
      below.<br>
      <br>
      With in the 'dasd' disklabel type, FBA disk is having special
      characteristics<br>
      like 'maxPrimaryPartitionCount' is '1' and having implicit
      partition. Some<br>
      extra code is required to support FBA disk and to differentiate
      normal
      DASD<br>
      and FBA disk.<br>
      <br>
      &gt;&gt; Patches originally submitted by Nageswara R Sastry<br>
      &gt;&gt; &lt;rnsastry at linux.vnet.ibm.com&gt;; I only refactored
      them
      as needed to mesh<br>
      &gt;&gt; with current code since they were written last year.<br>
      &gt;&gt; ---<br>
      &gt;&gt; &nbsp;blivet/__init__.py &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;| &nbsp;7
      +++++--<br>
      &gt;&gt; &nbsp;blivet/devices.py &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; | 17
      +++++++++--------<br>
      &gt;&gt; &nbsp;blivet/formats/disklabel.py | 23 ++++++++++++++++++++++-<br>
      &gt;&gt; &nbsp;blivet/partitioning.py &nbsp; &nbsp; &nbsp;| &nbsp;2 ++<br>
      &gt;&gt; &nbsp;4 files changed, 38 insertions(+), 11 deletions(-)<br>
      &gt;&gt;<br>
      &gt;&gt;<br>
      &gt;&gt; - &nbsp; &nbsp; &nbsp; &nbsp;if disk.partitioned and disk.format.partitions:<br>
      &gt;&gt; - &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;raise ValueError("cannot
      initialize a disk that has partitions")<br>
      &gt;&gt; + &nbsp; &nbsp; &nbsp; &nbsp;log.debug("Checking to see if
      device is LDL DASD")<br>
      &gt;&gt; + &nbsp; &nbsp; &nbsp; &nbsp;if not disk.format.isLDL():<br>
      &gt;&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;log.debug("Disk
      is not LDL DASD")<br>
      &gt;&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;if disk.partitioned
      and disk.format.partitions:<br>
      &gt;&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;raise
      ValueError("cannot initialize a disk that has partitions")<br>
      &gt;<br>
      &gt;I don't want the logs littered with strings about LDL DASD
      when 99.8%
      of<br>
      &gt;the user-base doesn't know or care what that is. If there must
      be<br>
      &gt;anything in the logs about these it should be only when we see
      one.<br>
      <br>
      Agree, The above two log.debug statements can be removed.<br>
      <br>
      &gt;&gt; --- a/blivet/devices.py<br>
      &gt;&gt; +++ b/blivet/devices.py<br>
      &gt;&gt; @@ -1484,15 +1484,16 @@ class
      PartitionDevice(StorageDevice):<br>
      &gt;&gt; &nbsp; &nbsp; &nbsp;def _create(self):<br>
      &gt;&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;""" Create the
      device. """<br>
      &gt;&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;log_method_call(self, self.name,
      status=self.status)<br>
      &gt;&gt; - &nbsp; &nbsp; &nbsp;
      &nbsp;self.disk.format.addPartition(self.partedPartition)<br>
      &gt;&gt; + &nbsp; &nbsp; &nbsp; &nbsp;# Do not create partition on LDL
      formatted<br>
      &gt;&gt; + &nbsp; &nbsp; &nbsp; &nbsp;if not self.disk.format.isLDL():<br>
      &gt;&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
      &nbsp;self.disk.format.addPartition(self.partedPartition)<br>
      &gt;<br>
      &gt;So, what -- we're just going to silently not add a partition
      we have<br>
      &gt;already planned to add? This isn't right.<br>
      <br>
      FBA disks already have implicit partition and adding a new
      partition is
      not<br>
      possible.<br>
      <br>
      &gt;&gt; @@ -165,7 +178,13 @@ class DiskLabel(DeviceFormat):<br>
      &gt;&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;#
      preexisting disklabels if the passed type was wrong<br>
      &gt;&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;self._labelType
      = self._partedDisk.type<br>
      &gt;&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;else:<br>
      &gt;&gt; - &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;self._partedDisk
      = self.freshPartedDisk()<br>
      &gt;&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;if (self._labelType
      == "dasd"):<br>
      &gt;&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
      &nbsp;self._partedDisk = self.newPartedDisk()<br>
      &gt;&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
      &nbsp;# Don't want to break for the normal DASD<br>
      &gt;&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
      &nbsp;if self._partedDisk.maxPrimaryPartitionCount != 1:<br>
      &gt;&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
      &nbsp; &nbsp; &nbsp;self._partedDisk = self.freshPartedDisk()<br>
      &gt;&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;else:<br>
      &gt;&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
      &nbsp;self.partedDisk = self.freshPartedDisk()<br>
      &gt;<br>
      &gt;Why does this special type of DASD require the use of newDisk
      instead
      of<br>
      &gt;freshDisk, which works for every other disk type?<br>
      <br>
      'ped_disk_new' function first does 'disk_probe' and calls function
      named<br>
      'ped_disk_new_fresh'.<br>
      <br>
      'ped_disk_new_fresh' function starts creating disk according to
      the supplied<br>
      'disk_label'.<br>
      <br>
      FBA disks first need a 'disk_probe' to get the correct meta-data
      assigned.<br>
      After the correct assignment of meta-data 'ped_disk_new_fresh'
      function
      works<br>
      fine.<br>
      <br>
      &gt; works for every other disk type?<br>
      no, it won't work for every other disk type that is the reason why
      it was<br>
      separated for FBA<br>
      <br>
      &gt;&gt; --- a/blivet/partitioning.py<br>
      &gt;&gt; +++ b/blivet/partitioning.py<br>
      &gt;&gt; @@ -425,6 +425,8 @@ def getNextPartitionType(disk,
      no_primary=None):<br>
      &gt;&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;part_type
      = parted.PARTITION_LOGICAL<br>
      &gt;&gt; &nbsp; &nbsp; &nbsp;elif extended and logical_count &lt; max_logicals:<br>
      &gt;&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;part_type = parted.PARTITION_LOGICAL<br>
      &gt;&gt; + &nbsp; &nbsp;elif (disk.type == "dasd" and
      disk.maxPrimaryPartitionCount
      == 1):<br>
      &gt;&gt; + &nbsp; &nbsp; &nbsp; &nbsp;part_type = parted.PARTITION_NORMAL<br>
      &gt;<br>
      &gt;Is this really necessary? If these disks have special
      requirements<br>
      &gt;and/or limitations they should have code in parted (if not a
      new<br>
      &gt;disklabel type) to convey these instead of junking up blivet
      with all<br>
      &gt;this special-case code.<br>
      <br>
      This condition was taken care in 'parted' tool. But
      'getNextPartitionType'<br>
      function is not returning the correct partition type for FBA disk
      because
      this<br>
      kind of disk is not known to it. So added a condition to return
      the correct<br>
      partition type.<br>
      &gt;<br>
      &gt;&gt;<br>
      &gt;&gt; &nbsp; &nbsp; &nbsp;return part_type<br>
      &gt;&gt;<br>
      <br>
      <font size="3">Regards<br>
        <font size="3">Nageswara R Sastry</font><br>
      </font></font>
  </body>
</html>