----- Original Message -----
> From: "Vratislav Podzimek" <vpodzime(a)redhat.com>
> To: anaconda-patches(a)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(a)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(a)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