[Libguestfs] [PATCH 1/4] launch: appliance: Use socketpair instead of pipe for qemu stdin/stdout.

Richard W.M. Jones rjones at redhat.com
Thu Mar 7 11:44:15 UTC 2013


From: "Richard W.M. Jones" <rjones at redhat.com>

The libvirt backend already uses a Unix socket for the appliance
console, and so for the libvirt backend the fields g->fd[0] == g->fd[1].

Change the appliance backend to use a socketpair, so we need just a
single file descriptor for qemu stdin/stdout (ie. appliance console).

Consequently we can remove the array int fd[2] in the handle and
replace it with a single file descriptor.
---
 src/guestfs-internal.h |  2 +-
 src/handle.c           | 12 ++++--------
 src/launch-appliance.c | 49 +++++++++++++++++++------------------------------
 src/launch-libvirt.c   | 18 ++++--------------
 src/launch-unix.c      |  7 +++----
 src/proto.c            | 36 +++++++++++++++++-------------------
 6 files changed, 48 insertions(+), 76 deletions(-)

diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 793549f..4c632e8 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -263,7 +263,7 @@ struct guestfs_h
   int unique;
 
   /*** Protocol. ***/
-  int fd[2];			/* Stdin/stdout of qemu. */
+  int fd;			/* Stdin/stdout of qemu. */
   int sock;			/* Daemon communications socket. */
   int msg_next_serial;
 
diff --git a/src/handle.c b/src/handle.c
index 0e66d1a..cefe385 100644
--- a/src/handle.c
+++ b/src/handle.c
@@ -91,8 +91,7 @@ guestfs_create_flags (unsigned flags, ...)
 
   g->state = CONFIG;
 
-  g->fd[0] = -1;
-  g->fd[1] = -1;
+  g->fd = -1;
   g->sock = -1;
 
   guestfs___init_error_handler (g);
@@ -367,14 +366,11 @@ shutdown_backend (guestfs_h *g, int check_for_errors)
   }
 
   /* Close sockets. */
-  if (g->fd[0] >= 0)
-    close (g->fd[0]);
-  if (g->fd[1] >= 0)
-    close (g->fd[1]);
+  if (g->fd >= 0)
+    close (g->fd);
   if (g->sock >= 0)
     close (g->sock);
-  g->fd[0] = -1;
-  g->fd[1] = -1;
+  g->fd = -1;
   g->sock = -1;
 
   if (g->attach_ops->shutdown (g, check_for_errors) == -1)
