Re: [PATCH] Fix cio_ignore handling for zFCP devices (#533492).

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

 



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

On Thu, 18 Mar 2010, Steffen Maier wrote:

Did you test this successfully?

No.  This week our z9 was decommissioned and everything was moved to the z10.
Whoever was in charge of the migration was kind enough to not tell me where my
VM ended up or what the devices I was using changed to.

On 03/18/2010 03:45 PM, David Cantrell wrote:
The cio_ignore handling for zFCP devices was not complete.  Modify
linuxrc.s390 to write out the FCP_* lines from the CMS conf file to
/etc/zfcp.conf.  In storage/zfcp.py, modify the startup() method to run
/sbin/zfcp_cio_free, which reads /etc/zfcp.conf and removes them from
the device blacklist.

This only treats zFCP LUNs, that were provided in a parm/conf file on
boot of the installer. However, the user can always add (more) LUNs
through anaconda's advanced storage UI or the zfcp kickstart command.

Right.  And all of those methods go right back to this code.

Modify the onlineDevice() method to not free the
device, but rather set up the other properties.

The use of zfcp_cio_free should handle blocking on the device free
operation until it becomes available.
---
 loader/linuxrc.s390  |    4 +-
 scripts/upd-instroot |    1 +
 storage/zfcp.py      |  108 ++++++++++++++------------------------------------
 3 files changed, 33 insertions(+), 80 deletions(-)

diff --git a/loader/linuxrc.s390 b/loader/linuxrc.s390
index 83f5c94..64b38c9 100644
--- a/loader/linuxrc.s390
+++ b/loader/linuxrc.s390
@@ -2950,9 +2950,9 @@ fi
 syntax_check_fcp
 # currently we ignore failed syntax checks since FCP parms are non-interactive
 for i in ${!FCP_*}; do
-    echo "${!i}" >> /tmp/fcpconfig
+    echo "${!i}" >> /etc/zfcp.conf

This is OK as a workaround since device_cio_free currently does not
accept device specifications on the command line but only through well
known config files (https://bugzilla.redhat.com/show_bug.cgi?id=558881#c17).

It would be nice for the zfcp_cio_free command to accept command line
parameters, but since we don't have that now, I'm inclined to use the method
that works.

 done
-# cio_ignore handling for FCP should happen when the content of /tmp/fcpconfig
+# cio_ignore handling for FCP should happen when the content of /etc/zfcp.conf
 # will actually be processed which is in anaconda's zfcp.py ZFCP::readConfig()

 # TODO/FIXME: also need to pass IPv6 decision to loader/anaconda
diff --git a/scripts/upd-instroot b/scripts/upd-instroot
index 88ff473..fad0071 100755
--- a/scripts/upd-instroot
+++ b/scripts/upd-instroot
@@ -735,6 +735,7 @@ sbin/qetharp
 sbin/qetharp-2.6
 sbin/qethconf
 sbin/sysctl
+sbin/zfcp_cio_free

zfcp_cio_free is only a symlink to device_cio_free and we need all the

I know it's a symlink.

other *_cio_free sooner or later anyway, so I'd suggest:

+sbin/*_cio_free

OK.

 usr/bin/dirname
 usr/bin/expr
 usr/bin/getopt
diff --git a/storage/zfcp.py b/storage/zfcp.py
index 7692cad..48e326b 100644
--- a/storage/zfcp.py
+++ b/storage/zfcp.py
@@ -27,9 +27,9 @@ from udev import udev_settle
 import gettext
 _ = lambda x: gettext.ldgettext("anaconda", x)

+import iutil
 import logging
 log = logging.getLogger("anaconda")
-import warnings

 def loggedWriteLineToFile(fn, value):
     f = open(fn, "w")
@@ -39,6 +39,7 @@ def loggedWriteLineToFile(fn, value):

 zfcpsysfs = "/sys/bus/ccw/drivers/zfcp"
 scsidevsysfs = "/sys/bus/scsi/devices"
+zfcpconf = "/etc/zfcp.conf"

 class ZFCPDevice:
     def __init__(self, devnum, wwpn, fcplun):
@@ -115,41 +116,12 @@ class ZFCPDevice:
     checkValidWWPN = checkValidFCPLun = checkValid64BitHex

     def onlineDevice(self):
-        online = "%s/%s/online" %(zfcpsysfs, self.devnum)

This must stay. *_cio_free does only wait for the device to be bound to
its device driver but it does NOT set devices online and it must not do
so, since depending on the environment there are different mechanisms to
set devices online (in installer it's linuxrc and parts of anaconda,
while in the installed system it's udev).

So basically the zfcp_cio_free command gains us nothing.  Oh well.

         portadd = "%s/%s/port_add" %(zfcpsysfs, self.devnum)
         portdir = "%s/%s/%s" %(zfcpsysfs, self.devnum, self.wwpn)
         unitadd = "%s/unit_add" %(portdir)
         unitdir = "%s/%s" %(portdir, self.fcplun)
         failed = "%s/failed" %(unitdir)

-        try:
-            if not os.path.exists(online):
-                loggedWriteLineToFile("/proc/cio_ignore",
-                                      "free %s" %(self.devnum,))
-                udev_settle()I put this here deliberately, since startup() is called multiple times
-        except IOError as e:
-            raise ValueError, _("Could not free zFCP device %(devnum)s from "
-                                "device ignore list (%(e)s).") \
-                              % {'devnum': self.devnum, 'e': e}
-
-        if not os.path.exists(online):
-            raise ValueError, _(
-                "zFCP device %s not found, not even in device ignore list."
-                %(self.devnum,))
-

That's the part that gets replaced by zfcp_cio_free.

At least something can be removed.

-        try:
-            f = open(online, "r")
-            devonline = f.readline().strip()
-            f.close()
-            if devonline != "1":
-                loggedWriteLineToFile(online, "1")
-            else:
-                log.info("zFCP device %s already online." %(self.devnum,))
-        except IOError as e:
-            raise ValueError, _("Could not set zFCP device %(devnum)s "
-                                "online (%(e)s).") \
-                              % {'devnum': self.devnum, 'e': e}
-
         if not os.path.exists(portdir):
             if os.path.exists(portadd):
                 # older zfcp sysfs interface

This must stay to set the zfcp host bus adapter online.

Right, I mis-skimmed device_cio_free.  I see now that it only waits for the
existence of 'online' and not the value it contains.

@@ -326,59 +298,44 @@ class ZFCP:
     """ ZFCP utility class.

         This class will automatically online to ZFCP drives configured in
-        /tmp/fcpconfig when the startup() method gets called. It can also be
+        /etc/zfcp.conf when the startup() method gets called. It can also be
         used to manually configure ZFCP devices through the addFCP() method.

-        As this class needs to make sure that /tmp/fcpconfig configured
+        As this class needs to make sure that /etc/zfcp.conf configured
         drives are only onlined once and as it keeps a global list of all ZFCP
         devices it is implemented as a Singleton.
     """

     def __init__(self):
         self.fcpdevs = []
-        self.hasReadConfig = False
         self.down = True

-    # So that users can write zfcp() to get the singleton instance
-    def __call__(self):
-        return self
+        if os.path.isfile(zfcpconf):

Can we keep the log.info, if there is no zfcpconf?
It helps users and us with debugging.

linuxrc.s390 does that, which is the only place where we would be reading
user-provided zfcp.conf data anyway.  If we get malformed lines this far in to
the installer, I don't care.

+            log.info("Enabling zFCP devices listed in %s" % (zfcpconf,))
+            iutil.execWithRedirect("zfcp_cio_free", ["-V"],
+                                   stdout="/dev/tty5", stderr="/dev/tty5")

-    def readConfig(self):

Removing this capsulation function and the sentinel hasReadConfig, is it
ensured that __init__() is only ever called at most once?
It has to be, otherwise the LUN handling does not work, because addFCP
or onlineDevice must not be called more than once for the same LUN
without offlineDevice in between.

That's the way the storage subsystem in anaconda is set up.  For iscsi, fcoe,
zfcp, and dasd, we initialize a singleton for that utility class for the life
of the installer (see storage/__init__.py around line 263 or so).  The line at
the end of storage/zfcp.py needs to be removed, so I'll fix that in the patch.

-        try:
-            f = open("/tmp/fcpconfig", "r")
-        except IOError:
-            log.info("no /tmp/fcpconfig; not configuring zfcp")
-            return
+            # read in all zFCP devices
+            f = open(zfcpconf, "r")
+            lines = map(lambda x: x.strip(), f.readlines())
+            f.close()

-        lines = f.readlines()
-        f.close()
-        for line in lines:
-            # each line is a string separated list of values to describe a dev
-            # there are two valid formats for the line:
-            #   devnum scsiid wwpn scsilun fcplun    (scsiid + scsilun ignored)
-            #   devnum wwpn fcplun
-            line = string.strip(line).lower()
-            if line.startswith("#"):
-                continue
-            fcpconf = string.split(line)
-            if len(fcpconf) == 3:
-                devnum = fcpconf[0]
-                wwpn = fcpconf[1]
-                fcplun = fcpconf[2]
-            elif len(fcpconf) == 5:
-                warnings.warn("SCSI ID and SCSI LUN values for ZFCP devices are ignored and deprecated.", DeprecationWarning)
-                devnum = fcpconf[0]
-                wwpn = fcpconf[2]
-                fcplun = fcpconf[4]
-            else:
-                log.warn("Invalid line found in /tmp/fcpconfig!")
-                continue
+            for line in lines:
+                if line.startswith("#") or line == '':
+                    continue

-            try:
-                self.addFCP(devnum, wwpn, fcplun)
-            except ValueError, e:
-                log.warn(str(e))
-                continue
+                fields = line.split()
+
+                if len(fields) == 3:
+                    self.addFCP(fields[0], fields[1], fields[2])
+                elif len(fields) == 5:
+                    # support old syntax of:
+                    # devno scsiid wwpn scsilun fcplun
+                    self.addFCP(fields[0], fields[2], fields[4])

Can we keep the warning about invalid lines in zfcp.conf?
It helps the user and us to debug things.

Again, the syntax is validated in linuxrc.s390 and the user is told it's
wrong.

+
+    # So that users can write zfcp() to get the singleton instance
+    def __call__(self):
+        return self

     def addFCP(self, devnum, wwpn, fcplun):
         d = ZFCPDevice(devnum, wwpn, fcplun)

How do you wait for LUNs added interactively by the user through the
advanced storage UI or by the zfcp kickstart command, both ending up
also calling addFCP()?

Yes, the addFCP() method is called on storage.zfcp, which creates a new
ZFCPDevice object and enables it.

Do you want to update zfcp.conf everytime in addFCP and call
zfcp_cio_free here instead or rather really want a device_cio_free
accepting command line arguments for devices?

I want the command to accept arguments on the command, but since we don't have
that now, I am ok with appending to /etc/zfcp.conf and running zfcp_cio_free
again.  Given that the command _only_ frees devices not already free and then
waits for the existence of the 'online' property, I don't see that as a
problem for now.  Once zfcp_cio_free can accept command line arguments, we can
fix up these lines in storage/zfcp.py

@@ -401,12 +358,7 @@ class ZFCP:
         if not self.down:
             return
         self.down = False
-        if not self.hasReadConfig:
-            self.readConfig()
-            self.hasReadConfig = True
-            # readConfig calls addFCP which calls onlineDevice already
-            return
-
+
         if len(self.fcpdevs) == 0:
             return
         for d in self.fcpdevs:

I rather like encapsulating reading the config file which is really just
one of three methods to add zFCP LUNs.

It's not really necessary.

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

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

iEYEARECAAYFAkuijG4ACgkQ5hsjjIy1VkkLFwCcCF+sCe9Q+eqvRZ5BQ4E+q0jB
LocAnRFbhA2qKnNPEeNBj0ceFGd6aFOF
=CN9M
-----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