Add test for qemu broken -machine option (RHBZ#748266).
[libguestfs.git] / src / launch.c
index 1a47363..7b3372c 100644 (file)
@@ -63,6 +63,7 @@
 #include <netinet/in.h>
 
 #include "c-ctype.h"
+#include "ignore-value.h"
 #include "glthread/lock.h"
 
 #include "guestfs.h"
 
 static int launch_appliance (guestfs_h *g);
 static int64_t timeval_diff (const struct timeval *x, const struct timeval *y);
+static void print_qemu_command_line (guestfs_h *g, char **argv);
 static int connect_unix_socket (guestfs_h *g, const char *sock);
 static int qemu_supports (guestfs_h *g, const char *option);
+static char *qemu_drive_param (guestfs_h *g, const struct drive *drv);
+
+#if 0
+static int qemu_supports_re (guestfs_h *g, const pcre *option_regex);
+
+static void compile_regexps (void) __attribute__((constructor));
+static void free_regexps (void) __attribute__((destructor));
 
-/* Add a string to the current command line. */
 static void
-incr_cmdline_size (guestfs_h *g)
+compile_regexps (void)
+{
+  const char *err;
+  int offset;
+
+#define COMPILE(re,pattern,options)                                     \
+  do {                                                                  \
+    re = pcre_compile ((pattern), (options), &err, &offset, NULL);      \
+    if (re == NULL) {                                                   \
+      ignore_value (write (2, err, strlen (err)));                      \
+      abort ();                                                         \
+    }                                                                   \
+  } while (0)
+}
+
+static void
+free_regexps (void)
+{
+}
+#endif
+
+/* Functions to add a string to the current command line. */
+static void
+alloc_cmdline (guestfs_h *g)
 {
   if (g->cmdline == NULL) {
     /* g->cmdline[0] is reserved for argv[0], set in guestfs_launch. */
@@ -85,7 +116,12 @@ incr_cmdline_size (guestfs_h *g)
     g->cmdline = safe_malloc (g, sizeof (char *));
     g->cmdline[0] = NULL;
   }
+}
 
+static void
+incr_cmdline_size (guestfs_h *g)
+{
+  alloc_cmdline (g);
   g->cmdline_size++;
   g->cmdline = safe_realloc (g, g->cmdline, sizeof (char *) * g->cmdline_size);
 }
@@ -104,23 +140,18 @@ add_cmdline (guestfs_h *g, const char *str)
   return 0;
 }
 
-size_t
-guestfs___checkpoint_cmdline (guestfs_h *g)
+struct drive **
+guestfs___checkpoint_drives (guestfs_h *g)
 {
-  return g->cmdline_size;
+  struct drive **i = &g->drives;
+  while (*i != NULL) i = &((*i)->next);
+  return i;
 }
 
 void
-guestfs___rollback_cmdline (guestfs_h *g, size_t pos)
+guestfs___rollback_drives (guestfs_h *g, struct drive **i)
 {
-  size_t i;
-
-  assert (g->cmdline_size >= pos);
-
-  for (i = pos; i < g->cmdline_size; ++i)
-    free (g->cmdline[i]);
-
-  g->cmdline_size = pos;
+  guestfs___free_drives(i);
 }
 
 /* Internal command to return the command line. */
@@ -130,11 +161,7 @@ guestfs__debug_cmdline (guestfs_h *g)
   size_t i;
   char **r;
 
-  if (g->cmdline == NULL) {
-    r = safe_malloc (g, sizeof (char *) * 1);
-    r[0] = NULL;
-    return r;
-  }
+  alloc_cmdline (g);
 
   r = safe_malloc (g, sizeof (char *) * (g->cmdline_size + 1));
   r[0] = safe_strdup (g, g->qemu); /* g->cmdline[0] is always NULL */
@@ -147,6 +174,27 @@ guestfs__debug_cmdline (guestfs_h *g)
   return r;                     /* caller frees */
 }
 
+/* Internal command to return the list of drives. */
+char **
+guestfs__debug_drives (guestfs_h *g)
+{
+  size_t i, count;
+  char **ret;
+  struct drive *drv;
+
+  for (count = 0, drv = g->drives; drv; count++, drv = drv->next)
+    ;
+
+  ret = safe_malloc (g, sizeof (char *) * (count + 1));
+
+  for (i = 0, drv = g->drives; drv; i++, drv = drv->next)
+    ret[i] = qemu_drive_param (g, drv);
+
+  ret[count] = NULL;
+
+  return ret;                   /* caller frees */
+}
+
 int
 guestfs__config (guestfs_h *g,
                  const char *qemu_param, const char *qemu_value)