diff --git a/src/launch-appliance.c b/src/launch-appliance.c
index a86a8cc..4016d61 100644
--- a/src/launch-appliance.c
+++ b/src/launch-appliance.c
@@ -167,7 +167,7 @@ static int
 launch_appliance (guestfs_h *g, const char *arg)
 {
   int r;
-  int wfd[2], rfd[2];
+  int sv[2];
   char guestfsd_sock[256];
   struct sockaddr_un addr;
   CLEANUP_FREE char *kernel = NULL, *initrd = NULL, *appliance = NULL;
@@ -235,8 +235,8 @@ launch_appliance (guestfs_h *g, const char *arg)
   }
 
   if (!g->direct) {
-    if (pipe (wfd) == -1 || pipe (rfd) == -1) {
-      perrorf (g, "pipe");
+    if (socketpair (AF_LOCAL, SOCK_STREAM, 0, sv) == -1) {
+      perrorf (g, "socketpair");
       goto cleanup0;
     }
   }
@@ -248,10 +248,8 @@ launch_appliance (guestfs_h *g, const char *arg)
   if (r == -1) {
     perrorf (g, "fork");
     if (!g->direct) {
-      close (wfd[0]);
-      close (wfd[1]);
-      close (rfd[0]);
-      close (rfd[1]);
+      close (sv[0]);
+      close (sv[1]);
     }
     goto cleanup0;
   }
@@ -484,17 +482,16 @@ launch_appliance (guestfs_h *g, const char *arg)
       /* Set up stdin, stdout, stderr. */
       close (0);
       close (1);
-      close (wfd[1]);
-      close (rfd[0]);
+      close (sv[0]);
 
       /* Stdin. */
-      if (dup (wfd[0]) == -1) {
+      if (dup (sv[1]) == -1) {
       dup_failed:
         perror ("dup failed");
         _exit (EXIT_FAILURE);
       }
       /* Stdout. */
-      if (dup (rfd[1]) == -1)
+      if (dup (sv[1]) == -1)
         goto dup_failed;
 
       /* Particularly since qemu 0.15, qemu spews all sorts of debug
@@ -502,11 +499,10 @@ launch_appliance (guestfs_h *g, const char *arg)
        * not confuse casual users, so send stderr to the pipe as well.
        */
       close (2);
-      if (dup (rfd[1]) == -1)
+      if (dup (sv[1]) == -1)
         goto dup_failed;
 
-      close (wfd[0]);
-      close (rfd[1]);
+      close (sv[1]);
     }
 
     /* Dump the command line (after setting up stderr above). */
@@ -600,19 +596,16 @@ launch_appliance (guestfs_h *g, const char *arg)
   }
 
   if (!g->direct) {
-    /* Close the other ends of the pipe. */
-    close (wfd[0]);
-    close (rfd[1]);
+    /* Close the other end of the socketpair. */
+    close (sv[1]);
 
-    if (fcntl (wfd[1], F_SETFL, O_NONBLOCK) == -1 ||
-        fcntl (rfd[0], F_SETFL, O_NONBLOCK) == -1) {
+    if (fcntl (sv[0], F_SETFL, O_NONBLOCK) == -1) {
       perrorf (g, "fcntl");
       goto cleanup1;
     }
 
-    g->fd[0] = wfd[1];		/* stdin of child */
-    g->fd[1] = rfd[0];		/* stdout of child */
-    wfd[1] = rfd[0] = -1;
+    g->fd = sv[0];		/* stdin of child */
+    sv[0] = -1;
   }
 
   g->state = LAUNCHING;
@@ -680,18 +673,14 @@ launch_appliance (guestfs_h *g, const char *arg)
   return 0;
 
  cleanup1:
-  if (!g->direct) {
-    if (wfd[1] >= 0) close (wfd[1]);
-    if (rfd[1] >= 0) close (rfd[0]);
-  }
+  if (!g->direct && sv[0] >= 0)
+    close (sv[0]);
   if (g->app.pid > 0) kill (g->app.pid, 9);
   if (g->app.recoverypid > 0) kill (g->app.recoverypid, 9);
   if (g->app.pid > 0) waitpid (g->app.pid, NULL, 0);
   if (g->app.recoverypid > 0) waitpid (g->app.recoverypid, NULL, 0);
-  if (g->fd[0] >= 0) close (g->fd[0]);
-  if (g->fd[1] >= 0) close (g->fd[1]);
-  g->fd[0] = -1;
-  g->fd[1] = -1;
+  if (g->fd >= 0) close (g->fd);
+  g->fd = -1;
   g->app.pid = 0;
   g->app.recoverypid = 0;
   memset (&g->launch_t, 0, sizeof g->launch_t);
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index 5a69ced..caad5d4 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -399,13 +399,7 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
     goto cleanup;
   }
   console = -1;
-  g->fd[0] = r; /* This is the accepted console socket. */
-
-  g->fd[1] = dup (g->fd[0]);
-  if (g->fd[1] == -1) {
-    perrorf (g, "dup");
-    goto cleanup;
-  }
+  g->fd = r;                /* This is the accepted console socket. */
 
   /* Wait for libvirt domain to start and to connect back to us via
    * virtio-serial and send the GUESTFS_LAUNCH_FLAG message.
@@ -481,13 +475,9 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
 
   if (console >= 0)
     close (console);
-  if (g->fd[0] >= 0) {
-    close (g->fd[0]);
-    g->fd[0] = -1;
-  }
-  if (g->fd[1] >= 0) {
-    close (g->fd[1]);
-    g->fd[1] = -1;
+  if (g->fd >= 0) {
+    close (g->fd);
+    g->fd = -1;
   }
   if (g->sock >= 0) {
     close (g->sock);
diff --git a/src/launch-unix.c b/src/launch-unix.c
index 9bd64fa..a7a3ae2 100644
--- a/src/launch-unix.c
+++ b/src/launch-unix.c
@@ -46,11 +46,10 @@ launch_unix (guestfs_h *g, const char *sockpath)
     return -1;
   }
 
-  /* Set these to nothing so we don't try to read from random file
-   * descriptors.
+  /* Set this to nothing so we don't try to read from a random file
+   * descriptor.
    */
-  g->fd[0] = -1;
-  g->fd[1] = -1;
+  g->fd = -1;
 
   if (g->verbose)
     guestfs___print_timestamped_message (g, "connecting to %s", sockpath);
diff --git a/src/proto.c b/src/proto.c
index 3d00b2b..57cb609 100644
--- a/src/proto.c
+++ b/src/proto.c
@@ -161,11 +161,9 @@ child_cleanup (guestfs_h *g)
   debug (g, "child_cleanup: %p: child process died", g);
 
   g->attach_ops->shutdown (g, 0);
