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

Re: [master 5/6] iutil: execWithCallback() and execWithPulseProgress() return an object.



On Thu, 2010-05-20 at 17:22 +0200, 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 might rather use a new-style class for this, just because:

class ExecProduct(object):

I love seeing all that crap code gone from mdadm and lvm -- very nice.

Dave

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



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