[PATCH 0/5] fix zFCP in anaconda and also make it work with changed sysfs interface of device driver (#510524)

[Steffen Maier]

Steffen Maier (5):
  correctly activate zFCP LUN on s390
  correctly delete a SCSI device provided by a zFCP LUN on s390
  correctly deactivate zFCP LUN on s390
  error messages of zFCP on s390: log or pass to the UI
  prevent getting started up or shutdown again while already in such
    state

kickstart.py            |    5 +-
 storage/zfcp.py         |  177 ++++++++++++++++++++++++++++++++++++++++-------
 textw/partition_text.py |    5 +-
 3 files changed, 161 insertions(+), 26 deletions(-)

[PATCH 1/5] correctly activate zFCP LUN on s390

[Steffen Maier]

Activation happens in correct order and synchronizes with udev_settle.
Error cases are handled gracefully and report user readable error messages.
Supports both old (RHEL 5) and new sysfs zfcp device driver interface.
---
 storage/zfcp.py |   69 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 62 insertions(+), 7 deletions(-)

diff --git a/storage/zfcp.py b/storage/zfcp.py
index 2584268..f4cf339 100644
--- a/storage/zfcp.py
+++ b/storage/zfcp.py
@@ -22,6 +22,8 @@
 import string
 import os
 from constants import *
+from udev import udev_settle
+from time import sleep

import gettext
 _ = lambda x: gettext.ldgettext("anaconda", x)
@@ -117,17 +119,70 @@ class ZFCPDevice:
         online = "%s/%s/online" %(zfcpsysfs, self.devnum)
         portadd = "%s/%s/port_add" %(zfcpsysfs, self.devnum)
         unitadd = "%s/%s/%s/unit_add" %(zfcpsysfs, self.devnum, self.wwpn)
+        portdir = "%s/%s/%s" %(zfcpsysfs, self.devnum, self.wwpn)
+        unitdir = "%s/%s" %(portdir, self.fcplun)

try:
-            if not os.path.exists(unitadd):
-                loggedWriteLineToFile(portadd, self.wwpn)
+            if not os.path.exists(online):
+                loggedWriteLineToFile("/proc/cio_ignore",
+                                      "free %s" %(self.devnum,))
+                udev_settle()
+                sleep(1)
+        except Exception, e:
+            raise ValueError, _(
+                "could not free zfcp device %s from device ignore list (%s)"
+                %(self.devnum, e))
+
+        if not os.path.exists(online):
+            raise ValueError, _(
+                "zfcp device %s not found, not even in device ignore list"
+                %(self.devnum,))

-            loggedWriteLineToFile(unitadd, self.fcplun)
-            loggedWriteLineToFile(online, "1")
+        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 Exception, e:
-            log.warn("error bringing zfcp device %s online: %s"
-                     %(self.devnum, e))
-            return False
+            raise ValueError, _(
+                "could not set zfcp device %s online (%s)"
+                %(self.devnum, e))
+
+        if not os.path.exists(portdir):
+            if os.path.exists(portadd):
+                # older zfcp sysfs interface
+                try:
+                    loggedWriteLineToFile(portadd, self.wwpn)
+                    udev_settle()
+                except Exception, e:
+                    raise ValueError, _(
+                        "could not add wwpn %s to zfcp device %s (%s)"
+                        %(self.wwpn, self.devnum, e))
+            else:
+                # newer zfcp sysfs interface with auto port scan
+                raise ValueError, _("wwpn %s not found at zfcp device %s"
+                                    %(self.wwpn, self.devnum))
+        else:
+            if os.path.exists(portadd):
+                # older zfcp sysfs interface
+                log.info("wwpn %s at zfcp device %s already there"
+                         %(self.wwpn, self.devnum))
+
+        if not os.path.exists(unitdir):
+            try:
+                loggedWriteLineToFile(unitadd, self.fcplun)
+                udev_settle()
+            except Exception, e:
+                raise ValueError, _(
+                    "could not add lun %s to wwpn %s on zfcp device %s (%s)"
+                    %(self.fcplun, self.wwpn, self.devnum, e))
+        else:
+            raise ValueError, _(
+                "lun %s at wwpn %s on zfcp device %s already configured"
+                %(self.fcplun, self.wwpn, self.devnum))

return True

[PATCH 1/5] correctly activate zFCP LUN on s390

[David Cantrell]

Comments below.

On Fri, 10 Jul 2009, Steffen Maier wrote:
> Activation happens in correct order and synchronizes with udev_settle.
> Error cases are handled gracefully and report user readable error messages.
> Supports both old (RHEL 5) and new sysfs zfcp device driver interface.
>
> storage/zfcp.py |   69 +++++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 files changed, 62 insertions(+), 7 deletions(-)
>
> diff --git a/storage/zfcp.py b/storage/zfcp.py
> index 2584268..f4cf339 100644
> --- a/storage/zfcp.py
> +++ b/storage/zfcp.py
> @@ -22,6 +22,8 @@
> import string
> import os
> from constants import *
> +from udev import udev_settle
> +from time import sleep
>
> import gettext
> _ = lambda x: gettext.ldgettext("anaconda", x)
> @@ -117,17 +119,70 @@ class ZFCPDevice:
>         online = "%s/%s/online" %(zfcpsysfs, self.devnum)
>         portadd = "%s/%s/port_add" %(zfcpsysfs, self.devnum)
>         unitadd = "%s/%s/%s/unit_add" %(zfcpsysfs, self.devnum, self.wwpn)
>    portdir = "%s/%s/%s" %(zfcpsysfs, self.devnum, self.wwpn)
>    unitdir = "%s/%s" %(portdir, self.fcplun)
>
>   try:
>        if not os.path.exists(unitadd):
>            loggedWriteLineToFile(portadd, self.wwpn)
>        if not os.path.exists(online):
>            loggedWriteLineToFile("/proc/cio_ignore",
>                                  "free %s" %(self.devnum,))
>            udev_settle()
>            sleep(1)

Is the sleep(1) necessary?
>    except Exception, e:
>        raise ValueError, _(
>            "could not free zfcp device %s from device ignore list (%s)"
>            %(self.devnum, e))

Generic catch-all exception blocks like this can make debugging later on
difficult.  I would say we should be catching specific exceptions and taking
necessary action rather than having a catch-all and logging an error.  Why did
we get Exception here, for instance?  The same is true for the catch-all
exception blocks below.  If those could be changed to catch specific
exceptions and take the right action, I would feel better about these changes.
>    if not os.path.exists(online):
>        raise ValueError, _(
>            "zfcp device %s not found, not even in device ignore list"
>            %(self.devnum,))
>
>        loggedWriteLineToFile(unitadd, self.fcplun)
>        loggedWriteLineToFile(online, "1")
>    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 Exception, e:
>        log.warn("error bringing zfcp device %s online: %s"
>                 %(self.devnum, e))
>        return False
>        raise ValueError, _(
>            "could not set zfcp device %s online (%s)"
>            %(self.devnum, e))

Here.
>    if not os.path.exists(portdir):
>        if os.path.exists(portadd):
>            # older zfcp sysfs interface
>            try:
>                loggedWriteLineToFile(portadd, self.wwpn)
>                udev_settle()
>            except Exception, e:
>                raise ValueError, _(
>                    "could not add wwpn %s to zfcp device %s (%s)"
>                    %(self.wwpn, self.devnum, e))

Here.
>        else:
>            # newer zfcp sysfs interface with auto port scan
>            raise ValueError, _("wwpn %s not found at zfcp device %s"
>                                %(self.wwpn, self.devnum))
>    else:
>        if os.path.exists(portadd):
>            # older zfcp sysfs interface
>            log.info("wwpn %s at zfcp device %s already there"
>                     %(self.wwpn, self.devnum))
>    if not os.path.exists(unitdir):
>        try:
>            loggedWriteLineToFile(unitadd, self.fcplun)
>            udev_settle()
>        except Exception, e:
>            raise ValueError, _(
>                "could not add lun %s to wwpn %s on zfcp device %s (%s)"
>                %(self.fcplun, self.wwpn, self.devnum, e))

Here.
>    else:
>        raise ValueError, _(
>            "lun %s at wwpn %s on zfcp device %s already configured"
>            %(self.fcplun, self.wwpn, self.devnum))
>
>   return True

The blocks above are catching any exception from:

loggedWriteLineToFile()
     udev_settle()
     file opening/closing

loggedWriteLineToFile() could be modified to handle exceptions, if we even
care.  It opens a file, writes data, closes the file.

Likewise for the open file open and close.

udev_settle() we probably care about catching exceptions from it.

[PATCH 1/5] correctly activate zFCP LUN on s390

[Steffen Maier]

On 07/11/2009 01:44 AM, David Cantrell wrote:
> On Fri, 10 Jul 2009, Steffen Maier wrote:
> > Activation happens in correct order and synchronizes with udev_settle.
> > Error cases are handled gracefully and report user readable error
> > messages.
> > Supports both old (RHEL 5) and new sysfs zfcp device driver interface.
> >
> > storage/zfcp.py |   69
> > +++++++++++++++++++++++++++++++++++++++++++++++++-----
> > 1 files changed, 62 insertions(+), 7 deletions(-)
> >        if not os.path.exists(online):
> >            loggedWriteLineToFile("/proc/cio_ignore",
> >                                  "free %s" %(self.devnum,))
> >            udev_settle()
> >            sleep(1)
>
> Is the sleep(1) necessary?

Not sure, tried without and it succeeded, so let's go without and add it
back in, should it fail to access sysfs attributes in the future.
> >    except Exception, e:
> >        raise ValueError, _(
> >            "could not free zfcp device %s from device ignore
> >
> > list (%s)"
> >            %(self.devnum, e))
>
> Generic catch-all exception blocks like this can make debugging later on
> difficult.  I would say we should be catching specific exceptions and
> taking
> necessary action rather than having a catch-all and logging an error.

Agreed. I'll send reworked patches.
> The blocks above are catching any exception from:

Could someone please help me as a Python newbie identify which
exceptions may be thrown by the following functions?
After looking at the Python doc and some anaconda source, I came up with
this:
> loggedWriteLineToFile()

IOError
> udev_settle()

IOError, os.error, RuntimeError
> file opening/closing

IOError
> loggedWriteLineToFile() could be modified to handle exceptions, if we even
> care.  It opens a file, writes data, closes the file.

loggedWriteLineToFile does not know enough to throw a user readable
error message to be logged or displayed, so we don't handle exceptions
here but rather at the caller.

We do care very much about those exceptions. What you refer to as
accessing a "file" are sysfs attributes. They may be there or not, where
the latter is no reason to bail out from the installer. Writing to sysfs
attributes provided by device drivers is comparable to an ioctl and as
such they can return different errors, e.g. EINVAL, EBUSY, ENODEV, or
the like. Again we want to know about such errors, since knowing about
the device driver error will also help developers to determine user
problems.

BTW, ignoring errors is really a suboptimal habit at least for s390
specific code in anaconda. I wish people would become aware, that
correctly driving the device driver user space interface is important.
Bailing out of the installer or even silently dropping errors is
anything but helpful. I heard customers complaining that they could not
activate their FCP LUNs and didn't get any error. The "Add zFCP" dialog
just returned but their expected SCSI disk was still not there. I think
we don't want users to peek into /tmp/anaconda.log living in a volatile
ramdisk and hope to find something there, as long as we're able to
provide meaningful error messages. Users (of an enterprise Linux
distribution), especially on s390, expect resilience and right so.
> Likewise for the open file open and close.

Mostly sysfs attributes, of which I want to know if they were not there
but continue with the installer. An error here usually just means that
the user could not get his LUN activated.
> udev_settle() we probably care about catching exceptions from it.

Assuming IOError, os.error, and RuntimeError might be thrown by
udev_settle, I guess I'm not interested in catching RuntimeError, since
this seems to be thrown if things in execWithRedirect are really wrong.
Not sure about os.error, though. Any comments?

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

[PATCH 1/5] correctly activate zFCP LUN on s390

[David Cantrell]

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

On Sun, 16 Aug 2009, Steffen Maier wrote:
> On 07/11/2009 01:44 AM, David Cantrell wrote:
> > On Fri, 10 Jul 2009, Steffen Maier wrote:
> > > Activation happens in correct order and synchronizes with udev_settle.
> > > Error cases are handled gracefully and report user readable error
> > > messages.
> > > Supports both old (RHEL 5) and new sysfs zfcp device driver interface.
> > >
> > > storage/zfcp.py |   69
> > > +++++++++++++++++++++++++++++++++++++++++++++++++-----
> > > 1 files changed, 62 insertions(+), 7 deletions(-)
> > >        if not os.path.exists(online):
> > >            loggedWriteLineToFile("/proc/cio_ignore",
> > >                                  "free %s" %(self.devnum,))
> > >            udev_settle()
> > >            sleep(1)
> >
> > Is the sleep(1) necessary?
>
> Not sure, tried without and it succeeded, so let's go without and add it
> back in, should it fail to access sysfs attributes in the future.

Sounds fine.
> > >    except Exception, e:
> > >        raise ValueError, _(
> > >            "could not free zfcp device %s from device ignore
> > >
> > > list (%s)"
> > >            %(self.devnum, e))
> >
> > Generic catch-all exception blocks like this can make debugging later on
> > difficult.  I would say we should be catching specific exceptions and
> > taking
> > necessary action rather than having a catch-all and logging an error.
>
> Agreed. I'll send reworked patches.

The new patches look better.
> > The blocks above are catching any exception from:
>
> Could someone please help me as a Python newbie identify which
> exceptions may be thrown by the following functions?
> After looking at the Python doc and some anaconda source, I came up with
> this:
> > loggedWriteLineToFile()
>
> IOError
> > udev_settle()
>
> IOError, os.error, RuntimeError
> > file opening/closing
>
> IOError

Looks more or less complete to me.
> > loggedWriteLineToFile() could be modified to handle exceptions, if we even
> > care.  It opens a file, writes data, closes the file.
>
> loggedWriteLineToFile does not know enough to throw a user readable
> error message to be logged or displayed, so we don't handle exceptions
> here but rather at the caller.

OK.
> We do care very much about those exceptions. What you refer to as
> accessing a "file" are sysfs attributes. They may be there or not, where
> the latter is no reason to bail out from the installer. Writing to sysfs
> attributes provided by device drivers is comparable to an ioctl and as
> such they can return different errors, e.g. EINVAL, EBUSY, ENODEV, or
> the like. Again we want to know about such errors, since knowing about
> the device driver error will also help developers to determine user
> problems.
>
> BTW, ignoring errors is really a suboptimal habit at least for s390
> specific code in anaconda. I wish people would become aware, that
> correctly driving the device driver user space interface is important.
> Bailing out of the installer or even silently dropping errors is
> anything but helpful. I heard customers complaining that they could not
> activate their FCP LUNs and didn't get any error. The "Add zFCP" dialog
> just returned but their expected SCSI disk was still not there. I think
> we don't want users to peek into /tmp/anaconda.log living in a volatile
> ramdisk and hope to find something there, as long as we're able to
> provide meaningful error messages. Users (of an enterprise Linux
> distribution), especially on s390, expect resilience and right so.

That's fine and that's where your experience with s390 is helping us.  On no
other platform do we need to load modules and then fiddle with bits in sysfs
to tell the driver what we want it to do.  We're not generally used to that
sort of driver behavior.  But if that's what s390 requires, that's fine.  Your
new patches are done more in line with what I was saying.  It's the catch-all
for Exception that is a red flag to me.  From my point of view, if something
other than IOError occurs, we do want to see a traceback because it might
require a different recovery path than IOError does.
> > Likewise for the open file open and close.
>
> Mostly sysfs attributes, of which I want to know if they were not there
> but continue with the installer. An error here usually just means that
> the user could not get his LUN activated.

OK.
> > udev_settle() we probably care about catching exceptions from it.
>
> Assuming IOError, os.error, and RuntimeError might be thrown by
> udev_settle, I guess I'm not interested in catching RuntimeError, since
> this seems to be thrown if things in execWithRedirect are really wrong.
> Not sure about os.error, though. Any comments?

RuntimeError usually indicates a larger problem.  One that doesn't have a
recovery path in anaconda.

OSError probably also indicates something larger, but it does mean we are
still running and could possibly recover.  Catch it if you want to.

- -- 
David Cantrell 
Red Hat / Honolulu, HI
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkqLL0gACgkQ5hsjjIy1VknzMQCeIbnoIfmihyo6nz63iIQ4F2wk
> WWcAn1KYfEmMtQbfKY7KfuPYNO3Na639
> =53We
> -----END PGP SIGNATURE-----

[PATCH 1/5] correctly activate zFCP LUN on s390

[Steffen Maier]

Activation happens in correct order and synchronizes with udev_settle.
Error cases are handled gracefully and report user readable error messages.
Supports both old (RHEL 5) and new sysfs zfcp device driver interface.
---
 storage/zfcp.py |   89 ++++++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 79 insertions(+), 10 deletions(-)

diff --git a/storage/zfcp.py b/storage/zfcp.py
index b948eb5..8239206 100644
--- a/storage/zfcp.py
+++ b/storage/zfcp.py
@@ -22,6 +22,7 @@
 import string
 import os
 from constants import *
+from udev import udev_settle

import gettext
 _ = lambda x: gettext.ldgettext("anaconda", x)
@@ -116,18 +117,86 @@ class ZFCPDevice:
     def onlineDevice(self):
         online = "%s/%s/online" %(zfcpsysfs, self.devnum)
         portadd = "%s/%s/port_add" %(zfcpsysfs, self.devnum)
-        unitadd = "%s/%s/%s/unit_add" %(zfcpsysfs, self.devnum, self.wwpn)
+        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(unitadd):
-                loggedWriteLineToFile(portadd, self.wwpn)
+            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 %s from device ignore list (%s)."
+                %(self.devnum, e))

