Re: [PATCH] Offer to format unformatted DASD devices (#560702)

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

 



-----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@xxxxxxxxxx>
Red Hat / Honolulu, HI

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

iEYEARECAAYFAkvYmMwACgkQ5hsjjIy1VkmscQCfaskLhr0Za1plHoNqfuvVcM/o
Dc4AoKYRqelfjHBLq9OE5ePKiKvlTHNh
=jIBl
-----END PGP SIGNATURE-----

_______________________________________________
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