New API: Implement pwrite system call (partial fix for RHBZ#592883).
[libguestfs.git] / src / guestfs.c
index db291bd..0e4cb73 100644 (file)
@@ -1,5 +1,5 @@
 /* libguestfs
- * Copyright (C) 2009 Red Hat Inc.
+ * Copyright (C) 2009-2010 Red Hat Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -24,6 +24,8 @@
 #include <stdlib.h>
 #include <stdarg.h>
 #include <stddef.h>
+#include <stdint.h>
+#include <inttypes.h>
 #include <unistd.h>
 #include <string.h>
 #include <fcntl.h>
@@ -31,6 +33,7 @@
 #include <sys/stat.h>
 #include <sys/select.h>
 #include <dirent.h>
+#include <signal.h>
 
 #include <rpc/types.h>
 #include <rpc/xdr.h>
 #include <arpa/inet.h>
 #include <netinet/in.h>
 
+#include "c-ctype.h"
+#include "glthread/lock.h"
+#include "ignore-value.h"
+
 #include "guestfs.h"
+#include "guestfs-internal.h"
 #include "guestfs-internal-actions.h"
 #include "guestfs_protocol.h"
-#include "c-ctype.h"
-#include "ignore-value.h"
 
 #ifdef HAVE_GETTEXT
 #include "gettext.h"
 #define safe_strdup guestfs_safe_strdup
 //#define safe_memdup guestfs_safe_memdup
 
+#ifdef __linux__
+#define CAN_CHECK_PEER_EUID 1
+#else
+#define CAN_CHECK_PEER_EUID 0
+#endif
+
 static void default_error_cb (guestfs_h *g, void *data, const char *msg);
 static int send_to_daemon (guestfs_h *g, const void *v_buf, size_t n);
 static int recv_from_daemon (guestfs_h *g, uint32_t *size_rtn, void **buf_rtn);
 static int accept_from_daemon (guestfs_h *g);
 static int check_peer_euid (guestfs_h *g, int sock, uid_t *rtn);
 static void close_handles (void);
+static int qemu_supports (guestfs_h *g, const char *option);
 
 #define UNIX_PATH_MAX 108
 
-/* Also in guestfsd.c */
-#define GUESTFWD_ADDR "10.0.2.4"
+#ifndef MAX
+#define MAX(a,b) ((a)>(b)?(a):(b))
+#endif
+
+#ifdef __APPLE__
+#define xdr_uint32_t xdr_u_int32_t
+#endif
+
+/* Network configuration of the appliance.  Note these addresses are
+ * only meaningful within the context of the running appliance.  QEMU
+ * translates network connections to these magic addresses into
+ * userspace calls on the host (eg. connect(2)).  qemu-doc has a nice
+ * diagram which is also useful to refer to.
+ *
+ * NETWORK: The network.
+ *
+ * ROUTER: The address of the "host", ie. this library.
+ *
+ * [Note: If you change NETWORK and ROUTER then you also have to
+ * change the network configuration in appliance/init].
+ *
+ * GUESTFWD_ADDR, GUESTFWD_PORT: The guestfwd feature of qemu
+ * magically connects this pseudo-address to the guestfwd channel.  In
+ * typical Linux configurations of libguestfs, guestfwd is not
+ * actually used any more.
+ */
+#define NETWORK "169.254.0.0/16"
+#define ROUTER "169.254.2.2"
+#define GUESTFWD_ADDR "169.254.2.4"
 #define GUESTFWD_PORT "6666"
 
 /* GuestFS handle and connection. */
@@ -147,6 +187,7 @@ struct guestfs_h
   int msg_next_serial;
 };
 
+gl_lock_define_initialized (static, handles_lock);
 static guestfs_h *handles = NULL;
 static int atexit_handler_set = 0;
 
@@ -214,17 +255,15 @@ guestfs_create (void)
    */
   g->msg_next_serial = 0x00123400;
 
-  /* Link the handles onto a global list.  This is the one area
-   * where the library needs to be made thread-safe. (XXX)
-   */
-  /* acquire mutex (XXX) */
+  /* Link the handles onto a global list. */
+  gl_lock_lock (handles_lock);
   g->next = handles;
   handles = g;
   if (!atexit_handler_set) {
     atexit (close_handles);
     atexit_handler_set = 1;
   }