-            loggedWriteLineToFile(unitadd, self.fcplun)
-            loggedWriteLineToFile(online, "1")
-        except Exception, e:
-            log.warn("error bringing zfcp device %s online: %s"
-                     %(self.devnum, e))
-            return False
+        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()
+            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 %s online (%s)."
+                %(self.devnum, e))
+
+        if not os.path.exists(portdir):
+            if os.path.exists(portadd):
+                # older zfcp sysfs interface
+                try:
+                    loggedWriteLineToFile(portadd, self.wwpn)
+                    udev_settle()
+                except IOError as e:
+                    raise ValueError, _(
+                        "Could not add WWPN %s to zFCP device %s (%s)."
+                        %(self.wwpn, self.devnum, e))
+            else:
+                # newer zfcp sysfs interface with auto port scan
+                raise ValueError, _("WWPN %s not found at zFCP device %s."
+                                    %(self.wwpn, self.devnum))
+        else:
+            if os.path.exists(portadd):
+                # older zfcp sysfs interface
+                log.info("WWPN %s at zFCP device %s already there."
+                         %(self.wwpn, self.devnum))
+
+        if not os.path.exists(unitdir):
+            try:
+                loggedWriteLineToFile(unitadd, self.fcplun)
+                udev_settle()
+            except IOError as e:
+                raise ValueError, _(
+                    "Could not add LUN %s to WWPN %s on zFCP device %s (%s)."
+                    %(self.fcplun, self.wwpn, self.devnum, e))
+        else:
+            raise ValueError, _(
+                "LUN %s at WWPN %s on zFCP device %s already configured."
+                %(self.fcplun, self.wwpn, self.devnum))
+
+        fail = "0"
+        try:
+            f = open(failed, "r")
+            fail = f.readline().strip()
+            f.close()
+        except IOError as e:
+            raise ValueError, _(
+                "Could not read failed attribute of LUN %s at WWPN %s on zFCP device %s (%s)."
+                %(self.fcplun, self.wwpn, self.devnum, e))
+        if fail != "0":
+            self.offlineDevice()
+            raise ValueError, _(
+                "Failed LUN %s at WWPN %s on zFCP device %s removed again."
+                %(self.fcplun, self.wwpn, self.devnum))

