Libguestfs - Apr 2017 - [PATCH 0/4] common: Add a simple mini-library for handling qemu command and config files.

Currently we have an OCaml library for generating the qemu command
line (used only by ‘virt-v2v -o qemu’).  However we also generate a
qemu command line in ‘lib/launch-direct.c’, and we might in future
need to generate a ‘-readconfig’-compatible configuration file if we
want to go beyond 10,000 drives for scalability testing.

Therefore this patch series reimplements the qemu command line code as
a small C library (common/qemuopts), extends it to support qemu config
files, and then reimplements both ‘lib/launch-direct.c’ and ‘virt-v2v
-o qemu’ to use it.

It also fixes a few bugs.  In particular we don't properly comma-quote
qemu parameters in multiple places.  This fixes all of those places
properly.

It also drops support for ‘./configure --with-qemu-options’ which is
broken (see commit message).

Rich.

---
 .gitignore                       |   1 +
 Makefile.am                      |   2 +-
 common/qemuopts/Makefile.am      |  46 ++
 common/qemuopts/qemuopts-tests.c | 226 ++++++++++
 common/qemuopts/qemuopts.c       | 952 +++++++++++++++++++++++++++++++++++++++
 common/qemuopts/qemuopts.h       |  47 ++
 configure.ac                     |   1 +
 7 files changed, 1274 insertions(+), 1 deletion(-)
 create mode 100644 common/qemuopts/Makefile.am
 create mode 100644 common/qemuopts/qemuopts-tests.c
 create mode 100644 common/qemuopts/qemuopts.c
 create mode 100644 common/qemuopts/qemuopts.h

diff --git a/.gitignore b/.gitignore
index 152a400..d9a3d6d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -127,6 +127,7 @@ Makefile.in
 /common/protocol/guestfs_protocol.c
 /common/protocol/guestfs_protocol.h
 /common/protocol/guestfs_protocol.x
+/common/qemuopts/qemuopts-tests
 /common/utils/guestfs-internal-frontend-cleanups.h
 /common/utils/structs-cleanup.c
 /common/utils/structs-print.c
diff --git a/Makefile.am b/Makefile.am
index 6c072a1..53014e2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -38,7 +38,7 @@ SUBDIRS += gnulib/tests
 endif
 
 # Basic source for the library.
-SUBDIRS += common/errnostring common/protocol common/utils
+SUBDIRS += common/errnostring common/protocol common/qemuopts common/utils
 SUBDIRS += lib docs examples po
 
 # The daemon and the appliance.
