[PATCH] guestfish: Redirect stdout when executing remote commands
guestfish --listen necessarily redirects its stdout to /dev/null so as not to interfere with eval. The remote protocol doesn't contain any other provision for collecting stdout for the caller, so executing guestfish --remote will never generate any output. This patch fixes that by forwarding the caller's STDOUT to the listener over the unix socket connection. The listener redirects its STDOUT to the caller's STDOUT for the duration of the command, then closes it again. --- fish/rc.c | 134 +++++++++++++++++++++++++++++++++++++++++++++++++++++--- guestfish.pod | 7 --- 2 files changed, 126 insertions(+), 15 deletions(-) diff --git a/fish/rc.c b/fish/rc.c index 5423c22..5d64c70 100644 --- a/fish/rc.c +++ b/fish/rc.c @@ -27,6 +27,7 @@ #include <sys/types.h> #include <sys/un.h> #include <signal.h> +#include <sys/socket.h> #include <rpc/types.h> #include <rpc/xdr.h> @@ -49,6 +50,124 @@ create_sockpath (pid_t pid, char *sockpath, int len, struct sockaddr_un *addr) strcpy (addr->sun_path, sockpath); } +static const socklen_t controllen = CMSG_LEN (sizeof (int)); + +static void +receive_stdout (int s) +{ + static struct cmsghdr *cmptr = NULL, *h; + struct msghdr msg; + struct iovec iov[1]; + + /* Our 1 byte buffer */ + char buf[1]; + + if (NULL == cmptr) { + cmptr = malloc (controllen); + if (NULL == cmptr) { + perror ("malloc"); + exit (1); + } + } + + /* Don't specify a source */ + msg.msg_name = NULL; + msg.msg_namelen = 0; + + /* Initialise the msghdr to receive zero byte */ + iov[0].iov_base = buf; + iov[0].iov_len = 1; + msg.msg_iov = iov; + msg.msg_iovlen = 1; + + /* Initialise the control data */ + msg.msg_control = cmptr; + msg.msg_controllen = controllen; + + /* Read a message from the socket */ + ssize_t n = recvmsg (s, &msg, 0); + if (n < 0) { + perror ("recvmsg stdout fd"); + exit (1); + } + + h = CMSG_FIRSTHDR(&msg); + if (NULL == h) { + fprintf (stderr, "didn't receive a stdout file descriptor\n"); + } + + else { + /* Extract the transferred file descriptor from the control data */ + int fd = *(int *)CMSG_DATA (h); + + /* Duplicate the received file descriptor to stdout */ + dup2 (fd, STDOUT_FILENO); + close (fd); + } +} + +static void +send_stdout (int s) +{ + static struct cmsghdr *cmptr = NULL; + struct msghdr msg; + struct iovec iov[1]; + + /* Our 1 byte dummy buffer */ + char buf[1]; + + /* Don't specify a destination */ + msg.msg_name = NULL; + msg.msg_namelen = 0; + + /* Initialise the msghdr to send zero byte */ + iov[0].iov_base = buf; + iov[0].iov_len = 1; + msg.msg_iov = iov; + msg.msg_iovlen = 1; + + /* Initialize the zero byte */ + buf[0] = 0; + + /* Initialize the control data */ + if (NULL == cmptr) { + cmptr = malloc (controllen); + if (NULL == cmptr) { + perror ("malloc"); + exit (1); + } + } + cmptr->cmsg_level = SOL_SOCKET; + cmptr->cmsg_type = SCM_RIGHTS; + cmptr->cmsg_len = controllen; + + /* Add control header to the message */ + msg.msg_control = cmptr; + msg.msg_controllen = controllen; + + /* Add STDOUT to the control data */ + *(int *)CMSG_DATA (cmptr) = STDOUT_FILENO; + + if (sendmsg (s, &msg, 0) != 1) { + perror ("sendmsg stdout fd"); + exit (1); + } +} + +static void +close_stdout (void) +{ + int fd; + + fd = open ("/dev/null", O_WRONLY); + if (fd == -1) + perror ("/dev/null"); + else { + dup2 (fd, STDOUT_FILENO); + close (fd); + } +} + /* Remote control server. */ void rc_listen (void) @@ -56,7 +175,7 @@ rc_listen (void) char sockpath[128]; pid_t pid; struct sockaddr_un addr; - int sock, s, i, fd; + int sock, s, i; FILE *fp; XDR xdr, xdr2; guestfish_hello hello; @@ -111,13 +230,7 @@ rc_listen (void) /* Now close stdout and substitute /dev/null. This is necessary * so that eval `guestfish --listen` doesn't block forever. */ - fd = open ("/dev/null", O_WRONLY); - if (fd == -1) - perror ("/dev/null"); - else { - dup2 (fd, 1); - close (fd); - } + close_stdout(); /* Read commands and execute them. */ while (!quit) { @@ -125,6 +238,8 @@ rc_listen (void) if (s == -1) perror ("accept"); else { + receive_stdout(s); + fp = fdopen (s, "r+"); xdrstdio_create (&xdr, fp, XDR_DECODE); @@ -180,6 +295,7 @@ rc_listen (void) error: xdr_destroy (&xdr); /* NB. This doesn't close 'fp'. */ fclose (fp); /* Closes the underlying socket 's'. */ + close_stdout(); /* Re-close stdout */ } } @@ -227,6 +343,8 @@ rc_remote (int pid, const char *cmd, int argc, char *argv[], return -1; } + send_stdout(sock); + /* Send the greeting. */ fp = fdopen (sock, "r+"); xdrstdio_create (&xdr, fp, XDR_ENCODE); diff --git a/guestfish.pod b/guestfish.pod index d0d6839..affb83b 100644 --- a/guestfish.pod +++ b/guestfish.pod @@ -398,13 +398,6 @@ You can have several guestfish listener processes running using: guestfish --remote=$pid1 cmd guestfish --remote=$pid2 cmd -=head2 STANDARD OUTPUT DURING REMOTE CONTROL - -Because of limitations in the C<eval> statement, stdout from the -listener is currently redirected to C</dev/null>. - -Stderr is unchanged. - =head2 REMOTE CONTROL DETAILS Remote control happens over a Unix domain socket called -- 1.6.2.5
On Fri, Sep 11, 2009 at 05:13:15PM +0100, Matthew Booth wrote:
guestfish --listen necessarily redirects its stdout to /dev/null so as not to interfere with eval. The remote protocol doesn't contain any other provision for collecting stdout for the caller, so executing guestfish --remote will never generate any output.
This patch fixes that by forwarding the caller's STDOUT to the listener over the unix socket connection. The listener redirects its STDOUT to the caller's STDOUT for the duration of the command, then closes it again.
This is a clever use of sockets. However I can't accept the patch unless it contains some tests. Luckily this should be easy to test by dropping some shell script(s) into regressions/ Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://et.redhat.com/~rjones/libguestfs/ See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html
guestfish --listen necessarily redirects its stdout to /dev/null so as not to interfere with eval. The remote protocol doesn't contain any other provision for collecting stdout for the caller, so executing guestfish --remote will never generate any output. This patch fixes that by forwarding the caller's STDOUT to the listener over the unix socket connection. The listener redirects its STDOUT to the caller's STDOUT for the duration of the command, then closes it again. --- fish/rc.c | 134 +++++++++++++++++++++++++++++++++++++++++--- guestfish.pod | 7 -- regressions/test-remote.sh | 12 ++++ 3 files changed, 138 insertions(+), 15 deletions(-) diff --git a/fish/rc.c b/fish/rc.c index 5423c22..5d64c70 100644 --- a/fish/rc.c +++ b/fish/rc.c @@ -27,6 +27,7 @@ #include <sys/types.h> #include <sys/un.h> #include <signal.h> +#include <sys/socket.h> #include <rpc/types.h> #include <rpc/xdr.h> @@ -49,6 +50,124 @@ create_sockpath (pid_t pid, char *sockpath, int len, struct sockaddr_un *addr) strcpy (addr->sun_path, sockpath); } +static const socklen_t controllen = CMSG_LEN (sizeof (int)); + +static void +receive_stdout (int s) +{ + static struct cmsghdr *cmptr = NULL, *h; + struct msghdr msg; + struct iovec iov[1]; + + /* Our 1 byte buffer */ + char buf[1]; + + if (NULL == cmptr) { + cmptr = malloc (controllen); + if (NULL == cmptr) { + perror ("malloc"); + exit (1); + } + } + + /* Don't specify a source */ + msg.msg_name = NULL; + msg.msg_namelen = 0; + + /* Initialise the msghdr to receive zero byte */ + iov[0].iov_base = buf; + iov[0].iov_len = 1; + msg.msg_iov = iov; + msg.msg_iovlen = 1; + + /* Initialise the control data */ + msg.msg_control = cmptr; + msg.msg_controllen = controllen; + + /* Read a message from the socket */ + ssize_t n = recvmsg (s, &msg, 0); + if (n < 0) { + perror ("recvmsg stdout fd"); + exit (1); + } + + h = CMSG_FIRSTHDR(&msg); + if (NULL == h) { + fprintf (stderr, "didn't receive a stdout file descriptor\n"); + } + + else { + /* Extract the transferred file descriptor from the control data */ + int fd = *(int *)CMSG_DATA (h); + + /* Duplicate the received file descriptor to stdout */ + dup2 (fd, STDOUT_FILENO); + close (fd); + } +} + +static void +send_stdout (int s) +{ + static struct cmsghdr *cmptr = NULL; + struct msghdr msg; + struct iovec iov[1]; + + /* Our 1 byte dummy buffer */ + char buf[1]; + + /* Don't specify a destination */ + msg.msg_name = NULL; + msg.msg_namelen = 0; + + /* Initialise the msghdr to send zero byte */ + iov[0].iov_base = buf; + iov[0].iov_len = 1; + msg.msg_iov = iov; + msg.msg_iovlen = 1; + + /* Initialize the zero byte */ + buf[0] = 0; + + /* Initialize the control data */ + if (NULL == cmptr) { + cmptr = malloc (controllen); + if (NULL == cmptr) { + perror ("malloc"); + exit (1); + } + } + cmptr->cmsg_level = SOL_SOCKET; + cmptr->cmsg_type = SCM_RIGHTS; + cmptr->cmsg_len = controllen; + + /* Add control header to the message */ + msg.msg_control = cmptr; + msg.msg_controllen = controllen; + + /* Add STDOUT to the control data */ + *(int *)CMSG_DATA (cmptr) = STDOUT_FILENO; + + if (sendmsg (s, &msg, 0) != 1) { + perror ("sendmsg stdout fd"); + exit (1); + } +} + +static void +close_stdout (void) +{ + int fd; + + fd = open ("/dev/null", O_WRONLY); + if (fd == -1) + perror ("/dev/null"); + else { + dup2 (fd, STDOUT_FILENO); + close (fd); + } +} + /* Remote control server. */ void rc_listen (void) @@ -56,7 +175,7 @@ rc_listen (void) char sockpath[128]; pid_t pid; struct sockaddr_un addr; - int sock, s, i, fd; + int sock, s, i; FILE *fp; XDR xdr, xdr2; guestfish_hello hello; @@ -111,13 +230,7 @@ rc_listen (void) /* Now close stdout and substitute /dev/null. This is necessary * so that eval `guestfish --listen` doesn't block forever. */ - fd = open ("/dev/null", O_WRONLY); - if (fd == -1) - perror ("/dev/null"); - else { - dup2 (fd, 1); - close (fd); - } + close_stdout(); /* Read commands and execute them. */ while (!quit) { @@ -125,6 +238,8 @@ rc_listen (void) if (s == -1) perror ("accept"); else { + receive_stdout(s); + fp = fdopen (s, "r+"); xdrstdio_create (&xdr, fp, XDR_DECODE); @@ -180,6 +295,7 @@ rc_listen (void) error: xdr_destroy (&xdr); /* NB. This doesn't close 'fp'. */ fclose (fp); /* Closes the underlying socket 's'. */ + close_stdout(); /* Re-close stdout */ } } @@ -227,6 +343,8 @@ rc_remote (int pid, const char *cmd, int argc, char *argv[], return -1; } + send_stdout(sock); + /* Send the greeting. */ fp = fdopen (sock, "r+"); xdrstdio_create (&xdr, fp, XDR_ENCODE); diff --git a/guestfish.pod b/guestfish.pod index 5427b23..2e50873 100644 --- a/guestfish.pod +++ b/guestfish.pod @@ -394,13 +394,6 @@ You can have several guestfish listener processes running using: guestfish --remote=$pid1 cmd guestfish --remote=$pid2 cmd -=head2 STANDARD OUTPUT DURING REMOTE CONTROL - -Because of limitations in the C<eval> statement, stdout from the -listener is currently redirected to C</dev/null>. - -Stderr is unchanged. - =head2 REMOTE CONTROL DETAILS Remote control happens over a Unix domain socket called diff --git a/regressions/test-remote.sh b/regressions/test-remote.sh index f3c14b4..783dd6e 100755 --- a/regressions/test-remote.sh +++ b/regressions/test-remote.sh @@ -29,6 +29,18 @@ eval `../fish/guestfish --listen` ../fish/guestfish --remote sfdiskM /dev/sda , ../fish/guestfish --remote mkfs ext2 /dev/sda1 ../fish/guestfish --remote mount /dev/sda1 / + +# Failure of the above commands will cause the guestfish listener to exit. +# Incorrect return from echo_daemon will not, so need to ensure the listener +# exits in any case, while still reporting an error. +error=0 +echo=$(../fish/guestfish --remote echo_daemon "This is a test") +if [ "$echo" != "This is a test" ]; then + error=1; +fi + ../fish/guestfish --remote exit rm -f test.img + +exit $error -- 1.6.2.5
On Mon, Sep 14, 2009 at 10:11:30AM +0100, Matthew Booth wrote:
guestfish --listen necessarily redirects its stdout to /dev/null so as not to interfere with eval. The remote protocol doesn't contain any other provision for collecting stdout for the caller, so executing guestfish --remote will never generate any output. This patch fixes that by forwarding the caller's STDOUT to the listener over the unix socket connection. The listener redirects its STDOUT to the caller's STDOUT for the duration of the command, then closes it again.
ACK. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
On 12/09/09 11:03, Richard W.M. Jones wrote:
On Fri, Sep 11, 2009 at 05:13:15PM +0100, Matthew Booth wrote:
guestfish --listen necessarily redirects its stdout to /dev/null so as not to interfere with eval. The remote protocol doesn't contain any other provision for collecting stdout for the caller, so executing guestfish --remote will never generate any output.
This patch fixes that by forwarding the caller's STDOUT to the listener over the unix socket connection. The listener redirects its STDOUT to the caller's STDOUT for the duration of the command, then closes it again.
This is a clever use of sockets. However I can't accept the patch unless it contains some tests. Luckily this should be easy to test by dropping some shell script(s) into regressions/
Ok. I added a new test to the existing test-remote.sh. Patch posted. Matt -- Matthew Booth, RHCA, RHCSS Red Hat Engineering, Virtualisation Team M: +44 (0)7977 267231 GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
participants (2)
-
Matthew Booth -
Richard W.M. Jones