return True

[PATCH 1/5] correctly activate zFCP LUN on s390

[David Cantrell]

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

On Sun, 16 Aug 2009, Steffen Maier wrote:
> Activation happens in correct order and synchronizes with udev_settle.
> Error cases are handled gracefully and report user readable error messages.
> Supports both old (RHEL 5) and new sysfs zfcp device driver interface.
>
> storage/zfcp.py |   89 ++++++++++++++++++++++++++++++++++++++++++++++++------
> 1 files changed, 79 insertions(+), 10 deletions(-)
>
> diff --git a/storage/zfcp.py b/storage/zfcp.py
> index b948eb5..8239206 100644
> --- a/storage/zfcp.py
> +++ b/storage/zfcp.py
> @@ -22,6 +22,7 @@
> import string
> import os
> from constants import *
> +from udev import udev_settle
>
> import gettext
> _ = lambda x: gettext.ldgettext("anaconda", x)
> @@ -116,18 +117,86 @@ class ZFCPDevice:
>     def onlineDevice(self):
>         online = "%s/%s/online" %(zfcpsysfs, self.devnum)
>         portadd = "%s/%s/port_add" %(zfcpsysfs, self.devnum)
>    unitadd = "%s/%s/%s/unit_add" %(zfcpsysfs, self.devnum, self.wwpn)
>    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(unitadd):
>            loggedWriteLineToFile(portadd, self.wwpn)
>        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 %s from device ignore list (%s)."
>            %(self.devnum, e))
>
>        loggedWriteLineToFile(unitadd, self.fcplun)
>        loggedWriteLineToFile(online, "1")
>    except Exception, e:
>        log.warn("error bringing zfcp device %s online: %s"
>                 %(self.devnum, e))
>        return False
>    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()
>        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 %s online (%s)."
>            %(self.devnum, e))
>    if not os.path.exists(portdir):
>        if os.path.exists(portadd):
>            # older zfcp sysfs interface
>            try:
>                loggedWriteLineToFile(portadd, self.wwpn)
>                udev_settle()
>            except IOError as e:
>                raise ValueError, _(
>                    "Could not add WWPN %s to zFCP device %s (%s)."
>                    %(self.wwpn, self.devnum, e))
>        else:
>            # newer zfcp sysfs interface with auto port scan
>            raise ValueError, _("WWPN %s not found at zFCP device %s."
>                                %(self.wwpn, self.devnum))
>    else:
>        if os.path.exists(portadd):
>            # older zfcp sysfs interface
>            log.info("WWPN %s at zFCP device %s already there."
>                     %(self.wwpn, self.devnum))
>    if not os.path.exists(unitdir):
>        try:
>            loggedWriteLineToFile(unitadd, self.fcplun)
>            udev_settle()
>        except IOError as e:
>            raise ValueError, _(
>                "Could not add LUN %s to WWPN %s on zFCP device %s (%s)."
>                %(self.fcplun, self.wwpn, self.devnum, e))
>    else:
>        raise ValueError, _(
>            "LUN %s at WWPN %s on zFCP device %s already configured."
>            %(self.fcplun, self.wwpn, self.devnum))
>    fail = "0"
>    try:
>        f = open(failed, "r")
>        fail = f.readline().strip()
>        f.close()
>    except IOError as e:
>        raise ValueError, _(
>            "Could not read failed attribute of LUN %s at WWPN %s on zFCP device %s (%s)."
>            %(self.fcplun, self.wwpn, self.devnum, e))
>    if fail != "0":
>        self.offlineDevice()
>        raise ValueError, _(
>            "Failed LUN %s at WWPN %s on zFCP device %s removed again."
>            %(self.fcplun, self.wwpn, self.devnum))
>
>   return True