-  /* release mutex (XXX) */
+  gl_lock_unlock (handles_lock);
 
   if (g->verbose)
     fprintf (stderr, "new guestfs handle %p\n", g);
@@ -309,7 +348,7 @@ guestfs_close (guestfs_h *g)
   /* Mark the handle as dead before freeing it. */
   g->state = NO_HANDLE;
 
-  /* acquire mutex (XXX) */
+  gl_lock_lock (handles_lock);
   if (handles == g)
     handles = g->next;
   else {
@@ -317,7 +356,7 @@ guestfs_close (guestfs_h *g)
       ;
     gg->next = g->next;
   }
-  /* release mutex (XXX) */
+  gl_lock_unlock (handles_lock);
 
   free (g->last_error);
   free (g->path);
@@ -385,7 +424,7 @@ guestfs_perrorf (guestfs_h *g, const char *fs, ...)
 
   if (err < 0) return;
 
-#ifndef _GNU_SOURCE
+#if !defined(_GNU_SOURCE) || defined(__APPLE__)
   char buf[256];
   strerror_r (errnum, buf, sizeof buf);
 #else
@@ -429,7 +468,11 @@ guestfs_safe_malloc (guestfs_h *g, size_t nbytes)
 
 /* Technically we should add an autoconf test for this, testing for the desired
    functionality, like what's done in gnulib, but for now, this is fine.  */
+#if defined(__GLIBC__)
 #define HAVE_GNU_CALLOC (__GLIBC__ >= 2)
+#else
+#define HAVE_GNU_CALLOC 0
+#endif
 
 /* Allocate zeroed memory for N elements of S bytes, with error
    checking.  S must be nonzero.  */
@@ -744,7 +787,8 @@ guestfs__config (guestfs_h *g,
 }
 
 int
