-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On Wed, 28 Apr 2010, Steffen Maier wrote:
The patch most probably fixes the bug but I've got some comments inline
below.
On 04/28/2010 12:00 AM, David Cantrell wrote:
Originally worked up in November 2009, I've updated the patch a bit to
work with our current code. Tested in text mode and gui mode. If you
have any unformatted DASD devices, anaconda prompts you and asks if you
want to format them.
---
iw/filter_gui.py | 2 +-
storage/__init__.py | 2 +-
storage/dasd.py | 56 ++++++++++++++++++++++----------------------------
3 files changed, 27 insertions(+), 33 deletions(-)
diff --git a/iw/filter_gui.py b/iw/filter_gui.py
index 6fed1a9..9fee628 100644
--- a/iw/filter_gui.py
+++ b/iw/filter_gui.py
@@ -550,7 +550,7 @@ class FilterWindow(InstallWindow):
storage.iscsi.iscsi().startup(anaconda.intf)
storage.fcoe.fcoe().startup(anaconda.intf)
storage.zfcp.ZFCP().startup()
- storage.dasd.DASD().startup(anaconda.intf)
+ storage.dasd.DASD().startup(anaconda.intf, anaconda.id.storage.zeroMbr)
disks = filter(udev_device_is_disk, udev_get_block_devices())
(singlepaths, mpaths, partitions) = identifyMultipaths(disks)
diff --git a/storage/__init__.py b/storage/__init__.py
index cbf155b..0da8795 100644
--- a/storage/__init__.py
+++ b/storage/__init__.py
@@ -348,7 +348,7 @@ class Storage(object):
self.iscsi.startup(self.anaconda.intf)
self.fcoe.startup(self.anaconda.intf)
self.zfcp.startup()
- self.dasd.startup(intf=self.anaconda.intf, zeroMbr=self.zeroMbr)
+ self.dasd.startup(self.anaconda.intf, self.zeroMbr)
if self.anaconda.id.getUpgrade():
clearPartType = CLEARPART_TYPE_NONE
else:
diff --git a/storage/dasd.py b/storage/dasd.py
index 469957e..ba4f565 100644
--- a/storage/dasd.py
+++ b/storage/dasd.py
@@ -1,7 +1,7 @@
#
# dasd.py - DASD class
#
-# Copyright (C) 2009 Red Hat, Inc. All rights reserved.
+# Copyright (C) 2009, 2010 Red Hat, Inc. All rights reserved.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -44,15 +44,17 @@ class DASD:
def __init__(self):
self._dasdlist = []
self._devices = [] # list of DASDDevice objects
- 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 __call__(self):
return self
- def startup(self, *args, **kwargs):
+ def startup(self, intf, zeroMbr):
""" Look for any unformatted DASDs in the system and offer the user
the option for format them with dasdfmt or exit the installer.
"""
@@ -64,9 +66,6 @@ class DASD:
if not iutil.isS390():
return
- intf = kwargs.get("intf")
- zeroMbr = kwargs.get("zeroMbr")
-
log.info("Checking for unformatted DASD devices:")
for device in os.listdir("/sys/block"):
@@ -81,8 +80,9 @@ class DASD:
status = f.read().strip()
f.close()
Although it's not required to fix the bug at hand, I'd like to remind us
of the following:
https://www.redhat.com/archives/anaconda-devel-list/2009-October/msg00469.html
Right. I've added in a test to make sure device is not in the exclusiveDisks
list.
On 10/28/2009 09:00 PM, David Cantrell wrote:
On Wed, 28 Oct 2009, Chris Lumens wrote:
Under RHEL-5, this process was serial and the user had to click Yes for
each unformatted DASD found, which could take a really long time if
you had thousands of DASDs.
The idea now is that if the DASD is seen by anaconda, we want to use it
for installation. The stage 1 device initialization routines as well as
the CMS conf file provided at boot time allow the user to restrict the
range of devices we see during installation. If any of the devices we
see are unformatted, run dasdfmt before building the devicetree.
I just realized that this is going to need to change when I get my
storage filtering patches in. At the least, DASD.startup is going to
have to respect storage.exclusiveDisks, or we could be formatting things
that the user explicitly said to not use.
That's fine, let me know what needs to be added and I can fix it up in a
future patch.
- if status == "unformatted":
- log.info(" %s is an unformatted DASD" % (device,))
+ if status in ["unformatted"]:
+ log.info(" %s status is %s, needs dasdfmt" % (device,
+ status,))
In order for the log message to be helpful, it should also contain the
PATH_ID so we can see the DASD's devno and not just the arbitrary kernel
device name.
Done. I'm logging the /dev/disk/by-path alias as well as the dasdX name.
self._dasdlist.append(device)
if not len(self._dasdlist):
@@ -116,7 +116,21 @@ class DASD:
log.info(" rescue mode: not running dasdfmt")
return
- 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")
In order to detect such bugs more easily in the future, it would be
good, if you'd check the error level of external commands and react
appropriately.
The iutil exec functions are intended to do this for us. Exceptions are
caught in those functions and messages logged and/or exceptions raised
depending on what happened.
+ 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)
@@ -126,7 +140,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)
In order to detect such bugs more easily in the future, it would be
good, if you'd check the error level of external commands and react
appropriately.
See above.
@@ -135,7 +149,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 addDASD(self, dasd):
@@ -168,26 +182,6 @@ 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
-
# Create DASD singleton
DASD = DASD()
Revised patch:
[PATCH] Offer to format unformatted DASD devices (#560702)
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.
Honor the exclusiveDisks list in the storage object and log the
/dev/disk/by-path alias for the device when logging the info message
about what device was found that was unformatted.
[Originally worked up in November 2009, I've updated the patch a bit to
work with our current code. Tested in text mode and gui mode. If you
have any unformatted DASD devices, anaconda prompts you and asks if you
want to format them.]
- ---
iw/filter_gui.py | 4 ++-
storage/__init__.py | 2 +-
storage/dasd.py | 58 +++++++++++++++++++++++---------------------------
3 files changed, 31 insertions(+), 33 deletions(-)
diff --git a/iw/filter_gui.py b/iw/filter_gui.py
index 6fed1a9..6291018 100644
- --- a/iw/filter_gui.py
+++ b/iw/filter_gui.py
@@ -550,7 +550,9 @@ class FilterWindow(InstallWindow):
storage.iscsi.iscsi().startup(anaconda.intf)
storage.fcoe.fcoe().startup(anaconda.intf)
storage.zfcp.ZFCP().startup()
- - storage.dasd.DASD().startup(anaconda.intf)
+ storage.dasd.DASD().startup(anaconda.intf,
+ anaconda.id.storage.exclusiveDisks,
+ anaconda.id.storage.zeroMbr)
disks = filter(udev_device_is_disk, udev_get_block_devices())
(singlepaths, mpaths, partitions) = identifyMultipaths(disks)
diff --git a/storage/__init__.py b/storage/__init__.py
index cbf155b..e4139c0 100644
- --- a/storage/__init__.py
+++ b/storage/__init__.py
@@ -348,7 +348,7 @@ class Storage(object):
self.iscsi.startup(self.anaconda.intf)
self.fcoe.startup(self.anaconda.intf)
self.zfcp.startup()
- - self.dasd.startup(intf=self.anaconda.intf, zeroMbr=self.zeroMbr)
+ self.dasd.startup(self.anaconda.intf, self.exclusiveDisks, self.zeroMbr)
if self.anaconda.id.getUpgrade():
clearPartType = CLEARPART_TYPE_NONE
else:
diff --git a/storage/dasd.py b/storage/dasd.py
index 469957e..8eea176 100644
- --- a/storage/dasd.py
+++ b/storage/dasd.py
@@ -1,7 +1,7 @@
#
# dasd.py - DASD class
#
- -# Copyright (C) 2009 Red Hat, Inc. All rights reserved.
+# Copyright (C) 2009, 2010 Red Hat, Inc. All rights reserved.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -44,15 +44,17 @@ class DASD:
def __init__(self):
self._dasdlist = []
self._devices = [] # list of DASDDevice objects
- - 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 __call__(self):
return self
- - def startup(self, *args, **kwargs):
+ def startup(self, intf, exclusiveDisks, zeroMbr):
""" Look for any unformatted DASDs in the system and offer the user
the option for format them with dasdfmt or exit the installer.
"""
@@ -64,9 +66,6 @@ class DASD:
if not iutil.isS390():
return
- - intf = kwargs.get("intf")
- - zeroMbr = kwargs.get("zeroMbr")
- -
log.info("Checking for unformatted DASD devices:")
for device in os.listdir("/sys/block"):
@@ -81,8 +80,11 @@ class DASD:
status = f.read().strip()
f.close()
- - if status == "unformatted":
- - log.info(" %s is an unformatted DASD" % (device,))
+ if status in ["unformatted"] and device not in exclusiveDisks:
+ bypath = deviceNameToDiskByPath("/dev/" + device)
+ log.info(" %s (%s) status is %s, needs dasdfmt" % (device,
+ bypath,
+ status,))
self._dasdlist.append(device)
if not len(self._dasdlist):
@@ -116,7 +118,21 @@ class DASD:
log.info(" rescue mode: not running dasdfmt")
return
- - 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)
@@ -126,7 +142,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)
@@ -135,7 +151,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 addDASD(self, dasd):
@@ -168,26 +184,6 @@ 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
- -
# Create DASD singleton
DASD = DASD()
- --
David Cantrell <dcantrell@xxxxxxxxxx>
Red Hat / Honolulu, HI
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
iEYEARECAAYFAkvXq/UACgkQ5hsjjIy1VknBZACeJVzJ3tm2CfHKCA7Cgwxmxxn1
wokAn3wNZcRYsL4TJdsAbsfBniOMjfmz
=79qQ
-----END PGP SIGNATURE-----
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