Looks better, applying to master.  Thanks.

- -- 
David Cantrell 
Red Hat / Honolulu, HI
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkqLLKgACgkQ5hsjjIy1Vklf9wCcDFYKx4FBUlLIzHePGWldnzBa
> jKAAn3HgWbiE9SMXpNiNhQfXopC+SKqZ
> =pkEP
> -----END PGP SIGNATURE-----

[PATCH 2/5] correctly delete a SCSI device provided by a zFCP LUN on s390

[Steffen Maier]

Parsing of /proc/scsi/scsi updated to work with latest zFCP.
Matching SCSI ID correctly with all of zFCP device, WWPN, and LUN.
Synchronization with udev_settle.
Error case is handled gracefully and reports user readable error message.
---
 storage/zfcp.py |   31 ++++++++++++++++++++++---------
 1 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/storage/zfcp.py b/storage/zfcp.py
index 2584268..7bb31b9 100644
--- a/storage/zfcp.py
+++ b/storage/zfcp.py
@@ -135,26 +135,39 @@ class ZFCPDevice:
         f = open("/proc/scsi/scsi", "r")
         lines = f.readlines()
         f.close()
+        # alternatively iterate over /sys/bus/scsi/devices/*:0:*:*/

for line in lines:
             if not line.startswith("Host"):
                 continue
             scsihost = string.split(line)
             host = scsihost[1]
-            channel = scsihost[3]
+            channel = "0"
             id = scsihost[5]
             lun = scsihost[7]
-            scsidev = "%s:%s:%s:%s" % (host[4], channel[1], id[1], lun[1])
-            fcpsysfs = "%s/%s/fcp_lun" % (scsidevsysfs, scsidev)
-            fcpdel = "%s/%s/delete" % (scsidevsysfs, scsidev)
+            scsidev = "%s:%s:%s:%s" % (host[4:], channel, id, lun)
+            fcpsysfs = "%s/%s" % (scsidevsysfs, scsidev)
+            scsidel = "%s/%s/delete" % (scsidevsysfs, scsidev)

-            f = open(fcpsysfs, "r")
-            fcplunsysfs = f.readline()
+            f = open("%s/hba_id" %(fcpsysfs), "r")
+            fcphbasysfs = f.readline().strip()
             f.close()
+            f = open("%s/wwpn" %(fcpsysfs), "r")
+            fcpwwpnsysfs = f.readline().strip()
+            f.close()
+            f = open("%s/fcp_lun" %(fcpsysfs), "r")
+            fcplunsysfs = f.readline().strip()
+            f.close()
+
+            if fcphbasysfs == self.devnum \
+                    and fcpwwpnsysfs == self.wwpn \
+                    and fcplunsysfs == self.fcplun:
+                loggedWriteLineToFile(scsidel, "1")
+                udev_settle()
+                return

-            if fcplunsysfs[:6] == self.fcplun[:6]:
-                loggedWriteLineToFile(fcpdel, "1")
-                break
+        log.warn("no scsi device found to delete for zfcp %s %s %s"
+                 %(self.devnum, self.wwpn, self.fcplun))

def offlineDevice(self):
         offline = "%s/%s/online" %(zfcpsysfs, self.devnum)

[PATCH 2/5] correctly delete a SCSI device provided by a zFCP LUN on s390

[David Cantrell]

This one looks good, applying it to master.