-guestfs__add_drive (guestfs_h *g, const char *filename)
+guestfs__add_drive_with_if (guestfs_h *g, const char *filename,
+                            const char *drive_if)
 {
   size_t len = strlen (filename) + 64;
   char buf[len];
@@ -767,12 +811,12 @@ guestfs__add_drive (guestfs_h *g, const char *filename)
   int fd = open (filename, O_RDONLY|O_DIRECT);
   if (fd >= 0) {
     close (fd);
-    snprintf (buf, len, "file=%s,cache=off,if=" DRIVE_IF, filename);
+    snprintf (buf, len, "file=%s,cache=off,if=%s", filename, drive_if);
   } else {
     fd = open (filename, O_RDONLY);
     if (fd >= 0) {
       close (fd);
-      snprintf (buf, len, "file=%s,if=" DRIVE_IF, filename);
+      snprintf (buf, len, "file=%s,if=%s", filename, drive_if);
     } else {
       perrorf (g, "%s", filename);
       return -1;
@@ -783,11 +827,9 @@ guestfs__add_drive (guestfs_h *g, const char *filename)
 }
 
 int
-guestfs__add_drive_ro (guestfs_h *g, const char *filename)
+guestfs__add_drive_ro_with_if (guestfs_h *g, const char *filename,
+                               const char *drive_if)
 {
-  size_t len = strlen (filename) + 64;
-  char buf[len];
-
   if (strchr (filename, ',') != NULL) {
     error (g, _("filename cannot contain ',' (comma) character"));
     return -1;
@@ -798,12 +840,41 @@ guestfs__add_drive_ro (guestfs_h *g, const char *filename)
     return -1;
   }
 
-  snprintf (buf, len, "file=%s,snapshot=on,if=%s", filename, DRIVE_IF);
+  if (qemu_supports (g, NULL) == -1)
+    return -1;
+
+  /* Only SCSI and virtio drivers support readonly mode.
+   * This is only supported as a QEMU feature since 2010/01.
+   */
+  int supports_ro = 0;
+  if ((STREQ (drive_if, "scsi") || STREQ (drive_if, "virtio")) &&
+      qemu_supports (g, "readonly=on"))
+    supports_ro = 1;
+
+  size_t len = strlen (filename) + 100;
+  char buf[len];
+
+  snprintf (buf, len, "file=%s,snapshot=on,%sif=%s",
+            filename,
+            supports_ro ? "readonly=on," : "",
+            drive_if);
 
   return guestfs__config (g, "-drive", buf);
 }
 
 int
+guestfs__add_drive (guestfs_h *g, const char *filename)
+{
+  return guestfs__add_drive_with_if (g, filename, DRIVE_IF);
+}
+
+int
+guestfs__add_drive_ro (guestfs_h *g, const char *filename)
+{
+  return guestfs__add_drive_ro_with_if (g, filename, DRIVE_IF);
+}
+
+int
 guestfs__add_cdrom (guestfs_h *g, const char *filename)
 {
   if (strchr (filename, ',') != NULL) {
@@ -850,17 +921,13 @@ dir_contains_files (const char *dir, ...)
   return 1;
 }
 
+static void print_timestamped_message (guestfs_h *g, const char *fs, ...);
 static int build_supermin_appliance (guestfs_h *g, const char *path, char **kernel, char **initrd);
-static int test_qemu (guestfs_h *g);
-static int qemu_supports (guestfs_h *g, const char *option);
+static int is_openable (guestfs_h *g, const char *path, int flags);
 static void print_cmdline (guestfs_h *g);
 
 static const char *kernel_name = "vmlinuz." REPO "." host_cpu;
 static const char *initrd_name = "initramfs." REPO "." host_cpu ".img";
-static const char *supermin_name =
-  "initramfs." REPO "." host_cpu ".supermin.img";
-static const char *supermin_hostfiles_name =
-  "initramfs." REPO "." host_cpu ".supermin.hostfiles";
 
 int
 guestfs__launch (guestfs_h *g)
@@ -877,9 +944,21 @@ guestfs__launch (guestfs_h *g)
   char unixsock[256];
   struct sockaddr_un addr;
 
+  /* Configured? */
+  if (!g->cmdline) {
+    error (g, _("you must call guestfs_add_drive before guestfs_launch"));
+    return -1;
+  }
+
+  if (g->state != CONFIG) {
+    error (g, _("the libguestfs handle has already been launched"));
+    return -1;
+  }
+
   /* Start the clock ... */
   gettimeofday (&g->launch_t, NULL);
 
+  /* Make the temporary directory. */
 #ifdef P_tmpdir
   tmpdir = P_tmpdir;
 #else
@@ -889,18 +968,6 @@ guestfs__launch (guestfs_h *g)
   tmpdir = getenv ("TMPDIR") ? : tmpdir;
   snprintf (dir_template, sizeof dir_template, "%s/libguestfsXXXXXX", tmpdir);
 
-  /* Configured? */
-  if (!g->cmdline) {
-    error (g, _("you must call guestfs_add_drive before guestfs_launch"));
-    return -1;
-  }
-
-  if (g->state != CONFIG) {
-    error (g, _("qemu has already been launched"));
-    return -1;
-  }
-
-  /* Make the temporary directory. */
   if (!g->tmpdir) {
     g->tmpdir = safe_strdup (g, dir_template);
     if (mkdtemp (g->tmpdir) == NULL) {
@@ -927,8 +994,7 @@ guestfs__launch (guestfs_h *g)
         fprintf (stderr,
                  "looking for supermin appliance in current directory\n");
       if (dir_contains_files (".",
-                              supermin_name, supermin_hostfiles_name,
-                              "kmod.whitelist", NULL)) {
+                              "supermin.d", "kmod.whitelist", NULL)) {
         if (build_supermin_appliance (g, ".", &kernel, &initrd) == -1)
           return -1;
         break;
@@ -940,8 +1006,7 @@ guestfs__launch (guestfs_h *g)
         fprintf (stderr, "looking for supermin appliance in %s\n", pelem);
 
       if (dir_contains_files (pelem,
-                              supermin_name, supermin_hostfiles_name,
-                              "kmod.whitelist", NULL)) {
+                              "supermin.d", "kmod.whitelist", NULL)) {
         if (build_supermin_appliance (g, pelem, &kernel, &initrd) == -1)
           return -1;
         break;
@@ -999,12 +1064,15 @@ guestfs__launch (guestfs_h *g)
     goto cleanup0;
   }
 
+  if (g->verbose)
+    print_timestamped_message (g, "begin testing qemu features");
+
   /* Get qemu help text and version. */
-  if (test_qemu (g) == -1)
+  if (qemu_supports (g, NULL) == -1)
     goto cleanup0;
 
   /* Choose which vmchannel implementation to use. */
-  if (qemu_supports (g, "-net user")) {
+  if (CAN_CHECK_PEER_EUID && qemu_supports (g, "-net user")) {
     /* The "null vmchannel" implementation.  Requires SLIRP (user mode
      * networking in qemu) but no other vmchannel support.  The daemon
      * will connect back to a random port number on localhost.
@@ -1059,6 +1127,9 @@ guestfs__launch (guestfs_h *g)
     }
   }
 
+  if (g->verbose)
+    print_timestamped_message (g, "finished testing qemu features");
+
   r = fork ();
   if (r == -1) {
     perrorf (g, "fork");
@@ -1080,15 +1151,44 @@ guestfs__launch (guestfs_h *g)
      */
     g->cmdline[0] = g->qemu;
 
-    snprintf (buf, sizeof buf, "%d", g->memsize);
-    add_cmdline (g, "-m");
-    add_cmdline (g, buf);
+    /* qemu sometimes needs this option to enable hardware
+     * virtualization, but some versions of 'qemu-kvm' will use KVM
+     * regardless (even where this option appears in the help text).
+     * It is rumoured that there are versions of qemu where supplying
+     * this option when hardware virtualization is not available will
+     * cause qemu to fail, so we we have to check at least that
+     * /dev/kvm is openable.  That's not reliable, since /dev/kvm
+     * might be openable by qemu but not by us (think: SELinux) in
+     * which case the user would not get hardware virtualization,
+     * although at least shouldn't fail.  A giant clusterfuck with the
+     * qemu command line, again.
+     */
+    if (qemu_supports (g, "-enable-kvm") &&
+        is_openable (g, "/dev/kvm", O_RDWR))
+      add_cmdline (g, "-enable-kvm");
+
+    /* Newer versions of qemu (from around 2009/12) changed the
+     * behaviour of monitors so that an implicit '-monitor stdio' is
+     * assumed if we are in -nographic mode and there is no other
+     * -monitor option.  Only a single stdio device is allowed, so
+     * this broke the '-serial stdio' option.  There is a new flag
+     * called -nodefaults which gets rid of all this default crud, so
+     * let's use that to avoid this and any future surprises.
+     */
+    if (qemu_supports (g, "-nodefaults"))
+      add_cmdline (g, "-nodefaults");
 
-    add_cmdline (g, "-no-reboot"); /* Force exit instead of reboot on panic */
     add_cmdline (g, "-nographic");
     add_cmdline (g, "-serial");
     add_cmdline (g, "stdio");
 
+    snprintf (buf, sizeof buf, "%d", g->memsize);
+    add_cmdline (g, "-m");
+    add_cmdline (g, buf);
+
+    /* Force exit instead of reboot on panic */
+    add_cmdline (g, "-no-reboot");
+
     /* These options recommended by KVM developers to improve reliability. */
     if (qemu_supports (g, "-no-hpet"))
       add_cmdline (g, "-no-hpet");
@@ -1102,10 +1202,11 @@ guestfs__launch (guestfs_h *g)
      */
     if (null_vmchannel_sock) {
       add_cmdline (g, "-net");
-      add_cmdline (g, "user,vlan=0,net=10.0.2.0/8");
+      add_cmdline (g, "user,vlan=0,net=" NETWORK);
 
       snprintf (buf, sizeof buf,
-                "guestfs_vmchannel=tcp:10.0.2.2:%d", null_vmchannel_sock);
+                "guestfs_vmchannel=tcp:" ROUTER ":%d",
+                null_vmchannel_sock);
       vmchannel = strdup (buf);
     }
 
@@ -1128,7 +1229,7 @@ guestfs__launch (guestfs_h *g)
       add_cmdline (g, buf);
 
       snprintf (buf, sizeof buf,
-                "user,vlan=0,net=10.0.2.0/8,"
+                "user,vlan=0,net=" NETWORK ","
                 "guestfwd=tcp:" GUESTFWD_ADDR ":" GUESTFWD_PORT
                 "-chardev:guestfsvmc");
 
@@ -1148,7 +1249,7 @@ guestfs__launch (guestfs_h *g)
       add_cmdline (g, "-net");
       add_cmdline (g, buf);
       add_cmdline (g, "-net");
-      add_cmdline (g, "user,vlan=0,net=10.0.2.0/8");
+      add_cmdline (g, "user,vlan=0,net=" NETWORK);
 
       vmchannel = "guestfs_vmchannel=tcp:" GUESTFWD_ADDR ":" GUESTFWD_PORT;
     }
@@ -1170,10 +1271,12 @@ guestfs__launch (guestfs_h *g)
               "%s "             /* (selinux) */
               "%s "             /* (vmchannel) */
               "%s "             /* (verbose) */
+              "TERM=%s "        /* (TERM environment variable) */
               "%s",             /* (append) */
               g->selinux ? "selinux=1 enforcing=0" : "selinux=0",
               vmchannel ? vmchannel : "",
               g->verbose ? "guestfs_verbose=1" : "",
+              getenv ("TERM") ? : "linux",
               g->append ? g->append : "");
 
     add_cmdline (g, "-kernel");
@@ -1200,7 +1303,7 @@ guestfs__launch (guestfs_h *g)
       if (dup (wfd[0]) == -1) {
       dup_failed:
         perror ("dup failed");
-        _exit (1);
+        _exit (EXIT_FAILURE);
       }
       if (dup (rfd[1]) == -1)
         goto dup_failed;
@@ -1220,7 +1323,7 @@ guestfs__launch (guestfs_h *g)
 
     execv (g->qemu, g->cmdline); /* Run qemu. */
     perror (g->qemu);
-    _exit (1);
+    _exit (EXIT_FAILURE);
   }
 
   /* Parent (library). */
@@ -1252,11 +1355,11 @@ guestfs__launch (guestfs_h *g)
        */
       for (;;) {
         if (kill (qemu_pid, 0) == -1) /* qemu's gone away, we aren't needed */
-          _exit (0);
+          _exit (EXIT_SUCCESS);
         if (kill (parent_pid, 0) == -1) {
           /* Parent's gone away, qemu still around, so kill qemu. */
           kill (qemu_pid, 9);
-          _exit (0);
+          _exit (EXIT_SUCCESS);
         }
         sleep (2);
       }
@@ -1316,6 +1419,7 @@ guestfs__launch (guestfs_h *g)
                  "libguestfs: warning: unexpected connection from UID %d to port %d\n",
                  uid, null_vmchannel_sock);
         close (sock);
+        sock = -1;
         continue;
       }
     }
