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

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

 



Did you test this successfully?

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.

> 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).

>  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
other *_cio_free sooner or later anyway, so I'd suggest:

+sbin/*_cio_free

>  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).

>          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.

> -        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.

> @@ -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.

> +            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.

> -        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.

> +
> +    # 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()?

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?

> @@ -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.

> @@ -426,11 +378,11 @@ 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()


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


[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