Comments inline.
One question at the bottom to you, Hans.
On 03/18/2010 10:03 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
linuxrc.s390 to write out the FCP_* lines from the parm or 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. 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 | 114 ++++++++++++++++++++++----------------------------
> 3 files changed, 53 insertions(+), 66 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
> 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 722ca5b..8186221 100755
> --- a/scripts/upd-instroot
> +++ b/scripts/upd-instroot
> @@ -733,6 +733,7 @@ sbin/qetharp
> sbin/qetharp-2.6
> sbin/qethconf
> sbin/sysctl
> +sbin/*_cio_free
> usr/bin/dirname
> usr/bin/expr
> usr/bin/getopt
> diff --git a/storage/zfcp.py b/storage/zfcp.py
> index 7692cad..55681e6 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):
> @@ -123,21 +124,6 @@ class ZFCPDevice:
> failed = "%s/failed" %(unitdir)
>
> try:
> - if not os.path.exists(online):
> - loggedWriteLineToFile("/proc/cio_ignore",
> - "free %s" %(self.devnum,))
> - udev_settle()
> - 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,))
> -
> - try:
> f = open(online, "r")
> devonline = f.readline().strip()
> f.close()
> @@ -326,64 +312,72 @@ 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):
# track entries that are already in zfcpconf to prevent duplicates
> + self.conf = []
> self.fcpdevs = []
> - self.hasReadConfig = False
> self.down = True
>
> - # So that users can write zfcp() to get the singleton instance
> - def __call__(self):
> - return self
Don't we still need __call__() along with the part at the bottom?
Please see below.
> + # go ahead and enable devices the user provided at boot time
> + # in a CMS conf file (in FCP_x variables)
# in a parm or conf file (in FCP_* variables)
It does not have to be in a conf file. It also, and especially in the general
case, works in a parm file, which is the only thing available in LPAR,
if there is no z/VM involved. In fact, there is even another method under z/VM
to pass kernel command line arguments besides parm/conf file (CP IPL <devno> PARM <args>).
> + if os.path.isfile(zfcpconf):
> + log.info("Enabling zFCP devices listed in %s" % (zfcpconf,))
> + iutil.execWithRedirect("zfcp_cio_free", ["-V"],
> + stdout="/dev/tty5", stderr="/dev/tty5")
>
> - def readConfig(self):
> - try:
> - f = open("/tmp/fcpconfig", "r")
> - except IOError:
> - log.info("no /tmp/fcpconfig; not configuring zfcp")
> + self._readZfcpConf()
> + for (devno, wwpn, fcplun) in self.conf:
> + self.addFCP(devno, wwpn, fcplun)
> +
> + def _readZfcpConf(self):
> + if not os.path.isfile(zfcpconf):
> return
>
> - lines = f.readlines()
> + f = open(zfcpconf, "r")
> + lines = map(lambda x: x.strip(), 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!")
> + 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.conf.append((fields[0], fields[1], fields[2]))
> + elif len(fields) == 5:
> + # support old syntax of:
> + # devno scsiid wwpn scsilun fcplun
> + self.conf.append((fields[0], fields[2], fields[4]))
> +
> + return
>
> def addFCP(self, devnum, wwpn, fcplun):
> + if (devnum, wwpn, fcplun) in self.conf:
> + return
> +
d = ZFCPDevice(devnum, wwpn, fcplun)
This does syntax and sanity checking and I guess we want to do that
first before even continuing with the triplet. If it is wrong, then
this will throw an exception (and tell the user, if he used the UI).
> + log.info("Enabling the following zFCP device:")
> + log.info(" devno = %s" % (devnum,))
> + log.info(" wwpn = %s" % (wwpn,))
> + log.info(" fcplun = %s" % (fcplun,))
> +
> + f = open(zfcpconf, "w+")
> + f.write("%s %s %s\n" % (devnum, wwpn, fcplun))
> + f.close()
> + iutil.execWithRedirect("zfcp_cio_free", ["-V"],
> + stdout="/dev/tty5", stderr="/dev/tty5")
> +
> + self.conf.append((devnum, wwpn, fcplun))
> d = ZFCPDevice(devnum, wwpn, fcplun)
remove construction of ZFCPDevice here and do it above
> if d.onlineDevice():
> - self.fcpdevs.append(d)
> + self.fcpdevs.add(d)
I guess your suggested tracking with self.conf works.
It's a bit of a pity, that almost the same
information also ends up in self.fcpdevs. Although, it only ends up in
the latter, if fcplun could be activated successfully
but we also need to track the writing to fcpconf.
>
> def shutdown(self):
> if self.down:
> @@ -401,12 +395,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:
> @@ -426,16 +415,13 @@ class ZFCP:
> def write(self, instPath):
> if len(self.fcpdevs) == 0:
> return
> - f = open(instPath + "/etc/zfcp.conf", "w")
> + f = open(os.path.join(instPath, zfcpconf), "w")
> for d in self.fcpdevs:
> f.write("%s\n" %(d,))
> f.close()
> -
> +
> f = open(instPath + "/etc/modprobe.conf", "a")
> f.write("alias scsi_hostadapter zfcp\n")
> f.close()
>
> -# Create ZFCP singleton
> -ZFCP = ZFCP()
> -
> # vim:tw=78:ts=4:et:sw=4
Isn't that required to instantiate the singleton?
storage/__init__.py:Storage.__init__() only calls zfcp.ZFCP() in order to get the already existing singleton object by means of ZFCP.__call__().
Is this correct, Hans?
Steffen
Linux on System z Development
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
