FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Debian > Debian Development

 
 
LinkBack Thread Tools
 
Old 04-27-2010, 10:00 PM
David Cantrell
 
Default Offer to format unformatted DASD devices (#560702)

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

- 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,))
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")
+ 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)
@@ -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()

--
1.6.6.1

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 04-27-2010, 11:21 PM
Steffen Maier
 
Default Offer to format unformatted DASD devices (#560702)

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

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

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

> @@ -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()
>


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@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 04-28-2010, 03:31 AM
David Cantrell
 
Default Offer to format unformatted DASD devices (#560702)

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

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@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 04-28-2010, 03:10 PM
Steffen Maier
 
Default Offer to format unformatted DASD devices (#560702)

On 04/28/2010 05:31 AM, David Cantrell wrote:
> 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/storage/__init__.py b/storage/__init__.py

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

Erm, no.

All iutil.execWith* only throw an exception, if they could not even
fork/exec the command.

iutil.execWithRedirect returns the error level no matter what its value
was. In this case here, dasdfmt most probably returned with !=0 and we
did not see this, since you ignore the return value.

iutil.execWithCapture returns the program output but proc.returncode not
at all, which is bad since we cannot detect if the executed command
exited with !=0.

iutil.execWithCallback returns os.WEXITSTATUS(status) and thus behaves
similar to execWithRedirect regarding the return of the programs error
level.

Those return values containing the program's error level need to be
checked and acted appropriately (fatal vs. non-fatal). Relying on a
follow-up exception because of an accidental division by zero is not the
right thing to do.

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

Dito.


> Revised patch:

Looks good with regard to fixing the bug, but still I think the two
comments above should eventually be integrated---could be a different
patch, though.

> [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()


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@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 04-28-2010, 05:20 PM
David Cantrell
 
Default Offer to format unformatted DASD devices (#560702)

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

On Wed, 28 Apr 2010, Steffen Maier wrote:


On 04/28/2010 05:31 AM, David Cantrell wrote:

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/storage/__init__.py b/storage/__init__.py



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


Erm, no.

All iutil.execWith* only throw an exception, if they could not even
fork/exec the command.

iutil.execWithRedirect returns the error level no matter what its value
was. In this case here, dasdfmt most probably returned with !=0 and we
did not see this, since you ignore the return value.

iutil.execWithCapture returns the program output but proc.returncode not
at all, which is bad since we cannot detect if the executed command
exited with !=0.

iutil.execWithCallback returns os.WEXITSTATUS(status) and thus behaves
similar to execWithRedirect regarding the return of the programs error
level.

Those return values containing the program's error level need to be
checked and acted appropriately (fatal vs. non-fatal). Relying on a
follow-up exception because of an accidental division by zero is not the
right thing to do.


I'm sorry, I was only thinking of instances where we'd get an exception from
the iutil.exec* functions, which would happen if we were missing the command
or there was an OSError starting the subprocess. I was not thinking of the
return values of dasdfmt.

That said, I do not want to change the calling semantics of execWithCapture()
because we use that all over the place. I've updated the patch to catch
exceptions and check the return value from execWithRedirect() and
execWithCallback(), much like we do in the storage/formats/fs.py code. On
error, I raise a DasdFormatError with the details collected. This is in line
with what we do with the existing storage code.




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


Dito.



Revised patch:


Looks good with regard to fixing the bug, but still I think the two
comments above should eventually be integrated---could be a different
patch, though.


Third revision:

[PATCH 1/2] 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 | 109 +++++++++++++++++++++++++++++---------------------
storage/errors.py | 3 +
4 files changed, 70 insertions(+), 48 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..a76943b 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.
"""
@@ -60,13 +62,12 @@ class DASD:
return

self.started = True
+ out = "/dev/tty5"
+ err = "/dev/tty5"

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 +82,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,27 +120,60 @@ 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=err)
+ 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
+ update = self._updateProgressWindow
+
+ title = P_("Formatting DASD Device", "Formatting DASD Devices", c)
+ msg = P_("Preparing %d DASD device for use with Linux..." % c,
+ "Preparing %d DASD devices for use with Linux..." % c, c)

if intf:
- - title = P_("Formatting DASD Device", "Formatting DASD Devices", c)
- - msg = P_("Preparing %d DASD device for use with Linux..." % c,
- - "Preparing %d DASD devices for use with Linux..." % c, c)
- - pw = intf.progressWindow(title, msg, 1.0)
+ if self.totalCylinders:
+ pw = intf.progressWindow(title, msg, 1.0)
+ else:
+ pw = intf.progressWindow(title, msg, 100, pulse=True)

- - for dasd in self._dasdlist:
- - log.info("Running dasdfmt on %s" % (dasd,))
- - iutil.execWithCallback("/sbin/dasdfmt", argv + [dasd],
- - stdout="/dev/tty5", stderr="/dev/tty5",
- - callback=self._updateProgressWindow,
- - callback_data=pw, echo=False)
- -
- - pw.pop()
- - else:
- - for dasd in self._dasdlist:
- - log.info("Running dasdfmt on %s" % (dasd,))
- - iutil.execWithRedirect("/sbin/dasdfmt", argv + [dasd],
- - stdout="/dev/tty5", stderr="/dev/tty5")
+ for dasd in self._dasdlist:
+ bypath = deviceNameToDiskByPath("/dev/" + dasd)
+ log.info("Running dasdfmt on %s" % (bypath,))
+ arglist = argv + [dasd]
+
+ try:
+ if intf and self.totalCylinders:
+ rc = iutil.execWithCallback(self.dasdfmt, arglist,
+ stdout=out, stderr=err,
+ callback=update,
+ callback_data=pw,
+ echo=False)
+ elif intf:
+ rc = iutil.execWithPulseProgress(self.dasdfmt, arglist,
+ stdout=out, stderr=err,
+ progress=pw)
+ else:
+ rc = iutil.execWithRedirect(self.dasdfmt, arglist,
+ stdout=out, stderr=err)
+ except Exception as e:
+ raise DasdFormatError(e, bypath)
+
+ if rc:
+ raise DasdFormatError("dasdfmt failed: %s" % rc, bypath)
+
+ if intf:
+ pw.pop()

def addDASD(self, dasd):
""" Adds a DASDDevice to the internal list of DASDs. """
@@ -168,26 +205,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()

diff --git a/storage/errors.py b/storage/errors.py
index a0f5f60..c4d4313 100644
- --- a/storage/errors.py
+++ b/storage/errors.py
@@ -148,3 +148,6 @@ class UdevError(StorageError):
class UnrecognizedFSTabEntryError(StorageError):
pass

+# dasd
+class DasdFormatError(StorageError):
+ pass

- --
David Cantrell <dcantrell@redhat.com>

Red Hat / Honolulu, HI

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAkvYbk0ACgkQ5hsjjIy1VkkbzACfZjMkUL/AViQlh4ARkRhjDG0J
ojIAoNo9JW6RH26JpV3eK7mRwotB/qHj
=ijKr
-----END PGP SIGNATURE-----

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 04-28-2010, 07:15 PM
Steffen Maier
 
Default Offer to format unformatted DASD devices (#560702)

On 04/28/2010 07:20 PM, David Cantrell wrote:
> On Wed, 28 Apr 2010, Steffen Maier wrote:
>> On 04/28/2010 05:31 AM, David Cantrell wrote:
>>> On Wed, 28 Apr 2010, Steffen Maier wrote:
>>>> On 04/28/2010 12:00 AM, David Cantrell wrote:

>>>>> diff --git a/storage/__init__.py b/storage/__init__.py
>>
>>>>> @@ -116,7 +116,21 @@ class DASD:

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

> That said, I do not want to change the calling semantics of
> execWithCapture() because we use that all over the place.

Sure, that would be too intrusive.

> I've updated the patch to catch
> exceptions and check the return value from execWithRedirect() and
> execWithCallback(), much like we do in the storage/formats/fs.py code. On
> error, I raise a DasdFormatError with the details collected. This is in
> line with what we do with the existing storage code.

Looks cool now, I just got one question regarding syntax at the end below.

> Third revision:
>
> [PATCH 1/2] Offer to format unformatted DASD devices (#560702)

> iw/filter_gui.py | 4 +-
> storage/__init__.py | 2 +-
> storage/dasd.py | 109 +++++++++++++++++++++++++++++---------------------
> storage/errors.py | 3 +
> 4 files changed, 70 insertions(+), 48 deletions(-)

> diff --git a/storage/dasd.py b/storage/dasd.py

> @@ -116,27 +120,60 @@ class DASD:

> + for dasd in self._dasdlist:
> + bypath = deviceNameToDiskByPath("/dev/" + dasd)
> + log.info("Running dasdfmt on %s" % (bypath,))
> + arglist = argv + [dasd]
> +
> + try:
> + if intf and self.totalCylinders:
> + rc = iutil.execWithCallback(self.dasdfmt, arglist,
> + stdout=out, stderr=err,
> + callback=update,
> + callback_data=pw,
> + echo=False)
> + elif intf:
> + rc = iutil.execWithPulseProgress(self.dasdfmt, arglist,
> + stdout=out, stderr=err,
> + progress=pw)

Very nice, now it even works if no totalCylinders could be determined.

> + else:
> + rc = iutil.execWithRedirect(self.dasdfmt, arglist,
> + stdout=out, stderr=err)
> + except Exception as e:
> + raise DasdFormatError(e, bypath)
> +
> + if rc:
> + raise DasdFormatError("dasdfmt failed: %s" % rc, bypath)

Supposing StorageError takes one string as argument in its constructur,
is this missing one more format specifier in the string
since it's got two arguments?


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@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 04-28-2010, 08:21 PM
David Cantrell
 
Default Offer to format unformatted DASD devices (#560702)

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

On Wed, 28 Apr 2010, Steffen Maier wrote:


On 04/28/2010 07:20 PM, David Cantrell wrote:

On Wed, 28 Apr 2010, Steffen Maier wrote:

On 04/28/2010 05:31 AM, David Cantrell wrote:

On Wed, 28 Apr 2010, Steffen Maier wrote:

On 04/28/2010 12:00 AM, David Cantrell wrote:



diff --git a/storage/__init__.py b/storage/__init__.py



@@ -116,7 +116,21 @@ class DASD:



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



That said, I do not want to change the calling semantics of
execWithCapture() because we use that all over the place.


Sure, that would be too intrusive.


I've updated the patch to catch
exceptions and check the return value from execWithRedirect() and
execWithCallback(), much like we do in the storage/formats/fs.py code. On
error, I raise a DasdFormatError with the details collected. This is in
line with what we do with the existing storage code.


Looks cool now, I just got one question regarding syntax at the end below.


Third revision:

[PATCH 1/2] Offer to format unformatted DASD devices (#560702)



iw/filter_gui.py | 4 +-
storage/__init__.py | 2 +-
storage/dasd.py | 109 +++++++++++++++++++++++++++++---------------------
storage/errors.py | 3 +
4 files changed, 70 insertions(+), 48 deletions(-)



diff --git a/storage/dasd.py b/storage/dasd.py



@@ -116,27 +120,60 @@ class DASD:



+ for dasd in self._dasdlist:
+ bypath = deviceNameToDiskByPath("/dev/" + dasd)
+ log.info("Running dasdfmt on %s" % (bypath,))
+ arglist = argv + [dasd]
+
+ try:
+ if intf and self.totalCylinders:
+ rc = iutil.execWithCallback(self.dasdfmt, arglist,
+ stdout=out, stderr=err,
+ callback=update,
+ callback_data=pw,
+ echo=False)
+ elif intf:
+ rc = iutil.execWithPulseProgress(self.dasdfmt, arglist,
+ stdout=out, stderr=err,
+ progress=pw)


Very nice, now it even works if no totalCylinders could be determined.


+ else:
+ rc = iutil.execWithRedirect(self.dasdfmt, arglist,
+ stdout=out, stderr=err)
+ except Exception as e:
+ raise DasdFormatError(e, bypath)
+
+ if rc:
+ raise DasdFormatError("dasdfmt failed: %s" % rc, bypath)


Supposing StorageError takes one string as argument in its constructur,
is this missing one more format specifier in the string
since it's got two arguments?



This is ok syntax. What happens is the two arguments passed to
DasdFormatError will become the 'args' property as a tuple on the actual
exception. We do the same thing for DeviceFormatError.

- --
David Cantrell <dcantrell@redhat.com>

Red Hat / Honolulu, HI

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAkvYmMwACgkQ5hsjjIy1VkmscQCfaskLhr0Za1 plHoNqfuvVcM/o
Dc4AoKYRqelfjHBLq9OE5ePKiKvlTHNh
=jIBl
-----END PGP SIGNATURE-----

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 

Thread Tools




All times are GMT. The time now is 07:21 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright ©2007 - 2008, www.linux-archive.org