comments inline
On 10/27/2009 03:07 AM, David Cantrell wrote:
> Before the device tree is built in the storage code, run through the
> DASD devices on the system and format any with a status of
> 'unformatted'. We have to run dasdfmt on these devices before we can
> partition them.
>
> 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.
>
> This process is implemented in a DASD class that works similar to the
> iscsi and zfcp classes. That is, devices may need to be started or
> brought in to a working state before the anaconda storage code can use
> them.
> ---
> storage/__init__.py | 3 +
> storage/dasd.py | 167 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 170 insertions(+), 0 deletions(-)
> create mode 100644 storage/dasd.py
>
> diff --git a/storage/__init__.py b/storage/__init__.py
> index 0a9dad0..e1569c8 100644
> --- a/storage/__init__.py
> +++ b/storage/__init__.py
> @@ -51,6 +51,7 @@ from udev import *
> import iscsi
> import fcoe
> import zfcp
> +import dasd
>
> import gettext
> _ = lambda x: gettext.ldgettext("anaconda", x)
> @@ -252,6 +253,7 @@ class Storage(object):
> self.iscsi = iscsi.iscsi()
> self.fcoe = fcoe.fcoe()
> self.zfcp = zfcp.ZFCP()
> + self.dasd = dasd.DASD()
>
> self._nextID = 0
> self.defaultFSType = get_default_filesystem_type()
> @@ -320,6 +322,7 @@ class Storage(object):
> self.iscsi.startup(self.anaconda.intf)
> self.fcoe.startup(self.anaconda.intf)
> self.zfcp.startup()
> + self.dasd.startup(self.anaconda.intf)
> if self.anaconda.id.getUpgrade():
> clearPartType = CLEARPART_TYPE_NONE
> else:
> diff --git a/storage/dasd.py b/storage/dasd.py
> new file mode 100644
> index 0000000..f4df8ca
> --- /dev/null
> +++ b/storage/dasd.py
> @@ -0,0 +1,167 @@
> +#
> +# dasd.py - DASD class
> +#
> +# Copyright (C) 2009 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
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +#
> +# Red Hat Author(s): David Cantrell <dcantrell@xxxxxxxxxx>
> +#
> +
> +import iutil
> +import sys
> +import os
> +from constants import *
> +from flags import flags
> +
> +import logging
> +log = logging.getLogger("anaconda")
> +
> +import gettext
> +_ = lambda x: gettext.ldgettext("anaconda", x)
> +P_ = lambda x, y, z: gettext.ldngettext("anaconda", x, y, z)
> +
> +class DASD:
> + """ Controlling class for DASD interaction before the storage code in
> + anaconda has initialized.
> +
> + The DASD class can determine if any DASD devices on the system are
> + unformatted and can perform a dasdfmt on them.
> + """
> +
> + def __init__(self):
> + self._dasdlist = []
> + self._totalCylinders = 0
> + self._completedCylinders = 0.0
> + self._maxFormatJobs = 0
> + self._progressBuffer = ''
> + self.started = False
> +
> + def startup(self, intf=None):
> + """ Look for any unformatted DASDs in the system and offer the user
> + the option for format them with dasdfmt or exit the installer.
> + """
> + if self.started:
> + return
> +
> + self.started = True
It's funny that we need this latch but we introduced it for zFCP as well
and this is fine. With this we don't have to make sure the callers behave.
> +
> + if not iutil.isS390():
> + return
> +
> + log.info("Checking for unformatted DASD devices:")
> +
> + for device in os.listdir("/sys/block"):
> + if not device.startswith("dasd"):
> + continue
> +
> + statusfile = "/sys/block/%s/device/status" % (device,)
> + if not os.path.isfile(statusfile):
> + continue
> +
> + f = open(statusfile, "r")
> + status = f.read().strip()
> + f.close()
> +
> + if status == "unformatted":
Even though this is currently undocumented in the device drivers book,
this is a valid and simple clean check. Eventually this use case of the
status sysfs attribute for DASD will be documented.
> + log.info(" %s is an unformatted DASD" % (device,))
> + self._dasdlist.append(device)
> +
> + if not len(self._dasdlist):
> + log.info(" no unformatted DASD devices found")
> + return
> +
> + if intf:
> + c = len(self._dasdlist)
> +
> + title = P_("Unformatted DASD Device Found",
> + "Unformatted DASD Devices Found", c)
> + msg = P_("Format uninitialized DASD device?\n\n"
> + "There is %d uninitialized DASD device on this "
> + "system. To continue installation, the device must "
> + "be formatted. Formatting will remove any data on "
> + "this device. If you are unsure of your DASD "
> + "configuration, select No to exit the installer." % c,
> + "Format uninitialized DASD devices?\n\n"
> + "There are %d uninitialized DASD devices on this "
> + "system. To continue installation, the devices must "
> + "be formatted. Formatting will remove any data on "
> + "these devices. If you are unsure of your DASD "
> + "configuration, select No to exit the installer." % c,
> + c)
We should show the list of DASDs to be formatted to the user and not
just how many. Otherwise he can hardly make a decision whether to
continue or not. We should have a string of the device bus IDs or simply
the symlink names under /dev/disk/by-path/ for the DASDs in
self._dasdlist. This is a similar use case to the one we had a while
back where we show a dialog box at the end of the partitioner on which
partitions will be formatted with a file system.
> +
> + rc = intf.messageWindow(title, msg,
> + custom_icon="error", type="yesno")
> + if rc == 0:
> + sys.exit(0)
I would like to see an else case here when there is no UI. As Hans
already pointed out, silently low-level formatting DASDs which do not
have any known format, is a bit hard. Suppose such DASD has an unknown
format with important data on it. Then we should not silently format it
and loose the data. A kickstart command as suggested by Hans sounds good.
How was this case handled with RHEL5?
> +
> + self._dasdlist = map(lambda s: "/dev/" + s, self._dasdlist)
> + dasdlist = " ".join(self._dasdlist)
> + log.info("Running dasdfmt on %s" % (dasdlist,))
Unfortunately, dasdlist based on the kernel device names dasdX is not
very helpful. We should have the device bus ID or simply the symlink
name under /dev/disk/by-path/ for the DASDs in self._dasdlist.
> + argv = ["-y", "-P", "-d", "cdl", "-b", "4096"]
> +
> + 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)
> +
> + for dasd in self._dasdlist:
> + iutil.execWithCallback("/sbin/dasdfmt", argv + [dasd],
> + stdout="/dev/tty5", stderr="/dev/tty5",
> + callback=self._updateProgressWindow,
> + callback_data=pw)
> +
> + pw.pop()
> + else:
> + for dasd in self._dasdlist:
> + iutil.execWithRedirect("/sbin/dasdfmt", argv + [dasd],
> + stdout="/dev/tty5", stderr="/dev/tty5")
> +
> + def _updateProgressWindow(self, data, callback_data=None):
> + """ Reads progress output from dasdfmt and collects the number of
> + cylinders completed so the progress window can update.
> + """
> + if not callback_data:
> + return
> +
> + if data == '\n':
> + # each newline we see in this output means one more cylinder done
Are you sure that dasdfmt only ever prints a newline if one cylinder has
been formatted but in no other cases? If so, then this is OK. Otherwise
I would have expected to parse self._progressBuffer for a regex or the
like which matches one line of real progress output of dasdfmt.
> + self._completedCylinders += 1.0
> + self._progressBuffer = ''
> + callback_data.set(self._completedCylinders / self.totalCylinders)
> + else:
> + self._progressBuffer += data
> +
> + @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
Very nice. This provides us with a good progress bar for sequential calls.
> + break
> +
> + return self._totalCylinders
> +
> +# vim:tw=78:ts=4:et:sw=4
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
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