On Fri, 10 Jul 2009, Steffen Maier wrote:
> Parsing of /proc/scsi/scsi updated to work with latest zFCP.
> Matching SCSI ID correctly with all of zFCP device, WWPN, and LUN.
> Synchronization with udev_settle.
> Error case is handled gracefully and reports user readable error message.
>
> storage/zfcp.py |   31 ++++++++++++++++++++++---------
> 1 files changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/storage/zfcp.py b/storage/zfcp.py
> index 2584268..7bb31b9 100644
> --- a/storage/zfcp.py
> +++ b/storage/zfcp.py
> @@ -135,26 +135,39 @@ class ZFCPDevice:
>         f = open("/proc/scsi/scsi", "r")
>         lines = f.readlines()
>         f.close()
>    # alternatively iterate over /sys/bus/scsi/devices/*:0:*:*/
>
>   for line in lines:
>       if not line.startswith("Host"):
>           continue
>       scsihost = string.split(line)
>       host = scsihost[1]
>        channel = scsihost[3]
>        channel = "0"
>       id = scsihost[5]
>       lun = scsihost[7]
>        scsidev = "%s:%s:%s:%s" % (host[4], channel[1], id[1], lun[1])
>        fcpsysfs = "%s/%s/fcp_lun" % (scsidevsysfs, scsidev)
>        fcpdel = "%s/%s/delete" % (scsidevsysfs, scsidev)
>        scsidev = "%s:%s:%s:%s" % (host[4:], channel, id, lun)
>        fcpsysfs = "%s/%s" % (scsidevsysfs, scsidev)
>        scsidel = "%s/%s/delete" % (scsidevsysfs, scsidev)
>
>        f = open(fcpsysfs, "r")
>        fcplunsysfs = f.readline()
>        f = open("%s/hba_id" %(fcpsysfs), "r")
>        fcphbasysfs = f.readline().strip()
>       f.close()
>        f = open("%s/wwpn" %(fcpsysfs), "r")
>        fcpwwpnsysfs = f.readline().strip()
>        f.close()
>        f = open("%s/fcp_lun" %(fcpsysfs), "r")
>        fcplunsysfs = f.readline().strip()
>        f.close()
>        if fcphbasysfs == self.devnum \
>                and fcpwwpnsysfs == self.wwpn \
>                and fcplunsysfs == self.fcplun:
>            loggedWriteLineToFile(scsidel, "1")
>            udev_settle()
>            return
>
>        if fcplunsysfs[:6] == self.fcplun[:6]:
>            loggedWriteLineToFile(fcpdel, "1")
>            break
>    log.warn("no scsi device found to delete for zfcp %s %s %s"
>             %(self.devnum, self.wwpn, self.fcplun))
>
> def offlineDevice(self):
>       offline = "%s/%s/online" %(zfcpsysfs, self.devnum)

[PATCH 3/5] correctly deactivate zFCP LUN on s390

[Steffen Maier]

Deactivation happens in correct order and synchronizes with udev_settle.
Error cases are handled gracefully and report user readable error messages.
Supports both old (RHEL 5) and new sysfs zfcp device driver interface.
---
 storage/zfcp.py |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/storage/zfcp.py b/storage/zfcp.py
index 2584268..ee72580 100644
--- a/storage/zfcp.py
+++ b/storage/zfcp.py
@@ -158,18 +158,64 @@ class ZFCPDevice:

def offlineDevice(self):
         offline = "%s/%s/online" %(zfcpsysfs, self.devnum)
+        portadd = "%s/%s/port_add" %(zfcpsysfs, self.devnum)
         portremove = "%s/%s/port_remove" %(zfcpsysfs, self.devnum)
         unitremove = "%s/%s/%s/unit_remove" %(zfcpsysfs, self.devnum, self.wwpn)
+        portdir = "%s/%s/%s" %(zfcpsysfs, self.devnum, self.wwpn)
+        devdir = "%s/%s" %(zfcpsysfs, self.devnum)

try:
             self.offlineSCSIDevice()
-            loggedWriteLineToFile(offline, "0")
+        except Exception, e:
+            raise ValueError, _(
+                "could not delete scsi device of zfcp %s %s %s (%s)"
+                %(self.devnum, self.wwpn, self.fcplun, e))
+
+        try:
             loggedWriteLineToFile(unitremove, self.fcplun)
-            loggedWriteLineToFile(portremove, self.wwpn)
         except Exception, e:
-            log.warn("error bringing zfcp device %s offline: %s"
-                     %(self.devnum, e))
-            return False
+            raise ValueError, _(
+                "could not remove lun %s at wwpn %s on zfcp device %s (%s)"
+                %(self.fcplun, self.wwpn, self.devnum, e))
+
+        if os.path.exists(portadd):
+            # only try to remove ports with older zfcp sysfs interface
+            for lun in os.listdir(portdir):
+                if lun.startswith("0x") and \
+                        os.path.isdir(os.path.join(portdir, lun)):
+                    log.info("not removing wwpn %s at zfcp device %s since port still has other luns, e.g. %s"
+                             %(self.wwpn, self.devnum, lun))
+                    return True
+
+            try:
+                loggedWriteLineToFile(portremove, self.wwpn)
+            except Exception, e:
+                raise ValueError, _("could not remove wwpn %s on zfcp device %s (%s)"
+                                    %(self.wwpn, self.devnum, e))
+
+        if os.path.exists(portadd):
+            # older zfcp sysfs interface
+            for port in os.listdir(devdir):
+                if port.startswith("0x") and \
+                        os.path.isdir(os.path.join(devdir, port)):
+                    log.info("not setting zfcp device %s offline since it still has other ports, e.g. %s"
+                             %(self.devnum, port))
+                    return True
+        else:
+            # newer zfcp sysfs interface with auto port scan
+            import glob
+            luns = glob.glob("%s/0x????????????????/0x????????????????"
+                          %(devdir,))
+            if len(luns) != 0:
+                log.info("not setting zfcp device %s offline since it still has other luns, e.g. %s"
+                         %(self.devnum, luns[0]))
+                return True
+
+        try:
+            loggedWriteLineToFile(offline, "0")
+        except Exception, e:
+            raise ValueError, _("could not set zfcp device %s offline (%s)"
+                                %(self.devnum, e))

return True

[PATCH 3/5] correctly deactivate zFCP LUN on s390

[David Cantrell]

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

Same comments as from the first patch.  Like what's going on here, but change
the generic Exception blocks to catch specific exceptions and take appropriate
action.