-  if (g->fd[0] >= 0) close (g->fd[0]);
-  if (g->fd[1] >= 0) close (g->fd[1]);
+  if (g->fd >= 0) close (g->fd);
   close (g->sock);
-  g->fd[0] = -1;
-  g->fd[1] = -1;
+  g->fd = -1;
   g->sock = -1;
   memset (&g->launch_t, 0, sizeof g->launch_t);
   guestfs___free_drives (g);
@@ -385,12 +383,12 @@ send_to_daemon (guestfs_h *g, const void *v_buf, size_t n)
   FD_ZERO (&rset);
   FD_ZERO (&wset);
 
-  if (g->fd[1] >= 0)            /* Read qemu stdout for log messages & EOF. */
-    FD_SET (g->fd[1], &rset);
+  if (g->fd >= 0)               /* Read qemu stdout for log messages & EOF. */
+    FD_SET (g->fd, &rset);
   FD_SET (g->sock, &rset);      /* Read socket for cancellation & EOF. */
   FD_SET (g->sock, &wset);      /* Write to socket to send the data. */
 
-  int max_fd = MAX (g->sock, g->fd[1]);
+  int max_fd = MAX (g->sock, g->fd);
 
   while (n > 0) {
     rset2 = rset;
@@ -403,8 +401,8 @@ send_to_daemon (guestfs_h *g, const void *v_buf, size_t n)
       return -1;
     }
 
-    if (g->fd[1] >= 0 && FD_ISSET (g->fd[1], &rset2)) {
-      if (read_log_message_or_eof (g, g->fd[1], 0) == -1)
+    if (g->fd >= 0 && FD_ISSET (g->fd, &rset2)) {
+      if (read_log_message_or_eof (g, g->fd, 0) == -1)
         return -1;
     }
     if (FD_ISSET (g->sock, &rset2)) {
@@ -516,11 +514,11 @@ recv_from_daemon (guestfs_h *g, uint32_t *size_rtn, void **buf_rtn)
 
   FD_ZERO (&rset);
 
-  if (g->fd[1] >= 0)            /* Read qemu stdout for log messages & EOF. */
-    FD_SET (g->fd[1], &rset);
+  if (g->fd >= 0)               /* Read qemu stdout for log messages & EOF. */
+    FD_SET (g->fd, &rset);
   FD_SET (g->sock, &rset);      /* Read socket for data & EOF. */
 
-  max_fd = MAX (g->sock, g->fd[1]);
+  max_fd = MAX (g->sock, g->fd);
 
   *size_rtn = 0;
   *buf_rtn = NULL;
@@ -548,8 +546,8 @@ recv_from_daemon (guestfs_h *g, uint32_t *size_rtn, void **buf_rtn)
       return -1;
     }
 
-    if (g->fd[1] >= 0 && FD_ISSET (g->fd[1], &rset2)) {
-      if (read_log_message_or_eof (g, g->fd[1], 0) == -1) {
+    if (g->fd >= 0 && FD_ISSET (g->fd, &rset2)) {
+      if (read_log_message_or_eof (g, g->fd, 0) == -1) {
         free (*buf_rtn);
         *buf_rtn = NULL;
         return -1;
@@ -728,11 +726,11 @@ guestfs___accept_from_daemon (guestfs_h *g)
 
   FD_ZERO (&rset);
 
-  if (g->fd[1] >= 0)            /* Read qemu stdout for log messages & EOF. */
-    FD_SET (g->fd[1], &rset);
+  if (g->fd >= 0)               /* Read qemu stdout for log messages & EOF. */
+    FD_SET (g->fd, &rset);
   FD_SET (g->sock, &rset);      /* Read socket for accept. */
 
-  int max_fd = MAX (g->sock, g->fd[1]);
+  int max_fd = MAX (g->sock, g->fd);
   int sock = -1;
 
   while (sock == -1) {
@@ -760,8 +758,8 @@ guestfs___accept_from_daemon (guestfs_h *g)
       return -1;
     }
 
-    if (g->fd[1] >= 0 && FD_ISSET (g->fd[1], &rset2)) {
-      if (read_log_message_or_eof (g, g->fd[1], 1) == -1)
+    if (g->fd >= 0 && FD_ISSET (g->fd, &rset2)) {
+      if (read_log_message_or_eof (g, g->fd, 1) == -1)
         return -1;
     }
     if (FD_ISSET (g->sock, &rset2)) {
-- 
1.8.1.4




More information about the Libguestfs mailing list