-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On Tue, 27 Oct 2009, Steffen Maier wrote:
comments inline
On 10/27/2009 03:07 AM, David Cantrell wrote:
+ 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.
A string listing all of the /dev/disk/by-path/ entries for the unformatted
DASDs could be too large. The only thing I think is reasonable here is to
introduce a scrolling listbox and add the /dev/disk/by-path/ entries there.
+
+ 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?
I agree with Chris here, we can just make use of 'clearpart --initlabel'
rather than introducing a new kickstart command.
+
+ 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.
Right, I'll change this to /dev/disk/by-path/ listings for the log file as
well.
+ 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.
Yes, with the -P option (and without -v), a newline means another cylinder
has been formatted. Originally I was parsing self._progressBuffer, but deemed
that unnecessary. In fact, the self._progressBuffer lines shouldn't even be
there anymore, as well as the else block that appends to self._progressBuffer.
- --
David Cantrell <dcantrell@xxxxxxxxxx>
Red Hat / Honolulu, HI
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
iEYEARECAAYFAkrnThgACgkQ5hsjjIy1VklxNACff2KEvCZgGRZ5tEyr3/6NjLLI
CbYAoNZ3p7ydiMEgzCGXaX/WWOtTNXat
=MeWx
-----END PGP SIGNATURE-----
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
