search for: flag_copy_fd

...; diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c index 0a29aa6..47245f7 100644 --- a/daemon/guestfsd.c +++ b/daemon/guestfsd.c @@ -932,21 +932,44 @@ commandrvf (char **stdoutput, char **stderror, int flags, signal (SIGPIPE, SIG_DFL); close (0); if (flag_copy_stdin) { - dup2 (flag_copy_fd, STDIN_FILENO); + if (dup2 (flag_copy_fd, STDIN_FILENO) == -1) { + perror ("dup2/flag_copy_fd"); + _exit (EXIT_FAILURE); + } } else { - /* Set stdin to /dev/null (ignore failure) */ - ignore_value (open ("/dev/null", O_RDONLY|O_CLOEXEC)); +...

...cgi?id=1280029 the problem was that the target program was being executed in a chroot which did not have /dev/null populated, hence the open in commandrvf failed and the process was left without fd 0. commandrvf does the following in the child: close (0); if (flag_copy_stdin) { dup2 (flag_copy_fd, STDIN_FILENO); } else { /* Set stdin to /dev/null (ignore failure) */ ignore_value (open ("/dev/null", O_RDONLY|O_CLOEXEC)); } [..] execvp (argv[0], (void *) argv); First observation is that regardless of whether this open("/dev/null", ..) succeeds,...

...dev/null. So you fork, open /dev/null and only then proceed to chroot. But as noted earlier, unpopulated /dev seems to be the real bug here. > > Current patch seems to work around shortcomings of the current API. > > > > Side content: > > if (flag_copy_stdin) close (flag_copy_fd); > > waitpid (pid, NULL, 0); > > return -1; > > This is done only in the main "while (quit < 2)" loop, and only on > error there, which will return with -1. > > > but some lines below there is: > > if (flag_copy_stdin && clos...

...the target program was being executed in a chroot which did not have > /dev/null populated, hence the open in commandrvf failed and the process > was left without fd 0. > > commandrvf does the following in the child: > close (0); > if (flag_copy_stdin) { > dup2 (flag_copy_fd, STDIN_FILENO); > } else { > /* Set stdin to /dev/null (ignore failure) */ > ignore_value (open ("/dev/null", O_RDONLY|O_CLOEXEC)); > } > [..] > execvp (argv[0], (void *) argv); > > First observation is that regardless of whether this o...

When running commands in the mounted guest (using the "command" API, and APIs based on it), provide the /dev/null from the appliance as open fd for stdin. Commands usually assume stdin is open if they didn't close it explicitly, so this should avoid crashes or misbehavings due to that. --- daemon/command.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)

...n/guestfsd.c > index 0a29aa6..47245f7 100644 > --- a/daemon/guestfsd.c > +++ b/daemon/guestfsd.c > @@ -932,21 +932,44 @@ commandrvf (char **stdoutput, char **stderror, int flags, > signal (SIGPIPE, SIG_DFL); > close (0); > if (flag_copy_stdin) { > - dup2 (flag_copy_fd, STDIN_FILENO); > + if (dup2 (flag_copy_fd, STDIN_FILENO) == -1) { > + perror ("dup2/flag_copy_fd"); > + _exit (EXIT_FAILURE); > + } close(0) explicitly assumes that stdin is 0, which is fine, but dup2 uses STDIN_FILENO (which itself is also fine), but...

...p.c for an + * example of usage. + */ +int +commandrvf (char **stdoutput, char **stderror, unsigned flags, + char const* const *argv) +{ + size_t so_size = 0, se_size = 0; + int so_fd[2], se_fd[2]; + unsigned flag_copy_stdin = flags & COMMAND_FLAG_CHROOT_COPY_FILE_TO_STDIN; + int flag_copy_fd = (int) (flags & COMMAND_FLAG_FD_MASK); + pid_t pid; + int r, quit, i; + fd_set rset, rset2; + char buf[256]; + char *p; + + if (stdoutput) *stdoutput = NULL; + if (stderror) *stderror = NULL; + + if (verbose) { + printf ("commandrvf: stdout=%s stderr=%s flags=0x%x\n", +...

Improve readability of commandrvf() by replacing bare int values for file descriptors with their symbolic names STD{IN,OUT,ERR}_FILENO. Also add PIPE_READ and PIPE_WRITE for referencing relevant ends of a pipe. --- daemon/guestfsd.c | 79 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 42 insertions(+), 37 deletions(-) diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c

...and.c b/daemon/command.c index 73fce56..2423a4e 100644 --- a/daemon/command.c +++ b/daemon/command.c @@ -185,6 +185,7 @@ commandrvf (char **stdoutput, char **stderror, unsigned flags, int so_fd[2], se_fd[2]; unsigned flag_copy_stdin = flags & COMMAND_FLAG_CHROOT_COPY_FILE_TO_STDIN; int flag_copy_fd = (int) (flags & COMMAND_FLAG_FD_MASK); + unsigned flag_out_on_err = flags & COMMAND_FLAG_FOLD_STDOUT_ON_STDERR; pid_t pid; int r, quit, i; fd_set rset, rset2; @@ -196,7 +197,8 @@ commandrvf (char **stdoutput, char **stderror, unsigned flags, if (verbose) { printf (&quot...

For background on this change, see: https://rwmj.wordpress.com/2015/12/06/inspection-now-with-added-prolog/ v2 was previously posted here: https://www.redhat.com/archives/libguestfs/2015-December/msg00038.html To test this patch series on a real guest, you can do: $ ./run guestfish -v -x -a /var/tmp/centos-6.img ><fs> run ><fs> debug sh "guestfs-inspection

This is a more working version. Inspection (partially) succeeds on a real guest this time :-) You can test it out on a real guest (in this case, a CentOS disk image located at /tmp/centos-6.img) by doing: $ ./run guestfish -v -x -a /tmp/centos-6.img ><fs> run ><fs> debug sh "guestfs-inspection --verbose" which will print lots of debugging, and at the end the