[PATCH 1/2] Fixup kickstart script logging

Brian C. Lane bcl at redhat.com
Thu Feb 14 14:37:11 UTC 2013


On Thu, Feb 14, 2013 at 10:25:09AM +0100, Vratislav Podzimek wrote:
> On Wed, 2013-02-13 at 14:21 -0800, Brian C. Lane wrote:
> > From: "Brian C. Lane" <bcl at redhat.com>
> > 
> > Changes to the execWith* methods removed the ability to handle a string
> > as stdin/out/err. Change the callers to first open where they want
> > output to be sent to and pass that instead.
> > Only stdout is needed because _run_program combines stdout and stderr,
> > pass stderr through but ignore it.
> > 
> > Also clean up a couple places that were passing strings that don't need
> > to.
> > ---
> >  anaconda                |  3 +--
> >  pyanaconda/iutil.py     | 61 ++++++++++++++++++++++++++++++-------------------
> >  pyanaconda/kickstart.py | 35 +++++++++++++---------------
> >  pyanaconda/network.py   |  3 +--
> >  4 files changed, 55 insertions(+), 47 deletions(-)
> > 
> > diff --git a/anaconda b/anaconda
> > index 324d31e..06773ca 100755
> > --- a/anaconda
> > +++ b/anaconda
> > @@ -85,8 +85,7 @@ def startMetacityWM():
> >          try:
> >              args = ['--display', ':1',
> >                      '--sm-disable']
> > -            iutil.execWithRedirect('metacity', args,
> > -                                   stdout='/dev/null', stderr='/dev/null')
> > +            iutil.execWithRedirect('metacity', args)
> >          except BaseException as e:
> >              # catch all possible exceptions
> >              log.error("Problems running the window manager: %s" % str(e))
> > diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py
> > index dbc7c3b..2d2b08c 100644
> > --- a/pyanaconda/iutil.py
> > +++ b/pyanaconda/iutil.py
> > @@ -49,7 +49,15 @@ def augmentEnv():
> >                 })
> >      return env
> >  
> > -def _run_program(argv, root='/', stdin=None, env_prune=None):
> > +def _run_program(argv, root='/', stdin=None, stdout=None, env_prune=None):
> > +    """ Run an external program, log the output and return it to the caller
> > +        @param argv The command to run and argument
> > +        @param root The directory to chroot to before running command.
> > +        @param stdin The file descriptor to read stdin from.
> > +        @param stdout Optional file descriptor to write stdout and stderr to.
> These should be both 'file object' instead of 'file descriptor'. The
> same goes for the other doc strings as well.

Thanks.

> 
> Otherwise these patches both look good to me. My only concern is about
> using proc.communicate() instead of the 'tee' class. It is a story much
> older than my history in the team but I'm quite sure there was a reason
> for writing and using that class. Do we support e.g. interactive
> scripts? Or don't we want output of the program logged even before the
> program exits so that it can be easily seen, that it e.g. expects input?
> Are we going to use _run_program for something with really long output?
> Because in all those cases proc.communicate() is not a good idea.

Well, this patch doesn't change that :) But I think it was related to
wanting to log things to /dev/tty5 as well as program.log and dlehman
fixed that by adding it to the program_logger, which is a pretty elegant
solution.

-- 
Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA (PST8PDT)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 482 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/anaconda-patches/attachments/20130214/28a110b0/attachment.sig>


More information about the anaconda-patches mailing list