2011/1/29 Eric Blake <eblake@xxxxxxxxxx>:
> * src/fdstream.c (virFDStreamOpenFile, virFDStreamCreateFile):
> Use VIR_FORCE_CLOSE instead of close.
> * tests/commandtest.c (mymain): Likewise.
> * tools/virsh.c (editFile): Use virCommand instead of system.
> * src/util/util.c (__virExec): Special case preservation of std
> file descriptors to child.
> ---
>
> v2: make virCommand behave more like system. ÂSo I didn't do
> any signal handling like system, but at least now you can
> actually edit interactively. ÂvirCommandRun doesn't like
> interactive sessions, so I have to use virCommandRunAsync
> followed by virCommandWait. ÂI also had to fix virExec.
>
> Âsrc/fdstream.c   Â|  Â6 ++--
> Âsrc/util/util.c   |  12 +++++----
> Âtests/commandtest.c | Â 12 ++++++---
> Âtools/virsh.c    |  65 ++++++++++++++++++++++++++------------------------
> Â4 files changed, 52 insertions(+), 43 deletions(-)
> diff --git a/src/util/util.c b/src/util/util.c
> index f412a83..af14b2e 100644
> --- a/src/util/util.c
> +++ b/src/util/util.c
> @@ -593,14 +593,16 @@ __virExec(const char *const*argv,
> Â Â Â Â goto fork_error;
> Â Â }
>
> - Â ÂVIR_FORCE_CLOSE(infd);
> + Â Âif (infd != STDIN_FILENO)
> + Â Â Â ÂVIR_FORCE_CLOSE(infd);
> Â Â VIR_FORCE_CLOSE(null);
> - Â Âtmpfd = childout; Â /* preserve childout value */
> - Â ÂVIR_FORCE_CLOSE(tmpfd);
> - Â Âif (childerr > 0 &&
> + Â Âif (childout != STDOUT_FILENO) {
> + Â Â Â Âtmpfd = childout; Â /* preserve childout value */
> + Â Â Â ÂVIR_FORCE_CLOSE(tmpfd);
Took me a moment to understand this. I think in case you pass parent's
std{in,out,err} to child's std{in,out,err} this works. But when you
exchange stdout and stderr like this: parent std{in,err,out} -> child
std{in,out,err}, then childout != STDOUT_FILENO is wrong and should be
childout > STDERR_FILENO, otherwise you could close the parent's
stderr here.
> + }
> + if (childerr > STDERR_FILENO &&
> childerr != childout) {
> Â Â Â Â VIR_FORCE_CLOSE(childerr);
> - Â Â Â Âchildout = -1;
Technically it's okay to remove this like as childout isn't accessed
afterwards anymore. But by using the tmpfd variable we tricked
VIR_FORCE_CLOSE and should finish it's job here.
ACK, with that comments addressed.
Mathias
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list