@@ -1390,6 +1494,9 @@ guestfs__launch (guestfs_h *g)
     goto cleanup1;
   }
 
+  if (g->verbose)
+    print_timestamped_message (g, "appliance is up");
+
   /* This is possible in some really strange situations, such as
    * guestfsd starts up OK but then qemu immediately exits.  Check for
    * it because the caller is probably expecting to be able to send
@@ -1407,7 +1514,7 @@ guestfs__launch (guestfs_h *g)
     close (wfd[1]);
     close (rfd[0]);
   }
-  kill (g->pid, 9);
+  if (g->pid > 0) kill (g->pid, 9);
   if (g->recoverypid > 0) kill (g->recoverypid, 9);
   waitpid (g->pid, NULL, 0);
   if (g->recoverypid > 0) waitpid (g->recoverypid, NULL, 0);
@@ -1468,6 +1575,9 @@ build_supermin_appliance (guestfs_h *g, const char *path,
   char cmd[4096];
   int r, len;
 
+  if (g->verbose)
+    print_timestamped_message (g, "begin building supermin appliance");
+
   len = strlen (g->tmpdir);
   *kernel = safe_malloc (g, len + 8);
   snprintf (*kernel, len+8, "%s/kernel", g->tmpdir);
@@ -1475,10 +1585,17 @@ build_supermin_appliance (guestfs_h *g, const char *path,
   snprintf (*initrd, len+8, "%s/initrd", g->tmpdir);
 
   snprintf (cmd, sizeof cmd,
-            "PATH='%s':$PATH "
-            "libguestfs-supermin-helper '%s' %s %s",
+            "febootstrap-supermin-helper%s "
+            "-k '%s/kmod.whitelist' "
+            "'%s/supermin.d' "
+            host_cpu " "
+            "%s %s",
+            g->verbose ? " --verbose" : "",
+            path,
             path,
-            path, *kernel, *initrd);
+            *kernel, *initrd);
+  if (g->verbose)
+    print_timestamped_message (g, "%s", cmd);
 
   r = system (cmd);
   if (r == -1 || WEXITSTATUS(r) != 0) {
@@ -1489,9 +1606,48 @@ build_supermin_appliance (guestfs_h *g, const char *path,
     return -1;
   }
 
+  if (g->verbose)
+    print_timestamped_message (g, "finished building supermin appliance");
+
   return 0;
 }
 
+/* Compute Y - X and return the result in milliseconds.
+ * Approximately the same as this code:
+ * http://www.mpp.mpg.de/~huber/util/timevaldiff.c
+ */
+static int64_t
+timeval_diff (const struct timeval *x, const struct timeval *y)
+{
+  int64_t msec;
+
+  msec = (y->tv_sec - x->tv_sec) * 1000;
+  msec += (y->tv_usec - x->tv_usec) / 1000;
+  return msec;
+}
+
+static void
+print_timestamped_message (guestfs_h *g, const char *fs, ...)
+{
+  va_list args;
+  char *msg;
+  int err;
+  struct timeval tv;
+
+  va_start (args, fs);
+  err = vasprintf (&msg, fs, args);
+  va_end (args);
+
+  if (err < 0) return;
+
+  gettimeofday (&tv, NULL);
+
+  fprintf (stderr, "[%05" PRIi64 "ms] %s\n",
+           timeval_diff (&g->launch_t, &tv), msg);
+
+  free (msg);
+}
+
 static int read_all (guestfs_h *g, FILE *fp, char **ret);
 
 /* Test qemu binary (or wrapper) runs, and do 'qemu -help' and
@@ -1504,12 +1660,7 @@ test_qemu (guestfs_h *g)
   char cmd[1024];
   FILE *fp;
 
-  free (g->qemu_help);
-  free (g->qemu_version);
-  g->qemu_help = NULL;
-  g->qemu_version = NULL;
-
-  snprintf (cmd, sizeof cmd, "LC_ALL=C '%s' -help", g->qemu);
+  snprintf (cmd, sizeof cmd, "LC_ALL=C '%s' -nographic -help", g->qemu);
 
   fp = popen (cmd, "r");
   /* qemu -help should always work (qemu -version OTOH wasn't
@@ -1531,7 +1682,8 @@ test_qemu (guestfs_h *g)
   if (pclose (fp) == -1)
     goto error;
 
-  snprintf (cmd, sizeof cmd, "LC_ALL=C '%s' -version 2>/dev/null", g->qemu);
+  snprintf (cmd, sizeof cmd, "LC_ALL=C '%s' -nographic -version 2>/dev/null",
+           g->qemu);
 
   fp = popen (cmd, "r");
   if (fp) {
@@ -1569,11 +1721,40 @@ read_all (guestfs_h *g, FILE *fp, char **ret)
 
 /* Test if option is supported by qemu command line (just by grepping
  * the help text).
+ *
+ * The first time this is used, it has to run the external qemu
+ * binary.  If that fails, it returns -1.
+ *
+ * To just do the first-time run of the qemu binary, call this with
+ * option == NULL, in which case it will return -1 if there was an
+ * error doing that.
  */
 static int
 qemu_supports (guestfs_h *g, const char *option)
 {
-  return g->qemu_help && strstr (g->qemu_help, option) != NULL;
+  if (!g->qemu_help) {
+    if (test_qemu (g) == -1)
+      return -1;
+  }
+
+  if (option == NULL)
+    return 1;
+
+  return strstr (g->qemu_help, option) != NULL;
+}
+
+/* Check if a file can be opened. */
+static int
+is_openable (guestfs_h *g, const char *path, int flags)
+{
+  int fd = open (path, flags);
+  if (fd == -1) {
+    if (g->verbose)
+      perror (path);
+    return 0;
+  }
+  close (fd);
+  return 1;
 }
 
 /* Check the peer effective UID for a TCP socket.  Ideally we'd like
@@ -1583,6 +1764,7 @@ qemu_supports (guestfs_h *g, const char *option)
 static int
 check_peer_euid (guestfs_h *g, int sock, uid_t *rtn)
 {
+#if CAN_CHECK_PEER_EUID
   struct sockaddr_in peer;
   socklen_t addrlen = sizeof peer;
 
@@ -1651,6 +1833,12 @@ check_peer_euid (guestfs_h *g, int sock, uid_t *rtn)
   error (g, "check_peer_euid: no matching TCP connection found in /proc/net/tcp");
   fclose (fp);
   return -1;
+#else /* !CAN_CHECK_PEER_EUID */
+  /* This function exists but should never be called in this
+   * configuration.
+   */
+  abort ();
+#endif /* !CAN_CHECK_PEER_EUID */
 }
 
 /* You had to call this function after launch in versions <= 1.0.70,
@@ -1678,7 +1866,7 @@ guestfs__kill_subprocess (guestfs_h *g)
   if (g->verbose)
     fprintf (stderr, "sending SIGTERM to process %d\n", g->pid);
 
-  kill (g->pid, SIGTERM);
+  if (g->pid > 0) kill (g->pid, SIGTERM);
   if (g->recoverypid > 0) kill (g->recoverypid, 9);
 
   return 0;
@@ -1829,7 +2017,7 @@ child_cleanup (guestfs_h *g)
   if (g->verbose)
     fprintf (stderr, "child_cleanup: %p: child process died\n", g);
 
-  /*kill (g->pid, SIGTERM);*/
+  /*if (g->pid > 0) kill (g->pid, SIGTERM);*/
   if (g->recoverypid > 0) kill (g->recoverypid, 9);
   waitpid (g->pid, NULL, 0);
   if (g->recoverypid > 0) waitpid (g->recoverypid, NULL, 0);
