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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