On Fri, 10 Jul 2009, Steffen Maier wrote:
> Deactivation happens in correct order and synchronizes with udev_settle.
> Error cases are handled gracefully and report user readable error messages.
> Supports both old (RHEL 5) and new sysfs zfcp device driver interface.
>
> storage/zfcp.py |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++----
> 1 files changed, 51 insertions(+), 5 deletions(-)
>
> diff --git a/storage/zfcp.py b/storage/zfcp.py
> index 2584268..ee72580 100644
> --- a/storage/zfcp.py
> +++ b/storage/zfcp.py
> @@ -158,18 +158,64 @@ class ZFCPDevice:
> def offlineDevice(self):
>     offline = "%s/%s/online" %(zfcpsysfs, self.devnum)
>    portadd = "%s/%s/port_add" %(zfcpsysfs, self.devnum)
>   portremove = "%s/%s/port_remove" %(zfcpsysfs, self.devnum)
>   unitremove = "%s/%s/%s/unit_remove" %(zfcpsysfs, self.devnum, self.wwpn)
>    portdir = "%s/%s/%s" %(zfcpsysfs, self.devnum, self.wwpn)
>    devdir = "%s/%s" %(zfcpsysfs, self.devnum)
>
>   try:
>       self.offlineSCSIDevice()
>        loggedWriteLineToFile(offline, "0")
>    except Exception, e:
>        raise ValueError, _(
>            "could not delete scsi device of zfcp %s %s %s (%s)"
>            %(self.devnum, self.wwpn, self.fcplun, e))
>    try:
>       loggedWriteLineToFile(unitremove, self.fcplun)
>        loggedWriteLineToFile(portremove, self.wwpn)
>   except Exception, e:
>        log.warn("error bringing zfcp device %s offline: %s"
>                 %(self.devnum, e))
>        return False
>        raise ValueError, _(
>            "could not remove lun %s at wwpn %s on zfcp device %s (%s)"
>            %(self.fcplun, self.wwpn, self.devnum, e))
>    if os.path.exists(portadd):
>        # only try to remove ports with older zfcp sysfs interface
>        for lun in os.listdir(portdir):
>            if lun.startswith("0x") and \
>                    os.path.isdir(os.path.join(portdir, lun)):
>                log.info("not removing wwpn %s at zfcp device %s since port still has other luns, e.g. %s"
>                         %(self.wwpn, self.devnum, lun))
>                return True
>        try:
>            loggedWriteLineToFile(portremove, self.wwpn)
>        except Exception, e:
>            raise ValueError, _("could not remove wwpn %s on zfcp device %s (%s)"
>                                %(self.wwpn, self.devnum, e))
>    if os.path.exists(portadd):
>        # older zfcp sysfs interface
>        for port in os.listdir(devdir):
>            if port.startswith("0x") and \
>                    os.path.isdir(os.path.join(devdir, port)):
>                log.info("not setting zfcp device %s offline since it still has other ports, e.g. %s"
>                         %(self.devnum, port))
>                return True
>    else:
>        # newer zfcp sysfs interface with auto port scan
>        import glob
>        luns = glob.glob("%s/0x????????????????/0x????????????????"
>                      %(devdir,))
>        if len(luns) != 0:
>            log.info("not setting zfcp device %s offline since it still has other luns, e.g. %s"
>                     %(self.devnum, luns[0]))
>            return True
>    try:
>        loggedWriteLineToFile(offline, "0")
>    except Exception, e:
>        raise ValueError, _("could not set zfcp device %s offline (%s)"
>                            %(self.devnum, e))
>
>   return True

- -- 
David Cantrell 
Red Hat / Honolulu, HI
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkpX0ysACgkQ5hsjjIy1Vkk/gQCeJGVORjGuTI9RxVXMzaWfyUI7
> UiMAoKn6ljWJSmKHPRIOa3hSzIAm3JXu
> =nOu9
> -----END PGP SIGNATURE-----

[PATCH 3/5] correctly deactivate zFCP LUN on s390

[Steffen Maier]

Deactivation happens in correct order and synchronizes with udev_settle.
Error cases are handled gracefully and report user readable error messages.
Supports both old (RHEL 5) and new sysfs zfcp device driver interface.
---
 storage/zfcp.py |   58 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/storage/zfcp.py b/storage/zfcp.py
index b948eb5..40803fb 100644
--- a/storage/zfcp.py
+++ b/storage/zfcp.py
@@ -171,18 +171,64 @@ class ZFCPDevice:

def offlineDevice(self):
         offline = "%s/%s/online" %(zfcpsysfs, self.devnum)
+        portadd = "%s/%s/port_add" %(zfcpsysfs, self.devnum)
         portremove = "%s/%s/port_remove" %(zfcpsysfs, self.devnum)
         unitremove = "%s/%s/%s/unit_remove" %(zfcpsysfs, self.devnum, self.wwpn)
+        portdir = "%s/%s/%s" %(zfcpsysfs, self.devnum, self.wwpn)
+        devdir = "%s/%s" %(zfcpsysfs, self.devnum)

try:
             self.offlineSCSIDevice()
-            loggedWriteLineToFile(offline, "0")
+        except IOError as e:
+            raise ValueError, _(
+                "Could not correctly delete SCSI device of zFCP %s %s %s (%s)."
+                %(self.devnum, self.wwpn, self.fcplun, e))
+
+        try:
             loggedWriteLineToFile(unitremove, self.fcplun)
-            loggedWriteLineToFile(portremove, self.wwpn)
-        except Exception, e:
-            log.warn("error bringing zfcp device %s offline: %s"
-                     %(self.devnum, e))
-            return False
+        except IOError as e:
+            raise ValueError, _(
+                "Could not remove LUN %s at WWPN %s on zFCP device %s (%s)."
+                %(self.fcplun, self.wwpn, self.devnum, e))
+
+        if os.path.exists(portadd):
+            # only try to remove ports with older zfcp sysfs interface
+            for lun in os.listdir(portdir):
+                if lun.startswith("0x") and \
+                        os.path.isdir(os.path.join(portdir, lun)):
+                    log.info("Not removing WWPN %s at zFCP device %s since port still has other LUNs, e.g. %s."
+                             %(self.wwpn, self.devnum, lun))
+                    return True
+
+            try:
+                loggedWriteLineToFile(portremove, self.wwpn)
+            except IOError as e:
+                raise ValueError, _("Could not remove WWPN %s on zFCP device %s (%s)."
+                                    %(self.wwpn, self.devnum, e))
+
+        if os.path.exists(portadd):
+            # older zfcp sysfs interface
+            for port in os.listdir(devdir):
+                if port.startswith("0x") and \
+                        os.path.isdir(os.path.join(devdir, port)):
+                    log.info("Not setting zFCP device %s offline since it still has other ports, e.g. %s."
+                             %(self.devnum, port))
+                    return True
+        else:
+            # newer zfcp sysfs interface with auto port scan
+            import glob
+            luns = glob.glob("%s/0x????????????????/0x????????????????"
+                          %(devdir,))
+            if len(luns) != 0:
+                log.info("Not setting zFCP device %s offline since it still has other LUNs, e.g. %s."
+                         %(self.devnum, luns[0]))
+                return True
+
+        try:
+            loggedWriteLineToFile(offline, "0")
+        except IOError as e:
+            raise ValueError, _("Could not set zFCP device %s offline (%s)."
+                                %(self.devnum, e))

return True

[PATCH 3/5] correctly deactivate zFCP LUN on s390

[David Cantrell]

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