@@ -1848,7 +2036,7 @@ child_cleanup (guestfs_h *g)
 }
 
 static int
-read_log_message_or_eof (guestfs_h *g, int fd)
+read_log_message_or_eof (guestfs_h *g, int fd, int error_if_eof)
 {
   char buf[BUFSIZ];
   int n;
@@ -1869,6 +2057,13 @@ read_log_message_or_eof (guestfs_h *g, int fd)
   if (n == 0) {
     /* Hopefully this indicates the qemu child process has died. */
     child_cleanup (g);
+
+    if (error_if_eof) {
+      /* We weren't expecting eof here (called from launch) so place
+       * something in the error buffer.  RHBZ#588851.
+       */
+      error (g, "child process died unexpectedly");
+    }
     return -1;
   }
 
@@ -1976,7 +2171,7 @@ send_to_daemon (guestfs_h *g, const void *v_buf, size_t n)
     }
 
     if (FD_ISSET (g->fd[1], &rset2)) {
-      if (read_log_message_or_eof (g, g->fd[1]) == -1)
+      if (read_log_message_or_eof (g, g->fd[1], 0) == -1)
         return -1;
     }
     if (FD_ISSET (g->sock, &rset2)) {
@@ -2059,7 +2254,7 @@ recv_from_daemon (guestfs_h *g, uint32_t *size_rtn, void **buf_rtn)
     }
 
     if (FD_ISSET (g->fd[1], &rset2)) {
-      if (read_log_message_or_eof (g, g->fd[1]) == -1) {
+      if (read_log_message_or_eof (g, g->fd[1], 0) == -1) {
         free (*buf_rtn);
         *buf_rtn = NULL;
         return -1;
@@ -2196,8 +2391,16 @@ accept_from_daemon (guestfs_h *g)
   int sock = -1;
 
   while (sock == -1) {
+    /* If the qemu process has died, clean up the zombie (RHBZ#579155).
+     * By partially polling in the select below we ensure that this
+     * function will be called eventually.
+     */
+    waitpid (g->pid, NULL, WNOHANG);
+
     rset2 = rset;
-    int r = select (max_fd+1, &rset2, NULL, NULL, NULL);
+
+    struct timeval tv = { .tv_sec = 1, .tv_usec = 0 };
+    int r = select (max_fd+1, &rset2, NULL, NULL, &tv);
     if (r == -1) {
       if (errno == EINTR || errno == EAGAIN)
         continue;
@@ -2206,7 +2409,7 @@ accept_from_daemon (guestfs_h *g)
     }
 
     if (FD_ISSET (g->fd[1], &rset2)) {
-      if (read_log_message_or_eof (g, g->fd[1]) == -1)
+      if (read_log_message_or_eof (g, g->fd[1], 1) == -1)
         return -1;
     }
     if (FD_ISSET (g->sock, &rset2)) {
@@ -2517,7 +2720,6 @@ guestfs___recv_file (guestfs_h *g, const char *filename)
 {
   void *buf;
   int fd, r;
-  size_t len;
 
   fd = open (filename, O_WRONLY|O_CREAT|O_TRUNC|O_NOCTTY, 0666);
   if (fd == -1) {