2011/1/29 Eric Blake <eblake@redhat.com>:
* 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