[blivet][master] Do not mix stdout and stderr when running utilities

Vratislav Podzimek vpodzime at redhat.com
Fri Oct 31 12:44:02 UTC 2014


On Thu, 2014-10-30 at 09:51 -0400, Anne Mulhern wrote:
> 
> 
> 
> ----- Original Message -----
> > From: "Vratislav Podzimek" <vpodzime at redhat.com>
> > To: anaconda-patches at lists.fedorahosted.org
> > Sent: Thursday, October 30, 2014 8:01:57 AM
> > Subject: [blivet][master] Do not mix stdout and stderr when running utilities
> > 
> > We only gather data from stdout and stderr contents may confuse our parsing
> > code. But we may still be interested in the stderr so log it.
> > 
> > Signed-off-by: Vratislav Podzimek <vpodzime at redhat.com>
> > ---
> >  blivet/util.py | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/blivet/util.py b/blivet/util.py
> > index a69b694..01a4548 100644
> > --- a/blivet/util.py
> > +++ b/blivet/util.py
> > @@ -44,15 +44,21 @@ def _run_program(argv, root='/', stdin=None,
> > env_prune=None):
> >              proc = subprocess.Popen(argv,
> >                                      stdin=stdin,
> >                                      stdout=subprocess.PIPE,
> > -                                    stderr=subprocess.STDOUT,
> > +                                    stderr=subprocess.PIPE,
> >                                      close_fds=True,
> >                                      preexec_fn=chroot, cwd=root, env=env)
> >  
> > -            out = proc.communicate()[0]
> > +            out, err = proc.communicate()
> >              if out:
> > +                program_log.info("stdout:")
> >                  for line in out.splitlines():
> >                      program_log.info("%s", line)
> >  
> > +            if err:
> > +                program_log.info("stderr:")
> > +                for line in err.splitlines():
> > +                    program_log.info("%s", line)
> > +
> >          except OSError as e:
> >              program_log.error("Error running %s: %s", argv[0], e.strerror)
> >              raise
> > --
> > 1.9.3
> > 
> > _______________________________________________
> > anaconda-patches mailing list
> > anaconda-patches at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> > 
> 
> I do not like conflating standard out and standard error.
> So, generally speaking, I'm in favor of this change.
> 
> I also do not see any methods in devicelibs that seem to expect to read
> stderr from stdout. This doesn't mean there are none, but it's heartening.
> 
> I looked around outside devicelibs and found a number of calls to 
> util.*capture_output* methods that were worth investigating.
> However, only fcoe.fcoe.addSan() seems to me to contain
> some calls which seem to expect that
> out will contain stderr. 
> 
> This patch isn't really complete until at the least those calls have
> been investigated and appropriate action taken, however.
Okay so the addSan method returns an error message in case of failure
which is constructed from the mangled stdout+stderr of 'fipvlan'
invocation. Such message is then prepended with the string "Activating
FCoE SAN failed:" and shown to the user by Anaconda. I can confirm that
the 'fipvlan' utility really uses its stderr for error messages.

So we can:
1) drop this proposed patch and pretend that it is okay to mangle stdout
and stderr
2) apply it as it is and live with the fact that users won't see the
fipvlan's stderr output (we will, in the program.log)
3) add a 'err_to_out' parameter to run_program used only by addSan

Any suggestions? I think 3) is easy enough to be the best choice, but
YMMV.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list