Re: [PATCH 3/3] Find and format any unformatted DASD devices (#528386).

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Only two minor comments below.

On 10/28/2009 04:23 AM, David Cantrell wrote:
> diff --git a/storage/dasd.py b/storage/dasd.py
> new file mode 100644
> index 0000000..efaed40
> --- /dev/null
> +++ b/storage/dasd.py
> @@ -0,0 +1,173 @@

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

Although parallel formatting is not yet implemented, initializing this
to zero almost sounds like unbounded to me. Especially with many devices
this is not desirable since it takes too much compute power. Typically,
there are many virtual machines running in parallel, even in production
mode and we should leave most of the compute power to them (and also not
just put this burden on the VM scheduler). Initializing
self._maxFormatJobs to 1 seems like a sane value to begin with.

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

> +        for dasd in self._dasdlist:
> +            log.info("Running dasdfmt on %s" % (dasd,))

Hm, this logs all future jobs which have not even started, so the user
or developers can hardly determine the progress from the logs, e.g.
during problem determination. Can we move this to the two places below?

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

here

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

and here

> +                iutil.execWithRedirect("/sbin/dasdfmt", argv + [dasd],
> +                                       stdout="/dev/tty5", stderr="/dev/tty5")

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

[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux