The object contains all the products of the exec: return call, stdout and
stderr. The remaining execWith*() methods can migrate to this too one
day. The advantage now is that lvm() and mdadm() in devicelibs/ don't have
to resort to (possibly broken) hacks to get the error message from their
calls.
---
iutil.py | 15 +++++++++------
storage/dasd.py | 18 ++++++++++--------
storage/devicelibs/lvm.py | 15 +++------------
storage/devicelibs/mdraid.py | 15 +++------------
storage/devicelibs/swap.py | 4 ++--
storage/formats/fs.py | 22 +++++++++++-----------
6 files changed, 38 insertions(+), 51 deletions(-)

diff --git a/iutil.py b/iutil.py
index d026bcf..fb3d6d1 100644
--- a/iutil.py
+++ b/iutil.py
@@ -39,6 +39,12 @@ import logging
log = logging.getLogger("anaconda")
program_log = logging.getLogger("program")

+class ExecProduct:
+ def __init__(self, rc, stdout, stderr):
+ self.rc = rc
+ self.stdout = stdout
+ self.stderr = stderr
+
#Python reimplementation of the shell tee process, so we can
#feed the pipe output into two places at the same time
class tee(threading.Thread):
@@ -348,14 +354,11 @@ def execWithCallback(command, argv, stdin = None, stdout = None,
log.critical("exception from waitpid: %s %s" %(e.errno, e.strerror))

closefds()
- # *shrug* no clue why this would happen, but hope that things are fine
- if status is None:
- return 0

+ rc = 1
if os.WIFEXITED(status):
- return os.WEXITSTATUS(status)
-
- return 1
+ rc = os.WEXITSTATUS(status)
+ return ExecProduct(rc, log_output , log_errors)

def _pulseProgressCallback(data, callback_data=None):
if callback_data:
diff --git a/storage/dasd.py b/storage/dasd.py
index f113cfb..3e8fefe 100644
--- a/storage/dasd.py
+++ b/storage/dasd.py
@@ -168,15 +168,17 @@ class DASD:

try:
if intf and self.totalCylinders:
- rc = iutil.execWithCallback(self.dasdfmt, arglist,
- stdout=out, stderr=err,
- callback=update,
- callback_data=pw,
- echo=False)
+ ret = iutil.execWithCallback(self.dasdfmt, arglist,
+ stdout=out, stderr=err,
+ callback=update,
+ callback_data=pw,
+ echo=False)
+ rc = ret.rc
elif intf:
- rc = iutil.execWithPulseProgress(self.dasdfmt, arglist,
- stdout=out, stderr=err,
- progress=pw)
+ ret = iutil.execWithPulseProgress(self.dasdfmt, arglist,
+ stdout=out, stderr=err,
+ progress=pw)
+ rc = ret.rc
else:
rc = iutil.execWithRedirect(self.dasdfmt, arglist,
stdout=out, stderr=err)
diff --git a/storage/devicelibs/lvm.py b/storage/devicelibs/lvm.py
index fd74f56..726d209 100644
--- a/storage/devicelibs/lvm.py
+++ b/storage/devicelibs/lvm.py
@@ -159,21 +159,12 @@ def clampSize(size, pesize, roundup=None):
return long(round(float(size)/float(pesize)) * pesize)

def lvm(args, progress=None):
- rc = iutil.execWithPulseProgress("lvm", args,
+ ret = iutil.execWithPulseProgress("lvm", args,
stdout = "/dev/tty5",
stderr = "/dev/tty5",
progress=progress)
- if not rc:
- return
-
- try:
- # grab the last line of program.log and strip off the timestamp
- msg = open("/tmp/program.log").readlines()[-1]
- msg = msg.split("program: ", 1)[1].strip()
- except Exception:
- msg = ""
-
- raise LVMError(msg)
+ if ret.rc:
+ raise LVMError(ret.stderr)

def pvcreate(device, progress=None):
args = ["pvcreate"] +
diff --git a/storage/devicelibs/mdraid.py b/storage/devicelibs/mdraid.py
index a04965d..851f88f 100644
--- a/storage/devicelibs/mdraid.py
+++ b/storage/devicelibs/mdraid.py
@@ -125,21 +125,12 @@ def get_raid_max_spares(raidlevel, nummembers):
raise ValueError, "invalid raid level %d" % raidlevel

def mdadm(args, progress=None):
- rc = iutil.execWithPulseProgress("mdadm", args,
+ ret = iutil.execWithPulseProgress("mdadm", args,
stdout = "/dev/tty5",
stderr = "/dev/tty5",
progress=progress)
- if not rc:
- return
-
- try:
- # grab the last line of program.log and strip off the timestamp
- msg = open("/tmp/program.log").readlines()[-1]
- msg = msg.split("program: ", 1)[1].strip()
- except Exception:
- msg = ""
-
- raise MDRaidError(msg)
+ if ret.rc:
+ raise MDRaidError(ret.stderr)

def mdcreate(device, level, disks, spares=0, metadataVer=None, bitmap=False,
progress=None):
diff --git a/storage/devicelibs/swap.py b/storage/devicelibs/swap.py
index 92dfe93..ef0f4c2 100644
--- a/storage/devicelibs/swap.py
+++ b/storage/devicelibs/swap.py
@@ -40,12 +40,12 @@ def mkswap(device, label=', progress=None):
argv.extend(["-L", label])
argv.append(device)

- rc = iutil.execWithPulseProgress("mkswap", argv,
+ ret = iutil.execWithPulseProgress("mkswap", argv,
stderr = "/dev/tty5",
stdout = "/dev/tty5",
progress=progress)

- if rc:
+ if ret.rc:
raise SwapError("mkswap failed for '%s'" % device)

def swapon(device, priority=None):
diff --git a/storage/formats/fs.py b/storage/formats/fs.py
index 7377471..7c0b19b 100644
--- a/storage/formats/fs.py
+++ b/storage/formats/fs.py
@@ -357,18 +357,18 @@ class FS(DeviceFormat):
100, pulse = True)

try:
- rc = iutil.execWithPulseProgress(self.mkfsProg,
- argv,
- stdout="/dev/tty5",
- stderr="/dev/tty5",
- progress=w)
+ ret = iutil.execWithPulseProgress(self.mkfsProg,
+ argv,
+ stdout="/dev/tty5",
+ stderr="/dev/tty5",
+ progress=w)
except Exception as e:
raise FormatCreateError(e, self.device)
finally:
if w:
w.pop()

- if rc:
+ if ret.rc:
raise FormatCreateError("format failed: %s" % rc, self.device)

self.exists = True
@@ -466,7 +466,7 @@ class FS(DeviceFormat):
100, pulse = True)

try:
- rc = iutil.execWithPulseProgress(self.resizefsProg,
+ ret = iutil.execWithPulseProgress(self.resizefsProg,
self.resizeArgs,
stdout="/dev/tty5",
stderr="/dev/tty5",
@@ -477,7 +477,7 @@ class FS(DeviceFormat):
if w:
w.pop()

- if rc:
+ if ret.rc:
raise FSResizeError("resize failed: %s" % rc, self.device)

self.doCheck(intf=intf)
@@ -516,7 +516,7 @@ class FS(DeviceFormat):
100, pulse = True)

try:
- rc = iutil.execWithPulseProgress(self.fsckProg,
+ ret = iutil.execWithPulseProgress(self.fsckProg,
self._getCheckArgs(),
stdout="/dev/tty5",
stderr="/dev/tty5",
@@ -527,11 +527,11 @@ class FS(DeviceFormat):
if w:
w.pop()

- if self._fsckFailed(rc):
+ if self._fsckFailed(ret.rc):
hdr = _("%(type)s filesystem check failure on %(device)s: ") %
{"type": self.type, "device": self.device}

- msg = self._fsckErrorMessage(rc)
+ msg = self._fsckErrorMessage(ret.rc)

if intf:
help = _("Errors like this usually mean there is a problem "
--
1.6.6

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/20/2010 08:22 AM, Ales Kozumplik wrote:
> The object contains all the products of the exec: return call, stdout and
> stderr. The remaining execWith*() methods can migrate to this too one
> day. The advantage now is that lvm() and mdadm() in devicelibs/ don't have
> to resort to (possibly broken) hacks to get the error message from their
> calls.
> ---
> iutil.py | 15 +++++++++------
> storage/dasd.py | 18 ++++++++++--------
> storage/devicelibs/lvm.py | 15 +++------------
> storage/devicelibs/mdraid.py | 15 +++------------
> storage/devicelibs/swap.py | 4 ++--
> storage/formats/fs.py | 22 +++++++++++-----------
> 6 files changed, 38 insertions(+), 51 deletions(-)
>
> diff --git a/iutil.py b/iutil.py
> index d026bcf..fb3d6d1 100644
> --- a/iutil.py
> +++ b/iutil.py
> @@ -39,6 +39,12 @@ import logging
> log = logging.getLogger("anaconda")
> program_log = logging.getLogger("program")
>
> +class ExecProduct:
> + def __init__(self, rc, stdout, stderr):
> + self.rc = rc
> + self.stdout = stdout
> + self.stderr = stderr
> +

I would probably just use a dict for something like this. To me classes
are for data structures and the methods that act on them, not just an
easy container for data -- that's what dicts are for. But that is likely
a personal preference

I do like the idea of returning all of this info, either way.

- --
Brian C. Lane <bcl@redhat.com>
Red Hat / Port Orchard, WA
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iQEVAwUBS/WiXRF+jBaO/jp/AQK/Kgf+OWRjEE8joIHTldPiaKgLKD1PA4bEXax6
sz6DC56Cf11vfXerbBLggckMz9xDWh4PeMA5BG9IUGSHjYSDky ccvqbUYt9z99bS
D7aJ3W8oNPY1lAEErK51Pz6deHLCbhYT/epdIC5pK7MHSABiPH77RNEJMk9yRKwa
lPU5qvP+Xud8wtUGB2nzDOGTr23YV0948R5aR1SCotNDS6alxo spl8l+urOe55dV
mepm832Epo9Pg7RZLGq02Cv6Twfi0GAMSQViZ59fEXSJUb48uO QKM3Aq39muwCne
Mb2WCUEgSb+e21Jlv5LtkLCXXxzvrL4DgjuIsrYBZbjN8SyPg4 hIcg==
=D8va
-----END PGP SIGNATURE-----

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

On 05/20/2010 10:58 PM, Brian C. Lane wrote:


I would probably just use a dict for something like this. To me classes
are for data structures and the methods that act on them, not just an
easy container for data -- that's what dicts are for. But that is likely
a personal preference



I am going to use objects, simply because accessing object.parameter
looks (to me) more aesthetic than object["parameter"]. Plus I know
returning an object here will always be sufficient whatever we decide to
do with it in the future. The only thing a dictionary could offer over
an object is that it's easier to iterate over its keys/values or test
for their presence. But in this case we know all the 'keys' in advance.


Ales

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list