On Sun, 16 Aug 2009, Steffen Maier wrote:
> Deactivation happens in correct order and synchronizes with udev_settle.
> Error cases are handled gracefully and report user readable error messages.
> Supports both old (RHEL 5) and new sysfs zfcp device driver interface.
>
> storage/zfcp.py |   58 +++++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 files changed, 52 insertions(+), 6 deletions(-)
>
> diff --git a/storage/zfcp.py b/storage/zfcp.py
> index b948eb5..40803fb 100644
> --- a/storage/zfcp.py
> +++ b/storage/zfcp.py
> @@ -171,18 +171,64 @@ class ZFCPDevice:
> def offlineDevice(self):
>     offline = "%s/%s/online" %(zfcpsysfs, self.devnum)
>    portadd = "%s/%s/port_add" %(zfcpsysfs, self.devnum)
>   portremove = "%s/%s/port_remove" %(zfcpsysfs, self.devnum)
>   unitremove = "%s/%s/%s/unit_remove" %(zfcpsysfs, self.devnum, self.wwpn)
>    portdir = "%s/%s/%s" %(zfcpsysfs, self.devnum, self.wwpn)
>    devdir = "%s/%s" %(zfcpsysfs, self.devnum)
>
>   try:
>       self.offlineSCSIDevice()
>        loggedWriteLineToFile(offline, "0")
>    except IOError as e:
>        raise ValueError, _(
>            "Could not correctly delete SCSI device of zFCP %s %s %s (%s)."
>            %(self.devnum, self.wwpn, self.fcplun, e))
>    try:
>       loggedWriteLineToFile(unitremove, self.fcplun)
>        loggedWriteLineToFile(portremove, self.wwpn)
>    except Exception, e:
>        log.warn("error bringing zfcp device %s offline: %s"
>                 %(self.devnum, e))
>        return False
>    except IOError as e:
>        raise ValueError, _(
>            "Could not remove LUN %s at WWPN %s on zFCP device %s (%s)."
>            %(self.fcplun, self.wwpn, self.devnum, e))
>    if os.path.exists(portadd):
>        # only try to remove ports with older zfcp sysfs interface
>        for lun in os.listdir(portdir):
>            if lun.startswith("0x") and \
>                    os.path.isdir(os.path.join(portdir, lun)):
>                log.info("Not removing WWPN %s at zFCP device %s since port still has other LUNs, e.g. %s."
>                         %(self.wwpn, self.devnum, lun))
>                return True
>        try:
>            loggedWriteLineToFile(portremove, self.wwpn)
>        except IOError as e:
>            raise ValueError, _("Could not remove WWPN %s on zFCP device %s (%s)."
>                                %(self.wwpn, self.devnum, e))
>    if os.path.exists(portadd):
>        # older zfcp sysfs interface
>        for port in os.listdir(devdir):
>            if port.startswith("0x") and \
>                    os.path.isdir(os.path.join(devdir, port)):
>                log.info("Not setting zFCP device %s offline since it still has other ports, e.g. %s."
>                         %(self.devnum, port))
>                return True
>    else:
>        # newer zfcp sysfs interface with auto port scan
>        import glob
>        luns = glob.glob("%s/0x????????????????/0x????????????????"
>                      %(devdir,))
>        if len(luns) != 0:
>            log.info("Not setting zFCP device %s offline since it still has other LUNs, e.g. %s."
>                     %(self.devnum, luns[0]))
>            return True
>    try:
>        loggedWriteLineToFile(offline, "0")
>    except IOError as e:
>        raise ValueError, _("Could not set zFCP device %s offline (%s)."
>                            %(self.devnum, e))
>
>   return True

Applying to master, thanks.

- -- 
David Cantrell 
Red Hat / Honolulu, HI
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkqLLMIACgkQ5hsjjIy1VklGjwCg5QYWypn5KpmdwvHS5+c12Qyj
> ObUAoPFvGNnT4MKqsk9m/CRvxSUgsFb5
> =9Mev
> -----END PGP SIGNATURE-----

[PATCH 4/5] error messages of zFCP on s390: log or pass to the UI

[Steffen Maier]

---
 kickstart.py            |    5 ++++-
 storage/zfcp.py         |   12 +++++++++---
 textw/partition_text.py |    5 ++++-
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/kickstart.py b/kickstart.py
index a53dac9..bf61333 100644
--- a/kickstart.py
+++ b/kickstart.py
@@ -966,7 +966,10 @@ class ZFCP(commands.zfcp.FC3_ZFCP):
     def parse(self, args):
         retval = commands.zfcp.FC3_ZFCP.parse(self, args)
         for fcp in self.zfcp:
-            self.handler.id.zfcp.addFCP(fcp.devnum, fcp.wwpn, fcp.fcplun)
+            try:
+                self.handler.id.zfcp.addFCP(fcp.devnum, fcp.wwpn, fcp.fcplun)
+            except ValueError, e:
+                log.warning(str(e))

isys.flushDriveDict()
         return retval
diff --git a/storage/zfcp.py b/storage/zfcp.py
index 2584268..4606bb3 100644
--- a/storage/zfcp.py
+++ b/storage/zfcp.py
@@ -212,7 +212,7 @@ class ZFCP:
             try:
                 self.addFCP(devnum, wwpn, fcplun)
             except ValueError, e:
-                log.warn("Invalid FCP device configuration: %s" %(e,))
+                log.warn(str(e))
                 continue

def addFCP(self, devnum, wwpn, fcplun):
@@ -224,7 +224,10 @@ class ZFCP:
         if len(self.fcpdevs) == 0:
             return
         for d in self.fcpdevs:
-            d.offlineDevice()
+            try:
+                d.offlineDevice()
+            except ValueError, e:
+                log.warn(str(e))

def startup(self):
         if not self.hasReadConfig:
@@ -234,7 +237,10 @@ class ZFCP:
         if len(self.fcpdevs) == 0:
             return
         for d in self.fcpdevs:
-            d.onlineDevice()
+            try:
+                d.onlineDevice()
+            except ValueError, e:
+                log.warn(str(e))

def writeKS(self, f):
         if len(self.fcpdevs) == 0:
diff --git a/textw/partition_text.py b/textw/partition_text.py
index f14a273..412a7d7 100644
--- a/textw/partition_text.py
+++ b/textw/partition_text.py
@@ -211,7 +211,10 @@ class PartitionTypeWindow:
         devnum = entries[0].strip()
         wwpn = entries[1].strip()
         fcplun = entries[2].strip()
-        self.anaconda.id.storage.zfcp.addFCP(devnum, wwpn, fcplun)
+        try:
+            self.anaconda.id.storage.zfcp.addFCP(devnum, wwpn, fcplun)
+        except ValueError, e:
+            log.warn(str(e)) # alternatively popup error dialog instead

return INSTALL_OK

[PATCH 4/5] error messages of zFCP on s390: log or pass to the UI

[David Cantrell]

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

Looks good, applying to master.

On Fri, 10 Jul 2009, Steffen Maier wrote:
> kickstart.py            |    5 ++++-
> storage/zfcp.py         |   12 +++++++++---
> textw/partition_text.py |    5 ++++-
> 3 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/kickstart.py b/kickstart.py
> index a53dac9..bf61333 100644
> --- a/kickstart.py
> +++ b/kickstart.py
> @@ -966,7 +966,10 @@ class ZFCP(commands.zfcp.FC3_ZFCP):
>     def parse(self, args):
>         retval = commands.zfcp.FC3_ZFCP.parse(self, args)
>         for fcp in self.zfcp:
>        self.handler.id.zfcp.addFCP(fcp.devnum, fcp.wwpn, fcp.fcplun)
>        try:
>            self.handler.id.zfcp.addFCP(fcp.devnum, fcp.wwpn, fcp.fcplun)
>        except ValueError, e:
>            log.warning(str(e))
>
>   isys.flushDriveDict()
>   return retval
>
> diff --git a/storage/zfcp.py b/storage/zfcp.py
> index 2584268..4606bb3 100644
> --- a/storage/zfcp.py
> +++ b/storage/zfcp.py
> @@ -212,7 +212,7 @@ class ZFCP:
>             try:
>                 self.addFCP(devnum, wwpn, fcplun)
>             except ValueError, e:
>            log.warn("Invalid FCP device configuration: %s" %(e,))
>            log.warn(str(e))
>           continue
>
> def addFCP(self, devnum, wwpn, fcplun):
>
> @@ -224,7 +224,10 @@ class ZFCP:
>         if len(self.fcpdevs) == 0:
>             return
>         for d in self.fcpdevs:
>        d.offlineDevice()
>        try:
>            d.offlineDevice()
>        except ValueError, e:
>            log.warn(str(e))
>
> def startup(self):
>       if not self.hasReadConfig:
>
> @@ -234,7 +237,10 @@ class ZFCP:
>         if len(self.fcpdevs) == 0:
>             return
>         for d in self.fcpdevs:
>        d.onlineDevice()
>        try:
>            d.onlineDevice()
>        except ValueError, e:
>            log.warn(str(e))
>
> def writeKS(self, f):
>       if len(self.fcpdevs) == 0:
>
> diff --git a/textw/partition_text.py b/textw/partition_text.py
> index f14a273..412a7d7 100644
> --- a/textw/partition_text.py
> +++ b/textw/partition_text.py
> @@ -211,7 +211,10 @@ class PartitionTypeWindow:
>         devnum = entries[0].strip()
>         wwpn = entries[1].strip()
>         fcplun = entries[2].strip()
>    self.anaconda.id.storage.zfcp.addFCP(devnum, wwpn, fcplun)
>    try:
>        self.anaconda.id.storage.zfcp.addFCP(devnum, wwpn, fcplun)
>    except ValueError, e:
>        log.warn(str(e)) # alternatively popup error dialog instead
>
>   return INSTALL_OK