@@ -185,17 +233,21 @@ guestfs__config (guestfs_h *g,
  * O_DIRECT.  This fails on some filesystem types (notably tmpfs).
  * So we check if we can open the file with or without O_DIRECT,
  * and use cache=off (or not) accordingly.
+ *
+ * NB: This function is only called on the !readonly path.  We must
+ * try to open with O_RDWR to test that the file is readable and
+ * writable here.
  */
 static int
 test_cache_off (guestfs_h *g, const char *filename)
 {
-  int fd = open (filename, O_RDONLY|O_DIRECT);
+  int fd = open (filename, O_RDWR|O_DIRECT);
   if (fd >= 0) {
     close (fd);
     return 1;
   }
 
-  fd = open (filename, O_RDONLY);
+  fd = open (filename, O_RDWR);
   if (fd >= 0) {
     close (fd);
     return 0;
@@ -228,8 +280,10 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
                          const struct guestfs_add_drive_opts_argv *optargs)
 {
   int readonly;
-  const char *format;
-  const char *iface;
+  char *format;
+  char *iface;
+  char *name;
+  int use_cache_off;
 
   if (strchr (filename, ',') != NULL) {
     error (g, _("filename cannot contain ',' (comma) character"));
@@ -239,18 +293,26 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
   readonly = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_READONLY_BITMASK
              ? optargs->readonly : 0;
   format = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_FORMAT_BITMASK
-           ? optargs->format : NULL;
+           ? safe_strdup (g, optargs->format) : NULL;
   iface = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_IFACE_BITMASK
-          ? optargs->iface : DRIVE_IF;
+          ? safe_strdup (g, optargs->iface) : safe_strdup (g, DRIVE_IF);
+  name = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_NAME_BITMASK
+          ? safe_strdup (g, optargs->name) : NULL;
 
   if (format && !valid_format_iface (format)) {
     error (g, _("%s parameter is empty or contains disallowed characters"),
            "format");
+    free (format);
+    free (iface);
+    free (name);
     return -1;
   }
   if (!valid_format_iface (iface)) {
     error (g, _("%s parameter is empty or contains disallowed characters"),
            "iface");
+    free (format);
+    free (iface);
+    free (name);
     return -1;
   }
 
@@ -258,31 +320,37 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
    * checks for the existence of the file.  For readonly we have
    * to do the check explicitly.
    */
-  int use_cache_off = readonly ? 0 : test_cache_off (g, filename);
-  if (use_cache_off == -1)
+  use_cache_off = readonly ? 0 : test_cache_off (g, filename);
+  if (use_cache_off == -1) {
+    free (format);
+    free (iface);
+    free (name);
     return -1;
+  }
 
   if (readonly) {
-    if (access (filename, F_OK) == -1) {
+    if (access (filename, R_OK) == -1) {
       perrorf (g, "%s", filename);
+      free (format);
+      free (iface);
+      free (name);
       return -1;
     }
   }
 
-  /* Construct the final -drive parameter. */
-  size_t len = 64 + strlen (filename) + strlen (iface);
-  if (format) len += strlen (format);
-  char buf[len];
+  struct drive **i = &(g->drives);
+  while (*i != NULL) i = &((*i)->next);
 
-  snprintf (buf, len, "file=%s%s%s%s%s,if=%s",
-            filename,
-            readonly ? ",snapshot=on" : "",
-            use_cache_off ? ",cache=off" : "",
-            format ? ",format=" : "",
-            format ? format : "",
-            iface);
+  *i = safe_malloc (g, sizeof (struct drive));
+  (*i)->next = NULL;
+  (*i)->path = safe_strdup (g, filename);
+  (*i)->readonly = readonly;
+  (*i)->format = format;
+  (*i)->iface = iface;
+  (*i)->name = name;
+  (*i)->use_cache_off = use_cache_off;
 
-  return guestfs__config (g, "-drive", buf);
+  return 0;
 }
 
 int
@@ -359,6 +427,8 @@ guestfs__launch (guestfs_h *g)
     return -1;
   }
 
+  TRACE0 (launch_start);
+
   /* Make the temporary directory. */
   if (!g->tmpdir) {
     TMP_TEMPLATE_ON_STACK (dir_template);
@@ -400,7 +470,7 @@ launch_appliance (guestfs_h *g)
   /* At present you must add drives before starting the appliance.  In
    * future when we enable hotplugging you won't need to do this.
    */
-  if (!g->cmdline) {
+  if (!g->drives) {
     error (g, _("you must call guestfs_add_drive before guestfs_launch"));
     return -1;
   }
@@ -409,11 +479,15 @@ launch_appliance (guestfs_h *g)
   gettimeofday (&g->launch_t, NULL);
   guestfs___launch_send_progress (g, 0);
 
+  TRACE0 (launch_build_appliance_start);
+
   /* Locate and/or build the appliance. */
   char *kernel = NULL, *initrd = NULL, *appliance = NULL;
   if (guestfs___build_appliance (g, &kernel, &initrd, &appliance) == -1)
     return -1;
 
+  TRACE0 (launch_build_appliance_end);
+
   guestfs___launch_send_progress (g, 3);
 
   if (g->verbose)
@@ -482,8 +556,27 @@ launch_appliance (guestfs_h *g)
     /* Set up the full command line.  Do this in the subprocess so we
      * don't need to worry about cleaning up.
      */
+
+    /* Set g->cmdline[0] to the name of the qemu process.  However
+     * it is possible that no g->cmdline has been allocated yet so
+     * we must do that first.
+     */
+    alloc_cmdline (g);
     g->cmdline[0] = g->qemu;
 
+    /* Add drives */
+    struct drive *drv = g->drives;
+    while (drv != NULL) {
+      /* Construct the final -drive parameter. */
+      char *buf = qemu_drive_param (g, drv);
+
+      add_cmdline (g, "-drive");
+      add_cmdline (g, buf);
+      free (buf);
+
+      drv = drv->next;
+    }
+
     if (qemu_supports (g, "-nodefconfig"))
       add_cmdline (g, "-nodefconfig");
 
@@ -493,7 +586,19 @@ launch_appliance (guestfs_h *g)
      */
     if (qemu_supports (g, "-machine")) {
       add_cmdline (g, "-machine");
+#if QEMU_MACHINE_TYPE_IS_BROKEN
+      /* Workaround for qemu 0.15: We have to add the '[type=]pc'
+       * since there is no default.  This is not a permanent solution
+       * because this only works on PC-like hardware.  Other platforms
+       * like ppc would need a different machine type.
+       *
+       * This bug is fixed in qemu commit 2645c6dcaf6ea2a51a, and was
+       * not a problem in qemu < 0.15.
+       */
+      add_cmdline (g, "pc,accel=kvm:tcg");
+#else
       add_cmdline (g, "accel=kvm:tcg");
+#endif
     } else {
       /* qemu sometimes needs this option to enable hardware
        * virtualization, but some versions of 'qemu-kvm' will use KVM
@@ -525,6 +630,12 @@ launch_appliance (guestfs_h *g)
 
     add_cmdline (g, "-nographic");
 
+    if (g->smp > 1) {
+      snprintf (buf, sizeof buf, "%d", g->smp);
+      add_cmdline (g, "-smp");
+      add_cmdline (g, buf);
+    }
+
     snprintf (buf, sizeof buf, "%d", g->memsize);
     add_cmdline (g, "-m");
     add_cmdline (g, buf);
@@ -576,7 +687,7 @@ launch_appliance (guestfs_h *g)
     "panic=1 "         /* force kernel to panic if daemon exits */     \
     "console=ttyS0 "   /* serial console */                            \
     "udevtimeout=300 " /* good for very slow systems (RHBZ#480319) */  \
-    "noapic "          /* workaround for RHBZ#502058 - ok if not SMP */ \
+    "no_timer_check "  /* fix for RHBZ#502058 */                        \
     "acpi=off "        /* we don't need ACPI, turn it off */           \
     "printk.time=1 "   /* display timestamp before kernel messages */   \
     "cgroup_disable=memory " /* saves us about 5 MB of RAM */
@@ -621,21 +732,28 @@ launch_appliance (guestfs_h *g)
     incr_cmdline_size (g);
     g->cmdline[g->cmdline_size-1] = NULL;
 
-    if (g->verbose)
-      guestfs___print_timestamped_argv (g, (const char **)g->cmdline);
-
     if (!g->direct) {
-      /* Set up stdin, stdout. */
+      /* Set up stdin, stdout, stderr. */
       close (0);
       close (1);
       close (wfd[1]);
       close (rfd[0]);
 
+      /* Stdin. */
       if (dup (wfd[0]) == -1) {
       dup_failed:
         perror ("dup failed");
         _exit (EXIT_FAILURE);
       }
+      /* Stdout. */
+      if (dup (rfd[1]) == -1)
+        goto dup_failed;
+
+      /* Particularly since qemu 0.15, qemu spews all sorts of debug
+       * information on stderr.  It is useful to both capture this and
+       * not confuse casual users, so send stderr to the pipe as well.
+       */
+      close (2);
       if (dup (rfd[1]) == -1)
         goto dup_failed;
 
@@ -643,12 +761,18 @@ launch_appliance (guestfs_h *g)
       close (rfd[1]);
     }
 
+    /* Dump the command line (after setting up stderr above). */
+    if (g->verbose)
+      print_qemu_command_line (g, g->cmdline);
+
     /* Put qemu in a new process group. */
     if (g->pgroup)
       setpgid (0, 0);
 
     setenv ("LC_ALL", "C", 1);
 
+    TRACE0 (launch_run_qemu);
+
     execv (g->qemu, g->cmdline); /* Run qemu. */
     perror (g->qemu);
     _exit (EXIT_FAILURE);
@@ -683,7 +807,7 @@ launch_appliance (guestfs_h *g)
         setpgid (0, 0);
 
       /* Writing to argv is hideously complicated and error prone.  See:
-       * http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/misc/ps_status.c?rev=1.33.2.1;content-type=text%2Fplain
+       * http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/utils/misc/ps_status.c;hb=HEAD
        */
 
       /* Loop around waiting for one or both of the other processes to
@@ -745,6 +869,13 @@ launch_appliance (guestfs_h *g)
   if (r == -1)
     goto cleanup1;
 
+  /* NB: We reach here just because qemu has opened the socket.  It
+   * does not mean the daemon is up until we read the
+   * GUESTFS_LAUNCH_FLAG below.  Failures in qemu startup can still
+   * happen even if we reach here, even early failures like not being
+   * able to open a drive.
+   */
+
   close (g->sock); /* Close the listening socket. */
   g->sock = r; /* This is the accepted data socket. */
 
@@ -778,6 +909,8 @@ launch_appliance (guestfs_h *g)
     goto cleanup1;
   }
 
+  TRACE0 (launch_end);
+
   guestfs___launch_send_progress (g, 12);
 
   return 0;
@@ -963,47 +1096,9 @@ timeval_diff (const struct timeval *x, const struct timeval *y)
   return msec;
 }
 
-void
-guestfs___print_timestamped_argv (guestfs_h *g, const char * argv[])
-{
-  int i = 0;
-  int needs_quote;
-  char *buf = NULL;
-  size_t len;
-  FILE *fp;
-
-  fp = open_memstream (&buf, &len);
-  if (fp == NULL) {
-    warning (g, "open_memstream: %m");
-    return;
-  }
-
-  struct timeval tv;
-  gettimeofday (&tv, NULL);
-  fprintf (fp, "[%05" PRIi64 "ms] ", timeval_diff (&g->launch_t, &tv));
-
-  while (argv[i]) {
-    if (argv[i][0] == '-') /* -option starts a new line */
-      fprintf (fp, " \\\n   ");
-
-    if (i > 0) fputc (' ', fp);
-
-    /* Does it need shell quoting?  This only deals with simple cases. */
-    needs_quote = strcspn (argv[i], " ") != strlen (argv[i]);
-
-    if (needs_quote) fputc ('\'', fp);
-    fprintf (fp, "%s", argv[i]);
-    if (needs_quote) fputc ('\'', fp);
-    i++;
-  }
-
-  fclose (fp);
-
-  debug (g, "%s", buf);
-
-  free (buf);
-}
-
+/* Note that since this calls 'debug' it should only be called
+ * from the parent process.
+ */
 void
 guestfs___print_timestamped_message (guestfs_h *g, const char *fs, ...)
 {
@@ -1025,6 +1120,37 @@ guestfs___print_timestamped_message (guestfs_h *g, const char *fs, ...)
   free (msg);
 }
 
+/* This is called from the forked subprocess just before qemu runs, so
+ * it can just print the message straight to stderr, where it will be
+ * picked up and funnelled through the usual appliance event API.
+ */
+static void
+print_qemu_command_line (guestfs_h *g, char **argv)
+{
+  int i = 0;
+  int needs_quote;
+
+  struct timeval tv;
+  gettimeofday (&tv, NULL);
+  fprintf (stderr, "[%05" PRIi64 "ms] ", timeval_diff (&g->launch_t, &tv));
+
+  while (argv[i]) {
+    if (argv[i][0] == '-') /* -option starts a new line */
+      fprintf (stderr, " \\\n   ");
+
+    if (i > 0) fputc (' ', stderr);
+
+    /* Does it need shell quoting?  This only deals with simple cases. */
+    needs_quote = strcspn (argv[i], " ") != strlen (argv[i]);
+
+    if (needs_quote) fputc ('\'', stderr);
+    fprintf (stderr, "%s", argv[i]);
+    if (needs_quote) fputc ('\'', stderr);
+    i++;
+  }
+}
+
+static int test_qemu_cmd (guestfs_h *g, const char *cmd, char **ret);
 static int read_all (guestfs_h *g, FILE *fp, char **ret);
 
 /* Test qemu binary (or wrapper) runs, and do 'qemu -help' and
@@ -1037,38 +1163,49 @@ test_qemu (guestfs_h *g)
   char cmd[1024];
   FILE *fp;
 
+  free (g->qemu_help);
+  g->qemu_help = NULL;
+  free (g->qemu_version);
+  g->qemu_version = NULL;
+
   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
    * supported by qemu 0.9).  If this command doesn't work then it
    * probably indicates that the qemu binary is missing.
    */
-  if (!fp) {
-    /* XXX This error is never printed, even if the qemu binary
-     * doesn't exist.  Why?
-     */
-  error:
-    perrorf (g, _("%s: command failed: If qemu is located on a non-standard path, try setting the LIBGUESTFS_QEMU environment variable."), cmd);
+  if (test_qemu_cmd (g, cmd, &g->qemu_help) == -1) {
+    error (g, _("command failed: %s\n\nIf qemu is located on a non-standard path, try setting the LIBGUESTFS_QEMU\nenvironment variable.  There may also be errors printed above."),
+           cmd);
     return -1;
   }
 
-  if (read_all (g, fp, &g->qemu_help) == -1)
-    goto error;
-
-  if (pclose (fp) == -1)
-    goto error;
-
   snprintf (cmd, sizeof cmd, "LC_ALL=C '%s' -nographic -version 2>/dev/null",
             g->qemu);
 
+  /* Intentionally ignore errors from qemu -version. */
+  ignore_value (test_qemu_cmd (g, cmd, &g->qemu_version));
+
+  return 0;
+}
+
+static int
+test_qemu_cmd (guestfs_h *g, const char *cmd, char **ret)
+{
+  FILE *fp;
+
   fp = popen (cmd, "r");
-  if (fp) {
-    /* Intentionally ignore errors. */
-    read_all (g, fp, &g->qemu_version);
+  if (fp == NULL)
+    return -1;
+
+  if (read_all (g, fp, ret) == -1) {
     pclose (fp);
+    return -1;
   }
 
+  if (pclose (fp) != 0)
+    return -1;
+
   return 0;
 }
 
@@ -1120,6 +1257,20 @@ qemu_supports (guestfs_h *g, const char *option)
   return strstr (g->qemu_help, option) != NULL;
 }
 
+#if 0
+/* As above but using a regex instead of a fixed string. */
+static int
+qemu_supports_re (guestfs_h *g, const pcre *option_regex)
+{
+  if (!g->qemu_help) {
+    if (test_qemu (g) == -1)
+      return -1;
+  }
+
+  return match (g, g->qemu_help, option_regex);
+}
+#endif
+
 /* Check if a file can be opened. */
 static int
 is_openable (guestfs_h *g, const char *path, int flags)
@@ -1133,6 +1284,30 @@ is_openable (guestfs_h *g, const char *path, int flags)
   return 1;
 }
 
+static char *
+qemu_drive_param (guestfs_h *g, const struct drive *drv)
+{
+  size_t len = 64;
+  char *r;
+
+  len += strlen (drv->path);
+  len += strlen (drv->iface);
+  if (drv->format)
+    len += strlen (drv->format);
+
+  r = safe_malloc (g, len);
+
+  snprintf (r, len, "file=%s%s%s%s%s,if=%s",
+            drv->path,
+            drv->readonly ? ",snapshot=on" : "",
+            drv->use_cache_off ? ",cache=off" : "",
+            drv->format ? ",format=" : "",
+            drv->format ? drv->format : "",
+            drv->iface);
+
+  return r;                     /* caller frees */
+}
+
 /* You had to call this function after launch in versions <= 1.0.70,
  * but it is now a no-op.
  */