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

Re: [PATCH] Add dracutSetupString() method to ZFCPDiskDevice (#526354).



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

On Wed, 14 Oct 2009, Steffen Maier wrote:

Small comments below.

Changes incorporated, tested, and patch committed to master.


On 10/14/2009 04:28 AM, David Cantrell wrote:
Collect CCW bus ID, WWPN, and FCP LUN values for zFCP devices when
building the device tree.  Store these in the ZFCPDiskDevice object and
use them to generate the rd_ZFCP= string for dracut.

Expand storage/udev.py with functions to determine if a device is zFCP,
get a zFCP device's bus ID, and get arbitrary attribute values.
---
 storage/devices.py    |   23 +++++++++++------------
 storage/devicetree.py |    8 ++++++++
 storage/udev.py       |   35 +++++++++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/storage/devices.py b/storage/devices.py
index b76402c..19b4c3c 100644
--- a/storage/devices.py
+++ b/storage/devices.py
@@ -3135,23 +3135,22 @@ class ZFCPDiskDevice(DiskDevice):
     """ A mainframe ZFCP disk. """
     _type = "zfcp"

-    def __init__(self, name, size=None, major=None, minor=None,
-                 devnum=None, wwpn=None, fcplun=None,
-                 parents=None, sysfsPath=''):
-        self.devnum = devnum
-        self.wwpn = wwpn
-        self.fcplun = fcplun
-        name = "zfcp://%s/%s/%s" % (self.devnum, self.wwpn, self.fcplun)
-        DiskDevice.__init__(self, name, size=size,
-                            major=major, minor=minor,
-                            parents=parents, sysfsPath=sysfsPath)
+    def __init__(self, name, **kwargs):
+        self.busid = kwargs.get("busid")
+        self.wwpn = kwargs.get("wwpn")
+        self.fcp_lun = kwargs.get("fcp_lun")
+        name = "zfcp://%s/%s/%s" % (self.busid, self.wwpn, self.fcplun,)
+        DiskDevice.__init__(self, name, **kwargs)

     def __str__(self):
         s = DiskDevice.__str__(self)
-        s += ("  devnum = %(devnum)s  wwpn = %(wwpn)s  fcplun = %(fcplun)s" %
-              {"devnum": self.devnum, "wwpn": self.wwpn, "fcplun": self.fcplun})
+        s += ("  busid = %(busid)s  wwpn = %(wwpn)s  fcp_lun = %(fcp_lun)s" %
+              {"busid": self.busid, "wwpn": self.wwpn, "fcp_lun": self.fcp_lun})
         return s

+    def dracutSetupString(self):
+        return "rd_ZFCP=%s,%s,%s" % (self.busid, self.wwpn, self.fcp_lun,)
+

 class DASDDevice(DiskDevice):
     """ A mainframe DASD. """
diff --git a/storage/devicetree.py b/storage/devicetree.py
index 1aae2a9..1b5358a 100644
--- a/storage/devicetree.py
+++ b/storage/devicetree.py
@@ -1199,6 +1199,14 @@ class DeviceTree(object):
                 kwargs["opts"][attr] = udev_device_get_dasd_flag(info, attr)

             log.debug("%s is a dasd device" % name)
+        elif udev_device_is_zfcp(info):
+            diskType = ZFCPDiskDevice

+            kwargs["busid"] = udev_device_get_zfcp_bus_id(info)

This special handling can be dropped, see below.

+
+            for attr in ['wwpn', 'fcp_lun']:

for attr in ['hba_id', 'wwpn', 'fcp_lun']:

Saves the extra udev_device_get_zfcp_bus_id(info) before the loop.
That would require one more renaming of "busid" to "hba_id" in the whole
patch. Then it would be consistent and we would use the sysfs attribute
names all over as in the DASD case.

+                kwargs[attr] = udev_device_get_zfcp_attribute(info, attr=attr)
+
+            log.debug("%s is a zfcp device" % name)
         else:
             diskType = DiskDevice
             log.debug("%s is a disk" % name)
diff --git a/storage/udev.py b/storage/udev.py
index 1af2289..3318602 100644
--- a/storage/udev.py
+++ b/storage/udev.py
@@ -180,6 +180,41 @@ def udev_device_get_dasd_flag(info, flag=None):

     return open(path, 'r').read().strip()

+def udev_device_is_zfcp(info):
+    """ Return True if the device is a zfcp device. """
+    bypath = False
+    devdir = os.path.realpath("/sys" + info.get("sysfs_path") + "/device")
+    wwpn = os.path.realpath(devdir + "/wwpn")
+    fcplun = os.path.realpath(devdir + "/fcp_lun")
+
+    for symlink in info.get("symlinks"):
+        if symlink.startswith("disk/by-path") and symlink.find("zfcp") != -1:

As with PATH_ID for DASD, this relies on an external tool called by udev
to create this symlink. I'd like to get the driver information more
directly in order to avoid such dependency.

+            bypath = True
+            break
+
+    if bypath and os.path.isdir(devdir) and \
+       os.path.isfile(wwpn) and os.path.isfile(fcplun):

Wouldn't the above test for zfcp in the by-path symlink be sufficient?

+        return True
+
+    return False
+

Even though it looks strange and currently given no better alternative,
I could suggest the following mechanism to check if a device is zfcp:

According to
http://lxr.linux.no/#linux+v2.6.31/Documentation/sysfs-rules.txt#L159
we start with "/sys" + info.get("sysfs_path") and get the canonical path.
In a loop we check if the last part of the subsystem symlink (if
available) matches ccw and if not we go up one directory level until we
reached /sys/devices.
If both the subsystem is ccw and the last part of the driver symlink
matches zfcp, then we return true else false.

E.g.
# cd /sys/class/block/sda
# cd $(pwd -P)
[/sys/devices/css0/0.0.0015/0.0.3c1b/host0/rport-0:0-16/target0:0:16/0:0:16:1089355792/block/sda]# readlink subsystem
../../../../../../../../../../class/block
[/sys/devices/css0/0.0.0015/0.0.3c1b/host0/rport-0:0-16/target0:0:16/0:0:16:1089355792/block/sda]# cd ..
[/sys/devices/css0/0.0.0015/0.0.3c1b/host0/rport-0:0-16/target0:0:16/0:0:16:1089355792/block]# readlink subsystem
[/sys/devices/css0/0.0.0015/0.0.3c1b/host0/rport-0:0-16/target0:0:16/0:0:16:1089355792/block]# cd ..
[/sys/devices/css0/0.0.0015/0.0.3c1b/host0/rport-0:0-16/target0:0:16/0:0:16:1089355792]# readlink subsystem
../../../../../../../../bus/scsi
[/sys/devices/css0/0.0.0015/0.0.3c1b/host0/rport-0:0-16/target0:0:16/0:0:16:1089355792]# cd ..
[/sys/devices/css0/0.0.0015/0.0.3c1b/host0/rport-0:0-16/target0:0:16]# readlink subsystem
../../../../../../../bus/scsi
[/sys/devices/css0/0.0.0015/0.0.3c1b/host0/rport-0:0-16/target0:0:16]# cd ..
[/sys/devices/css0/0.0.0015/0.0.3c1b/host0/rport-0:0-16]# readlink subsystem
[/sys/devices/css0/0.0.0015/0.0.3c1b/host0/rport-0:0-16]# cd ..
[/sys/devices/css0/0.0.0015/0.0.3c1b/host0]# readlink subsystem
../../../../../bus/scsi
[/sys/devices/css0/0.0.0015/0.0.3c1b/host0]# cd ..
[/sys/devices/css0/0.0.0015/0.0.3c1b]# readlink subsystem
../../../../bus/ccw
[/sys/devices/css0/0.0.0015/0.0.3c1b]# readlink driver
../../../../bus/ccw/drivers/zfcp
Hit found.

A short cut hack would be to take the first five parts of the canonical
path /sys/devices/css0/0.0.0015/0.0.3c1b and call readlink driver
directly on that hoping the sysfs layout won't change too much to break
this.

+def udev_device_get_zfcp_bus_id(info):
+    """ Return the CCW bus ID of the zfcp device. """
+    return info.get("sysfs_path").split("/")[4]

Don't ask me why, but we only need this "hack" for DASD. zFCP does
provide the device bus ID in its sysfs attribute hba_id. Let's drop this
special method and use hba_id above, just as we already do in
storage.ZFCPDevice.offlineSCSIDevice().

+
+def udev_device_get_zfcp_attribute(info, attr=None):
+    """ Return the value of the specified attribute of the zfcp device. """
+    if not attr:
+        return None
+
+    attribute = "/sys%s/devices/%s" % (info.get("sysfs_path"), attr,)
+    attribute = os.path.realpath(attribute)
+
+    if not os.path.isfile(attribute):
+        return None

Could we at least log some warning here? If we ever get here, then
either the sysfs has changed and/or we have a bug in anaconda and we
should be able to determine this problem afterwards. Maybe an exception
would even be appropriate, so we would notice immediately as soon as it
happened the first time. The sysfs attributes we read here are vital for
the dracut string to work on boot. In contrast to the DASD case, where
we read just options, here we read the three essential parts of the
triplet that identify a zFCP LUN.

+
+    return open(attribute, "r").read().strip()
+
 def udev_device_is_cdrom(info):
     """ Return True if the device is an optical drive. """
     # FIXME: how can we differentiate USB drives from CD-ROM drives?

Steffen

Linux on System z Development

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Erich Baier
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


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


- -- David Cantrell <dcantrell redhat com>
Red Hat / Honolulu, HI

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

iEYEARECAAYFAkrX1fcACgkQ5hsjjIy1VkkUHQCeJibOwvyo6FjZ5olpBOE123Aj
O2cAoNdhux6o9acIC0boHml0MJYB8Ovg
=gkbH
-----END PGP SIGNATURE-----

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