Everyone test this out on their mainframe....oh wait...
On s390, if you had a single DASD that needed dasdfmt run, you would get a ZeroDivisionError traceback because the self._totalCylinders value was zero. The problem was caused by the DASD.totalCylinders property and how it initialized. It would initialize itself on the first call, but since we'd already fired off a dasdfmt process for the device, the one running to capture the cylinder count couldn't get access to the device.
Users with more than one DASD needing a dasdfmt would not see the traceback, but would have an incorrect total cylinder count, so 100% would be reached when n-1 devices had been formatted.
Lastly, in certain instances, the status for the DASD could be "basic" rather than "unformatted", so treat those the same and perform a dasdfmt. --- storage/dasd.py | 49 +++++++++++++++++++++++-------------------------- 1 files changed, 23 insertions(+), 26 deletions(-)
diff --git a/storage/dasd.py b/storage/dasd.py index 19b8488..eebae53 100644 --- a/storage/dasd.py +++ b/storage/dasd.py @@ -43,9 +43,11 @@ class DASD:
def __init__(self): self._dasdlist = [] - self._totalCylinders = 0 + self.totalCylinders = 0 self._completedCylinders = 0.0 self._maxFormatJobs = 0 + self.dasdfmt = "/sbin/dasdfmt" + self.commonArgv = ["-y", "-d", "cdl", "-b", "4096"] self.started = False
def startup(self, *args, **kwargs): @@ -77,8 +79,9 @@ class DASD: status = f.read().strip() f.close()
- if status == "unformatted": - log.info(" %s is an unformatted DASD" % (device,)) + if status in ["unformatted", "basic"]: + log.info(" %s status is %s, needs dasdfmt" % (device, + status,)) self._dasdlist.append(device)
if not len(self._dasdlist): @@ -127,7 +130,21 @@ class DASD: log.info(" not running dasdfmt, exiting installer") sys.exit(0)
- argv = ["-y", "-P", "-d", "cdl", "-b", "4096"] + # gather total cylinder count + argv = ["-t", "-v"] + self.commonArgv + for dasd in self._dasdlist: + buf = iutil.execWithCapture(self.dasdfmt, argv + [dasd], + stderr="/dev/tty5") + for line in buf.splitlines(): + if line.startswith("Drive Geometry: "): + # line will look like this: + # Drive Geometry: 3339 Cylinders * 15 Heads = 50085 Tracks + cyls = long(filter(lambda s: s, line.split(' '))[2]) + self.totalCylinders += cyls + break + + # format DASDs + argv = ["-P"] + self.commonArgv
if intf: title = P_("Formatting DASD Device", "Formatting DASD Devices", c) @@ -137,7 +154,7 @@ class DASD:
for dasd in self._dasdlist: log.info("Running dasdfmt on %s" % (dasd,)) - iutil.execWithCallback("/sbin/dasdfmt", argv + [dasd], + iutil.execWithCallback(self.dasdfmt, argv + [dasd], stdout="/dev/tty5", stderr="/dev/tty5", callback=self._updateProgressWindow, callback_data=pw, echo=False) @@ -146,7 +163,7 @@ class DASD: else: for dasd in self._dasdlist: log.info("Running dasdfmt on %s" % (dasd,)) - iutil.execWithRedirect("/sbin/dasdfmt", argv + [dasd], + iutil.execWithRedirect(self.dasdfmt, argv + [dasd], stdout="/dev/tty5", stderr="/dev/tty5")
def _updateProgressWindow(self, data, callback_data=None): @@ -161,24 +178,4 @@ class DASD: self._completedCylinders += 1.0 callback_data.set(self._completedCylinders / self.totalCylinders)
- @property - def totalCylinders(self): - """ Total number of cylinders of all unformatted DASD devices. """ - if self._totalCylinders: - return self._totalCylinders - - argv = ["-t", "-v", "-y", "-d", "cdl", "-b", "4096"] - for dasd in self._dasdlist: - buf = iutil.execWithCapture("/sbin/dasdfmt", argv + [dasd], - stderr="/dev/tty5") - for line in buf.splitlines(): - if line.startswith("Drive Geometry: "): - # line will look like this: - # Drive Geometry: 3339 Cylinders * 15 Heads = 50085 Tracks - cyls = long(filter(lambda s: s, line.split(' '))[2]) - self._totalCylinders += cyls - break - - return self._totalCylinders - # vim:tw=78:ts=4:et:sw=4
ack
On 11/07/2009 12:38 AM, David Cantrell wrote:
On s390, if you had a single DASD that needed dasdfmt run, you would get a ZeroDivisionError traceback because the self._totalCylinders value was zero. The problem was caused by the DASD.totalCylinders property and how it initialized. It would initialize itself on the first call, but since we'd already fired off a dasdfmt process for the device, the one running to capture the cylinder count couldn't get access to the device.
Users with more than one DASD needing a dasdfmt would not see the traceback, but would have an incorrect total cylinder count, so 100% would be reached when n-1 devices had been formatted.
Lastly, in certain instances, the status for the DASD could be "basic" rather than "unformatted", so treat those the same and perform a dasdfmt.
storage/dasd.py | 49 +++++++++++++++++++++++-------------------------- 1 files changed, 23 insertions(+), 26 deletions(-)
diff --git a/storage/dasd.py b/storage/dasd.py index 19b8488..eebae53 100644 --- a/storage/dasd.py +++ b/storage/dasd.py @@ -43,9 +43,11 @@ class DASD:
def __init__(self): self._dasdlist = []
self._totalCylinders = 0
self.totalCylinders = 0 self._completedCylinders = 0.0 self._maxFormatJobs = 0
self.dasdfmt = "/sbin/dasdfmt"
self.commonArgv = ["-y", "-d", "cdl", "-b", "4096"] self.started = False def startup(self, *args, **kwargs):
@@ -77,8 +79,9 @@ class DASD: status = f.read().strip() f.close()
if status == "unformatted":
log.info(" %s is an unformatted DASD" % (device,))
if status in ["unformatted", "basic"]:
log.info(" %s status is %s, needs dasdfmt" % (device,
status,)) self._dasdlist.append(device) if not len(self._dasdlist):
@@ -127,7 +130,21 @@ class DASD: log.info(" not running dasdfmt, exiting installer") sys.exit(0)
argv = ["-y", "-P", "-d", "cdl", "-b", "4096"]
# gather total cylinder count
argv = ["-t", "-v"] + self.commonArgv
for dasd in self._dasdlist:
buf = iutil.execWithCapture(self.dasdfmt, argv + [dasd],
stderr="/dev/tty5")
for line in buf.splitlines():
if line.startswith("Drive Geometry: "):
# line will look like this:
# Drive Geometry: 3339 Cylinders * 15 Heads = 50085 Tracks
cyls = long(filter(lambda s: s, line.split(' '))[2])
self.totalCylinders += cyls
break
# format DASDs
argv = ["-P"] + self.commonArgv if intf: title = P_("Formatting DASD Device", "Formatting DASD Devices", c)
@@ -137,7 +154,7 @@ class DASD:
for dasd in self._dasdlist: log.info("Running dasdfmt on %s" % (dasd,))
iutil.execWithCallback("/sbin/dasdfmt", argv + [dasd],
iutil.execWithCallback(self.dasdfmt, argv + [dasd], stdout="/dev/tty5", stderr="/dev/tty5", callback=self._updateProgressWindow, callback_data=pw, echo=False)
@@ -146,7 +163,7 @@ class DASD: else: for dasd in self._dasdlist: log.info("Running dasdfmt on %s" % (dasd,))
iutil.execWithRedirect("/sbin/dasdfmt", argv + [dasd],
iutil.execWithRedirect(self.dasdfmt, argv + [dasd], stdout="/dev/tty5", stderr="/dev/tty5") def _updateProgressWindow(self, data, callback_data=None):
@@ -161,24 +178,4 @@ class DASD: self._completedCylinders += 1.0 callback_data.set(self._completedCylinders / self.totalCylinders)
- @property
- def totalCylinders(self):
""" Total number of cylinders of all unformatted DASD devices. """
if self._totalCylinders:
return self._totalCylinders
argv = ["-t", "-v", "-y", "-d", "cdl", "-b", "4096"]
for dasd in self._dasdlist:
buf = iutil.execWithCapture("/sbin/dasdfmt", argv + [dasd],
stderr="/dev/tty5")
for line in buf.splitlines():
if line.startswith("Drive Geometry: "):
# line will look like this:
# Drive Geometry: 3339 Cylinders * 15 Heads = 50085 Tracks
cyls = long(filter(lambda s: s, line.split(' '))[2])
self._totalCylinders += cyls
break
return self._totalCylinders
- # vim:tw=78:ts=4:et:sw=4
On 11/07/2009 12:38 AM, David Cantrell wrote:
On s390, if you had a single DASD that needed dasdfmt run, you would get a ZeroDivisionError traceback because the self._totalCylinders value was zero. The problem was caused by the DASD.totalCylinders property and how it initialized. It would initialize itself on the first call, but since we'd already fired off a dasdfmt process for the device, the one running to capture the cylinder count couldn't get access to the device.
Users with more than one DASD needing a dasdfmt would not see the traceback, but would have an incorrect total cylinder count, so 100% would be reached when n-1 devices had been formatted.
Determining the total cylinder count before starting the actual low-level formatting is exactly the right way to do it. Somehow I already misinterpreted this into the previous code.
Lastly, in certain instances, the status for the DASD could be "basic" rather than "unformatted", so treat those the same and perform a dasdfmt.
Only DASDs with status "unformatted" are eligible for low-level formatting. Otherwise we risk formatting disks that are already formatted and thus risk data loss. The reason is that the status attribute shows the current state of a transition through different states during device bringup.
An already formatted DASD shows the following state transition: 'unkown new detected basic ready online (*)'
An unformatted DASD shows roughly the following state transition: 'unknown new detected basic unformatted (*)' On running dasdfmt, the transition continues as follows: 'basic ready online'
(*) is the spot your code should find the DASDs in. I suppose you only saw the 'basic' state because the already running dasdfmt on that particular DASD had set its state from 'unformatted' back to 'basic' which is a preparation it does before formatting. If still DASDs appear in state 'basic', we have to look at it again from a different perspective. When dasd.py accesses DASD sysfs attributes, the DASDs should have long gone into either state 'online' or 'unformatted' since the onlining and detection has happened long enough before in linuxrc.s390.
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
On 11/09/2009 03:41 PM, Steffen Maier wrote:
On 11/07/2009 12:38 AM, David Cantrell wrote:
On s390, if you had a single DASD that needed dasdfmt run, you would get a ZeroDivisionError traceback because the self._totalCylinders value was zero. The problem was caused by the DASD.totalCylinders property and how it initialized. It would initialize itself on the first call, but since we'd already fired off a dasdfmt process for the device, the one running to capture the cylinder count couldn't get access to the device.
Users with more than one DASD needing a dasdfmt would not see the traceback, but would have an incorrect total cylinder count, so 100% would be reached when n-1 devices had been formatted.
Lastly, in certain instances, the status for the DASD could be "basic" rather than "unformatted", so treat those the same and perform a dasdfmt.
Only DASDs with status "unformatted" are eligible for low-level formatting. Otherwise we risk formatting disks that are already formatted and thus risk data loss. The reason is that the status attribute shows the current state of a transition through different states during device bringup.
An already formatted DASD shows the following state transition: 'unkown new detected basic ready online (*)'
An unformatted DASD shows roughly the following state transition: 'unknown new detected basic unformatted (*)' On running dasdfmt, the transition continues as follows: 'basic ready online'
(*) is the spot your code should find the DASDs in. I suppose you only saw the 'basic' state because the already running dasdfmt on that particular DASD had set its state from 'unformatted' back to 'basic' which is a preparation it does before formatting. If still DASDs appear in state 'basic', we have to look at it again from a different perspective. When dasd.py accesses DASD sysfs attributes, the DASDs should have long gone into either state 'online' or 'unformatted' since the onlining and detection has happened long enough before in linuxrc.s390.
David, I still see the check for "basic" in the rhel6-branch from commit43db7a8b4374a0728a91a8aab260253ccfc7a5a0. Can you please remove the check and only compare for "unformatted"? In master it is correct, although that seems because this commit was only applied to rhel6-branch.
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@lists.stg.fedoraproject.org