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.
--
David Cantrell <dcantrell@xxxxxxxxxx>
Red Hat / Honolulu, HI
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
