<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>
>> Fixed Block Access (FBA) DASDs are mainframe-specific
disk devices<br>
>> which are layed out as a sequence of 512-byte sectors. In
contrast<br>
>> to ECKD DASDs, these disks do not require formatting and
resemble<br>
>> the LBA layout of non-mainframe disks. Despite this
resemblance,<br>
>> the Linux kernel applies special handling during
partition detection<br>
>> for FBA DASDs, resulting in a single, immutable partition
being<br>
>> reported.<br>
>><br>
>> While actual FBA DASD hardware is no longer available,
the z/VM<br>
>> hypervisor can simulate FBA DASD disks, backed by either
ECKD
or<br>
>> SCSI devices.<br>
><br>
>This should really not require _any_ special code in blivet
unless
to<br>
>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>
>> Patches originally submitted by Nageswara R Sastry<br>
>> <rnsastry at linux.vnet.ibm.com>; I only refactored
them
as needed to mesh<br>
>> with current code since they were written last year.<br>
>> ---<br>
>> blivet/__init__.py | 7
+++++--<br>
>> blivet/devices.py | 17
+++++++++--------<br>
>> blivet/formats/disklabel.py | 23 ++++++++++++++++++++++-<br>
>> blivet/partitioning.py | 2 ++<br>
>> 4 files changed, 38 insertions(+), 11 deletions(-)<br>
>><br>
>><br>
>> - if disk.partitioned and disk.format.partitions:<br>
>> - raise ValueError("cannot
initialize a disk that has partitions")<br>
>> + log.debug("Checking to see if
device is LDL DASD")<br>
>> + if not disk.format.isLDL():<br>
>> + log.debug("Disk
is not LDL DASD")<br>
>> + if disk.partitioned
and disk.format.partitions:<br>
>> + raise
ValueError("cannot initialize a disk that has partitions")<br>
><br>
>I don't want the logs littered with strings about LDL DASD
when 99.8%
of<br>
>the user-base doesn't know or care what that is. If there must
be<br>
>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>
>> --- a/blivet/devices.py<br>
>> +++ b/blivet/devices.py<br>
>> @@ -1484,15 +1484,16 @@ class
PartitionDevice(StorageDevice):<br>
>> def _create(self):<br>
>> """ Create the
device. """<br>
>> log_method_call(self, self.name,
status=self.status)<br>
>> -
self.disk.format.addPartition(self.partedPartition)<br>
>> + # Do not create partition on LDL
formatted<br>
>> + if not self.disk.format.isLDL():<br>
>> +
self.disk.format.addPartition(self.partedPartition)<br>
><br>
>So, what -- we're just going to silently not add a partition
we have<br>
>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>
>> @@ -165,7 +178,13 @@ class DiskLabel(DeviceFormat):<br>
>> #
preexisting disklabels if the passed type was wrong<br>
>> self._labelType
= self._partedDisk.type<br>
>> else:<br>
>> - self._partedDisk
= self.freshPartedDisk()<br>
>> + if (self._labelType
== "dasd"):<br>
>> +
self._partedDisk = self.newPartedDisk()<br>
>> +
# Don't want to break for the normal DASD<br>
>> +
if self._partedDisk.maxPrimaryPartitionCount != 1:<br>
>> +
self._partedDisk = self.freshPartedDisk()<br>
>> + else:<br>
>> +
self.partedDisk = self.freshPartedDisk()<br>
><br>
>Why does this special type of DASD require the use of newDisk
instead
of<br>
>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>
> 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>
>> --- a/blivet/partitioning.py<br>
>> +++ b/blivet/partitioning.py<br>
>> @@ -425,6 +425,8 @@ def getNextPartitionType(disk,
no_primary=None):<br>
>> part_type
= parted.PARTITION_LOGICAL<br>
>> elif extended and logical_count < max_logicals:<br>
>> part_type = parted.PARTITION_LOGICAL<br>
>> + elif (disk.type == "dasd" and
disk.maxPrimaryPartitionCount
== 1):<br>
>> + part_type = parted.PARTITION_NORMAL<br>
><br>
>Is this really necessary? If these disks have special
requirements<br>
>and/or limitations they should have code in parted (if not a
new<br>
>disklabel type) to convey these instead of junking up blivet
with all<br>
>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>
><br>
>><br>
>> return part_type<br>
>><br>
<br>
<font size="3">Regards<br>
<font size="3">Nageswara R Sastry</font><br>
</font></font>
</body>
</html>