Re: [PATCH 2/3] Collect DASD kernel parameter information during device tree scan (#526354).

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Tue, 13 Oct 2009, Steffen Maier wrote:

Comments inline

On 10/13/2009 03:51 AM, David Cantrell wrote:
Expand the DASDDevice class to hold the device bus ID and flags that are
passed at boot time.  Add udev functions to return the bus ID and flag
values for DASD devices.  When building the device tree, read the DASD
information and pass that to the DASDDevice object.
---
 storage/devices.py    |    9 ++++-----
 storage/devicetree.py |   10 ++++++++++
 storage/udev.py       |   30 ++++++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/storage/devices.py b/storage/devices.py
index 5f9d068..7d31b26 100644
--- a/storage/devices.py
+++ b/storage/devices.py
@@ -3156,11 +3156,10 @@ class DASDDevice(DiskDevice):
     """ A mainframe DASD. """
     _type = "dasd"

-    def __init__(self, device, size=None, major=None, minor=None,
-                 parents=None, sysfsPath=''):
-        DiskDevice.__init__(self, device, size=size,
-                            major=major, minor=minor,
-                            parents=parents, sysfsPath=sysfsPath)
+    def __init__(self, device, **kwargs):
+        self.busid = kwargs.get('busid')
+        self.opts = kwargs.get('opts')
+        DiskDevice.__init__(self, device, kwargs)


 class NFSDevice(StorageDevice, NetworkStorageDevice):
diff --git a/storage/devicetree.py b/storage/devicetree.py
index d355804..cff7591 100644
--- a/storage/devicetree.py
+++ b/storage/devicetree.py
@@ -1192,6 +1192,16 @@ class DeviceTree(object):
             kwargs["exists"]  = True
         elif udev_device_is_dasd(info):
             diskType = DASDDevice
+            kwargs["busid"] = udev_device_get_dasd_bus_id(info)
+            kwargs["opts"] = {"ro": udev_device_get_dasd_flag(info,
+                                                              "readonly"),
+                              "diag": udev_device_get_dasd_flag(info,
+                                                                "use_diag"),
+                              "erplog": udev_device_get_dasd_flag(info,
+                                                                  "erplog"),
+                              "failfast": udev_device_get_dasd_flag(info,
+                                                                    "failfast")

I would prefer no to do any translation to the "old" DASD kernel
parameter naming scheme for options. Instead we could just have a list
of sysfs attribute names and our option names would exactly be the same.
The code just loops over the list to collect the option values from
sysfs. This way we could easily extend the list in the future without
having to add a single line of code.

That's fine.  I was misunderstanding what we were handing to dracut.  I
thought dracut still needed a dasd= parameter for just the root device.

What would be useful here is a Python module that provided a JSON interface
to sysfs.

Maybe we want to have a dict of sysfs attribute names along with the
default value for each attribute. If the read value from sysfs equals
the default value, we would not even emit this attribute and its value
to kwargs["opts"].

Not interested in that.  The default is already in sysfs and reading it and
carrying it around doesn't hurt much.  Storing defaults in anaconda means
another place to maintain that information.

+                             }
             log.debug("%s is a dasd device" % name)
         else:
             diskType = DiskDevice
diff --git a/storage/udev.py b/storage/udev.py
index d051157..aeb9062 100644
--- a/storage/udev.py
+++ b/storage/udev.py
@@ -165,6 +165,36 @@ def udev_device_is_dasd(info):
     else:
         return False

+def udev_device_get_dasd_bus_id(info):
+    """ Return the CCW bus ID of the dasd device. """
+    ccwdevices = "/sys/bus/ccw/devices"
+
+    if not os.path.isdir(ccwdevices):
+        return None
+
+    for busid in os.listdir(ccwdevices):

Oh, this linear search might become a problem when having thousands of
devices. It even searches all kinds of devices on the ccw bus, so not
only DASDs but also zFCP HBAs, network devices, consoles, printers,
readers, etc.

[root@XYZ:/sys/class/block/dasda]# ll /sys/class/block/dasda
lrwxrwxrwx 1 root root 0 2009-10-11 18:00 /sys/class/block/dasda ->
../../devices/css0/0.0.0005/0.0.eb26/block/dasda
[root@H4245013:/sys/class/block/dasda]# pwd -P
/sys/devices/css0/0.0.0005/0.0.eb26/block/dasda

info.get("link_ref") should contain:
../../devices/css0/0.0.0005/0.0.eb26/block/dasda
info.get("sysfs_path") should contain:
/sys/devices/css0/0.0.0005/0.0.eb26/block/dasda

This should do all this method needs:
return info.get("sysfs_path").split("/")[-3]

OK, changing it to this line.

+def udev_device_get_dasd_flag(info, flag=None):
+    """ Return the specified flag for the dasd device. """
+    if flag is None:
+        return None
+
+    path = "/sys" + info.get("sysfs_path") + "/device/" + flag
+    if not os.path.isfile(path):
+        return None
+
+    return bool(open(path, 'r').read().strip())

We shouldn't imply that the sysfs attributes are of boolean type. While
this happens to be the case for our current set, it might change in the
future. I'd prefer to "blindly" read the sysfs attribute value string
and use this without knowing what it is (except for the above mentioned
default value, but that would just be an equal operation on two strings
whose syntax and semantics anaconda doesn't have to know).

That's fine.  When I was working on this for just the dasd= parameter,
booleans were more useful because I could then use then to emit the flag on
the parameter line or not.

- -- David Cantrell <dcantrell@xxxxxxxxxx>
Red Hat / Honolulu, HI

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAkrU5j0ACgkQ5hsjjIy1VknFQACgzoTHSfx/T614bD1zBhmEHCwY
ArEAnjPTv35Yw6nptu7oawV6w5k7vv+H
=f0Pq
-----END PGP SIGNATURE-----

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux