Libguestfs - Sep 2009 - [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