- -- 
David Cantrell 
Red Hat / Honolulu, HI
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkpX01AACgkQ5hsjjIy1VkmxFgCgjSOrSLracBboI06s1RqPpFRp
> ryQAoIl7BFFk5KZY4wn2e+UUhmYDJySU
> =zKR7
> -----END PGP SIGNATURE-----

[PATCH 5/5] prevent getting started up or shutdown again while already in such state

[Steffen Maier]

Essential with all the correct error handling in place.
---
 storage/zfcp.py |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/storage/zfcp.py b/storage/zfcp.py
index 2584268..ea7e4f4 100644
--- a/storage/zfcp.py
+++ b/storage/zfcp.py
@@ -177,6 +177,7 @@ class ZFCP:
     def __init__(self):
         self.fcpdevs = []
         self.hasReadConfig = False
+        self.down = True

def readConfig(self):
         try:
@@ -221,15 +222,23 @@ class ZFCP:
             self.fcpdevs.append(d)

def shutdown(self):
+        if self.down:
+            return
+        self.down = True
         if len(self.fcpdevs) == 0:
             return
         for d in self.fcpdevs:
             d.offlineDevice()

def startup(self):
+        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

[PATCH 5/5] prevent getting started up or shutdown again while already in such state

[David Cantrell]

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

Looks good, applying to master.

On Fri, 10 Jul 2009, Steffen Maier wrote:
> Essential with all the correct error handling in place.
>
> storage/zfcp.py |    9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/storage/zfcp.py b/storage/zfcp.py
> index 2584268..ea7e4f4 100644
> --- a/storage/zfcp.py
> +++ b/storage/zfcp.py
> @@ -177,6 +177,7 @@ class ZFCP:
>     def __init__(self):
>         self.fcpdevs = []
>         self.hasReadConfig = False
>    self.down = True
>
> def readConfig(self):
>       try:
>
> @@ -221,15 +222,23 @@ class ZFCP:
>             self.fcpdevs.append(d)
> def shutdown(self):
>    if self.down:
>        return
>    self.down = True
>   if len(self.fcpdevs) == 0:
>       return
>   for d in self.fcpdevs:
>       d.offlineDevice()
>
> def startup(self):
>    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

- -- 
David Cantrell 
Red Hat / Honolulu, HI
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkpX03cACgkQ5hsjjIy1VknG6wCglQzQ1lkn7McZHqviHo/UXRV2
> 6IoAn1p+okeBxTOqNhfo9YsV6Y1/0tl0
> =5pFh
> -----END PGP SIGNATURE-----

[PATCH 5/5] prevent getting started up or shutdown again while already in such state

[David Cantrell]

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

Or I would apply it.  Looks like I need to wait for the other updated patches
from you.

On Fri, 10 Jul 2009, David Cantrell wrote:
> Looks good, applying to master.
>
> On Fri, 10 Jul 2009, Steffen Maier wrote:
> > Essential with all the correct error handling in place.
> >
> > storage/zfcp.py |    9 +++++++++
> > 1 files changed, 9 insertions(+), 0 deletions(-)

- -- 
David Cantrell 
Red Hat / Honolulu, HI
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkpX1LsACgkQ5hsjjIy1VknFBQCg3ayo+D/uivzfDeuiW2hXeq7i
> KwcAnip8+0AzG1hab++t2m+5+UCvCLpw
> =FO2p
> -----END PGP SIGNATURE-----

[PATCH 5/5] prevent getting started up or shutdown again while already in such state

[Steffen Maier]

Essential with all the correct error handling in place.
---
 storage/zfcp.py |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/storage/zfcp.py b/storage/zfcp.py
index b948eb5..c98c3ca 100644
--- a/storage/zfcp.py
+++ b/storage/zfcp.py
@@ -190,6 +190,7 @@ class ZFCP:
     def __init__(self):
         self.fcpdevs = []
         self.hasReadConfig = False
+        self.down = True

def readConfig(self):
         try:
@@ -234,6 +235,9 @@ class ZFCP:
             self.fcpdevs.append(d)

def shutdown(self):
+        if self.down:
+            return
+        self.down = True
         if len(self.fcpdevs) == 0:
             return
         for d in self.fcpdevs:
@@ -243,9 +247,14 @@ class ZFCP:
                 log.warn(str(e))

def startup(self):
+        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

[PATCH 5/5] prevent getting started up or shutdown again while already in such state

[David Cantrell]

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

On Sun, 16 Aug 2009, Steffen Maier wrote:
> Essential with all the correct error handling in place.
>
> storage/zfcp.py |    9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/storage/zfcp.py b/storage/zfcp.py
> index b948eb5..c98c3ca 100644
> --- a/storage/zfcp.py
> +++ b/storage/zfcp.py
> @@ -190,6 +190,7 @@ class ZFCP:
>     def __init__(self):
>         self.fcpdevs = []
>         self.hasReadConfig = False
>    self.down = True
>
> def readConfig(self):
>       try:
>
> @@ -234,6 +235,9 @@ class ZFCP:
>             self.fcpdevs.append(d)
> def shutdown(self):
>    if self.down:
>        return
>    self.down = True
>   if len(self.fcpdevs) == 0:
>       return
>   for d in self.fcpdevs:
>
> @@ -243,9 +247,14 @@ class ZFCP:
>                 log.warn(str(e))
> def startup(self):
>    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

Applied to master.  Thanks.

- -- 
David Cantrell 
Red Hat / Honolulu, HI
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkqLKz8ACgkQ5hsjjIy1VkmW8gCg2REdZ0XvlK+uizMpgbXYiYFS
> UsMAoK4bonS5CxBRfc+eiUSElp66ZAsa
> =9sn2
> -----END PGP SIGNATURE-----