diff --git a/common/qemuopts/Makefile.am b/common/qemuopts/Makefile.am
new file mode 100644
index 0000000..ff643be
--- /dev/null
+++ b/common/qemuopts/Makefile.am
@@ -0,0 +1,46 @@
+# libguestfs
+# Copyright (C) 2017 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+include $(top_srcdir)/subdir-rules.mk
+
+noinst_LTLIBRARIES = libqemuopts.la
+
+libqemuopts_la_SOURCES = \
+	qemuopts.c \
+	qemuopts.h
+libqemuopts_la_CPPFLAGS = \
+	-I$(srcdir) -I.
+libqemuopts_la_CFLAGS = \
+	$(WARN_CFLAGS) $(WERROR_CFLAGS) \
+	$(GCC_VISIBILITY_HIDDEN)
+
+TESTS_ENVIRONMENT = $(top_builddir)/run --test
+LOG_COMPILER = $(VG)
+TESTS = qemuopts-tests
+
+check_PROGRAMS = qemuopts-tests
+
+qemuopts_tests_SOURCES = qemuopts-tests.c
+qemuopts_tests_CPPFLAGS = \
+	-I$(srcdir) -I.
+qemuopts_tests_CFLAGS = \
+	$(WARN_CFLAGS) $(WERROR_CFLAGS)
+qemuopts_tests_LDADD = \
+	libqemuopts.la
+
+check-valgrind:
+	make VG="@VG@" check
diff --git a/common/qemuopts/qemuopts-tests.c b/common/qemuopts/qemuopts-tests.c
new file mode 100644
index 0000000..b4e7bcc
--- /dev/null
+++ b/common/qemuopts/qemuopts-tests.c
@@ -0,0 +1,226 @@
+/* libguestfs
+ * Copyright (C) 2014-2017 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+/**
+ * Unit tests of internal functions.
+ *
+ * These tests may use a libguestfs handle, but must not launch the
+ * handle.  Also, avoid long-running tests.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdarg.h>
+#include <string.h>
+#include <errno.h>
+#include <assert.h>
+
+#include "qemuopts.h"
+
+#define CHECK_ERROR(r,call,expr)                \
+  do {                                          \
+    if ((expr) == (r)) {                        \
+      perror (call);                            \
+      exit (EXIT_FAILURE);                      \
+    }                                           \
+  } while (0)
+
+int
+main (int argc, char *argv[])
+{
+  struct qemuopts *qopts;
+  FILE *fp;
+  char *actual;
+  size_t i, len;
+  char **actual_argv;
+
+  qopts = qemuopts_create ();
+
+  if (qemuopts_set_binary_by_arch (qopts, NULL) == -1) {
+    if (errno == ENXIO) {
+      fprintf (stderr, "qemuopts: This architecture does not support
KVM.\n");
+      fprintf (stderr, "If this architecture *does* support KVM, then
please modify qemuopts.c\n");
+      fprintf (stderr, "and send us a patch.\n");
+      exit (77);                /* Skip the test. */
+    }
+    perror ("qemuopts_set_binary_by_arch");
+    exit (EXIT_FAILURE);
+  }
+  /* ... but for the purposes of testing, it's easier if we
+   * set this to a known string.
+   */
+  CHECK_ERROR (-1, "qemuopts_set_binary",
+               qemuopts_set_binary (qopts, "qemu-system-x86_64"));
+
+  CHECK_ERROR (-1, "qemuopts_add_flag",
+               qemuopts_add_flag (qopts, "-nodefconfig"));
+  CHECK_ERROR (-1, "qemuopts_add_arg",
+               qemuopts_add_arg (qopts, "-m", "1024"));
+  CHECK_ERROR (-1, "qemuopts_add_arg_format",
+               qemuopts_add_arg_format (qopts, "-smp",
"%d", 4));
+
+  CHECK_ERROR (-1, "qemuopts_start_arg_list",
+               qemuopts_start_arg_list (qopts, "-drive"));
+  CHECK_ERROR (-1, "qemuopts_append_arg_list",
+               qemuopts_append_arg_list (qopts, "file=/tmp/foo"));
+  CHECK_ERROR (-1, "qemuopts_append_arg_list_format",
+               qemuopts_append_arg_list_format (qopts, "if=%s",
"ide"));
+  CHECK_ERROR (-1, "qemuopts_end_arg_list",
+               qemuopts_end_arg_list (qopts));
+  CHECK_ERROR (-1, "qemuopts_add_arg_list",
+               qemuopts_add_arg_list (qopts, "-drive",
+                                      "file=/tmp/bar",
"serial=123",
+                                      NULL));
+
+  /* Test qemu comma-quoting. */
+  CHECK_ERROR (-1, "qemuopts_add_arg",
+               qemuopts_add_arg (qopts, "-name",
"foo,bar"));
+  CHECK_ERROR (-1, "qemuopts_add_arg_list",
+               qemuopts_add_arg_list (qopts, "-drive",
+                                      "file=comma,in,name",
+                                      "serial=$dollar$",
+                                      NULL));
+
+  /* Test shell quoting. */
+  CHECK_ERROR (-1, "qemuopts_add_arg",
+               qemuopts_add_arg (qopts, "-cdrom",
"\"$quoted\".iso"));
+
+  fp = open_memstream (&actual, &len);
+  if (fp == NULL) {
+    perror ("open_memstream");
+    exit (EXIT_FAILURE);
+  }
+  CHECK_ERROR (-1, "qemuopts_to_channel",
+               qemuopts_to_channel (qopts, fp));
+  if (fclose (fp) == EOF) {
+    perror ("fclose");
+    exit (EXIT_FAILURE);
+  }
+
+  const char *expected +    "qemu-system-x86_64 \\\n"
+    "    -nodefconfig \\\n"
+    "    -m 1024 \\\n"
+    "    -smp 4 \\\n"
+    "    -drive file=/tmp/foo,if=ide \\\n"
+    "    -drive file=/tmp/bar,serial=123 \\\n"
+    "    -name \"foo,,bar\" \\\n"
+    "    -drive
\"file=comma,,in,,name\",\"serial=\\$dollar\\$\" \\\n"
+    "    -cdrom \"\\\"\\$quoted\\\".iso\"\n";
+
+  if (strcmp (actual, expected) != 0) {
+    fprintf (stderr, "qemuopts: Serialized qemu command line does not
match expected\n");
+    fprintf (stderr, "Actual:\n%s", actual);
+    fprintf (stderr, "Expected:\n%s", expected);
+    exit (EXIT_FAILURE);
+  }
+
+  free (actual);
+
+  /* Test qemuopts_to_argv. */
+  CHECK_ERROR (NULL, "qemuopts_to_argv",
+               actual_argv = qemuopts_to_argv (qopts));
+  const char *expected_argv[] = {
+    "qemu-system-x86_64",
+    "-nodefconfig",
+    "-m", "1024",
+    "-smp", "4",
+    "-drive", "file=/tmp/foo,if=ide",
+    "-drive", "file=/tmp/bar,serial=123",
+    "-name", "foo,,bar",
+    "-drive", "file=comma,,in,,name,serial=$dollar$",
+    "-cdrom", "\"$quoted\".iso",
+    NULL
+  };
+
+  for (i = 0; actual_argv[i] != NULL; ++i) {
+    if (expected_argv[i] == NULL ||
+        strcmp (actual_argv[i], expected_argv[i])) {
+      fprintf (stderr, "qemuopts: actual != expected argv at position %zu,
%s != %s\n",
+               i, actual_argv[i], expected_argv[i]);
+      exit (EXIT_FAILURE);
+    }
+  }
+  assert (expected_argv[i] == NULL);
+
+  for (i = 0; actual_argv[i] != NULL; ++i)
+    free (actual_argv[i]);
+  free (actual_argv);
+
+  qemuopts_free (qopts);
+
+  /* Test qemuopts_to_config_channel. */
+  qopts = qemuopts_create ();
+
+  CHECK_ERROR (-1, "qemuopts_start_arg_list",
+               qemuopts_start_arg_list (qopts, "-drive"));
+  CHECK_ERROR (-1, "qemuopts_append_arg_list",
+               qemuopts_append_arg_list (qopts, "file=/tmp/foo"));
+  CHECK_ERROR (-1, "qemuopts_append_arg_list",
+               qemuopts_append_arg_list (qopts, "id=id"));
+  CHECK_ERROR (-1, "qemuopts_append_arg_list_format",
+               qemuopts_append_arg_list_format (qopts, "if=%s",
"ide"));
+  CHECK_ERROR (-1, "qemuopts_append_arg_list_format",
+               qemuopts_append_arg_list_format (qopts, "bool"));
+  CHECK_ERROR (-1, "qemuopts_end_arg_list",
+               qemuopts_end_arg_list (qopts));
+  CHECK_ERROR (-1, "qemuopts_add_arg_list",
+               qemuopts_add_arg_list (qopts, "-drive",
+                                      "file=/tmp/bar",
"serial=123",
+                                      NULL));
+
+  fp = open_memstream (&actual, &len);
+  if (fp == NULL) {
+    perror ("open_memstream");
+    exit (EXIT_FAILURE);
+  }
+  CHECK_ERROR (-1, "qemuopts_to_config_channel",
+               qemuopts_to_config_channel (qopts, fp));
+  if (fclose (fp) == EOF) {
+    perror ("fclose");
+    exit (EXIT_FAILURE);
+  }
+
+  const char *expected2 +    "# qemu config file\n"
+    "\n"
+    "[drive \"id\"]\n"
+    "  file = \"/tmp/foo\"\n"
+    "  if = \"ide\"\n"
+    "  bool = \"on\"\n"
+    "\n"
+    "[drive]\n"
+    "  file = \"/tmp/bar\"\n"
+    "  serial = \"123\"\n"
+    "\n";
+
+  if (strcmp (actual, expected2) != 0) {
+    fprintf (stderr, "qemuopts: Serialized qemu command line does not
match expected\n");
+    fprintf (stderr, "Actual:\n%s", actual);
+    fprintf (stderr, "Expected:\n%s", expected2);
+    exit (EXIT_FAILURE);
+  }
+
+  free (actual);
+
+  qemuopts_free (qopts);
+
+  exit (EXIT_SUCCESS);
+}
diff --git a/common/qemuopts/qemuopts.c b/common/qemuopts/qemuopts.c
new file mode 100644
index 0000000..acdfda8
--- /dev/null
+++ b/common/qemuopts/qemuopts.c
@@ -0,0 +1,952 @@
+/* libguestfs
+ * Copyright (C) 2009-2017 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
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * Mini-library for writing qemu command lines and qemu config files.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdarg.h>
+#include <string.h>
+#include <unistd.h>
+#include <assert.h>
+#include <errno.h>
+#include <sys/stat.h>
+
+#include "qemuopts.h"
+
+enum qopt_type {
+  QOPT_FLAG,
+  QOPT_ARG,
+  QOPT_ARG_NOQUOTE,
+  QOPT_ARG_LIST,
+};
+
+struct qopt {
+  enum qopt_type type;
+  char *flag;             /* eg. "-m" */
+  char *value;            /* Value, for QOPT_ARG, QOPT_ARG_NOQUOTE. */
+  char **values;          /* List of values, for QOPT_ARG_LIST. */
+};
+
+struct qemuopts {
+  char *binary;        /* NULL = qemuopts_set_binary not called yet */
+  struct qopt *options;
+  size_t nr_options, nr_alloc;
+};
+
+/**
+ * Create an empty list of qemu options.
+ *
+ * The caller must eventually free the list by calling
+ * C<qemuopts_free>.
+ *
+ * Returns C<NULL> on error, setting C<errno>.
+ */
+struct qemuopts *
+qemuopts_create (void)
+{
+  struct qemuopts *qopts;
+
+  qopts = malloc (sizeof *qopts);
+  if (qopts == NULL)
+    return NULL;
+
+  qopts->binary = NULL;
+  qopts->options = NULL;
+  qopts->nr_options = qopts->nr_alloc = 0;
+
+  return qopts;
+}
+
+static void
+free_string_list (char **argv)
+{
+  size_t i;
+
+  if (argv == NULL)
+    return;
+
+  for (i = 0; argv[i] != NULL; ++i)
+    free (argv[i]);
+  free (argv);
+}
+
+static size_t
+count_strings (char **argv)
+{
+  size_t i;
+
+  for (i = 0; argv[i] != NULL; ++i)
+    ;
+  return i;
+}
+
+/**
+ * Free the list of qemu options.
+ */
+void
+qemuopts_free (struct qemuopts *qopts)
+{
+  size_t i;
+
+  for (i = 0; i < qopts->nr_options; ++i) {
+    free (qopts->options[i].flag);
+    free (qopts->options[i].value);
+    free_string_list (qopts->options[i].values);
+  }
+  free (qopts->options);
+  free (qopts->binary);
+  free (qopts);
+}
+
+static struct qopt *
+extend_options (struct qemuopts *qopts)
+{
+  struct qopt *new_options;
+  struct qopt *ret;
+
+  if (qopts->nr_options >= qopts->nr_alloc) {
+    if (qopts->nr_alloc == 0)
+      qopts->nr_alloc = 1;
+    else
+      qopts->nr_alloc *= 2;
+    new_options = qopts->options;
+    new_options = realloc (new_options,
+                           qopts->nr_alloc * sizeof (struct qopt));
+    if (new_options == NULL)
+      return NULL;
+    qopts->options = new_options;
+  }
+
+  ret = &qopts->options[qopts->nr_options];
+  qopts->nr_options++;
+
+  ret->type = 0;
+  ret->flag = NULL;
+  ret->value = NULL;
+  ret->values = NULL;
+
+  return ret;
+}
+
+static struct qopt *
+last_option (struct qemuopts *qopts)
+{
+  assert (qopts->nr_options > 0);
+  return &qopts->options[qopts->nr_options-1];
+}
+
+/**
+ * Add a command line flag which has no argument. eg:
+ *
+ *  qemuopts_add_flag (qopts, "-nodefconfig");
+ *
+ * Returns C<0> on success.  Returns C<-1> on error, setting
C<errno>.
+ */
+int
+qemuopts_add_flag (struct qemuopts *qopts, const char *flag)
+{
+  struct qopt *qopt;
+  char *flag_copy;
+
+  if (flag[0] != '-') {
+    errno = EINVAL;
+    return -1;
+  }
+
+  flag_copy = strdup (flag);
+  if (flag_copy == NULL)
+    return -1;
+
+  if ((qopt = extend_options (qopts)) == NULL) {
+    free (flag_copy);
+    return -1;
+  }
+
+  qopt->type = QOPT_FLAG;
+  qopt->flag = flag_copy;
+  return 0;
+}
+
+/**
+ * Add a command line flag which has a single argument. eg:
+ *
+ *  qemuopts_add_arg (qopts, "-m", "1024");
+ *
+ * Don't use this if the argument is a comma-separated list, since
+ * quoting will not be done properly.  See C<qemuopts_add_arg_list>.
+ *
+ * Returns C<0> on success.  Returns C<-1> on error, setting
C<errno>.
+ */
+int
+qemuopts_add_arg (struct qemuopts *qopts, const char *flag, const char *value)
+{
+  struct qopt *qopt;
+  char *flag_copy;
+  char *value_copy;
+
+  if (flag[0] != '-') {
+    errno = EINVAL;
+    return -1;
+  }
+
+  flag_copy = strdup (flag);
+  if (flag_copy == NULL)
+    return -1;
+
+  value_copy = strdup (value);
+  if (value_copy == NULL) {
+    free (flag_copy);
+    return -1;
+  }
+
+  if ((qopt = extend_options (qopts)) == NULL) {
+    free (flag_copy);
+    free (value_copy);
+    return -1;
+  }
+
+  qopt->type = QOPT_ARG;
+  qopt->flag = flag_copy;
+  qopt->value = value_copy;
+  return 0;
+}
+
+/**
+ * Add a command line flag which has a single formatted argument. eg:
+ *
+ *  qemuopts_add_arg_format (qopts, "-m", "%d", 1024);
+ *
+ * Don't use this if the argument is a comma-separated list, since
+ * quoting will not be done properly.  See C<qemuopts_add_arg_list>.
+ *
+ * Returns C<0> on success.  Returns C<-1> on error, setting
C<errno>.
+ */
+int
+qemuopts_add_arg_format (struct qemuopts *qopts, const char *flag,
+                         const char *fs, ...)
+{
+  char *value;
+  int r;
+  va_list args;
+
+  if (flag[0] != '-') {
+    errno = EINVAL;
+    return -1;
+  }
+
+  va_start (args, fs);
+  r = vasprintf (&value, fs, args);
+  va_end (args);
+  if (r == -1)
+    return -1;
+
+  r = qemuopts_add_arg (qopts, flag, value);
+  free (value);
+  return r;
+}
+
+/**
+ * This is like C<qemuopts_add_arg> except that no quoting is done on
+ * the value.
+ *
+ * For C<qemuopts_to_script> and C<qemuopts_to_channel>, this
+ * means that neither shell quoting nor qemu comma quoting is done
+ * on the value.
+ *
+ * For C<qemuopts_to_argv> this means that qemu comma quoting is
+ * not done.
+ *
+ * C<qemuopts_to_config*> will fail.
+ *
+ * You should use this with great care.
+ */
+int
+qemuopts_add_arg_noquote (struct qemuopts *qopts, const char *flag,
+                          const char *value)
+{
+  struct qopt *qopt;
+  char *flag_copy;
+  char *value_copy;
+
+  if (flag[0] != '-') {
+    errno = EINVAL;
+    return -1;
+  }
+
+  flag_copy = strdup (flag);
+  if (flag_copy == NULL)
+    return -1;
+
+  value_copy = strdup (value);
+  if (value_copy == NULL) {
+    free (flag_copy);
+    return -1;
+  }
+
+  if ((qopt = extend_options (qopts)) == NULL) {
+    free (flag_copy);
+    free (value_copy);
+    return -1;
+  }
+
+  qopt->type = QOPT_ARG_NOQUOTE;
+  qopt->flag = flag_copy;
+  qopt->value = value_copy;
+  return 0;
+}
+
+/**
+ * Start an argument that takes a comma-separated list of fields.
+ *
+ * Typical usage is like this (with error handling omitted):
+ *
+ *  qemuopts_start_arg_list (qopts, "-drive");
+ *  qemuopts_append_arg_list (qopts, "file=foo");
+ *  qemuopts_append_arg_list_format (qopts, "if=%s",
"ide");
+ *  qemuopts_end_arg_list (qopts);
+ *
+ * which would construct C<-drive file=foo,if=ide>
+ *
+ * See also C<qemuopts_add_arg_list> for a way to do simple cases in
+ * one call.
+ *
+ * Returns C<0> on success.  Returns C<-1> on error, setting
C<errno>.
+ */
+int
+qemuopts_start_arg_list (struct qemuopts *qopts, const char *flag)
+{
+  struct qopt *qopt;
+  char *flag_copy;
+  char **values;
+
+  if (flag[0] != '-') {
+    errno = EINVAL;
+    return -1;
+  }
+
+  flag_copy = strdup (flag);
+  if (flag_copy == NULL)
+    return -1;
+
+  values = calloc (1, sizeof (char *));
+  if (values == NULL) {
+    free (flag_copy);
+    return -1;
+  }
+
+  if ((qopt = extend_options (qopts)) == NULL) {
+    free (flag_copy);
+    free (values);
+    return -1;
+  }
+
+  qopt->type = QOPT_ARG_LIST;
+  qopt->flag = flag_copy;
+  qopt->values = values;
+  return 0;
+}
+
+int
+qemuopts_append_arg_list (struct qemuopts *qopts, const char *value)
+{
+  struct qopt *qopt;
+  char **new_values;
+  char *value_copy;
+  size_t len;
+
+  qopt = last_option (qopts);
+  assert (qopt->type == QOPT_ARG_LIST);
+  len = count_strings (qopt->values);
+
+  value_copy = strdup (value);
+  if (value_copy == NULL)
+    return -1;
+
+  new_values = qopt->values;
+  new_values = realloc (new_values, (len+2) * sizeof (char *));
+  if (new_values == NULL) {
+    free (value_copy);
+    return -1;
+  }
+  qopt->values = new_values;
+  qopt->values[len] = value_copy;
+  qopt->values[len+1] = NULL;
+  return 0;
+}
+
+int
+qemuopts_append_arg_list_format (struct qemuopts *qopts,
+                                 const char *fs, ...)
+{
+  char *value;
+  int r;
+  va_list args;
+
+  va_start (args, fs);
+  r = vasprintf (&value, fs, args);
+  va_end (args);
+  if (r == -1)
+    return -1;
+
+  r = qemuopts_append_arg_list (qopts, value);
+  free (value);
+  return r;
+}
+
+int
+qemuopts_end_arg_list (struct qemuopts *qopts)
+{
+  /* Nothing to do, the list is already well-formed. */
+  return 0;
+}
+
+/**
+ * Add a command line flag which has a list of arguments. eg:
+ *
+ *  qemuopts_add_arg_list (qopts, "-drive", "file=foo",
"if=ide", NULL);
+ *
+ * This is turned into a comma-separated list, like:
+ * C<-drive file=foo,if=ide>.  Note that this handles qemu quoting
+ * properly, so individual elements may contain commas and this will
+ * do the right thing.
+ *
+ * Returns C<0> on success.  Returns C<-1> on error, setting
C<errno>.
+ */
+int
+qemuopts_add_arg_list (struct qemuopts *qopts, const char *flag,
+                       const char *elem0, ...)
+{
+  va_list args;
+  const char *elem;
+
+  if (qemuopts_start_arg_list (qopts, flag) == -1)
+    return -1;
+  if (qemuopts_append_arg_list (qopts, elem0) == -1)
+    return -1;
+  va_start (args, elem0);
+  elem = va_arg (args, const char *);
+  while (elem != NULL) {
+    if (qemuopts_append_arg_list (qopts, elem) == -1) {
+      va_end (args);
+      return -1;
+    }
+    elem = va_arg (args, const char *);
+  }
+  va_end (args);
+  if (qemuopts_end_arg_list (qopts) == -1)
+    return -1;
+  return 0;
+}
+
+/**
+ * Set the qemu binary name.
+ *
+ * Returns C<0> on success.  Returns C<-1> on error, setting
C<errno>.
+ */
+int
+qemuopts_set_binary (struct qemuopts *qopts, const char *binary)
+{
+  char *binary_copy;
+
+  binary_copy = strdup (binary);
+  if (binary_copy == NULL)
+    return -1;
+
+  free (qopts->binary);
+  qopts->binary = binary_copy;
+  return 0;
+}
+
+/**
+ * Set the qemu binary name to C<qemu-system-[arch]>.
+ *
+ * As a special case if C<arch> is C<NULL>, the binary is set to
the
+ * KVM binary for the current host architecture:
+ *
+ *  qemuopts_set_binary_by_arch (qopts, NULL);
+ *
+ * Returns C<0> on success.  Returns C<-1> on error, setting
C<errno>.
+ */
+int
+qemuopts_set_binary_by_arch (struct qemuopts *qopts, const char *arch)
+{
+  char *binary;
+
+  free (qopts->binary);
+  qopts->binary = NULL;
+
+  if (arch) {
+    if (asprintf (&binary, "qemu-system-%s", arch) == -1)
+      return -1;
+    qopts->binary = binary;
+  }
+  else {
+#if defined(__i386__) || defined(__x86_64__)
+    binary = strdup ("qemu-system-x86_64");
+#elif defined(__aarch64__)
+    binary = strdup ("qemu-system-aarch64");
+#elif defined(__arm__)
+    binary = strdup ("qemu-system-arm");
+#elif defined(__powerpc64__) || defined(__powerpc64le__)
+    binary = strdup ("qemu-system-ppc64");
+#elif defined(__s390x__)
+    binary = strdup ("qemu-system-s390x");
+#else
+    /* There is no KVM capability on this architecture. */
+    errno = ENXIO;
+    binary = NULL;
+#endif
+    if (binary == NULL)
+      return -1;
+    qopts->binary = binary;
+  }
+
+  return 0;
+}
+
+/**
+ * Write the qemu options to a script.
+ *
+ * C<qemuopts_set_binary*> must be called first.
+ *
+ * The script file will start with C<#!/bin/sh> and will be chmod to
+ * mode C<0755>.
+ *
+ * Returns C<0> on success.  Returns C<-1> on error, setting
C<errno>.
+ */
+int
+qemuopts_to_script (struct qemuopts *qopts, const char *filename)
+{
+  FILE *fp;
+  int saved_errno;
+
+  fp = fopen (filename, "w");
+  if (fp == NULL)
+    return -1;
+
+  fprintf (fp, "#!/bin/sh -\n\n");
+  if (qemuopts_to_channel (qopts, fp) == -1) {
+  error:
+    saved_errno = errno;
+    fclose (fp);
+    unlink (filename);
+    errno = saved_errno;
+    return -1;
+  }
+
+  if (fchmod (fileno (fp), 0755) == -1)
+    goto error;
+
+  if (fclose (fp) == EOF) {
+    saved_errno = errno;
+    unlink (filename);
+    errno = saved_errno;
+    return -1;
+  }
+
+  return 0;
+}
+
+/**
+ * Print C<str> to C<fp>, shell-quoting it if necessary.
+ */
+static void
+shell_quote (const char *str, FILE *fp)
+{
+  const char *safe_chars +   
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789.-_=,:/";
+  size_t i, len;
+
+  /* If the string consists only of safe characters, output it as-is. */
+  len = strlen (str);
+  if (len == strspn (str, safe_chars)) {
+    fputs (str, fp);
+    return;
+  }
+
+  /* Double-quote the string. */
+  fputc ('"', fp);
+  for (i = 0; i < len; ++i) {
+    switch (str[i]) {
+    case '$': case '`': case '\\': case
'"':
+      fputc ('\\', fp);
+      /*FALLTHROUGH*/
+    default:
+      fputc (str[i], fp);
+    }
+  }
+  fputc ('"', fp);
+}
+
+/**
+ * Print C<str> to C<fp> doing both shell and qemu comma quoting.
+ */
+static void
+shell_and_comma_quote (const char *str, FILE *fp)
+{
+  const char *safe_chars +   
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789.-_=:/";
+  size_t i, len;
+
+  /* If the string consists only of safe characters, output it as-is. */
+  len = strlen (str);
+  if (len == strspn (str, safe_chars)) {
+    fputs (str, fp);
+    return;
+  }
+
+  fputc ('"', fp);
+  for (i = 0; i < len; ++i) {
+    switch (str[i]) {
+    case ',':
+      /* qemu comma-quoting doubles commas. */
+      fputs (",,", fp);
+      break;
+    case '$': case '`': case '\\': case
'"':
+      fputc ('\\', fp);
+      /*FALLTHROUGH*/
+    default:
+      fputc (str[i], fp);
+    }
+  }
+  fputc ('"', fp);
+}
+
+/**
+ * Write the qemu options to a C<FILE *fp>.
+ *
+ * C<qemuopts_set_binary*> must be called first.
+ *
+ * Only the qemu command line is written.  The caller may need to add
+ * C<#!/bin/sh> and may need to chmod the resulting file to
C<0755>.
+ *
+ * Returns C<0> on success.  Returns C<-1> on error, setting
C<errno>.
+ */
+int
+qemuopts_to_channel (struct qemuopts *qopts, FILE *fp)
+{
+  size_t i, j;
+  const char *nl = " \\\n    ";
+
+  if (qopts->binary == NULL) {
+    errno = ENOENT;
+    return -1;
+  }
+
+  shell_quote (qopts->binary, fp);
+  for (i = 0; i < qopts->nr_options; ++i) {
+    switch (qopts->options[i].type) {
+    case QOPT_FLAG:
+      fprintf (fp, "%s%s", nl, qopts->options[i].flag);
+      break;
+
+    case QOPT_ARG_NOQUOTE:
+      fprintf (fp, "%s%s %s",
+               nl, qopts->options[i].flag, qopts->options[i].value);
+      break;
+
+    case QOPT_ARG:
+      fprintf (fp, "%s%s ",
+               nl, qopts->options[i].flag);
+      shell_and_comma_quote (qopts->options[i].value, fp);
+      break;
+
+    case QOPT_ARG_LIST:
+      fprintf (fp, "%s%s ",
+               nl, qopts->options[i].flag);
+      for (j = 0; qopts->options[i].values[j] != NULL; ++j) {
+        if (j > 0) fputc (',', fp);
+        shell_and_comma_quote (qopts->options[i].values[j], fp);
+      }
+      break;
+    }
+  }
+  fputc ('\n', fp);
+
+  return 0;
+}
+
+/**
+ * Return a NULL-terminated argument list, of the kind that can be
+ * passed directly to L<execv(3)>.
+ *
+ * C<qemuopts_set_binary*> must be called first.  It will be
+ * returned as C<argv[0]> in the returned list.
+ *
+ * The list of strings and the strings themselves must be freed by the
+ * caller.
+ *
+ * Returns C<NULL> on error, setting C<errno>.
+ */
+char **
+qemuopts_to_argv (struct qemuopts *qopts)
+{
+  char **ret, **values;
+  size_t n, i, j, k, len;
+
+  if (qopts->binary == NULL) {
+    errno = ENOENT;
+    return NULL;
+  }
+
+  /* Count how many arguments we will return.  It's not the same as
+   * the number of options because some options are flags (returning a
+   * single string) and others have a parameter (two strings).
+   */
+  n = 1; /* for the qemu binary */
+  for (i = 0; i < qopts->nr_options; ++i) {
+    switch (qopts->options[i].type) {
+    case QOPT_FLAG:
+      n++;
+      break;
+
+    case QOPT_ARG_NOQUOTE:
+    case QOPT_ARG:
+    case QOPT_ARG_LIST:
+      n += 2;
+    }
+  }
+
+  ret = calloc (n+1, sizeof (char *));
+  if (ret == NULL)
+    return NULL;
+
+  n = 0;
+  ret[n] = strdup (qopts->binary);
+  if (ret[n] == NULL) {
+  error:
+    for (i = 0; i < n; ++i)
+      free (ret[i]);
+    free (ret);
+    return NULL;
+  }
+  n++;
+
+  for (i = 0; i < qopts->nr_options; ++i) {
+    ret[n] = strdup (qopts->options[i].flag);
+    if (ret[n] == NULL) goto error;
+    n++;
+
+    switch (qopts->options[i].type) {
+    case QOPT_FLAG:
+      /* nothing */
+      break;
+
+    case QOPT_ARG_NOQUOTE:
+      ret[n] = strdup (qopts->options[i].value);
+      if (ret[n] == NULL) goto error;
+      n++;
+      break;
+
+    case QOPT_ARG:
+      /* We only have to do comma-quoting here. */
+      len = 0;
+      for (k = 0; k < strlen (qopts->options[i].value); ++k) {
+        if (qopts->options[i].value[k] == ',') len++;
+        len++;
+      }
+      ret[n] = malloc (len+1);
+      if (ret[n] == NULL) goto error;
+      len = 0;
+      for (k = 0; k < strlen (qopts->options[i].value); ++k) {
+        if (qopts->options[i].value[k] == ',') ret[n][len++] =
',';
+        ret[n][len++] = qopts->options[i].value[k];
+      }
+      ret[n][len] = '\0';
+      n++;
+      break;
+
+    case QOPT_ARG_LIST:
+      /* We only have to do comma-quoting here. */
+      values = qopts->options[i].values;
+      len = count_strings (values) - 1 /* one for each comma */;
+      for (j = 0; values[j] != NULL; ++j) {
+        for (k = 0; k < strlen (values[j]); ++k) {
+          if (values[j][k] == ',') len++;
+          len++;
+        }
+      }
+      ret[n] = malloc (len+1);
+      if (ret[n] == NULL) goto error;
+      len = 0;
+      for (j = 0; values[j] != NULL; ++j) {
+        if (j > 0) ret[n][len++] = ',';
+        for (k = 0; k < strlen (values[j]); ++k) {
+          if (values[j][k] == ',') ret[n][len++] = ',';
+          ret[n][len++] = values[j][k];
+        }
+      }
+      ret[n][len] = '\0';
+      n++;
+    }
+  }
+
+  return ret;
+}
+
+/**
+ * Write the qemu options to a qemu config file, suitable for reading
+ * in using C<qemu -readconfig filename>.
+ *
+ * Note that qemu config files have limitations on content and
+ * quoting, so not all qemuopts structs can be written (this function
+ * returns an error in these cases).  For more information see
+ * L<https://habkost.net/posts/2016/12/qemu-apis-qemuopts.html>
+ * L<https://bugs.launchpad.net/qemu/+bug/1686364>
+ *
+ * Also, command line argument names and config file sections
+ * sometimes have different names.  For example the equivalent of
+ * C<-m 1024> is:
+ *
+ *  [memory]
+ *    size = "1024"
+ *
+ * This code does I<not> attempt to convert between the two forms.
+ * You just need to know how to do that yourself.
+ *
+ * Returns C<0> on success.  Returns C<-1> on error, setting
C<errno>.
+ */
+int
+qemuopts_to_config_file (struct qemuopts *qopts, const char *filename)
+{
+  FILE *fp;
+  int saved_errno;
+
+  fp = fopen (filename, "w");
+  if (fp == NULL)
+    return -1;
+
+  if (qemuopts_to_config_channel (qopts, fp) == -1) {
+    saved_errno = errno;
+    fclose (fp);
+    unlink (filename);
+    errno = saved_errno;
+    return -1;
+  }
+
+  if (fclose (fp) == EOF) {
+    saved_errno = errno;
+    unlink (filename);
+    errno = saved_errno;
+    return -1;
+  }
+
+  return 0;
+}
+
+/**
+ * Same as C<qemuopts_to_config_file>, but this writes to a C<FILE
*fp>.
+ */
+int
+qemuopts_to_config_channel (struct qemuopts *qopts, FILE *fp)
+{
+  size_t i, j, k;
+  ssize_t id_param;
+  char **values;
+
+  /* Before starting, try to detect some illegal options which
+   * cannot be translated into a qemu config file.
+   */
+  for (i = 0; i < qopts->nr_options; ++i) {
+    switch (qopts->options[i].type) {
+    case QOPT_FLAG:
+      /* Single flags cannot be written to a config file.  It seems
+       * as if the file format simply does not support this notion.
+       */
+      errno = EINVAL;
+      return -1;
+
+    case QOPT_ARG_NOQUOTE:
+      /* arg_noquote is incompatible with this function. */
+      errno = EINVAL;
+      return -1;
+
+    case QOPT_ARG:
+      /* Single arguments can be expressed, but we would have to do
+       * special translation as outlined in the description of
+       * C<qemuopts_to_config_file> above.
+       */
+      errno = EINVAL;
+      return -1;
+
+    case QOPT_ARG_LIST:
+      /* If any value contains a double quote character, then qemu
+       * cannot parse it.  See
+       * https://bugs.launchpad.net/qemu/+bug/1686364.
+       */
+      values = qopts->options[i].values;
+      for (j = 0; values[j] != NULL; ++j) {
+        if (strchr (values[j], '"') != NULL) {
+          errno = EINVAL;
+          return -1;
+        }
+      }
+      break;
+    }
+  }
+
+  /* Write the output. */
+  fprintf (fp, "# qemu config file\n\n");
+
+  for (i = 0; i < qopts->nr_options; ++i) {
+    switch (qopts->options[i].type) {
+    case QOPT_FLAG:
+    case QOPT_ARG_NOQUOTE:
+    case QOPT_ARG:
+      abort ();
+
+    case QOPT_ARG_LIST:
+      values = qopts->options[i].values;
+      /* The id=... parameter is special. */
+      id_param = -1;
+      for (j = 0; values[j] != NULL; ++j) {
+        if (strncmp (values[j], "id=", 2) == 0) {
+          id_param = j;
+          break;
+        }
+      }
+
+      if (id_param >= 0)
+        fprintf (fp, "[%s \"%s\"]\n",
+                 &qopts->options[i].flag[1],
+                 &values[id_param][3]);
+      else
+        fprintf (fp, "[%s]\n", &qopts->options[i].flag[1]);
+
+      for (j = 0; values[j] != NULL; ++j) {
+        if ((ssize_t) j != id_param) {
+          k = strcspn (values[j], "=");
+          if (k < strlen (values[j])) {
+            fprintf (fp, "  %.*s = ", (int) k, values[j]);
+            fprintf (fp, "\"%s\"\n", &values[j][k+1]);
+          }
+          else
+            fprintf (fp, "  %s = \"on\"\n", values[j]);
+        }
+      }
+    }
+    fprintf (fp, "\n");
+  }
+
+  return 0;
+}
diff --git a/common/qemuopts/qemuopts.h b/common/qemuopts/qemuopts.h
new file mode 100644
index 0000000..7a7818b
--- /dev/null
+++ b/common/qemuopts/qemuopts.h
@@ -0,0 +1,47 @@
+/* libguestfs
+ * Copyright (C) 2009-2017 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
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/* See qemuopts.c for documentation on how to use the library. */
+
+#ifndef QEMUOPTS_H_
+#define QEMUOPTS_H_
+
+#include <stdarg.h>
+
+struct qemuopts;
+
+extern struct qemuopts *qemuopts_create (void);
+extern void qemuopts_free (struct qemuopts *qopts);
+extern int qemuopts_add_flag (struct qemuopts *qopts, const char *flag);
+extern int qemuopts_add_arg (struct qemuopts *qopts, const char *flag, const
char *value);
+extern int qemuopts_add_arg_format (struct qemuopts *qopts, const char *flag,
const char *fs, ...) __attribute__((format (printf,3,4)));
+extern int qemuopts_add_arg_noquote (struct qemuopts *qopts, const char *flag,
const char *value);
+extern int qemuopts_start_arg_list (struct qemuopts *qopts, const char *flag);
+extern int qemuopts_append_arg_list (struct qemuopts *qopts, const char
*value);
+extern int qemuopts_append_arg_list_format (struct qemuopts *qopts, const char
*fs, ...) __attribute__((format (printf,2,3)));
+extern int qemuopts_end_arg_list (struct qemuopts *qopts);
+extern int qemuopts_add_arg_list (struct qemuopts *qopts, const char *flag,
const char *elem0, ...) __attribute__((sentinel));
+extern int qemuopts_set_binary (struct qemuopts *qopts, const char *binary);
+extern int qemuopts_set_binary_by_arch (struct qemuopts *qopts, const char
*arch);
+extern int qemuopts_to_script (struct qemuopts *qopts, const char *filename);
+extern int qemuopts_to_channel (struct qemuopts *qopts, FILE *fp);
+extern char **qemuopts_to_argv (struct qemuopts *qopts);
+extern int qemuopts_to_config_file (struct qemuopts *qopts, const char
*filename);
+extern int qemuopts_to_config_channel (struct qemuopts *qopts, FILE *fp);
+
+#endif /* QEMUOPTS_H_ */
diff --git a/configure.ac b/configure.ac
index d47fe48..4b466f5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -187,6 +187,7 @@ AC_CONFIG_FILES([Makefile
                  common/parallel/Makefile
                  common/progress/Makefile
                  common/protocol/Makefile
+                 common/qemuopts/Makefile
                  common/utils/Makefile
                  common/visit/Makefile
                  common/windows/Makefile
-- 
2.9.3

In its current form this is very hard to implement because it requires
us to "unparse" the options, including removing any shell quoting.

It wasn't implemented at all for the libvirt backend.

Also contrary to the documentation, the configure script did not use
these options for testing, but constructed its own set of qemu test
options.
---
 configure.ac              |  2 +-
 docs/guestfs-building.pod |  5 ----
 lib/launch-direct.c       | 66 -----------------------------------------------
 m4/guestfs_qemu.m4        | 16 ------------
 4 files changed, 1 insertion(+), 88 deletions(-)

diff --git a/configure.ac b/configure.ac
index 4b466f5..b6ba6de 100644
--- a/configure.ac
+++ b/configure.ac
@@ -318,7 +318,7 @@ echo "This is how we have configured the optional
components for you today:"
 echo
 echo       "Daemon .............................. $enable_daemon"
 echo       "Appliance ........................... $ENABLE_APPLIANCE"
-echo       "QEMU ................................ $QEMU
$QEMU_OPTIONS"
+echo       "QEMU ................................ $QEMU"
 echo       "guestfish and C-based virt tools .... yes"
 echo       "FUSE filesystem ..................... $enable_fuse"
 AS_ECHO_N(["GNU gettext for i18n ................ "])
diff --git a/docs/guestfs-building.pod b/docs/guestfs-building.pod
index bfb46a0..b026667 100644
--- a/docs/guestfs-building.pod
+++ b/docs/guestfs-building.pod
@@ -663,11 +663,6 @@ Provide an alternate qemu binary (or list of binaries). 
This can be
 overridden at runtime by setting the C<LIBGUESTFS_HV> environment
 variable.
 
-=item B<--with-qemu-options=">-M ... -cpu ...B<">
-
-If qemu requires extra options to work on this platform, you can pass
-them here, and they will be used both when testing and running qemu.
-
 =item B<--with-supermin-packager-config=>I<yum.conf>
 
 This passes the I<--packager-config> option to L<supermin(1)>.
diff --git a/lib/launch-direct.c b/lib/launch-direct.c
index b0038c6..aa9c292 100644
--- a/lib/launch-direct.c
+++ b/lib/launch-direct.c
@@ -97,64 +97,6 @@ create_cow_overlay_direct (guestfs_h *g, void *datav, struct
drive *drv)
   return overlay;
 }
 
-#ifdef QEMU_OPTIONS
-/* Like 'add_cmdline' but allowing a shell-quoted string of zero or
- * more options.  XXX The unquoting is not very clever.
- */
-static void
-add_cmdline_shell_unquoted (guestfs_h *g, struct stringsbuf *sb,
-                            const char *options)
-{
-  char quote;
-  const char *startp, *endp, *nextp;
-
-  while (*options) {
-    quote = *options;
-    if (quote == '\'' || quote == '"')
-      startp = options+1;
-    else {
-      startp = options;
-      quote = ' ';
-    }
-
-    endp = strchr (options, quote);
-    if (endp == NULL) {
-      if (quote != ' ') {
-        fprintf (stderr,
-                 _("unclosed quote character (%c) in command line near:
%s"),
-                 quote, options);
-        _exit (EXIT_FAILURE);
-      }
-      endp = options + strlen (options);
-    }
-
-    if (quote == ' ') {
-      if (endp[0] == '\0')
-        nextp = endp;
-      else
-        nextp = endp+1;
-    }
-    else {
-      if (!endp[1])
-        nextp = endp+1;
-      else if (endp[1] == ' ')
-        nextp = endp+2;
-      else {
-        fprintf (stderr, _("cannot parse quoted string near: %s"),
options);
-        _exit (EXIT_FAILURE);
-      }
-    }
-    while (*nextp && *nextp == ' ')
-      nextp++;
-
-    guestfs_int_add_string_nodup (g, sb,
-				  safe_strndup (g, startp, endp-startp));
-
-    options = nextp;
-  }
-}
-#endif /* defined QEMU_OPTIONS */
-
 /* On Debian, /dev/kvm is mode 0660 and group kvm, so users need to
  * add themselves to the kvm group otherwise things are going to be
  * very slow (this is Debian bug 640328).  Warn about this.
@@ -604,14 +546,6 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
    * qemu -set parameters can modify previously added options.
    */
 
-  /* Add the extra options for the qemu command line specified
-   * at configure time.
-   */
-#ifdef QEMU_OPTIONS
-  if (STRNEQ (QEMU_OPTIONS, ""))
-    add_cmdline_shell_unquoted (g, &cmdline, QEMU_OPTIONS);
-#endif
-
   /* Add any qemu parameters. */
   for (hp = g->hv_params; hp; hp = hp->next) {
     ADD_CMDLINE (hp->hv_param);
diff --git a/m4/guestfs_qemu.m4 b/m4/guestfs_qemu.m4
index fb02854..e07e980 100644
--- a/m4/guestfs_qemu.m4
+++ b/m4/guestfs_qemu.m4
@@ -43,22 +43,6 @@ AS_IF([test "x$with_qemu" = "xno"],[
 
     AC_DEFINE_UNQUOTED([QEMU],["$QEMU"],[Location of qemu binary.])
 
-    dnl Does the user wish to specify -M, -cpu or other qemu options?
-    AC_MSG_CHECKING([if the user specified extra options for qemu command
line])
-    AC_ARG_WITH([qemu-options],
-        [AS_HELP_STRING([--with-qemu-options="-M ... -cpu ... etc"],
-            [pass extra options for qemu command line
@<:@default=no@:>@])],
-        [QEMU_OPTIONS="$withval"],
-        [QEMU_OPTIONS=no])
-    AS_IF([test "x$QEMU_OPTIONS" = "xno"],[
-        AC_MSG_RESULT([no])
-        QEMU_OPTIONS-    ],[
-        AC_MSG_RESULT([$QEMU_OPTIONS])
-    ])
-    AC_DEFINE_UNQUOTED([QEMU_OPTIONS],["$QEMU_OPTIONS"],
-                       [Extra options for qemu command line.])
-
     dnl Check that the chosen qemu has virtio-serial support.
     dnl For historical reasons this can be disabled by setting
     dnl vmchannel_test=no.
-- 
2.9.3

---
 lib/Makefile.am     |   3 +
 lib/launch-direct.c | 498 ++++++++++++++++++++++++++++++----------------------
 lib/qemu.c          |   4 +
 3 files changed, 292 insertions(+), 213 deletions(-)

diff --git a/lib/Makefile.am b/lib/Makefile.am
index 063706f..b22fbd1 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -56,6 +56,7 @@ lib_LTLIBRARIES = libguestfs.la
 libguestfs_la_SOURCES = \
 	../common/errnostring/errnostring.h \
 	../common/protocol/guestfs_protocol.h \
+	../common/qemuopts/qemuopts.h \
 	../common/utils/guestfs-internal-frontend.h \
 	../common/utils/guestfs-internal-frontend-cleanups.h \
 	guestfs.h \
@@ -136,6 +137,7 @@ libguestfs_la_CPPFLAGS = \
 	-DLIBOSINFO_DB_PATH='"$(datadir)/libosinfo/db"' \
 	-I$(top_srcdir)/common/errnostring -I$(top_builddir)/common/errnostring \
 	-I$(top_srcdir)/common/protocol -I$(top_builddir)/common/protocol \
+	-I$(top_srcdir)/common/qemuopts -I$(top_builddir)/common/qemuopts \
 	-I$(top_srcdir)/common/utils -I$(top_builddir)/common/utils \
 	-I$(top_srcdir)/gnulib/lib -I$(top_builddir)/gnulib/lib
 
@@ -151,6 +153,7 @@ libguestfs_la_CFLAGS = \
 libguestfs_la_LIBADD = \
 	../common/errnostring/liberrnostring.la \
 	../common/protocol/libprotocol.la \
+	../common/qemuopts/libqemuopts.la \
 	../common/utils/libutils.la \
 	$(PCRE_LIBS) $(MAGIC_LIBS) \
 	$(LIBVIRT_LIBS) $(LIBXML2_LIBS) \
diff --git a/lib/launch-direct.c b/lib/launch-direct.c
index aa9c292..4d5d6b9 100644
--- a/lib/launch-direct.c
+++ b/lib/launch-direct.c
@@ -47,6 +47,7 @@
 #include "guestfs.h"
 #include "guestfs-internal.h"
 #include "guestfs_protocol.h"
+#include "qemuopts.h"
 
 /* Per-handle data. */
 struct backend_direct_data {
@@ -61,7 +62,6 @@ struct backend_direct_data {
 
 static int is_openable (guestfs_h *g, const char *path, int flags);
 static char *make_appliance_dev (guestfs_h *g);
-static void print_qemu_command_line (guestfs_h *g, char **argv);
 
 static char *
 create_cow_overlay_direct (guestfs_h *g, void *datav, struct drive *drv)
@@ -157,11 +157,181 @@ debian_kvm_warning (guestfs_h *g)
 #endif /* __linux__ */
 }
 
+/* Some macros which make using qemuopts a bit easier. */
+#define flag(flag)                                                      \
+  do {                                                                  \
+    if (qemuopts_add_flag (qopts, (flag)) == -1) goto qemuopts_error;   \
+  } while (0)
+#define arg(flag, value)                                                \
+  do {                                                                  \
+    if (qemuopts_add_arg (qopts, (flag), (value)) == -1) goto qemuopts_error; \
+  } while (0)
+#define arg_format(flag, fs, ...)                                       \
+  do {                                                                  \
+    if (qemuopts_add_arg_format (qopts, (flag), (fs), ##__VA_ARGS__) == -1) \
+      goto qemuopts_error;                                              \
+  } while (0)
+#define arg_noquote(flag, value)                                        \
+  do {                                                                  \
+    if (qemuopts_add_arg_noquote (qopts, (flag), (value)) == -1)        \
+      goto qemuopts_error;                                              \
+  } while (0)
+#define start_list(flag)                                                \
+  if (qemuopts_start_arg_list (qopts, (flag)) == -1) goto qemuopts_error; \
+  do
+#define append_list(value)                                       \
+  do {                                                           \
+    if (qemuopts_append_arg_list (qopts, (value)) == -1)         \
+      goto qemuopts_error;                                       \
+  } while (0)
+#define append_list_format(fs, ...)                                     \
+  do {                                                                  \
+    if (qemuopts_append_arg_list_format (qopts, (fs), ##__VA_ARGS__) == -1) \
+      goto qemuopts_error;                                              \
+  } while (0)
+#define end_list()                                                      \
+  while (0);                                                            \
+  do {                                                                  \
+    if (qemuopts_end_arg_list (qopts) == -1) goto qemuopts_error;       \
+  } while (0)
+
+/**
+ * Add the standard elements of the C<-drive> parameter.
+ */
+static int
+add_drive_standard_params (guestfs_h *g, struct backend_direct_data *data,
+                           struct qemuopts *qopts,
+                           size_t i, struct drive *drv)
+{
+  if (!drv->overlay) {
+    CLEANUP_FREE char *file = NULL;
+
+    /* file= parameter. */
+    file = guestfs_int_drive_source_qemu_param (g, &drv->src);
+    append_list_format ("file=%s", file);
+
+    if (drv->readonly)
+      append_list ("snapshot=on");
+    append_list_format ("cache=%s",
+                        drv->cachemode ? drv->cachemode :
"writeback");
+    if (drv->src.format)
+      append_list_format ("format=%s", drv->src.format);
+    if (drv->disk_label)
+      append_list_format ("serial=%s", drv->disk_label);
+    if (drv->copyonread)
+      append_list ("copy-on-read=on");
+
+    /* Discard mode. */
+    switch (drv->discard) {
+    case discard_disable:
+      /* Since the default is always discard=ignore, don't specify it
+       * on the command line.  This also avoids unnecessary breakage
+       * with qemu < 1.5 which didn't have the option at all.
+       */
+      break;
+    case discard_enable:
+      if (!guestfs_int_discard_possible (g, drv, &data->qemu_version))
+        return -1;
+      /*FALLTHROUGH*/
+    case discard_besteffort:
+      /* I believe from reading the code that this is always safe as
+       * long as qemu >= 1.5.
+       */
+      if (guestfs_int_version_ge (&data->qemu_version, 1, 5, 0))
+        append_list ("discard=unmap");
+      break;
+    }
+  }
+  else {
+    /* Writable qcow2 overlay on top of read-only drive. */
+    append_list_format ("file=%s", drv->overlay);
+    append_list ("cache=unsafe");
+    append_list ("format=qcow2");
+    if (drv->disk_label)
+      append_list_format ("serial=%s", drv->disk_label);
+  }
+
+  append_list_format ("id=hd%zu", i);
+
+  return 0;
+
+  /* This label is called implicitly from the qemuopts macros on error. */
+ qemuopts_error:
+  perrorf (g, "qemuopts");
+  return -1;
+}
+
+static int
+add_drive (guestfs_h *g, struct backend_direct_data *data,
+           struct qemuopts *qopts, size_t i, struct drive *drv)
+{
+  /* If there's an explicit 'iface', use it.  Otherwise default to
+   * virtio-scsi.
+   */
+  if (drv->iface && STREQ (drv->iface, "virtio")) { /*
virtio-blk */
+    start_list ("-drive") {
+      if (add_drive_standard_params (g, data, qopts, i, drv) == -1)
+        return -1;
+      append_list ("if=none");
+    } end_list ();
+    start_list ("-device") {
+      append_list (VIRTIO_BLK);
+      append_list_format ("drive=hd%zu", i);
+    } end_list ();
+  }
+#if defined(__arm__) || defined(__aarch64__) || defined(__powerpc__)
+  else if (drv->iface && STREQ (drv->iface, "ide")) {
+    error (g, "'ide' interface does not work on ARM or
PowerPC");
+    return -1;
+  }
+#endif
+  else if (drv->iface) {
+    start_list ("-drive") {
+      if (add_drive_standard_params (g, data, qopts, i, drv) == -1)
+        return -1;
+      append_list_format ("if=%s", drv->iface);
+    } end_list ();
+  }
+  else /* default case: virtio-scsi */ {
+    start_list ("-drive") {
+      if (add_drive_standard_params (g, data, qopts, i, drv) == -1)
+        return -1;
+      append_list ("if=none");
+    } end_list ();
+    start_list ("-device") {
+      append_list ("scsi-hd");
+      append_list_format ("drive=hd%zu", i);
+    } end_list ();
+  }
+
+  return 0;
+
+  /* This label is called implicitly from the qemuopts macros on error. */
+ qemuopts_error:
+  perrorf (g, "qemuopts");
+  return -1;
+}
+
+static int
+add_drives (guestfs_h *g, struct backend_direct_data *data,
+            struct qemuopts *qopts)
+{
+  size_t i;
+  struct drive *drv;
+
+  ITER_DRIVES (g, i, drv) {
+    if (add_drive (g, data, qopts, i, drv) == -1)
+      return -1;
+  }
+
+  return 0;
+}
+
 static int
 launch_direct (guestfs_h *g, void *datav, const char *arg)
 {
   struct backend_direct_data *data = datav;
-  CLEANUP_FREE_STRINGSBUF DECLARE_STRINGSBUF (cmdline);
+  struct qemuopts *qopts = NULL;
   int daemon_accept_sock = -1, console_sock = -1;
   int r;
   int flags;
@@ -174,12 +344,12 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
   CLEANUP_FREE char *appliance_dev = NULL;
   uint32_t size;
   CLEANUP_FREE void *buf = NULL;
-  struct drive *drv;
-  size_t i;
   struct hv_param *hp;
   bool has_kvm;
   int force_tcg;
   const char *cpu_model;
+  CLEANUP_FREE char *append = NULL;
+  CLEANUP_FREE_STRING_LIST char **argv = NULL;
 
   /* At present you must add drives before starting the appliance.  In
    * future when we enable hotplugging you won't need to do this.
@@ -267,30 +437,28 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
    * forking, because after fork we are not allowed to use
    * non-signal-safe functions such as malloc.
    */
-#define ADD_CMDLINE(str)			\
-  guestfs_int_add_string (g, &cmdline, (str))
-#define ADD_CMDLINE_STRING_NODUP(str)			\
-  guestfs_int_add_string_nodup (g, &cmdline, (str))
-#define ADD_CMDLINE_PRINTF(fs,...)				\
-  guestfs_int_add_sprintf (g, &cmdline, (fs), ##__VA_ARGS__)
-
-  ADD_CMDLINE (g->hv);
+  qopts = qemuopts_create ();
+  if (qopts == NULL) {
+  qemuopts_error:
+    perrorf (g, "qemuopts");
+    goto cleanup0;
+  }
+  if (qemuopts_set_binary (qopts, g->hv) == -1) goto qemuopts_error;
 
   /* CVE-2011-4127 mitigation: Disable SCSI ioctls on virtio-blk
    * devices.
    */
-  ADD_CMDLINE ("-global");
-  ADD_CMDLINE (VIRTIO_BLK ".scsi=off");
+  arg ("-global", VIRTIO_BLK ".scsi=off");
 
   if (guestfs_int_qemu_supports (g, data->qemu_data,
"-nodefconfig"))
-    ADD_CMDLINE ("-nodefconfig");
+    flag ("-nodefconfig");
 
   /* This oddly named option doesn't actually enable FIPS.  It just
    * causes qemu to do the right thing if FIPS is enabled in the
    * kernel.  So like libvirt, we pass it unconditionally.
    */
   if (guestfs_int_qemu_supports (g, data->qemu_data,
"-enable-fips"))
-    ADD_CMDLINE ("-enable-fips");
+    flag ("-enable-fips");
 
   /* Newer versions of qemu (from around 2009/12) changed the
    * behaviour of monitors so that an implicit '-monitor stdio' is
@@ -301,60 +469,47 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
    * let's use that to avoid this and any future surprises.
    */
   if (guestfs_int_qemu_supports (g, data->qemu_data,
"-nodefaults"))
-    ADD_CMDLINE ("-nodefaults");
+    flag ("-nodefaults");
 
   /* This disables the host-side display (SDL, Gtk). */
-  ADD_CMDLINE ("-display");
-  ADD_CMDLINE ("none");
+  arg ("-display", "none");
 
   /* See guestfs.pod / gdb */
   if (guestfs_int_get_backend_setting_bool (g, "gdb") > 0) {
-    ADD_CMDLINE ("-S");
-    ADD_CMDLINE ("-s");
+    flag ("-S");
+    flag ("-s");
     warning (g, "qemu debugging is enabled, connect gdb to tcp::1234 to
begin");
   }
 
-  ADD_CMDLINE ("-machine");
-  ADD_CMDLINE_PRINTF (
+  start_list ("-machine") {
 #ifdef MACHINE_TYPE
-                      MACHINE_TYPE ","
+    append_list (MACHINE_TYPE);
 #endif
 #ifdef __aarch64__
-                      "%s"      /* gic-version */
+    if (has_kvm && !force_tcg)
+      append_list ("gic-version=host");
 #endif
-                      "accel=%s",
-#ifdef __aarch64__
-                      has_kvm && !force_tcg ?
"gic-version=host," : "",
-#endif
-                      !force_tcg ? "kvm:tcg" : "tcg");
+    append_list_format ("accel=%s", !force_tcg ? "kvm:tcg"
: "tcg");
+  } end_list ();
 
   cpu_model = guestfs_int_get_cpu_model (has_kvm && !force_tcg);
-  if (cpu_model) {
-    ADD_CMDLINE ("-cpu");
-    ADD_CMDLINE (cpu_model);
-  }
+  if (cpu_model)
+    arg ("-cpu", cpu_model);
 
-  if (g->smp > 1) {
-    ADD_CMDLINE ("-smp");
-    ADD_CMDLINE_PRINTF ("%d", g->smp);
-  }
+  if (g->smp > 1)
+    arg_format ("-smp", "%d", g->smp);
 
-  ADD_CMDLINE ("-m");
-  ADD_CMDLINE_PRINTF ("%d", g->memsize);
+  arg_format ("-m", "%d", g->memsize);
 
   /* Force exit instead of reboot on panic */
-  ADD_CMDLINE ("-no-reboot");
+  flag ("-no-reboot");
 
   /* These are recommended settings, see RHBZ#1053847. */
-  ADD_CMDLINE ("-rtc");
-  ADD_CMDLINE ("driftfix=slew");
-  if (guestfs_int_qemu_supports (g, data->qemu_data, "-no-hpet"))
{
-    ADD_CMDLINE ("-no-hpet");
-  }
-  if (guestfs_int_version_ge (&data->qemu_version, 1, 3, 0)) {
-    ADD_CMDLINE ("-global");
-    ADD_CMDLINE ("kvm-pit.lost_tick_policy=discard");
-  }
+  arg ("-rtc", "driftfix=slew");
+  if (guestfs_int_qemu_supports (g, data->qemu_data, "-no-hpet"))
+    flag ("-no-hpet");
+  if (guestfs_int_version_ge (&data->qemu_version, 1, 3, 0))
+    arg ("-global", "kvm-pit.lost_tick_policy=discard");
 
   /* UEFI (firmware) if required. */
   if (guestfs_int_get_uefi (g, &uefi_code, &uefi_vars, &uefi_flags)
== -1)
@@ -372,19 +527,24 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
     goto cleanup0;
   }
   if (uefi_code) {
-    ADD_CMDLINE ("-drive");
-    ADD_CMDLINE_PRINTF ("if=pflash,format=raw,file=%s,readonly",
uefi_code);
+    start_list ("-drive") {
+      append_list ("if=pflash");
+      append_list ("format=raw");
+      append_list_format ("file=%s", uefi_code);
+      append_list ("readonly");
+    } end_list ();
     if (uefi_vars) {
-      ADD_CMDLINE ("-drive");
-      ADD_CMDLINE_PRINTF ("if=pflash,format=raw,file=%s", uefi_vars);
+      start_list ("-drive") {
+        append_list ("if=pflash");
+        append_list ("format=raw");
+        append_list_format ("file=%s", uefi_vars);
+      } end_list ();
     }
   }
 
   /* Kernel and initrd. */
-  ADD_CMDLINE ("-kernel");
-  ADD_CMDLINE (kernel);
-  ADD_CMDLINE ("-initrd");
-  ADD_CMDLINE (initrd);
+  arg ("-kernel", kernel);
+  arg ("-initrd", initrd);
 
   /* Add a random number generator (backend for virtio-rng).  This
    * isn't strictly necessary but means we won't need to hang around
@@ -392,121 +552,50 @@ launch_direct (guestfs_h *g, void *datav, const char
*arg)
    */
   if (guestfs_int_qemu_supports_device (g, data->qemu_data,
                                         "virtio-rng-pci")) {
-    ADD_CMDLINE ("-object");
-    ADD_CMDLINE ("rng-random,filename=/dev/urandom,id=rng0");
-    ADD_CMDLINE ("-device");
-    ADD_CMDLINE ("virtio-rng-pci,rng=rng0");
+    start_list ("-object") {
+      append_list ("rng-random");
+      append_list ("filename=/dev/urandom");
+      append_list ("id=rng0");
+    } end_list ();
+    start_list ("-device") {
+      append_list ("virtio-rng-pci");
+      append_list ("rng=rng0");
+    } end_list ();
   }
 
   /* Create the virtio-scsi bus. */
-  ADD_CMDLINE ("-device");
-  ADD_CMDLINE (VIRTIO_SCSI ",id=scsi");
-
-  /* Add drives */
-  ITER_DRIVES (g, i, drv) {
-    CLEANUP_FREE char *file = NULL, *escaped_file = NULL, *param = NULL;
-
-    if (!drv->overlay) {
-      const char *discard_mode = "";
-
-      switch (drv->discard) {
-      case discard_disable:
-        /* Since the default is always discard=ignore, don't specify it
-         * on the command line.  This also avoids unnecessary breakage
-         * with qemu < 1.5 which didn't have the option at all.
-         */
-        break;
-      case discard_enable:
-        if (!guestfs_int_discard_possible (g, drv, &data->qemu_version))
-          goto cleanup0;
-        /*FALLTHROUGH*/
-      case discard_besteffort:
-        /* I believe from reading the code that this is always safe as
-         * long as qemu >= 1.5.
-         */
-        if (guestfs_int_version_ge (&data->qemu_version, 1, 5, 0))
-          discard_mode = ",discard=unmap";
-        break;
-      }
-
-      /* Make the file= parameter. */
-      file = guestfs_int_drive_source_qemu_param (g, &drv->src);
-      escaped_file = guestfs_int_qemu_escape_param (g, file);
-
-      /* Make the first part of the -drive parameter, everything up to
-       * the if=... at the end.
-       */
-      param = safe_asprintf
-        (g, "file=%s%s,cache=%s%s%s%s%s%s%s,id=hd%zu",
-         escaped_file,
-         drv->readonly ? ",snapshot=on" : "",
-         drv->cachemode ? drv->cachemode : "writeback",
-         discard_mode,
-         drv->src.format ? ",format=" : "",
-         drv->src.format ? drv->src.format : "",
-         drv->disk_label ? ",serial=" : "",
-         drv->disk_label ? drv->disk_label : "",
-         drv->copyonread ? ",copy-on-read=on" : "",
-         i);
-    }
-    else {
-      /* Writable qcow2 overlay on top of read-only drive. */
-      escaped_file = guestfs_int_qemu_escape_param (g, drv->overlay);
-      param = safe_asprintf
-        (g, "file=%s,cache=unsafe,format=qcow2%s%s,id=hd%zu",
-         escaped_file,
-         drv->disk_label ? ",serial=" : "",
-         drv->disk_label ? drv->disk_label : "",
-         i);
-    }
-
-    /* If there's an explicit 'iface', use it.  Otherwise default
to
-     * virtio-scsi.
-     */
-    if (drv->iface && STREQ (drv->iface, "virtio")) {
/* virtio-blk */
-      ADD_CMDLINE ("-drive");
-      ADD_CMDLINE_PRINTF ("%s,if=none" /* sic */, param);
-      ADD_CMDLINE ("-device");
-      ADD_CMDLINE_PRINTF (VIRTIO_BLK ",drive=hd%zu", i);
-    }
-#if defined(__arm__) || defined(__aarch64__) || defined(__powerpc__)
-    else if (drv->iface && STREQ (drv->iface, "ide")) {
-      error (g, "'ide' interface does not work on ARM or
PowerPC");
-      goto cleanup0;
-    }
-#endif
-    else if (drv->iface) {
-      ADD_CMDLINE ("-drive");
-      ADD_CMDLINE_PRINTF ("%s,if=%s", param, drv->iface);
-    }
-    else /* virtio-scsi */ {
-      ADD_CMDLINE ("-drive");
-      ADD_CMDLINE_PRINTF ("%s,if=none" /* sic */, param);
-      ADD_CMDLINE ("-device");
-      ADD_CMDLINE_PRINTF ("scsi-hd,drive=hd%zu", i);
-    }
-  }
+  start_list ("-device") {
+    append_list (VIRTIO_SCSI);
+    append_list ("id=scsi");
+  } end_list ();
+
+  /* Add drives (except for the appliance drive). */
+  if (add_drives (g, data, qopts) == -1)
+    goto cleanup0;
 
   /* Add the ext2 appliance drive (after all the drives). */
   if (has_appliance_drive) {
-    ADD_CMDLINE ("-drive");
-    ADD_CMDLINE_PRINTF ("file=%s,snapshot=on,id=appliance,"
-                        "cache=unsafe,if=none,format=raw",
-                        appliance);
-
-    ADD_CMDLINE ("-device");
-    ADD_CMDLINE ("scsi-hd,drive=appliance");
+    start_list ("-drive") {
+      append_list_format ("file=%s", appliance);
+      append_list ("snapshot=on");
+      append_list ("id=appliance");
+      append_list ("cache=unsafe");
+      append_list ("if=none");
+      append_list ("format=raw");
+    } end_list ();
+    start_list ("-device") {
+      append_list ("scsi-hd");
+      append_list ("drive=appliance");
+    } end_list ();
 
     appliance_dev = make_appliance_dev (g);
   }
 
   /* Create the virtio serial bus. */
-  ADD_CMDLINE ("-device");
-  ADD_CMDLINE (VIRTIO_SERIAL);
+  arg ("-device", VIRTIO_SERIAL);
 
   /* Create the serial console. */
-  ADD_CMDLINE ("-serial");
-  ADD_CMDLINE ("stdio");
+  arg ("-serial", "stdio");
 
   if (g->verbose &&
       guestfs_int_qemu_supports_device (g, data->qemu_data,
@@ -517,30 +606,39 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
      * https://bugs.launchpad.net/qemu/+bug/1021649
      * QEmu has included sgabios upstream since just before 1.0.
      */
-    ADD_CMDLINE ("-device");
-    ADD_CMDLINE ("sga");
+    arg ("-device", "sga");
   }
 
   /* Set up virtio-serial for the communications channel. */
-  ADD_CMDLINE ("-chardev");
-  ADD_CMDLINE_PRINTF ("socket,path=%s,id=channel0",
data->guestfsd_sock);
-  ADD_CMDLINE ("-device");
-  ADD_CMDLINE
("virtserialport,chardev=channel0,name=org.libguestfs.channel.0");
+  start_list ("-chardev") {
+    append_list ("socket");
+    append_list_format ("path=%s", data->guestfsd_sock);
+    append_list ("id=channel0");
+  } end_list ();
+  start_list ("-device") {
+    append_list ("virtserialport");
+    append_list ("chardev=channel0");
+    append_list ("name=org.libguestfs.channel.0");
+  } end_list ();
 
   /* Enable user networking. */
   if (g->enable_network) {
-    ADD_CMDLINE ("-netdev");
-    ADD_CMDLINE ("user,id=usernet,net=169.254.0.0/16");
-    ADD_CMDLINE ("-device");
-    ADD_CMDLINE (VIRTIO_NET ",netdev=usernet");
+    start_list ("-netdev") {
+      append_list ("user");
+      append_list ("id=usernet");
+      append_list ("net=169.254.0.0/16");
+    } end_list ();
+    start_list ("-device") {
+      append_list (VIRTIO_NET);
+      append_list ("netdev=usernet");
+    } end_list ();
   }
 
-  ADD_CMDLINE ("-append");
   flags = 0;
   if (!has_kvm || force_tcg)
     flags |= APPLIANCE_COMMAND_LINE_IS_TCG;
-  ADD_CMDLINE_STRING_NODUP
-    (guestfs_int_appliance_command_line (g, appliance_dev, flags));
+  append = guestfs_int_appliance_command_line (g, appliance_dev, flags);
+  arg ("-append", append);
 
   /* Note: custom command line parameters must come last so that
    * qemu -set parameters can modify previously added options.
@@ -548,13 +646,14 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
 
   /* Add any qemu parameters. */
   for (hp = g->hv_params; hp; hp = hp->next) {
-    ADD_CMDLINE (hp->hv_param);
-    if (hp->hv_value)
-      ADD_CMDLINE (hp->hv_value);
+    if (!hp->hv_value)
+      flag (hp->hv_param);
+    else
+      arg_noquote (hp->hv_param, hp->hv_value);
   }
 
-  /* Finish off the command line. */
-  guestfs_int_end_stringsbuf (g, &cmdline);
+  /* Get the argv list from the command line. */
+  argv = qemuopts_to_argv (qopts);
 
   r = fork ();
   if (r == -1) {
@@ -609,7 +708,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
 
     /* Dump the command line (after setting up stderr above). */
     if (g->verbose)
-      print_qemu_command_line (g, cmdline.argv);
+      qemuopts_to_channel (qopts, stderr);
 
     /* Put qemu in a new process group. */
     if (g->pgroup)
@@ -620,7 +719,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
 
     TRACE0 (launch_run_qemu);
 
-    execv (g->hv, cmdline.argv); /* Run qemu. */
+    execv (g->hv, argv);        /* Run qemu. */
     perror (g->hv);
     _exit (EXIT_FAILURE);
   }
@@ -628,6 +727,9 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
   /* Parent (library). */
   data->pid = r;
 
+  qemuopts_free (qopts);
+  qopts = NULL;
+
   /* Fork the recovery process off which will kill qemu if the parent
    * process fails to do so (eg. if the parent segfaults).
    */
@@ -635,6 +737,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
   if (g->recovery_proc) {
     r = fork ();
     if (r == 0) {
+      size_t i;
       struct sigaction sa;
       pid_t qemu_pid = data->pid;
       pid_t parent_pid = getppid ();
@@ -774,6 +877,8 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
   data->qemu_data = NULL;
 
  cleanup0:
+  if (qopts != NULL)
+    qemuopts_free (qopts);
   if (daemon_accept_sock >= 0)
     close (daemon_accept_sock);
   if (console_sock >= 0)
@@ -813,39 +918,6 @@ make_appliance_dev (guestfs_h *g)
   return safe_strdup (g, dev);  /* Caller frees. */
 }
 
-/* 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] ",
-           guestfs_int_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++;
-  }
-
-  fputc ('\n', stderr);
-}
-
 /* Check if a file can be opened. */
 static int
 is_openable (guestfs_h *g, const char *path, int flags)
diff --git a/lib/qemu.c b/lib/qemu.c
index 8bec6ba..41098a2 100644
--- a/lib/qemu.c
+++ b/lib/qemu.c
@@ -304,6 +304,10 @@ guestfs_int_qemu_supports_device (guestfs_h *g,
  * Escape a qemu parameter.
  *
  * Every C<,> becomes C<,,>.  The caller must free the returned
string.
+ *
+ * XXX This functionality is now only used when constructing a
+ * qemu-img command in F<lib/create.c>.  We should extend the qemuopts
+ * library to cover this use case.
  */
 char *
 guestfs_int_qemu_escape_param (guestfs_h *g, const char *param)
-- 
2.9.3

---
 v2v/Makefile.am                        |   8 +-
 v2v/output_qemu.ml                     |  88 +++++++-------
 v2v/qemu_command.ml                    | 101 ---------------
 v2v/qemuopts-c.c                       | 216 +++++++++++++++++++++++++++++++++
 v2v/qemuopts.ml                        |  35 ++++++
 v2v/{qemu_command.mli => qemuopts.mli} |  24 ++--
 6 files changed, 318 insertions(+), 154 deletions(-)
 delete mode 100644 v2v/qemu_command.ml
 create mode 100644 v2v/qemuopts-c.c
 create mode 100644 v2v/qemuopts.ml
 rename v2v/{qemu_command.mli => qemuopts.mli} (75%)

diff --git a/v2v/Makefile.am b/v2v/Makefile.am
index 8df8ca0..5b6618a 100644
--- a/v2v/Makefile.am
+++ b/v2v/Makefile.am
@@ -57,7 +57,7 @@ SOURCES_MLI = \
 	parse_ovf_from_ova.mli \
 	parse_libvirt_xml.mli \
 	parse_vmx.mli \
-	qemu_command.mli \
+	qemuopts.mli \
 	target_bus_assignment.mli \
 	types.mli \
 	uefi.mli \
@@ -85,7 +85,7 @@ SOURCES_ML = \
 	parse_vmx.ml \
 	parse_libvirt_xml.ml \
 	create_libvirt_xml.ml \
-	qemu_command.ml \
+	qemuopts.ml \
 	input_libvirtxml.ml \
 	input_libvirt_other.ml \
 	input_libvirt_vcenter_https.ml \
@@ -111,6 +111,7 @@ SOURCES_ML = \
 
 SOURCES_C = \
 	libvirt_utils-c.c \
+	qemuopts-c.c \
 	utils-c.c
 
 if HAVE_OCAML
@@ -122,6 +123,7 @@ virt_v2v_CPPFLAGS = \
 	-I. \
 	-I$(top_builddir) \
 	-I$(shell $(OCAMLC) -where) \
+	-I$(top_srcdir)/common/qemuopts \
 	-I$(top_srcdir)/common/utils \
 	-I$(top_srcdir)/lib
 virt_v2v_CFLAGS = \
@@ -140,6 +142,7 @@ XOBJECTS = $(BOBJECTS:.cmo=.cmx)
 OCAMLPACKAGES = \
 	-package str,unix \
 	-I $(top_builddir)/common/utils/.libs \
+	-I $(top_builddir)/common/qemuopts/.libs \
 	-I $(top_builddir)/lib/.libs \
 	-I $(top_builddir)/gnulib/lib/.libs \
 	-I $(top_builddir)/ocaml \
@@ -151,6 +154,7 @@ endif
 
 OCAMLCLIBS = \
 	-lutils \
+	-lqemuopts \
 	$(LIBVIRT_LIBS) \
 	$(LIBXML2_LIBS) \
 	$(LIBINTL) \
diff --git a/v2v/output_qemu.ml b/v2v/output_qemu.ml
index 7721f0b..031279c 100644
--- a/v2v/output_qemu.ml
+++ b/v2v/output_qemu.ml
@@ -69,29 +69,31 @@ object
     let machine_q35 = secure_boot_required in
     let smm = secure_boot_required in
 
-    (* Construct the command line.  Note that the [Qemu_command]
+    (* Construct the command line.  Note that the [Qemuopts]
      * module deals with shell and qemu comma quoting.
      *)
-    let cmd = Qemu_command.create ~arch:guestcaps.gcaps_arch () in
-    let flag = Qemu_command.flag cmd
-    and arg = Qemu_command.arg cmd
-    and arg_noquote = Qemu_command.arg_noquote cmd
-    and commas = Qemu_command.commas cmd in
+    let cmd = Qemuopts.create () in
+    Qemuopts.set_binary_by_arch cmd (Some guestcaps.gcaps_arch);
+
+    let flag = Qemuopts.flag cmd
+    and arg = Qemuopts.arg cmd
+    and arg_noquote = Qemuopts.arg_noquote cmd
+    and arg_list = Qemuopts.arg_list cmd in
 
     flag "-no-user-config"; flag "-nodefaults";
     arg "-name" source.s_name;
-    commas "-machine" (if machine_q35 then ["q35"] else []
@
-                       if smm then ["smm=on"] else [] @
-                       ["accel=kvm:tcg"]);
+    arg_list "-machine" (if machine_q35 then ["q35"] else
[] @
+                         if smm then ["smm=on"] else [] @
+                         ["accel=kvm:tcg"]);
 
     (match uefi_firmware with
      | None -> ()
      | Some { Uefi.code = code } ->
         if secure_boot_required then
-          commas "-global"
-                 ["driver=cfi.pflash01"; "property=secure";
"value=on"];
-        commas "-drive"
-               ["if=pflash"; "format=raw";
"file=" ^ code; "readonly"];
+          arg_list "-global"
+                   ["driver=cfi.pflash01";
"property=secure"; "value=on"];
+        arg_list "-drive"
+                 ["if=pflash"; "format=raw";
"file=" ^ code; "readonly"];
         arg_noquote "-drive"
"if=pflash,format=raw,file=\"$uefi_vars\"";
     );
 
@@ -113,7 +115,7 @@ object
                              (match source.s_cpu_threads with
                               | None -> 1
                               | Some v -> v));
-        commas "-smp" !a
+        arg_list "-smp" !a
       )
       else
         arg "-smp" (string_of_int source.s_vcpu);
@@ -123,17 +125,17 @@ object
     | BusSlotEmpty -> ()
 
     | BusSlotTarget t ->
-       commas "-drive" ["file=" ^ t.target_file;
"format=" ^ t.target_format;
-                        "if=" ^ if_name; "index=" ^
string_of_int i;
-                        "media=disk"]
+       arg_list "-drive" ["file=" ^ t.target_file;
"format=" ^ t.target_format;
+                          "if=" ^ if_name; "index=" ^
string_of_int i;
+                          "media=disk"]
 
     | BusSlotRemovable { s_removable_type = CDROM } ->
-       commas "-drive" ["format=raw"; "if=" ^
if_name;
-                        "index=" ^ string_of_int i;
"media=cdrom"]
+       arg_list "-drive" ["format=raw"; "if=" ^
if_name;
+                          "index=" ^ string_of_int i;
"media=cdrom"]
 
     | BusSlotRemovable { s_removable_type = Floppy } ->
-       commas "-drive" ["format=raw"; "if=" ^
if_name;
-                        "index=" ^ string_of_int i;
"media=floppy"]
+       arg_list "-drive" ["format=raw"; "if=" ^
if_name;
+                          "index=" ^ string_of_int i;
"media=floppy"]
     in
     Array.iteri (make_disk "virtio")
target_buses.target_virtio_blk_bus;
     Array.iteri (make_disk "ide") target_buses.target_ide_bus;
@@ -142,17 +144,17 @@ object
     | BusSlotEmpty -> ()
 
     | BusSlotTarget t ->
-       commas "-drive" ["file=" ^ t.target_file;
"format=" ^ t.target_format;
-                        "if=scsi"; "bus=0";
"unit=" ^ string_of_int i;
-                        "media=disk"]
+       arg_list "-drive" ["file=" ^ t.target_file;
"format=" ^ t.target_format;
+                          "if=scsi"; "bus=0";
"unit=" ^ string_of_int i;
+                          "media=disk"]
 
     | BusSlotRemovable { s_removable_type = CDROM } ->
-       commas "-drive" ["format=raw"; "if=scsi";
"bus=0";
-                        "unit=" ^ string_of_int i;
"media=cdrom"]
+       arg_list "-drive" ["format=raw";
"if=scsi"; "bus=0";
+                          "unit=" ^ string_of_int i;
"media=cdrom"]
 
     | BusSlotRemovable { s_removable_type = Floppy } ->
-       commas "-drive" ["format=raw"; "if=scsi";
"bus=0";
-                        "unit=" ^ string_of_int i;
"media=floppy"]
+       arg_list "-drive" ["format=raw";
"if=scsi"; "bus=0";
+                          "unit=" ^ string_of_int i;
"media=floppy"]
     in
     Array.iteri make_scsi target_buses.target_scsi_bus;
 
@@ -167,12 +169,12 @@ object
       | RTL8139 -> "rtl8139" in
     iteri (
       fun i nic ->
-        commas "-netdev" ["user"; "id=net" ^
string_of_int i];
-        commas "-device" [net_bus;
-                          sprintf "netdev=net%d%s" i
-                                  (match nic.s_mac with
-                                   | None -> ""
-                                   | Some mac -> "mac=" ^ mac)]
+        arg_list "-netdev" ["user"; "id=net" ^
string_of_int i];
+        arg_list "-device" [net_bus;
+                            sprintf "netdev=net%d%s" i
+                                    (match nic.s_mac with
+                                     | None -> ""
+                                     | Some mac -> "mac=" ^ mac)]
     ) source.s_nics;
 
     (* Add a display. *)
@@ -185,11 +187,11 @@ object
       | VNC ->
          arg "-display" "vnc=:0"
       | Spice ->
-         commas "-spice" [sprintf "port=%d"
-                                  (match display.s_port with
-                                   | None -> 5900
-                                   | Some p -> p);
-                          "addr=127.0.0.1"]
+         arg_list "-spice" [sprintf "port=%d"
+                                    (match display.s_port with
+                                     | None -> 5900
+                                     | Some p -> p);
+                            "addr=127.0.0.1"]
       );
       arg "-vga"
           (match guestcaps.gcaps_video with Cirrus -> "cirrus" |
QXL -> "qxl")
@@ -215,15 +217,15 @@ object
 
     (* Add the miscellaneous KVM devices. *)
     if guestcaps.gcaps_virtio_rng then (
-      commas "-object" ["rng-random";
"filename=/dev/urandom"; "id=rng0"];
-      commas "-device" ["virtio-rng-pci";
"rng=rng0"];
+      arg_list "-object" ["rng-random";
"filename=/dev/urandom"; "id=rng0"];
+      arg_list "-device" ["virtio-rng-pci";
"rng=rng0"];
     );
     if guestcaps.gcaps_virtio_balloon then
       arg "-balloon" "virtio"
     else
       arg "-balloon" "none";
     if guestcaps.gcaps_isa_pvpanic then
-      commas "-device" ["pvpanic";
"ioport=0x505"];
+      arg_list "-device" ["pvpanic";
"ioport=0x505"];
 
     (* Add a serial console to Linux guests. *)
     if inspect.i_type = "linux" then
@@ -244,7 +246,7 @@ object
         fpf "\n"
     );
 
-    Qemu_command.to_chan cmd chan;
+    Qemuopts.to_chan cmd chan;
     close_out chan;
 
     Unix.chmod file 0o755;
diff --git a/v2v/qemu_command.ml b/v2v/qemu_command.ml
deleted file mode 100644
index ccdda8a..0000000
--- a/v2v/qemu_command.ml
+++ /dev/null
@@ -1,101 +0,0 @@
-(* virt-v2v
- * Copyright (C) 2009-2017 Red Hat Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *)
-
-(** Generate a qemu command line, dealing with quoting. *)
-
-open Printf
-
-open Common_utils
-
-type t = {
-  cmd : string;
-  mutable args : arg list; (* list constructed in reverse order *)
-}
-and arg -  | Flag of string
-  | Arg of string * string * bool
-  | Commas of string * string list
-
-let create ?(arch = "x86_64") () -  { cmd = "qemu-system-"
^ arch; args = [] }
-
-let flag t k -  assert (String.is_prefix k "-");
-  t.args <- Flag k :: t.args
-
-let arg t k v -  assert (String.is_prefix k "-");
-  t.args <- Arg (k, v, true) :: t.args
-
-let arg_noquote t k v -  assert (String.is_prefix k "-");
-  t.args <- Arg (k, v, false) :: t.args
-
-let commas t k vs -  assert (String.is_prefix k "-");
-  List.iter (fun v -> assert (v <> "")) vs;
-  t.args <- Commas (k, vs) :: t.args
-
-let nl = " \\\n\t"
-
-(* If the value contains only simple characters then it doesn't
- * need quoting.  This keeps the output as similar as possible
- * to the old code.
- *)
-let do_quoting str -  let len = String.length str in
-  let ret = ref false in
-  for i = 0 to len-1 do
-    let c = String.unsafe_get str i in
-    if not (Char.isalnum c) &&
-         c <> '.' && c <> '-' && c
<> '_' &&
-         c <> '=' && c <> ',' && c
<> ':' && c <> '/'
-    then
-      ret := true
-  done;
-  !ret
-
-let print_quoted_param chan k v -  if not (do_quoting v) then
-    fprintf chan "%s%s %s" nl k v
-  else
-    fprintf chan "%s%s %s" nl k (quote v)
-
-let to_chan t chan -  fprintf chan "%s" t.cmd;
-  List.iter (
-    function
-    | Flag k ->
-       fprintf chan "%s%s" nl k
-    | Arg (k, v, true) ->
-       print_quoted_param chan k v
-    | Arg (k, v, false) ->
-       fprintf chan "%s%s %s" nl k v
-    | Commas (k, vs) ->
-       let vs = List.map (fun s -> String.replace s ","
",,") vs in
-       let v = String.concat "," vs in
-       print_quoted_param chan k v
-  ) (List.rev t.args);
-  fprintf chan "\n"
-
-let to_script t filename -  let chan = open_out filename in
-  fprintf chan "#!/bin/sh -\n";
-  to_chan t chan;
-  close_out chan;
-  Unix.chmod filename 0o755
diff --git a/v2v/qemuopts-c.c b/v2v/qemuopts-c.c
new file mode 100644
index 0000000..cfdb5f6
--- /dev/null
+++ b/v2v/qemuopts-c.c
@@ -0,0 +1,216 @@
+/* virt-v2v
+ * Copyright (C) 2009-2017 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <errno.h>
+
+#include <caml/alloc.h>
+#include <caml/custom.h>
+#include <caml/fail.h>
+#include <caml/memory.h>
+#include <caml/mlvalues.h>
+
+#ifdef HAVE_CAML_UNIXSUPPORT_H
+#include <caml/unixsupport.h>
+#else
+#define Nothing ((value) 0)
+extern void unix_error (int errcode, char * cmdname, value arg) Noreturn;
+#endif
+
+#include "qemuopts.h"
+
+#pragma GCC diagnostic ignored "-Wmissing-prototypes"
+
+#define Qopts_val(v) (*((struct qemuopts **)Data_custom_val(v)))
+
+static void
+qopts_finalize (value qoptsv)
+{
+  struct qemuopts *qopts = Qopts_val (qoptsv);
+
+  if (qopts)
+    qemuopts_free (qopts);
+}
+
+static struct custom_operations qemuopts_custom_operations = {
+  (char *) "qemuopts_custom_operations",
+  qopts_finalize,
+  custom_compare_default,
+  custom_hash_default,
+  custom_serialize_default,
+  custom_deserialize_default
+};
+
+value
+guestfs_int_qemuopts_create (value unitv)
+{
+  CAMLparam1 (unitv);
+  CAMLlocal1 (qoptsv);
+  struct qemuopts *qopts;
+
+  qopts = qemuopts_create ();
+  if (qopts == NULL)
+    unix_error (errno, (char *) "qemuopts_create", Nothing);
+
+  qoptsv = caml_alloc_custom (&qemuopts_custom_operations,
+                              sizeof (struct qemuopts *), 0, 1);
+  Qopts_val (qoptsv) = qopts;
+
+  CAMLreturn (qoptsv);
+}
+
+value
+guestfs_int_qemuopts_set_binary (value qoptsv, value strv)
+{
+  CAMLparam2 (qoptsv, strv);
+  struct qemuopts *qopts = Qopts_val (qoptsv);
+
+  if (qemuopts_set_binary (qopts, String_val (strv)) == -1)
+    unix_error (errno, (char *) "qemuopts_set_binary", strv);
+
+  CAMLreturn (Val_unit);
+}
+
+value
+guestfs_int_qemuopts_set_binary_by_arch (value qoptsv, value ostrv)
+{
+  CAMLparam2 (qoptsv, ostrv);
+  struct qemuopts *qopts = Qopts_val (qoptsv);
+  int r;
+
+  if (ostrv != Val_int (0))
+    r = qemuopts_set_binary_by_arch (qopts, NULL);
+  else
+    r = qemuopts_set_binary_by_arch (qopts, String_val (Field (ostrv, 0)));
+
+  if (r == -1)
+    unix_error (errno, (char *) "qemuopts_set_binary_by_arch",
Nothing);
+
+  CAMLreturn (Val_unit);
+}
+
+value
+guestfs_int_qemuopts_flag (value qoptsv, value flagv)
+{
+  CAMLparam2 (qoptsv, flagv);
+  struct qemuopts *qopts = Qopts_val (qoptsv);
+
+  if (qemuopts_add_flag (qopts, String_val (flagv)) == -1)
+    unix_error (errno, (char *) "qemuopts_add_flag", flagv);
+
+  CAMLreturn (Val_unit);
+}
+
+value
+guestfs_int_qemuopts_arg (value qoptsv, value flagv, value valv)
+{
+  CAMLparam3 (qoptsv, flagv, valv);
+  struct qemuopts *qopts = Qopts_val (qoptsv);
+
+  if (qemuopts_add_arg (qopts, String_val (flagv), String_val (valv)) == -1)
+    unix_error (errno, (char *) "qemuopts_add_arg", flagv);
+
+  CAMLreturn (Val_unit);
+}
+
+value
+guestfs_int_qemuopts_arg_noquote (value qoptsv, value flagv, value valv)
+{
+  CAMLparam3 (qoptsv, flagv, valv);
+  struct qemuopts *qopts = Qopts_val (qoptsv);
+
+  if (qemuopts_add_arg_noquote (qopts,
+                                String_val (flagv), String_val (valv)) == -1)
+    unix_error (errno, (char *) "qemuopts_add_arg_noquote", flagv);
+
+  CAMLreturn (Val_unit);
+}
+
+value
+guestfs_int_qemuopts_arg_list (value qoptsv, value flagv, value valuesv)
+{
+  CAMLparam3 (qoptsv, flagv, valuesv);
+  CAMLlocal1 (hd);
+  struct qemuopts *qopts = Qopts_val (qoptsv);
+
+  if (qemuopts_start_arg_list (qopts, String_val (flagv)) == -1)
+    unix_error (errno, (char *) "qemuopts_start_arg_list", flagv);
+
+  while (valuesv != Val_emptylist) {
+    hd = Field (valuesv, 0);
+    if (qemuopts_append_arg_list (qopts, String_val (hd)) == -1)
+      unix_error (errno, (char *) "qemuopts_append_arg_list", flagv);
+    valuesv = Field (valuesv, 1);
+  }
+
+  if (qemuopts_end_arg_list (qopts) == -1)
+    unix_error (errno, (char *) "qemuopts_end_arg_list", flagv);
+
+  CAMLreturn (Val_unit);
+}
+
+value
+guestfs_int_qemuopts_to_script (value qoptsv, value strv)
+{
+  CAMLparam2 (qoptsv, strv);
+  struct qemuopts *qopts = Qopts_val (qoptsv);
+
+  if (qemuopts_to_script (qopts, String_val (strv)) == -1)
+    unix_error (errno, (char *) "qemuopts_to_script", strv);
+
+  CAMLreturn (Val_unit);
+}
+
+value
+guestfs_int_qemuopts_to_chan (value qoptsv, value fdv)
+{
+  CAMLparam2 (qoptsv, fdv);
+  struct qemuopts *qopts = Qopts_val (qoptsv);
+  /* Note that Unix.file_descr is really just an int. */
+  int fd = Int_val (fdv);
+  int fd2;
+  FILE *fp;
+  int saved_errno;
+
+  /* Dup the file descriptor so we don't lose it in fclose. */
+  fd2 = dup (fd);
+  if (fd2 == -1)
+    unix_error (errno, (char *) "qemuopts_to_channel: dup", Nothing);
+
+  fp = fdopen (fd2, "w");
+  if (fp == NULL) {
+    saved_errno = errno;
+    close (fd2);
+    unix_error (saved_errno, (char *) "qemuopts_to_channel: fdopen",
Nothing);
+  }
+
+  if (qemuopts_to_channel (qopts, fp) == -1) {
+    saved_errno = errno;
+    fclose (fp);
+    unix_error (saved_errno, (char *) "qemuopts_to_channel",
Nothing);
+  }
+
+  if (fclose (fp) == EOF)
+    unix_error (errno, (char *) "qemuopts_to_channel: fclose",
Nothing);
+
+  CAMLreturn (Val_unit);
+}
diff --git a/v2v/qemuopts.ml b/v2v/qemuopts.ml
new file mode 100644
index 0000000..752e4d6
--- /dev/null
+++ b/v2v/qemuopts.ml
@@ -0,0 +1,35 @@
+(* virt-v2v
+ * Copyright (C) 2009-2017 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *)
+
+type t
+
+external create : unit -> t = "guestfs_int_qemuopts_create"
+external set_binary : t -> string -> unit =
"guestfs_int_qemuopts_set_binary"
+external set_binary_by_arch : t -> string option -> unit =
"guestfs_int_qemuopts_set_binary_by_arch"
+external flag : t -> string -> unit =
"guestfs_int_qemuopts_flag"
+external arg : t -> string -> string -> unit =
"guestfs_int_qemuopts_arg"
+external arg_noquote : t -> string -> string -> unit =
"guestfs_int_qemuopts_arg_noquote"
+external arg_list : t -> string -> string list -> unit =
"guestfs_int_qemuopts_arg_list"
+external to_script : t -> string -> unit =
"guestfs_int_qemuopts_to_script"
+
+external _to_chan : t -> Unix.file_descr -> unit =
"guestfs_int_qemuopts_to_chan"
+
+let to_chan t chan +  flush chan;
+  let fd = Unix.descr_of_out_channel chan in
+  _to_chan t fd
diff --git a/v2v/qemu_command.mli b/v2v/qemuopts.mli
similarity index 75%
rename from v2v/qemu_command.mli
rename to v2v/qemuopts.mli
index d993bd2..695acab 100644
--- a/v2v/qemu_command.mli
+++ b/v2v/qemuopts.mli
@@ -16,14 +16,22 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  *)
 
-(** Generate a qemu command line, dealing with quoting. *)
+(** OCaml bindings for the [common/qemuopts] library. *)
 
 type t
 
-val create : ?arch:string -> unit -> t
-(** Create an empty qemu command.  If the optional [?arch] parameter
-    is supplied then the command will be [qemu-system-<arch>],
-    otherwise it will be [qemu-system-x86_64]. *)
+val create : unit -> t
+(** Create an empty qemu command line.
+
+    In case of error, all these functions raise [Unix_error]. *)
+
+val set_binary : t -> string -> unit
+(** Set the qemu binary name. *)
+
+val set_binary_by_arch : t -> string option -> unit
+(** Set the qemu binary to [qemu-system-<arch>].  If [arch] is [None],
+    then this picks the right KVM binary for the current host
+    architecture. *)
 
 val flag : t -> string -> unit
 (** [flag t "-foo"] adds a parameter to the command line with no
argument. *)
@@ -34,13 +42,13 @@ val arg : t -> string -> string -> unit
     The value will shell-quoted if required, so you do not need to quote
     the string.  However if the value is a comma-separated list
     (eg. [-drive file=foo,if=ide]) then do {b not} use this function, call
-    {!commas} instead. *)
+    {!arg_list} instead. *)
 
 val arg_noquote : t -> string -> string -> unit
 (** Like {!arg} except no quoting is done on the value. *)
 
-val commas : t -> string -> string list -> unit
-(** [commas t "-drive" ["file=foo"; "if=ide"]]
adds a comma-separated
+val arg_list : t -> string -> string list -> unit
+(** [arg_list t "-drive" ["file=foo"; "if=ide"]]
adds a comma-separated
     list of parameters to the command line [-drive file=foo,if=ide].
 
     This does both qemu comma-quoting and shell-quoting as required. *)
-- 
2.9.3