[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2 0/8] virCommand: next round



On 11/23/2010 04:49 PM, Eric Blake wrote:
> This should address some of the review comments from the first round
> (https://www.redhat.com/archives/libvir-list/2010-November/msg00803.html).
> 
> Daniel P. Berrange (5):
>   Introduce new APIs for spawning processes
>   virCommand: docs for usage of new command APIs
>   Port hooks and iptables code to new command execution APIs
>   uml: convert to virCommand
>   Remove bogus includes
> 
> Eric Blake (3):
>   util: add virVasprintf
>   util: fix saferead type
>   qemu: convert to virCommand

Here's the interdiff from v1 (well, from v1 + untested uml patches), if
it helps review.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org
diff --git a/.x-sc_prohibit_asprintf b/.x-sc_prohibit_asprintf
index 614c238..d03b947 100644
--- a/.x-sc_prohibit_asprintf
+++ b/.x-sc_prohibit_asprintf
@@ -1,3 +1,5 @@
+ChangeLog
+^bootstrap.conf$
 ^gnulib/
 ^po/
-ChangeLog
+^src/util/util.c$
diff --git a/cfg.mk b/cfg.mk
index 2ac3c69..6312632 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -243,7 +243,7 @@ sc_prohibit_strncmp:

 # Use virAsprintf rather than as'printf since *strp is undefined on error.
 sc_prohibit_asprintf:
-	@prohibit='\<a[s]printf\>'					\
+	@prohibit='\<v?a[s]printf\>'					\
 	halt='use virAsprintf, not as'printf				\
 	  $(_sc_search_regexp)

diff --git a/docs/internals/command.html.in b/docs/internals/command.html.in
index 3d28cd4..c4fa800 100644
--- a/docs/internals/command.html.in
+++ b/docs/internals/command.html.in
@@ -111,6 +111,15 @@
 </pre>

     <p>
+      If an argument needs to be formatted as if by
+      <code>printf</code>:
+    </p>
+
+<pre>
+  virCommandAddArgFormat(cmd, "%d", count);
+</pre>
+
+    <p>
       To add a entire NULL terminated array of arguments in one go,
       there are two options.
     </p>
@@ -124,8 +133,8 @@
 </pre>

     <p>
-      This latter option can also be used at time of initial
-      construction of the <code>virCommandPtr</code> object
+      This can also be done at the time of initial construction of
+      the <code>virCommandPtr</code> object:
     </p>

 <pre>
@@ -134,7 +143,9 @@
       "--strict-order", "--except-interface",
       "lo", "--domain", "localdomain", NULL
   };
-  virCommandPtr cmd = virCommandNewArgs(cmd, args);
+  virCommandPtr cmd1 = virCommandNewArgs(cmd, args);
+  virCommandPtr cmd2 = virCommandNewArgList("/usr/bin/dnsmasq",
+                                            "--domain", "localdomain", NULL);
 </pre>

     <h3><a name="env">Setting up the environment</a></h3>
@@ -233,23 +244,31 @@
     <h3><a name="fds">Managing file handles</a></h3>

     <p>
-      To prevent unintended resource leaks to child processes,
-      all open file handles will be closed in the child, and
-      its stdin/out/err all connected to <code>/dev/null</code>.
-      It is possible to allow an open file handle to be passed
-      into the child:
+      To prevent unintended resource leaks to child processes, the
+      child defaults to closing all open file handles, and setting
+      stdin/out/err to <code>/dev/null</code>.  It is possible to
+      allow an open file handle to be passed into the child, while
+      controlling whether that handle remains open in the parent or
+      guaranteeing that the handle will be closed in the parent after
+      either virCommandRun or virCommandFree.
     </p>

 <pre>
-  virCommandPreserveFD(cmd, 5);
+  int sharedfd = open("cmd.log", "w+");
+  int childfd = open("conf.txt", "r");
+  virCommandPreserveFD(cmd, sharedfd);
+  virCommandTransferFD(cmd, childfd);
+  if (VIR_CLOSE(sharedfd) &lt; 0)
+      goto cleanup;
 </pre>

     <p>
-      With this file descriptor 5 in the current process remains
-      open as file descriptor 5 in the child. For stdin/out/err
-      it is usually necessary to map a file handle. To attach
-      file descriptor 7 in the current process to stdin in the
-      child:
+      With this, both file descriptors sharedfd and childfd in the
+      current process remain open as the same file descriptors in the
+      child. Meanwhile, after the child is spawned, sharedfd remains
+      open in the parent, while childfd is closed.  For stdin/out/err
+      it is usually necessary to map a file handle. To attach file
+      descriptor 7 in the current process to stdin in the child:
     </p>

 <pre>
@@ -284,16 +303,30 @@
     <p>
       Once the command is running, <code>outfd</code>
       and <code>errfd</code> will be initialized with
-      valid file handles that can be read from.
+      valid file handles that can be read from.  It is
+      permissible to pass the same pointer for both outfd
+      and errfd, in which case both standard streams in
+      the child will share the same fd in the parent.
     </p>

+    <p>
+      Normally, file descriptors opened to collect output from a child
+      process perform blocking I/O, but the parent process can request
+      non-blocking mode:
+    </p>
+
+<pre>
+  virCommandNonblockingFDs(cmd);
+</pre>
+
     <h3><a name="buffers">Feeding &amp; capturing strings to/from the child</a></h3>

     <p>
       Often dealing with file handles for stdin/out/err
       is unnecessarily complex. It is possible to specify
       a string buffer to act as the data source for the
-      child's stdin, if there are no embedded NUL bytes
+      child's stdin, if there are no embedded NUL bytes,
+      and if the command will be run with virCommandRun:
     </p>

 <pre>
@@ -304,7 +337,8 @@
     <p>
       Similarly it is possible to request that the child's
       stdout/err be redirected into a string buffer, if the
-      output is not expected to contain NUL bytes
+      output is not expected to contain NUL bytes, and if
+      the command will be run with virCommandRun:
     </p>

 <pre>
@@ -346,6 +380,31 @@
   virCommandSetPreExecHook(cmd, hook, opaque);
 </pre>

+    <h3><a name="logging">Logging commands</a></h3>
+
+    <p>
+      Sometimes, it is desirable to log what command will be run, or
+      even to use virCommand solely for creation of a single
+      consolidated string without running anything.
+    </p>
+
+<pre>
+  int logfd = ...;
+  char *timestamp = virTimestamp();
+  char *string = NULL;
+
+  dprintf(logfd, "%s: ", timestamp);
+  VIR_FREE(timestamp);
+  virCommandWriteArgLog(cmd, logfd);
+
+  string = virCommandToString(cmd);
+  if (string)
+      VIR_DEBUG("about to run %s", string);
+  VIR_FREE(string);
+  if (virCommandRun(cmd) &lt; 0)
+      return -1;
+</pre>
+
     <h3><a name="sync">Running commands synchronously</a></h3>

     <p>
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 63795b3..d7dc7b1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -85,6 +85,7 @@ virCgroupSetSwapHardLimit;

 # command.h
 virCommandAddArg;
+virCommandAddArgFormat;
 virCommandAddArgList;
 virCommandAddArgPair;
 virCommandAddArgSet;
@@ -96,7 +97,9 @@ virCommandClearCaps;
 virCommandDaemonize;
 virCommandFree;
 virCommandNew;
+virCommandNewArgList;
 virCommandNewArgs;
+virCommandNonblockingFDs;
 virCommandPreserveFD;
 virCommandRun;
 virCommandRunAsync;
@@ -109,7 +112,10 @@ virCommandSetOutputFD;
 virCommandSetPidFile;
 virCommandSetPreExecHook;
 virCommandSetWorkingDirectory;
+virCommandToString;
+virCommandTransferFD;
 virCommandWait;
+virCommandWriteArgLog;


 # conf.h
@@ -839,6 +845,7 @@ virStrToLong_ull;
 virStrcpy;
 virStrncpy;
 virTimestamp;
+virVasprintf;


 # uuid.h
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 35caccc..7c0db15 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1530,6 +1530,15 @@ int qemudExtractVersionInfo(const char *qemu,
     if (retversion)
         *retversion = 0;

+    /* Make sure the binary we are about to try exec'ing exists.
+     * Technically we could catch the exec() failure, but that's
+     * in a sub-process so it's hard to feed back a useful error.
+     */
+    if (access(qemu, X_OK) < 0) {
+        virReportSystemError(errno, _("Cannot find QEMU binary %s"), qemu);
+        return -1;
+    }
+
     if (virExec(qemuarg, qemuenv, NULL,
                 &child, -1, &newstdout, NULL, VIR_EXEC_CLEAR_CAPS) < 0)
         return -1;
@@ -3942,43 +3951,35 @@ qemuBuildSmpArgStr(const virDomainDefPtr def,
  * XXX 'conn' is only required to resolve network -> bridge name
  * figure out how to remove this requirement some day
  */
-int qemudBuildCommandLine(virConnectPtr conn,
-                          struct qemud_driver *driver,
-                          virDomainDefPtr def,
-                          virDomainChrDefPtr monitor_chr,
-                          int monitor_json,
-                          unsigned long long qemuCmdFlags,
-                          const char ***retargv,
-                          const char ***retenv,
-                          int **vmfds,
-                          int *nvmfds,
-                          const char *migrateFrom,
-                          virDomainSnapshotObjPtr current_snapshot)
+virCommandPtr
+qemudBuildCommandLine(virConnectPtr conn,
+                      struct qemud_driver *driver,
+                      virDomainDefPtr def,
+                      virDomainChrDefPtr monitor_chr,
+                      bool monitor_json,
+                      unsigned long long qemuCmdFlags,
+                      const char *migrateFrom,
+                      virDomainSnapshotObjPtr current_snapshot)
 {
     int i;
-    char memory[50];
     char boot[VIR_DOMAIN_BOOT_LAST+1];
     struct utsname ut;
     int disableKQEMU = 0;
     int enableKQEMU = 0;
     int disableKVM = 0;
     int enableKVM = 0;
-    int qargc = 0, qarga = 0;
-    const char **qargv = NULL;
-    int qenvc = 0, qenva = 0;
-    const char **qenv = NULL;
     const char *emulator;
     char uuid[VIR_UUID_STRING_BUFLEN];
-    char domid[50];
     char *cpu;
     char *smp;
     int last_good_net = -1;
     bool hasHwVirt = false;
+    virCommandPtr cmd;

     uname_normalize(&ut);

     if (qemuAssignDeviceAliases(def, qemuCmdFlags) < 0)
-        return -1;
+        return NULL;

     virUUIDFormat(def->uuid, uuid);

@@ -3990,7 +3991,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
             if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP)) {
                 qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                 "%s", _("TCP migration is not supported with this QEMU binary"));
-                return -1;
+                return NULL;
             }
         } else if (STREQ(migrateFrom, "stdio")) {
             if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC) {
@@ -3998,13 +3999,13 @@ int qemudBuildCommandLine(virConnectPtr conn,
             } else if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_KVM_STDIO)) {
                 qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                 "%s", _("STDIO migration is not supported with this QEMU binary"));
-                return -1;
+                return NULL;
             }
         } else if (STRPREFIX(migrateFrom, "exec")) {
             if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC)) {
                 qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                 "%s", _("STDIO migration is not supported with this QEMU binary"));
-                return -1;
+                return NULL;
             }
         }
     }
@@ -4065,134 +4066,45 @@ int qemudBuildCommandLine(virConnectPtr conn,
         break;
     }

-#define ADD_ARG_SPACE                                                   \
-    do { \
-        if (qargc == qarga) {                                           \
-            qarga += 10;                                                \
-            if (VIR_REALLOC_N(qargv, qarga) < 0)                        \
-                goto no_memory;                                         \
-        }                                                               \
-    } while (0)
-
-#define ADD_ARG(thisarg)                                                \
-    do {                                                                \
-        ADD_ARG_SPACE;                                                  \
-        qargv[qargc++] = thisarg;                                       \
-    } while (0)
-
-#define ADD_ARG_LIT(thisarg)                                            \
-    do {                                                                \
-        ADD_ARG_SPACE;                                                  \
-        if ((qargv[qargc++] = strdup(thisarg)) == NULL)                 \
-            goto no_memory;                                             \
-    } while (0)
-
-#define ADD_USBDISK(thisarg)                                            \
-    do {                                                                \
-        ADD_ARG_LIT("-usbdevice");                                      \
-        ADD_ARG_SPACE;                                                  \
-        if ((virAsprintf((char **)&(qargv[qargc++]),                    \
-                         "disk:%s", thisarg)) == -1) {                  \
-            goto no_memory;                                             \
-        }                                                               \
-    } while (0)
-
-#define ADD_ENV_SPACE                                                   \
-    do {                                                                \
-        if (qenvc == qenva) {                                           \
-            qenva += 10;                                                \
-            if (VIR_REALLOC_N(qenv, qenva) < 0)                         \
-                goto no_memory;                                         \
-        }                                                               \
-    } while (0)
-
-#define ADD_ENV(thisarg)                                                \
-    do {                                                                \
-        ADD_ENV_SPACE;                                                  \
-        qenv[qenvc++] = thisarg;                                        \
-    } while (0)
-
-#define ADD_ENV_LIT(thisarg)                                            \
-    do {                                                                \
-        ADD_ENV_SPACE;                                                  \
-        if ((qenv[qenvc++] = strdup(thisarg)) == NULL)                  \
-            goto no_memory;                                             \
-    } while (0)
-
-#define ADD_ENV_PAIR(envname, val)                                      \
-    do {                                                                \
-        char *envval;                                                   \
-        ADD_ENV_SPACE;                                                  \
-        if (virAsprintf(&envval, "%s=%s", envname, val) < 0)            \
-            goto no_memory;                                             \
-        qenv[qenvc++] = envval;                                         \
-    } while (0)
-
-    /* Make sure to unset or set all envvars in qemuxml2argvtest.c that
-     * are copied here using this macro, otherwise the test may fail */
-#define ADD_ENV_COPY(envname)                                           \
-    do {                                                                \
-        char *val = getenv(envname);                                    \
-        if (val != NULL) {                                              \
-            ADD_ENV_PAIR(envname, val);                                 \
-        }                                                               \
-    } while (0)
+    cmd = virCommandNewArgList(emulator, "-S", NULL);

-    /* Set '-m MB' based on maxmem, because the lower 'memory' limit
-     * is set post-startup using the balloon driver. If balloon driver
-     * is not supported, then they're out of luck anyway
-     */
-    snprintf(memory, sizeof(memory), "%lu", def->mem.max_balloon/1024);
-    snprintf(domid, sizeof(domid), "%d", def->id);
-
-    ADD_ENV_LIT("LC_ALL=C");
-
-    ADD_ENV_COPY("LD_PRELOAD");
-    ADD_ENV_COPY("LD_LIBRARY_PATH");
-    ADD_ENV_COPY("PATH");
-    ADD_ENV_COPY("HOME");
-    ADD_ENV_COPY("USER");
-    ADD_ENV_COPY("LOGNAME");
-    ADD_ENV_COPY("TMPDIR");
-
-    ADD_ARG_LIT(emulator);
-    ADD_ARG_LIT("-S");
+    virCommandAddEnvPassCommon(cmd);

     /* This should *never* be NULL, since we always provide
      * a machine in the capabilities data for QEMU. So this
      * check is just here as a safety in case the unexpected
      * happens */
-    if (def->os.machine) {
-        ADD_ARG_LIT("-M");
-        ADD_ARG_LIT(def->os.machine);
-    }
+    if (def->os.machine)
+        virCommandAddArgList(cmd, "-M", def->os.machine, NULL);

     if (qemuBuildCpuArgStr(driver, def, emulator, qemuCmdFlags,
                            &ut, &cpu, &hasHwVirt) < 0)
         goto error;

     if (cpu) {
-        ADD_ARG_LIT("-cpu");
-        ADD_ARG_LIT(cpu);
+        virCommandAddArgList(cmd, "-cpu", cpu, NULL);
         VIR_FREE(cpu);

         if ((qemuCmdFlags & QEMUD_CMD_FLAG_NESTING) &&
             hasHwVirt)
-            ADD_ARG_LIT("-enable-nesting");
+            virCommandAddArg(cmd, "-enable-nesting");
     }

     if (disableKQEMU)
-        ADD_ARG_LIT("-no-kqemu");
-    else if (enableKQEMU) {
-        ADD_ARG_LIT("-enable-kqemu");
-        ADD_ARG_LIT("-kernel-kqemu");
-    }
+        virCommandAddArg(cmd, "-no-kqemu");
+    else if (enableKQEMU)
+        virCommandAddArgList(cmd, "-enable-kqemu", "-kernel-kqemu", NULL);
     if (disableKVM)
-        ADD_ARG_LIT("-no-kvm");
+        virCommandAddArg(cmd, "-no-kvm");
     if (enableKVM)
-        ADD_ARG_LIT("-enable-kvm");
-    ADD_ARG_LIT("-m");
-    ADD_ARG_LIT(memory);
+        virCommandAddArg(cmd, "-enable-kvm");
+
+    /* Set '-m MB' based on maxmem, because the lower 'memory' limit
+     * is set post-startup using the balloon driver. If balloon driver
+     * is not supported, then they're out of luck anyway
+     */
+    virCommandAddArg(cmd, "-m");
+    virCommandAddArgFormat(cmd, "%lu", def->mem.max_balloon / 1024);
     if (def->mem.hugepage_backed) {
         if (!driver->hugetlbfs_mount) {
             qemuReportError(VIR_ERR_INTERNAL_ERROR,
@@ -4210,43 +4122,38 @@ int qemudBuildCommandLine(virConnectPtr conn,
                             def->emulator);
             goto error;
         }
-        ADD_ARG_LIT("-mem-prealloc");
-        ADD_ARG_LIT("-mem-path");
-        ADD_ARG_LIT(driver->hugepage_path);
+        virCommandAddArgList(cmd, "-mem-prealloc", "-mem-path",
+                             driver->hugepage_path, NULL);
     }

-    ADD_ARG_LIT("-smp");
+    virCommandAddArg(cmd, "-smp");
     if (!(smp = qemuBuildSmpArgStr(def, qemuCmdFlags)))
         goto error;
-    ADD_ARG(smp);
+    virCommandAddArg(cmd, smp);
+    VIR_FREE(smp);

     if (qemuCmdFlags & QEMUD_CMD_FLAG_NAME) {
-        ADD_ARG_LIT("-name");
+        virCommandAddArg(cmd, "-name");
         if (driver->setProcessName &&
             (qemuCmdFlags & QEMUD_CMD_FLAG_NAME_PROCESS)) {
-            char *name;
-            if (virAsprintf(&name, "%s,process=qemu:%s",
-                            def->name, def->name) < 0)
-                goto no_memory;
-            ADD_ARG_LIT(name);
+            virCommandAddArgFormat(cmd, "%s,process=qemu:%s",
+                                   def->name, def->name);
         } else {
-            ADD_ARG_LIT(def->name);
+            virCommandAddArg(cmd, def->name);
         }
     }
-    if (qemuCmdFlags & QEMUD_CMD_FLAG_UUID) {
-        ADD_ARG_LIT("-uuid");
-        ADD_ARG_LIT(uuid);
-    }
+    if (qemuCmdFlags & QEMUD_CMD_FLAG_UUID)
+        virCommandAddArgList(cmd, "-uuid", uuid, NULL);
     if (def->virtType == VIR_DOMAIN_VIRT_XEN ||
         STREQ(def->os.type, "xen") ||
         STREQ(def->os.type, "linux")) {
         if (qemuCmdFlags & QEMUD_CMD_FLAG_DOMID) {
-            ADD_ARG_LIT("-domid");
-            ADD_ARG_LIT(domid);
+            virCommandAddArg(cmd, "-domid");
+            virCommandAddArgFormat(cmd, "%d", def->id);
         } else if (qemuCmdFlags & QEMUD_CMD_FLAG_XEN_DOMID) {
-            ADD_ARG_LIT("-xen-attach");
-            ADD_ARG_LIT("-xen-domid");
-            ADD_ARG_LIT(domid);
+            virCommandAddArg(cmd, "-xen-attach");
+            virCommandAddArg(cmd, "-xen-domid");
+            virCommandAddArgFormat(cmd, "%d", def->id);
         } else {
             qemuReportError(VIR_ERR_INTERNAL_ERROR,
                             _("qemu emulator '%s' does not support xen"),
@@ -4288,13 +4195,13 @@ int qemudBuildCommandLine(virConnectPtr conn,

             smbioscmd = qemuBuildSmbiosBiosStr(source);
             if (smbioscmd != NULL) {
-                ADD_ARG_LIT("-smbios");
-                ADD_ARG(smbioscmd);
+                virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL);
+                VIR_FREE(smbioscmd);
             }
             smbioscmd = qemuBuildSmbiosSystemStr(source);
             if (smbioscmd != NULL) {
-                ADD_ARG_LIT("-smbios");
-                ADD_ARG(smbioscmd);
+                virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL);
+                VIR_FREE(smbioscmd);
             }
         }
     }
@@ -4307,12 +4214,14 @@ int qemudBuildCommandLine(virConnectPtr conn,
      * these defaults ourselves...
      */
     if (!def->graphics)
-        ADD_ARG_LIT("-nographic");
+        virCommandAddArg(cmd, "-nographic");

     if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
         if (qemuCmdFlags & QEMUD_CMD_FLAG_NODEFCONFIG)
-            ADD_ARG_LIT("-nodefconfig"); /* Disabling global config files */
-        ADD_ARG_LIT("-nodefaults");  /* Disabling default guest devices */
+            virCommandAddArg(cmd,
+                             "-nodefconfig"); /* Disable global config files */
+        virCommandAddArg(cmd,
+                         "-nodefaults");  /* Disable default guest devices */
     }

     if (monitor_chr) {
@@ -4320,39 +4229,40 @@ int qemudBuildCommandLine(virConnectPtr conn,
         /* Use -chardev if it's available */
         if (qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) {

-            ADD_ARG_LIT("-chardev");
+            virCommandAddArg(cmd, "-chardev");
             if (!(chrdev = qemuBuildChrChardevStr(monitor_chr)))
                 goto error;
-            ADD_ARG(chrdev);
+            virCommandAddArg(cmd, chrdev);
+            VIR_FREE(chrdev);

-            ADD_ARG_LIT("-mon");
-            if (monitor_json)
-                ADD_ARG_LIT("chardev=monitor,mode=control");
-            else
-                ADD_ARG_LIT("chardev=monitor,mode=readline");
+            virCommandAddArg(cmd, "-mon");
+            virCommandAddArgFormat(cmd, "chardev=monitor,mode=%s",
+                                   monitor_json ? "control" : "readline");
         } else {
             const char *prefix = NULL;
             if (monitor_json)
                 prefix = "control,";

-            ADD_ARG_LIT("-monitor");
+            virCommandAddArg(cmd, "-monitor");
             if (!(chrdev = qemuBuildChrArgStr(monitor_chr, prefix)))
                 goto error;
-            ADD_ARG(chrdev);
+            virCommandAddArg(cmd, chrdev);
+            VIR_FREE(chrdev);
         }
     }

     if (qemuCmdFlags & QEMUD_CMD_FLAG_RTC) {
         const char *rtcopt;
-        ADD_ARG_LIT("-rtc");
+        virCommandAddArg(cmd, "-rtc");
         if (!(rtcopt = qemuBuildClockArgStr(&def->clock)))
             goto error;
-        ADD_ARG(rtcopt);
+        virCommandAddArg(cmd, rtcopt);
+        VIR_FREE(rtcopt);
     } else {
         switch (def->clock.offset) {
         case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
         case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE:
-            ADD_ARG_LIT("-localtime");
+            virCommandAddArg(cmd, "-localtime");
             break;

         case VIR_DOMAIN_CLOCK_OFFSET_UTC:
@@ -4368,7 +4278,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
     }
     if (def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE &&
         def->clock.data.timezone) {
-        ADD_ENV_PAIR("TZ", def->clock.data.timezone);
+        virCommandAddEnvPair(cmd, "TZ", def->clock.data.timezone);
     }

     for (i = 0; i < def->clock.ntimers; i++) {
@@ -4392,7 +4302,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
                     /* the default - do nothing */
                     break;
                 case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP:
-                    ADD_ARG_LIT("-rtc-td-hack");
+                    virCommandAddArg(cmd, "-rtc-td-hack");
                     break;
                 case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE:
                 case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD:
@@ -4421,14 +4331,14 @@ int qemudBuildCommandLine(virConnectPtr conn,
                 /* delay is the default if we don't have kernel
                    (-no-kvm-pit), otherwise, the default is catchup. */
                 if (qemuCmdFlags & QEMUD_CMD_FLAG_NO_KVM_PIT)
-                    ADD_ARG_LIT("-no-kvm-pit-reinjection");
+                    virCommandAddArg(cmd, "-no-kvm-pit-reinjection");
                 break;
             case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP:
                 if (qemuCmdFlags & QEMUD_CMD_FLAG_NO_KVM_PIT) {
                     /* do nothing - this is default for kvm-pit */
                 } else if (qemuCmdFlags & QEMUD_CMD_FLAG_TDF) {
                     /* -tdf switches to 'catchup' with userspace pit. */
-                    ADD_ARG_LIT("-tdf");
+                    virCommandAddArg(cmd, "-tdf");
                 } else {
                     /* can't catchup if we have neither pit mode */
                     qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -4457,7 +4367,7 @@ int qemudBuildCommandLine(virConnectPtr conn,

             if (qemuCmdFlags & QEMUD_CMD_FLAG_NO_HPET) {
                 if (def->clock.timers[i]->present == 0)
-                    ADD_ARG_LIT("-no-hpet");
+                    virCommandAddArg(cmd, "-no-hpet");
             } else {
                 /* no hpet timer available. The only possible action
                    is to raise an error if present="yes" */
@@ -4472,10 +4382,10 @@ int qemudBuildCommandLine(virConnectPtr conn,

     if ((qemuCmdFlags & QEMUD_CMD_FLAG_NO_REBOOT) &&
         def->onReboot != VIR_DOMAIN_LIFECYCLE_RESTART)
-        ADD_ARG_LIT("-no-reboot");
+        virCommandAddArg(cmd, "-no-reboot");

     if (!(def->features & (1 << VIR_DOMAIN_FEATURE_ACPI)))
-        ADD_ARG_LIT("-no-acpi");
+        virCommandAddArg(cmd, "-no-acpi");

     if (!def->os.bootloader) {
         for (i = 0 ; i < def->os.nBootDevs ; i++) {
@@ -4499,7 +4409,8 @@ int qemudBuildCommandLine(virConnectPtr conn,
         }
         if (def->os.nBootDevs) {
             virBuffer boot_buf = VIR_BUFFER_INITIALIZER;
-            ADD_ARG_LIT("-boot");
+            char *bootstr;
+            virCommandAddArg(cmd, "-boot");

             boot[def->os.nBootDevs] = '\0';

@@ -4518,24 +4429,19 @@ int qemudBuildCommandLine(virConnectPtr conn,
                 goto error;
             }

-            ADD_ARG(virBufferContentAndReset(&boot_buf));
+            bootstr = virBufferContentAndReset(&boot_buf);
+            virCommandAddArg(cmd, bootstr);
+            VIR_FREE(bootstr);
         }

-        if (def->os.kernel) {
-            ADD_ARG_LIT("-kernel");
-            ADD_ARG_LIT(def->os.kernel);
-        }
-        if (def->os.initrd) {
-            ADD_ARG_LIT("-initrd");
-            ADD_ARG_LIT(def->os.initrd);
-        }
-        if (def->os.cmdline) {
-            ADD_ARG_LIT("-append");
-            ADD_ARG_LIT(def->os.cmdline);
-        }
+        if (def->os.kernel)
+            virCommandAddArgList(cmd, "-kernel", def->os.kernel, NULL);
+        if (def->os.initrd)
+            virCommandAddArgList(cmd, "-initrd", def->os.initrd, NULL);
+        if (def->os.cmdline)
+            virCommandAddArgList(cmd, "-append", def->os.cmdline, NULL);
     } else {
-        ADD_ARG_LIT("-bootloader");
-        ADD_ARG_LIT(def->os.bootloader);
+        virCommandAddArgList(cmd, "-bootloader", def->os.bootloader, NULL);
     }

     for (i = 0 ; i < def->ndisks ; i++) {
@@ -4568,13 +4474,14 @@ int qemudBuildCommandLine(virConnectPtr conn,
                 goto error;
             }

-            ADD_ARG_LIT("-device");
+            virCommandAddArg(cmd, "-device");

             char *devstr;
             if (!(devstr = qemuBuildControllerDevStr(def->controllers[i])))
                 goto no_memory;

-            ADD_ARG(devstr);
+            virCommandAddArg(cmd, devstr);
+            VIR_FREE(devstr);
         }
     }

@@ -4610,10 +4517,12 @@ int qemudBuildCommandLine(virConnectPtr conn,
             if ((disk->bus == VIR_DOMAIN_DISK_BUS_USB) &&
                 !(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
                 if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
-                    ADD_USBDISK(disk->src);
+                    virCommandAddArg(cmd, "-usbdevice");
+                    virCommandAddArgFormat(cmd, "disk:%s", disk->src);
                 } else {
                     qemuReportError(VIR_ERR_INTERNAL_ERROR,
-                                    _("unsupported usb disk type for '%s'"), disk->src);
+                                    _("unsupported usb disk type for '%s'"),
+                                    disk->src);
                     goto error;
                 }
                 continue;
@@ -4634,10 +4543,10 @@ int qemudBuildCommandLine(virConnectPtr conn,
                 break;
             }

-            ADD_ARG_LIT("-drive");
+            virCommandAddArg(cmd, "-drive");

-            /* Unfortunately it is nt possible to use
-               -device for floppys, or Xen paravirt
+            /* Unfortunately it is not possible to use
+               -device for floppies, or Xen paravirt
                devices. Fortunately, those don't need
                static PCI addresses, so we don't really
                care that we can't use -device */
@@ -4648,24 +4557,23 @@ int qemudBuildCommandLine(virConnectPtr conn,
                                              (withDeviceArg ? qemuCmdFlags :
                                               (qemuCmdFlags & ~QEMUD_CMD_FLAG_DEVICE)))))
                 goto error;
-            ADD_ARG(optstr);
+            virCommandAddArg(cmd, optstr);
+            VIR_FREE(optstr);

             if (withDeviceArg) {
                 if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) {
-                    char *fdc;
-                    ADD_ARG_LIT("-global");
-
-                    if (virAsprintf(&fdc, "isa-fdc.drive%c=drive-%s",
-                                    disk->info.addr.drive.unit ? 'B' : 'A',
-                                    disk->info.alias) < 0)
-                        goto no_memory;
-                    ADD_ARG(fdc);
+                    virCommandAddArg(cmd, "-global");
+                    virCommandAddArgFormat(cmd, "isa-fdc.drive%c=drive-%s",
+                                           disk->info.addr.drive.unit
+                                           ? 'B' : 'A',
+                                           disk->info.alias);
                 } else {
-                    ADD_ARG_LIT("-device");
+                    virCommandAddArg(cmd, "-device");

                     if (!(optstr = qemuBuildDriveDevStr(disk)))
                         goto error;
-                    ADD_ARG(optstr);
+                    virCommandAddArg(cmd, optstr);
+                    VIR_FREE(optstr);
                 }
             }
         }
@@ -4677,10 +4585,12 @@ int qemudBuildCommandLine(virConnectPtr conn,

             if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
                 if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
-                    ADD_USBDISK(disk->src);
+                    virCommandAddArg(cmd, "-usbdevice");
+                    virCommandAddArgFormat(cmd, "disk:%s", disk->src);
                 } else {
                     qemuReportError(VIR_ERR_INTERNAL_ERROR,
-                                    _("unsupported usb disk type for '%s'"), disk->src);
+                                    _("unsupported usb disk type for '%s'"),
+                                    disk->src);
                     goto error;
                 }
                 continue;
@@ -4726,8 +4636,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
                 snprintf(file, PATH_MAX, "%s", disk->src);
             }

-            ADD_ARG_LIT(dev);
-            ADD_ARG_LIT(file);
+            virCommandAddArgList(cmd, dev, file, NULL);
         }
     }

@@ -4736,15 +4645,17 @@ int qemudBuildCommandLine(virConnectPtr conn,
             char *optstr;
             virDomainFSDefPtr fs = def->fss[i];

-            ADD_ARG_LIT("-fsdev");
+            virCommandAddArg(cmd, "-fsdev");
             if (!(optstr = qemuBuildFSStr(fs, qemuCmdFlags)))
                 goto error;
-            ADD_ARG(optstr);
+            virCommandAddArg(cmd, optstr);
+            VIR_FREE(optstr);

-            ADD_ARG_LIT("-device");
+            virCommandAddArg(cmd, "-device");
             if (!(optstr = qemuBuildFSDevStr(fs)))
                 goto error;
-            ADD_ARG(optstr);
+            virCommandAddArg(cmd, optstr);
+            VIR_FREE(optstr);
         }
     } else {
         if (def->nfss) {
@@ -4756,10 +4667,8 @@ int qemudBuildCommandLine(virConnectPtr conn,

     if (!def->nnets) {
         /* If we have -device, then we set -nodefault already */
-        if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
-            ADD_ARG_LIT("-net");
-            ADD_ARG_LIT("none");
-        }
+        if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE))
+            virCommandAddArgList(cmd, "-net", "none", NULL);
     } else {
         for (i = 0 ; i < def->nnets ; i++) {
             virDomainNetDefPtr net = def->nets[i];
@@ -4777,21 +4686,16 @@ int qemudBuildCommandLine(virConnectPtr conn,

             if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK ||
                 net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
-                int tapfd = qemudNetworkIfaceConnect(conn, driver, net, qemuCmdFlags);
+                int tapfd = qemudNetworkIfaceConnect(conn, driver, net,
+                                                     qemuCmdFlags);
                 if (tapfd < 0)
                     goto error;

-                if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) {
-                    virDomainConfNWFilterTeardown(net);
-                    VIR_FORCE_CLOSE(tapfd);
-                    goto no_memory;
-                }
-
                 last_good_net = i;
+                virCommandTransferFD(cmd, tapfd);

-                (*vmfds)[(*nvmfds)++] = tapfd;
-
-                if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", tapfd) >= sizeof(tapfd_name))
+                if (snprintf(tapfd_name, sizeof(tapfd_name), "%d",
+                             tapfd) >= sizeof(tapfd_name))
                     goto no_memory;
             } else if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
                 int tapfd = qemudPhysIfaceConnect(conn, driver, net,
@@ -4800,17 +4704,11 @@ int qemudBuildCommandLine(virConnectPtr conn,
                 if (tapfd < 0)
                     goto error;

-                if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) {
-                    virDomainConfNWFilterTeardown(net);
-                    VIR_FORCE_CLOSE(tapfd);
-                    goto no_memory;
-                }
-
                 last_good_net = i;
+                virCommandTransferFD(cmd, tapfd);

-                (*vmfds)[(*nvmfds)++] = tapfd;
-
-                if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", tapfd) >= sizeof(tapfd_name))
+                if (snprintf(tapfd_name, sizeof(tapfd_name), "%d",
+                             tapfd) >= sizeof(tapfd_name))
                     goto no_memory;
             }

@@ -4821,14 +4719,10 @@ int qemudBuildCommandLine(virConnectPtr conn,
                    network device */
                 int vhostfd = qemudOpenVhostNet(net, qemuCmdFlags);
                 if (vhostfd >= 0) {
-                    if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) {
-                        VIR_FORCE_CLOSE(vhostfd);
-                        goto no_memory;
-                    }
+                    virCommandTransferFD(cmd, vhostfd);

-                    (*vmfds)[(*nvmfds)++] = vhostfd;
-                    if (snprintf(vhostfd_name, sizeof(vhostfd_name), "%d", vhostfd)
-                        >= sizeof(vhostfd_name))
+                    if (snprintf(vhostfd_name, sizeof(vhostfd_name), "%d",
+                                 vhostfd) >= sizeof(vhostfd_name))
                         goto no_memory;
                 }
             }
@@ -4842,40 +4736,42 @@ int qemudBuildCommandLine(virConnectPtr conn,
              */
             if ((qemuCmdFlags & QEMUD_CMD_FLAG_NETDEV) &&
                 (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
-                ADD_ARG_LIT("-netdev");
+                virCommandAddArg(cmd, "-netdev");
                 if (!(host = qemuBuildHostNetStr(net, ',', vlan,
                                                  tapfd_name, vhostfd_name)))
                     goto error;
-                ADD_ARG(host);
+                virCommandAddArg(cmd, host);
+                VIR_FREE(host);
             }
             if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
-                ADD_ARG_LIT("-device");
+                virCommandAddArg(cmd, "-device");
                 if (!(nic = qemuBuildNicDevStr(net, vlan)))
                     goto error;
-                ADD_ARG(nic);
+                virCommandAddArg(cmd, nic);
+                VIR_FREE(nic);
             } else {
-                ADD_ARG_LIT("-net");
+                virCommandAddArg(cmd, "-net");
                 if (!(nic = qemuBuildNicStr(net, "nic,", vlan)))
                     goto error;
-                ADD_ARG(nic);
+                virCommandAddArg(cmd, nic);
+                VIR_FREE(nic);
             }
             if (!((qemuCmdFlags & QEMUD_CMD_FLAG_NETDEV) &&
                   (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE))) {
-                ADD_ARG_LIT("-net");
+                virCommandAddArg(cmd, "-net");
                 if (!(host = qemuBuildHostNetStr(net, ',', vlan,
                                                  tapfd_name, vhostfd_name)))
                     goto error;
-                ADD_ARG(host);
+                virCommandAddArg(cmd, host);
+                VIR_FREE(host);
             }
         }
     }

     if (!def->nserials) {
         /* If we have -device, then we set -nodefault already */
-        if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
-            ADD_ARG_LIT("-serial");
-            ADD_ARG_LIT("none");
-        }
+        if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE))
+            virCommandAddArgList(cmd, "-serial", "none", NULL);
     } else {
         for (i = 0 ; i < def->nserials ; i++) {
             virDomainChrDefPtr serial = def->serials[i];
@@ -4884,30 +4780,29 @@ int qemudBuildCommandLine(virConnectPtr conn,
             /* Use -chardev with -device if they are available */
             if ((qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) &&
                 (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
-                ADD_ARG_LIT("-chardev");
+                virCommandAddArg(cmd, "-chardev");
                 if (!(devstr = qemuBuildChrChardevStr(serial)))
                     goto error;
-                ADD_ARG(devstr);
+                virCommandAddArg(cmd, devstr);
+                VIR_FREE(devstr);

-                ADD_ARG_LIT("-device");
-                if (virAsprintf(&devstr, "isa-serial,chardev=%s", serial->info.alias) < 0)
-                    goto no_memory;
-                ADD_ARG(devstr);
+                virCommandAddArg(cmd, "-device");
+                virCommandAddArgFormat(cmd, "isa-serial,chardev=%s",
+                                       serial->info.alias);
             } else {
-                ADD_ARG_LIT("-serial");
+                virCommandAddArg(cmd, "-serial");
                 if (!(devstr = qemuBuildChrArgStr(serial, NULL)))
                     goto error;
-                ADD_ARG(devstr);
+                virCommandAddArg(cmd, devstr);
+                VIR_FREE(devstr);
             }
         }
     }

     if (!def->nparallels) {
         /* If we have -device, then we set -nodefault already */
-        if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
-            ADD_ARG_LIT("-parallel");
-            ADD_ARG_LIT("none");
-        }
+        if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE))
+            virCommandAddArgList(cmd, "-parallel", "none", NULL);
     } else {
         for (i = 0 ; i < def->nparallels ; i++) {
             virDomainChrDefPtr parallel = def->parallels[i];
@@ -4916,20 +4811,21 @@ int qemudBuildCommandLine(virConnectPtr conn,
             /* Use -chardev with -device if they are available */
             if ((qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) &&
                 (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
-                ADD_ARG_LIT("-chardev");
+                virCommandAddArg(cmd, "-chardev");
                 if (!(devstr = qemuBuildChrChardevStr(parallel)))
                     goto error;
-                ADD_ARG(devstr);
+                virCommandAddArg(cmd, devstr);
+                VIR_FREE(devstr);

-                ADD_ARG_LIT("-device");
-                if (virAsprintf(&devstr, "isa-parallel,chardev=%s", parallel->info.alias) < 0)
-                    goto no_memory;
-                ADD_ARG(devstr);
+                virCommandAddArg(cmd, "-device");
+                virCommandAddArgFormat(cmd, "isa-parallel,chardev=%s",
+                                       parallel->info.alias);
             } else {
-                ADD_ARG_LIT("-parallel");
+                virCommandAddArg(cmd, "-parallel");
                 if (!(devstr = qemuBuildChrArgStr(parallel, NULL)))
                       goto error;
-                ADD_ARG(devstr);
+                virCommandAddArg(cmd, devstr);
+                VIR_FREE(devstr);
             }
         }
     }
@@ -4947,24 +4843,23 @@ int qemudBuildCommandLine(virConnectPtr conn,
                 goto error;
             }

-            ADD_ARG_LIT("-chardev");
+            virCommandAddArg(cmd, "-chardev");
             if (!(devstr = qemuBuildChrChardevStr(channel)))
                 goto error;
-            ADD_ARG(devstr);
+            virCommandAddArg(cmd, devstr);
+            VIR_FREE(devstr);

             char *addr = virSocketFormatAddr(channel->target.addr);
             if (!addr)
                 goto error;
             int port = virSocketGetPort(channel->target.addr);

-            ADD_ARG_LIT("-netdev");
-            if (virAsprintf(&devstr, "user,guestfwd=tcp:%s:%i,chardev=%s,id=user-%s",
-                            addr, port, channel->info.alias, channel->info.alias) < 0) {
-                VIR_FREE(addr);
-                goto no_memory;
-            }
+            virCommandAddArg(cmd, "-netdev");
+            virCommandAddArgFormat(cmd,
+                                   "user,guestfwd=tcp:%s:%i,chardev=%s,id=user-%s",
+                                   addr, port, channel->info.alias,
+                                   channel->info.alias);
             VIR_FREE(addr);
-            ADD_ARG(devstr);
             break;

         case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
@@ -4974,15 +4869,17 @@ int qemudBuildCommandLine(virConnectPtr conn,
                 goto error;
             }

-            ADD_ARG_LIT("-chardev");
+            virCommandAddArg(cmd, "-chardev");
             if (!(devstr = qemuBuildChrChardevStr(channel)))
                 goto error;
-            ADD_ARG(devstr);
+            virCommandAddArg(cmd, devstr);
+            VIR_FREE(devstr);

-            ADD_ARG_LIT("-device");
+            virCommandAddArg(cmd, "-device");
             if (!(devstr = qemuBuildVirtioSerialPortDevStr(channel)))
                 goto error;
-            ADD_ARG(devstr);
+            virCommandAddArg(cmd, devstr);
+            VIR_FREE(devstr);
             break;
         }
     }
@@ -5000,15 +4897,17 @@ int qemudBuildCommandLine(virConnectPtr conn,
                 goto error;
             }

-            ADD_ARG_LIT("-chardev");
+            virCommandAddArg(cmd, "-chardev");
             if (!(devstr = qemuBuildChrChardevStr(console)))
                 goto error;
-            ADD_ARG(devstr);
+            virCommandAddArg(cmd, devstr);
+            VIR_FREE(devstr);

-            ADD_ARG_LIT("-device");
+            virCommandAddArg(cmd, "-device");
             if (!(devstr = qemuBuildVirtioSerialPortDevStr(console)))
                 goto error;
-            ADD_ARG(devstr);
+            virCommandAddArg(cmd, devstr);
+            VIR_FREE(devstr);
             break;

         case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL:
@@ -5022,20 +4921,22 @@ int qemudBuildCommandLine(virConnectPtr conn,
         }
     }

-    ADD_ARG_LIT("-usb");
+    virCommandAddArg(cmd, "-usb");
     for (i = 0 ; i < def->ninputs ; i++) {
         virDomainInputDefPtr input = def->inputs[i];

         if (input->bus == VIR_DOMAIN_INPUT_BUS_USB) {
             if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
                 char *optstr;
-                ADD_ARG_LIT("-device");
+                virCommandAddArg(cmd, "-device");
                 if (!(optstr = qemuBuildUSBInputDevStr(input)))
                     goto error;
-                ADD_ARG(optstr);
+                virCommandAddArg(cmd, optstr);
+                VIR_FREE(optstr);
             } else {
-                ADD_ARG_LIT("-usbdevice");
-                ADD_ARG_LIT(input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ? "mouse" : "tablet");
+                virCommandAddArgList(cmd, "-usbdevice",
+                                     input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE
+                                     ? "mouse" : "tablet", NULL);
             }
         }
     }
@@ -5079,7 +4980,8 @@ int qemudBuildCommandLine(virConnectPtr conn,
                 virBufferAddLit(&opt, ",sasl");

                 if (driver->vncSASLdir)
-                    ADD_ENV_PAIR("SASL_CONF_DIR", driver->vncSASLdir);
+                    virCommandAddEnvPair(cmd, "SASL_CONF_DIR",
+                                         driver->vncSASLdir);

                 /* TODO: Support ACLs later */
             }
@@ -5094,11 +4996,11 @@ int qemudBuildCommandLine(virConnectPtr conn,

         optstr = virBufferContentAndReset(&opt);

-        ADD_ARG_LIT("-vnc");
-        ADD_ARG(optstr);
+        virCommandAddArgList(cmd, "-vnc", optstr, NULL);
+        VIR_FREE(optstr);
         if (def->graphics[0]->data.vnc.keymap) {
-            ADD_ARG_LIT("-k");
-            ADD_ARG_LIT(def->graphics[0]->data.vnc.keymap);
+            virCommandAddArgList(cmd, "-k", def->graphics[0]->data.vnc.keymap,
+                                 NULL);
         }

         /* Unless user requested it, set the audio backend to none, to
@@ -5106,45 +5008,33 @@ int qemudBuildCommandLine(virConnectPtr conn,
          * security issues and might not work when using VNC.
          */
         if (driver->vncAllowHostAudio) {
-            ADD_ENV_COPY("QEMU_AUDIO_DRV");
+            virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
         } else {
-            ADD_ENV_LIT("QEMU_AUDIO_DRV=none");
+            virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none");
         }
     } else if ((def->ngraphics == 1) &&
                def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) {
-        char *xauth = NULL;
-        char *display = NULL;
-
-        if (def->graphics[0]->data.sdl.xauth &&
-            virAsprintf(&xauth, "XAUTHORITY=%s",
-                        def->graphics[0]->data.sdl.xauth) < 0)
-            goto no_memory;
-        if (def->graphics[0]->data.sdl.display &&
-            virAsprintf(&display, "DISPLAY=%s",
-                        def->graphics[0]->data.sdl.display) < 0) {
-            VIR_FREE(xauth);
-            goto no_memory;
-        }
-
-        if (xauth)
-            ADD_ENV(xauth);
-        if (display)
-            ADD_ENV(display);
+        if (def->graphics[0]->data.sdl.xauth)
+            virCommandAddEnvPair(cmd, "XAUTHORITY",
+                                 def->graphics[0]->data.sdl.xauth);
+        if (def->graphics[0]->data.sdl.display)
+            virCommandAddEnvPair(cmd, "DISPLAY",
+                                 def->graphics[0]->data.sdl.display);
         if (def->graphics[0]->data.sdl.fullscreen)
-            ADD_ARG_LIT("-full-screen");
+            virCommandAddArg(cmd, "-full-screen");

         /* If using SDL for video, then we should just let it
          * use QEMU's host audio drivers, possibly SDL too
          * User can set these two before starting libvirtd
          */
-        ADD_ENV_COPY("QEMU_AUDIO_DRV");
-        ADD_ENV_COPY("SDL_AUDIODRIVER");
+        virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
+        virCommandAddEnvPass(cmd, "SDL_AUDIODRIVER");

         /* New QEMU has this flag to let us explicitly ask for
          * SDL graphics. This is better than relying on the
          * default, since the default changes :-( */
         if (qemuCmdFlags & QEMUD_CMD_FLAG_SDL)
-            ADD_ARG_LIT("-sdl");
+            virCommandAddArg(cmd, "-sdl");

     } else if ((def->ngraphics == 1) &&
                def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
@@ -5197,16 +5087,15 @@ int qemudBuildCommandLine(virConnectPtr conn,

         optstr = virBufferContentAndReset(&opt);

-        ADD_ARG_LIT("-spice");
-        ADD_ARG(optstr);
-        if (def->graphics[0]->data.spice.keymap) {
-            ADD_ARG_LIT("-k");
-            ADD_ARG_LIT(def->graphics[0]->data.spice.keymap);
-        }
+        virCommandAddArgList(cmd, "-spice", optstr, NULL);
+        VIR_FREE(optstr);
+        if (def->graphics[0]->data.spice.keymap)
+            virCommandAddArgList(cmd, "-k",
+                                 def->graphics[0]->data.spice.keymap, NULL);
         /* SPICE includes native support for tunnelling audio, so we
          * set the audio backend to point at SPICE's own driver
          */
-        ADD_ENV_LIT("QEMU_AUDIO_DRV=spice");
+        virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice");

     } else if ((def->ngraphics == 1)) {
         qemuReportError(VIR_ERR_INTERNAL_ERROR,
@@ -5235,18 +5124,17 @@ int qemudBuildCommandLine(virConnectPtr conn,
                     goto error;
                 }

-                ADD_ARG_LIT("-vga");
-                ADD_ARG_LIT(vgastr);
+                virCommandAddArgList(cmd, "-vga", vgastr, NULL);
             }
         } else {

             switch (def->videos[0]->type) {
             case VIR_DOMAIN_VIDEO_TYPE_VGA:
-                ADD_ARG_LIT("-std-vga");
+                virCommandAddArg(cmd, "-std-vga");
                 break;

             case VIR_DOMAIN_VIDEO_TYPE_VMVGA:
-                ADD_ARG_LIT("-vmwarevga");
+                virCommandAddArg(cmd, "-vmwarevga");
                 break;

             case VIR_DOMAIN_VIDEO_TYPE_XEN:
@@ -5273,12 +5161,13 @@ int qemudBuildCommandLine(virConnectPtr conn,
                         goto error;
                     }

-                    ADD_ARG_LIT("-device");
+                    virCommandAddArg(cmd, "-device");

                     if (!(str = qemuBuildVideoDevStr(def->videos[i])))
                         goto error;

-                    ADD_ARG(str);
+                    virCommandAddArg(cmd, str);
+                    VIR_FREE(str);
                 }
             } else {
                 qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -5290,10 +5179,8 @@ int qemudBuildCommandLine(virConnectPtr conn,
     } else {
         /* If we have -device, then we set -nodefault already */
         if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) &&
-            (qemuCmdFlags & QEMUD_CMD_FLAG_VGA)) {
-            ADD_ARG_LIT("-vga");
-            ADD_ARG_LIT("none");
-        }
+            (qemuCmdFlags & QEMUD_CMD_FLAG_VGA))
+            virCommandAddArgList(cmd, "-vga", "none", NULL);
     }

     /* Add sound hardware */
@@ -5307,15 +5194,15 @@ int qemudBuildCommandLine(virConnectPtr conn,
                  * we don't need to set any PCI address on it, so we don't
                  * mind too much */
                 if (sound->model == VIR_DOMAIN_SOUND_MODEL_PCSPK) {
-                    ADD_ARG_LIT("-soundhw");
-                    ADD_ARG_LIT("pcspk");
+                    virCommandAddArgList(cmd, "-soundhw", "pcspk", NULL);
                 } else {
-                    ADD_ARG_LIT("-device");
+                    virCommandAddArg(cmd, "-device");

                     if (!(str = qemuBuildSoundDevStr(sound)))
                         goto error;

-                    ADD_ARG(str);
+                    virCommandAddArg(cmd, str);
+                    VIR_FREE(str);
                 }
             }
         } else {
@@ -5338,8 +5225,8 @@ int qemudBuildCommandLine(virConnectPtr conn,
                 if (i < (def->nsounds - 1))
                     strncat(modstr, ",", size--);
             }
-            ADD_ARG_LIT("-soundhw");
-            ADD_ARG(modstr);
+            virCommandAddArgList(cmd, "-soundhw", modstr, NULL);
+            VIR_FREE(modstr);
         }
     }

@@ -5349,13 +5236,13 @@ int qemudBuildCommandLine(virConnectPtr conn,
         char *optstr;

         if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
-            ADD_ARG_LIT("-device");
+            virCommandAddArg(cmd, "-device");

             optstr = qemuBuildWatchdogDevStr(watchdog);
             if (!optstr)
                 goto error;
         } else {
-            ADD_ARG_LIT("-watchdog");
+            virCommandAddArg(cmd, "-watchdog");

             const char *model = virDomainWatchdogModelTypeToString(watchdog->model);
             if (!model) {
@@ -5367,7 +5254,8 @@ int qemudBuildCommandLine(virConnectPtr conn,
             if (!(optstr = strdup(model)))
                 goto no_memory;
         }
-        ADD_ARG(optstr);
+        virCommandAddArg(cmd, optstr);
+        VIR_FREE(optstr);

         const char *action = virDomainWatchdogActionTypeToString(watchdog->action);
         if (!action) {
@@ -5375,8 +5263,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
                             "%s", _("invalid watchdog action"));
             goto error;
         }
-        ADD_ARG_LIT("-watchdog-action");
-        ADD_ARG_LIT(action);
+        virCommandAddArgList(cmd, "-watchdog-action", action, NULL);
     }

     /* Add host passthrough hardware */
@@ -5389,15 +5276,17 @@ int qemudBuildCommandLine(virConnectPtr conn,
             hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {

             if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
-                ADD_ARG_LIT("-device");
+                virCommandAddArg(cmd, "-device");
                 if (!(devstr = qemuBuildUSBHostdevDevStr(hostdev)))
                     goto error;
-                ADD_ARG(devstr);
+                virCommandAddArg(cmd, devstr);
+                VIR_FREE(devstr);
             } else {
-                ADD_ARG_LIT("-usbdevice");
+                virCommandAddArg(cmd, "-usbdevice");
                 if (!(devstr = qemuBuildUSBHostdevUsbDevStr(hostdev)))
                     goto error;
-                ADD_ARG(devstr);
+                virCommandAddArg(cmd, devstr);
+                VIR_FREE(devstr);
             }
         }

@@ -5416,26 +5305,22 @@ int qemudBuildCommandLine(virConnectPtr conn,
                             goto no_memory;
                         }

-                        if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) {
-                            VIR_FREE(configfd_name);
-                            VIR_FORCE_CLOSE(configfd);
-                            goto no_memory;
-                        }
-
-                        (*vmfds)[(*nvmfds)++] = configfd;
+                        virCommandTransferFD(cmd, configfd);
                     }
                 }
-                ADD_ARG_LIT("-device");
+                virCommandAddArg(cmd, "-device");
                 devstr = qemuBuildPCIHostdevDevStr(hostdev, configfd_name);
                 VIR_FREE(configfd_name);
                 if (!devstr)
                     goto error;
-                ADD_ARG(devstr);
+                virCommandAddArg(cmd, devstr);
+                VIR_FREE(devstr);
             } else if (qemuCmdFlags & QEMUD_CMD_FLAG_PCIDEVICE) {
-                ADD_ARG_LIT("-pcidevice");
+                virCommandAddArg(cmd, "-pcidevice");
                 if (!(devstr = qemuBuildPCIHostdevPCIDevStr(hostdev)))
                     goto error;
-                ADD_ARG(devstr);
+                virCommandAddArg(cmd, devstr);
+                VIR_FREE(devstr);
             } else {
                 qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                                 _("PCI device assignment is not supported by this version of qemu"));
@@ -5444,10 +5329,8 @@ int qemudBuildCommandLine(virConnectPtr conn,
         }
     }

-    if (migrateFrom) {
-        ADD_ARG_LIT("-incoming");
-        ADD_ARG_LIT(migrateFrom);
-    }
+    if (migrateFrom)
+        virCommandAddArgList(cmd, "-incoming", migrateFrom, NULL);

     /* QEMU changed its default behavior to not include the virtio balloon
      * device.  Explicitly request it to ensure it will be present.
@@ -5465,76 +5348,43 @@ int qemudBuildCommandLine(virConnectPtr conn,
         }
         if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
             char *optstr;
-            ADD_ARG_LIT("-device");
+            virCommandAddArg(cmd, "-device");

             optstr = qemuBuildMemballoonDevStr(def->memballoon);
             if (!optstr)
                 goto error;
-            ADD_ARG(optstr);
+            virCommandAddArg(cmd, optstr);
+            VIR_FREE(optstr);
         } else if (qemuCmdFlags & QEMUD_CMD_FLAG_BALLOON) {
-            ADD_ARG_LIT("-balloon");
-            ADD_ARG_LIT("virtio");
+            virCommandAddArgList(cmd, "-balloon", "virtio", NULL);
         }
     }

-    if (current_snapshot && current_snapshot->def->active) {
-        ADD_ARG_LIT("-loadvm");
-        ADD_ARG_LIT(current_snapshot->def->name);
-    }
+    if (current_snapshot && current_snapshot->def->active)
+        virCommandAddArgList(cmd, "-loadvm", current_snapshot->def->name,
+                             NULL);

     if (def->namespaceData) {
-        qemuDomainCmdlineDefPtr cmd;
-
-        cmd = def->namespaceData;
-        for (i = 0; i < cmd->num_args; i++)
-            ADD_ARG_LIT(cmd->args[i]);
-        for (i = 0; i < cmd->num_env; i++) {
-            if (cmd->env_value[i])
-                ADD_ENV_PAIR(cmd->env_name[i], cmd->env_value[i]);
-            else
-                ADD_ENV_PAIR(cmd->env_name[i], "");
-        }
-    }
+        qemuDomainCmdlineDefPtr qemucmd;

-    ADD_ARG(NULL);
-    ADD_ENV(NULL);
+        qemucmd = def->namespaceData;
+        for (i = 0; i < qemucmd->num_args; i++)
+            virCommandAddArg(cmd, qemucmd->args[i]);
+        for (i = 0; i < qemucmd->num_env; i++)
+            virCommandAddEnvPair(cmd, qemucmd->env_name[i],
+                                 qemucmd->env_value[i]
+                                 ? qemucmd->env_value[i] : "");
+    }

-    *retargv = qargv;
-    *retenv = qenv;
-    return 0;
+    return cmd;

  no_memory:
     virReportOOMError();
  error:
     for (i = 0; i <= last_good_net; i++)
         virDomainConfNWFilterTeardown(def->nets[i]);
-    if (vmfds &&
-        *vmfds) {
-        for (i = 0; i < *nvmfds; i++)
-            VIR_FORCE_CLOSE((*vmfds)[i]);
-        VIR_FREE(*vmfds);
-        *nvmfds = 0;
-    }
-    if (qargv) {
-        for (i = 0 ; i < qargc ; i++)
-            VIR_FREE((qargv)[i]);
-        VIR_FREE(qargv);
-    }
-    if (qenv) {
-        for (i = 0 ; i < qenvc ; i++)
-            VIR_FREE((qenv)[i]);
-        VIR_FREE(qenv);
-    }
-    return -1;
-
-#undef ADD_ARG
-#undef ADD_ARG_LIT
-#undef ADD_ARG_SPACE
-#undef ADD_USBDISK
-#undef ADD_ENV
-#undef ADD_ENV_COPY
-#undef ADD_ENV_LIT
-#undef ADD_ENV_SPACE
+    virCommandFree(cmd);
+    return NULL;
 }


diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 790ce98..24df871 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -25,6 +25,7 @@
 # define __QEMUD_CONF_H

 # include <config.h>
+# include <stdbool.h>

 # include "ebtables.h"
 # include "internal.h"
@@ -40,6 +41,7 @@
 # include "cpu_conf.h"
 # include "driver.h"
 # include "bitmap.h"
+# include "command.h"

 # define qemudDebug(fmt, ...) do {} while(0)

@@ -227,16 +229,12 @@ int         qemudParseHelpStr           (const char *qemu,
                                          unsigned int *is_kvm,
                                          unsigned int *kvm_version);

-int         qemudBuildCommandLine       (virConnectPtr conn,
+virCommandPtr qemudBuildCommandLine     (virConnectPtr conn,
                                          struct qemud_driver *driver,
                                          virDomainDefPtr def,
                                          virDomainChrDefPtr monitor_chr,
-                                         int monitor_json,
+                                         bool monitor_json,
                                          unsigned long long qemuCmdFlags,
-                                         const char ***retargv,
-                                         const char ***retenv,
-                                         int **vmfds,
-                                         int *nvmfds,
                                          const char *migrateFrom,
                                          virDomainSnapshotObjPtr current_snapshot)
     ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f00d8a3..38d7bd2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3018,14 +3018,6 @@ qemuAssignPCIAddresses(virDomainDefPtr def)
     int ret = -1;
     unsigned long long qemuCmdFlags = 0;
     qemuDomainPCIAddressSetPtr addrs = NULL;
-    struct stat sb;
-
-    if (stat(def->emulator, &sb) < 0) {
-        virReportSystemError(errno,
-                             _("Cannot find QEMU binary %s"),
-                             def->emulator);
-        goto cleanup;
-    }

     if (qemudExtractVersionInfo(def->emulator,
                                 NULL,
@@ -3865,30 +3857,21 @@ static int qemudStartVMDaemon(virConnectPtr conn,
                               bool start_paused,
                               int stdin_fd,
                               const char *stdin_path) {
-    const char **argv = NULL, **tmp;
-    const char **progenv = NULL;
-    int i, ret, runflags;
-    struct stat sb;
-    int *vmfds = NULL;
-    int nvmfds = 0;
+    int ret;
     unsigned long long qemuCmdFlags;
-    fd_set keepfd;
-    const char *emulator;
-    pid_t child;
     int pos = -1;
     char ebuf[1024];
     char *pidfile = NULL;
     int logfile = -1;
     char *timestamp;
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    virCommandPtr cmd = NULL;

     struct qemudHookData hookData;
     hookData.conn = conn;
     hookData.vm = vm;
     hookData.driver = driver;

-    FD_ZERO(&keepfd);
-
     DEBUG0("Beginning VM startup process");

     if (virDomainObjIsActive(vm)) {
@@ -3979,21 +3962,8 @@ static int qemudStartVMDaemon(virConnectPtr conn,
     if ((logfile = qemudLogFD(driver, vm->def->name, false)) < 0)
         goto cleanup;

-    emulator = vm->def->emulator;
-
-    /* Make sure the binary we are about to try exec'ing exists.
-     * Technically we could catch the exec() failure, but that's
-     * in a sub-process so its hard to feed back a useful error
-     */
-    if (stat(emulator, &sb) < 0) {
-        virReportSystemError(errno,
-                             _("Cannot find QEMU binary %s"),
-                             emulator);
-        goto cleanup;
-    }
-
-    DEBUG0("Determing emulator version");
-    if (qemudExtractVersionInfo(emulator,
+    DEBUG0("Determining emulator version");
+    if (qemudExtractVersionInfo(vm->def->emulator,
                                 NULL,
                                 &qemuCmdFlags) < 0)
         goto cleanup;
@@ -4062,10 +4032,10 @@ static int qemudStartVMDaemon(virConnectPtr conn,

     DEBUG0("Building emulator command line");
     vm->def->id = driver->nextvmid++;
-    if (qemudBuildCommandLine(conn, driver, vm->def, priv->monConfig,
-                              priv->monJSON, qemuCmdFlags, &argv, &progenv,
-                              &vmfds, &nvmfds, migrateFrom,
-                              vm->current_snapshot) < 0)
+    if (!(cmd = qemudBuildCommandLine(conn, driver, vm->def, priv->monConfig,
+                                      priv->monJSON != 0, qemuCmdFlags,
+                                      migrateFrom,
+                                      vm->current_snapshot)))
         goto cleanup;

     if (qemuDomainSnapshotSetInactive(vm, driver->snapshotDir) < 0)
@@ -4100,50 +4070,28 @@ static int qemudStartVMDaemon(virConnectPtr conn,
         VIR_FREE(timestamp);
     }

-    tmp = progenv;
-
-    while (*tmp) {
-        if (safewrite(logfile, *tmp, strlen(*tmp)) < 0)
-            VIR_WARN("Unable to write envv to logfile: %s",
-                     virStrerror(errno, ebuf, sizeof ebuf));
-        if (safewrite(logfile, " ", 1) < 0)
-            VIR_WARN("Unable to write envv to logfile: %s",
-                     virStrerror(errno, ebuf, sizeof ebuf));
-        tmp++;
-    }
-    tmp = argv;
-    while (*tmp) {
-        if (safewrite(logfile, *tmp, strlen(*tmp)) < 0)
-            VIR_WARN("Unable to write argv to logfile: %s",
-                     virStrerror(errno, ebuf, sizeof ebuf));
-        if (safewrite(logfile, " ", 1) < 0)
-            VIR_WARN("Unable to write argv to logfile: %s",
-                     virStrerror(errno, ebuf, sizeof ebuf));
-        tmp++;
-    }
-    if (safewrite(logfile, "\n", 1) < 0)
-        VIR_WARN("Unable to write argv to logfile: %s",
-                 virStrerror(errno, ebuf, sizeof ebuf));
+    virCommandWriteArgLog(cmd, logfile);

     if ((pos = lseek(logfile, 0, SEEK_END)) < 0)
         VIR_WARN("Unable to seek to end of logfile: %s",
                  virStrerror(errno, ebuf, sizeof ebuf));

-    for (i = 0 ; i < nvmfds ; i++)
-        FD_SET(vmfds[i], &keepfd);
-
     VIR_DEBUG("Clear emulator capabilities: %d",
               driver->clearEmulatorCapabilities);
-    runflags = VIR_EXEC_NONBLOCK;
-    if (driver->clearEmulatorCapabilities) {
-        runflags |= VIR_EXEC_CLEAR_CAPS;
-    }
-
-    ret = virExecDaemonize(argv, progenv, &keepfd, &child,
-                           stdin_fd, &logfile, &logfile,
-                           runflags,
-                           qemudSecurityHook, &hookData,
-                           pidfile);
+    if (driver->clearEmulatorCapabilities)
+        virCommandClearCaps(cmd);
+
+    VIR_WARN("Executing %s", vm->def->emulator);
+    virCommandSetPreExecHook(cmd, qemudSecurityHook, &hookData);
+    virCommandSetInputFD(cmd, stdin_fd);
+    virCommandSetOutputFD(cmd, &logfile);
+    virCommandSetErrorFD(cmd, &logfile);
+    virCommandNonblockingFDs(cmd);
+    virCommandSetPidFile(cmd, pidfile);
+    virCommandDaemonize(cmd);
+
+    ret = virCommandRun(cmd, NULL);
+    VIR_WARN("Executing done %s", vm->def->emulator);
     VIR_FREE(pidfile);

     /* wait for qemu process to to show up */
@@ -4153,7 +4101,13 @@ static int qemudStartVMDaemon(virConnectPtr conn,
                             _("Domain %s didn't show up\n"), vm->def->name);
             ret = -1;
         }
+#if 0
     } else if (ret == -2) {
+        /*
+         * XXX this is bogus. It isn't safe to set vm->pid = child
+         * because the child no longer exists.
+         */
+
         /* The virExec process that launches the daemon failed. Pending on
          * when it failed (we can't determine for sure), there may be
          * extra info in the domain log (if the hook failed for example).
@@ -4163,30 +4117,16 @@ static int qemudStartVMDaemon(virConnectPtr conn,
          */
         vm->pid = child;
         ret = 0;
+#endif
     }

     if (migrateFrom)
         start_paused = true;
     vm->state = start_paused ? VIR_DOMAIN_PAUSED : VIR_DOMAIN_RUNNING;

-    for (i = 0 ; argv[i] ; i++)
-        VIR_FREE(argv[i]);
-    VIR_FREE(argv);
-
-    for (i = 0 ; progenv[i] ; i++)
-        VIR_FREE(progenv[i]);
-    VIR_FREE(progenv);
-
     if (ret == -1) /* The VM failed to start; tear filters before taps */
         virDomainConfVMNWFilterTeardown(vm);

-    if (vmfds) {
-        for (i = 0 ; i < nvmfds ; i++) {
-            VIR_FORCE_CLOSE(vmfds[i]);
-        }
-        VIR_FREE(vmfds);
-    }
-
     if (ret == -1) /* The VM failed to start */
         goto cleanup;

@@ -4240,6 +4180,7 @@ static int qemudStartVMDaemon(virConnectPtr conn,
     if (virDomainObjSetDefTransient(driver->caps, vm) < 0)
         goto cleanup;

+    virCommandFree(cmd);
     VIR_FORCE_CLOSE(logfile);

     return 0;
@@ -4248,9 +4189,9 @@ cleanup:
     /* We jump here if we failed to start the VM for any reason, or
      * if we failed to initialize the now running VM. kill it off and
      * pretend we never started it */
-    qemudShutdownVMDaemon(driver, vm, 0);
-
+    virCommandFree(cmd);
     VIR_FORCE_CLOSE(logfile);
+    qemudShutdownVMDaemon(driver, vm, 0);

     return -1;
 }
@@ -7312,13 +7253,8 @@ static char *qemuDomainXMLToNative(virConnectPtr conn,
     struct qemud_driver *driver = conn->privateData;
     virDomainDefPtr def = NULL;
     virDomainChrDef monConfig;
-    const char *emulator;
     unsigned long long qemuCmdFlags;
-    struct stat sb;
-    const char **retargv = NULL;
-    const char **retenv = NULL;
-    const char **tmp;
-    virBuffer buf = VIR_BUFFER_INITIALIZER;
+    virCommandPtr cmd = NULL;
     char *ret = NULL;
     int i;

@@ -7368,70 +7304,26 @@ static char *qemuDomainXMLToNative(virConnectPtr conn,
             def->graphics[i]->data.vnc.autoport)
             def->graphics[i]->data.vnc.port = 5900;
     }
-    emulator = def->emulator;
-
-    /* Make sure the binary we are about to try exec'ing exists.
-     * Technically we could catch the exec() failure, but that's
-     * in a sub-process so its hard to feed back a useful error
-     */
-    if (stat(emulator, &sb) < 0) {
-        virReportSystemError(errno,
-                             _("Cannot find QEMU binary %s"),
-                             emulator);
-        goto cleanup;
-    }

-    if (qemudExtractVersionInfo(emulator,
+    if (qemudExtractVersionInfo(def->emulator,
                                 NULL,
-                                &qemuCmdFlags) < 0) {
-        qemuReportError(VIR_ERR_INTERNAL_ERROR,
-                        _("Cannot determine QEMU argv syntax %s"),
-                        emulator);
+                                &qemuCmdFlags) < 0)
         goto cleanup;
-    }

     if (qemuPrepareMonitorChr(driver, &monConfig, def->name) < 0)
         goto cleanup;

-    if (qemudBuildCommandLine(conn, driver, def,
-                              &monConfig, 0, qemuCmdFlags,
-                              &retargv, &retenv,
-                              NULL, NULL, /* Don't want it to create TAP devices */
-                              NULL, NULL) < 0) {
-        goto cleanup;
-    }
-
-    tmp = retenv;
-    while (*tmp) {
-        virBufferAdd(&buf, *tmp, strlen(*tmp));
-        virBufferAddLit(&buf, " ");
-        tmp++;
-    }
-    tmp = retargv;
-    while (*tmp) {
-        virBufferAdd(&buf, *tmp, strlen(*tmp));
-        virBufferAddLit(&buf, " ");
-        tmp++;
-    }
-
-    if (virBufferError(&buf)) {
-        virBufferFreeAndReset(&buf);
-        virReportOOMError();
+    if (!(cmd = qemudBuildCommandLine(conn, driver, def,
+                                      &monConfig, false, qemuCmdFlags,
+                                      NULL, NULL)))
         goto cleanup;
-    }

-    ret = virBufferContentAndReset(&buf);
+    ret = virCommandToString(cmd);

 cleanup:
     qemuDriverUnlock(driver);
-    for (tmp = retargv ; tmp && *tmp ; tmp++)
-        VIR_FREE(*tmp);
-    VIR_FREE(retargv);
-
-    for (tmp = retenv ; tmp && *tmp ; tmp++)
-        VIR_FREE(*tmp);
-    VIR_FREE(retenv);

+    virCommandFree(cmd);
     virDomainDefFree(def);
     return ret;
 }
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
index 947a6d8..55b3ef6 100644
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -308,7 +308,7 @@ error:
 static char *
 umlBuildCommandLineChr(virDomainChrDefPtr def,
                        const char *dev,
-                       fd_set *keepfd)
+                       virCommandPtr cmd)
 {
     char *ret = NULL;

@@ -372,7 +372,7 @@ umlBuildCommandLineChr(virDomainChrDefPtr def,
                 VIR_FORCE_CLOSE(fd_out);
                 return NULL;
             }
-            FD_SET(fd_out, keepfd);
+            virCommandTransferFD(cmd, fd_out);
         }
         break;
    case VIR_DOMAIN_CHR_TYPE_PIPE:
@@ -425,20 +425,17 @@ virCommandPtr umlBuildCommandLine(virConnectPtr conn,
                                   virDomainObjPtr vm)
 {
     int i, j;
-    char memory[50];
     struct utsname ut;
     virCommandPtr cmd;

     uname(&ut);

-    snprintf(memory, sizeof(memory), "%luK", vm->def->mem.cur_balloon);
-
     cmd = virCommandNew(vm->def->os.kernel);

     virCommandAddEnvPassCommon(cmd);

     //virCommandAddArgPair(cmd, "con0", "fd:0,fd:1");
-    virCommandAddArgPair(cmd, "mem", memory);
+    virCommandAddArgFormat(cmd, "mem=%luK", vm->def->mem.cur_balloon);
     virCommandAddArgPair(cmd, "umid", vm->def->name);
     virCommandAddArgPair(cmd, "uml_dir", driver->monitorDir);

@@ -468,7 +465,7 @@ virCommandPtr umlBuildCommandLine(virConnectPtr conn,
     for (i = 0 ; i < UML_MAX_CHAR_DEVICE ; i++) {
         char *ret = NULL;
         if (i == 0 && vm->def->console)
-            ret = umlBuildCommandLineChr(vm->def->console, "con", keepfd);
+            ret = umlBuildCommandLineChr(vm->def->console, "con", cmd);
         if (!ret)
             if (virAsprintf(&ret, "con%d=none", i) < 0)
                 goto no_memory;
@@ -483,7 +480,7 @@ virCommandPtr umlBuildCommandLine(virConnectPtr conn,
             if (vm->def->serials[j]->target.port == i)
                 chr = vm->def->serials[j];
         if (chr)
-            ret = umlBuildCommandLineChr(chr, "ssl", keepfd);
+            ret = umlBuildCommandLineChr(chr, "ssl", cmd);
         if (!ret)
             if (virAsprintf(&ret, "ssl%d=none", i) < 0)
                 goto no_memory;
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 6c28c76..546e960 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -812,18 +812,11 @@ static int umlCleanupTapDevices(virConnectPtr conn ATTRIBUTE_UNUSED,
 static int umlStartVMDaemon(virConnectPtr conn,
                             struct uml_driver *driver,
                             virDomainObjPtr vm) {
-    const char **argv = NULL, **tmp;
-    const char **progenv = NULL;
-    int i, ret;
-    pid_t pid;
+    int ret;
     char *logfile;
     int logfd = -1;
-    struct stat sb;
-    fd_set keepfd;
-    char ebuf[1024];
     umlDomainObjPrivatePtr priv = vm->privateData;
-
-    FD_ZERO(&keepfd);
+    virCommandPtr cmd = NULL;

     if (virDomainObjIsActive(vm)) {
         umlReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -840,7 +833,7 @@ static int umlStartVMDaemon(virConnectPtr conn,
      * Technically we could catch the exec() failure, but that's
      * in a sub-process so its hard to feed back a useful error
      */
-    if (stat(vm->def->os.kernel, &sb) < 0) {
+    if (access(vm->def->os.kernel, X_OK) < 0) {
         virReportSystemError(errno,
                              _("Cannot find UML kernel %s"),
                              vm->def->os.kernel);
@@ -877,67 +870,30 @@ static int umlStartVMDaemon(virConnectPtr conn,
         return -1;
     }

-    if (umlBuildCommandLine(conn, driver, vm, &keepfd,
-                            &argv, &progenv) < 0) {
+    if (!(cmd = umlBuildCommandLine(conn, driver, vm))) {
         VIR_FORCE_CLOSE(logfd);
         virDomainConfVMNWFilterTeardown(vm);
         umlCleanupTapDevices(conn, vm);
         return -1;
     }

-    tmp = progenv;
-    while (*tmp) {
-        if (safewrite(logfd, *tmp, strlen(*tmp)) < 0)
-            VIR_WARN("Unable to write envv to logfile: %s",
-                   virStrerror(errno, ebuf, sizeof ebuf));
-        if (safewrite(logfd, " ", 1) < 0)
-            VIR_WARN("Unable to write envv to logfile: %s",
-                   virStrerror(errno, ebuf, sizeof ebuf));
-        tmp++;
-    }
-    tmp = argv;
-    while (*tmp) {
-        if (safewrite(logfd, *tmp, strlen(*tmp)) < 0)
-            VIR_WARN("Unable to write argv to logfile: %s",
-                   virStrerror(errno, ebuf, sizeof ebuf));
-        if (safewrite(logfd, " ", 1) < 0)
-            VIR_WARN("Unable to write argv to logfile: %s",
-                   virStrerror(errno, ebuf, sizeof ebuf));
-        tmp++;
-    }
-    if (safewrite(logfd, "\n", 1) < 0)
-        VIR_WARN("Unable to write argv to logfile: %s",
-                 virStrerror(errno, ebuf, sizeof ebuf));
+    virCommandWriteArgLog(cmd, logfd);

     priv->monitor = -1;

-    ret = virExecDaemonize(argv, progenv, &keepfd, &pid,
-                           -1, &logfd, &logfd,
-                           VIR_EXEC_CLEAR_CAPS,
-                           NULL, NULL, NULL);
+    virCommandClearCaps(cmd);
+    virCommandSetOutputFD(cmd, &logfd);
+    virCommandSetErrorFD(cmd, &logfd);
+    virCommandDaemonize(cmd);
+
+    ret = virCommandRun(cmd, NULL);
     VIR_FORCE_CLOSE(logfd);
     if (ret < 0)
         goto cleanup;

     ret = virDomainObjSetDefTransient(driver->caps, vm);
 cleanup:
-    /*
-     * At the moment, the only thing that populates keepfd is
-     * umlBuildCommandLineChr. We want to close every fd it opens.
-     */
-    for (i = 0; i < FD_SETSIZE; i++)
-        if (FD_ISSET(i, &keepfd)) {
-            int tmpfd = i;
-            VIR_FORCE_CLOSE(tmpfd);
-        }
-
-    for (i = 0 ; argv[i] ; i++)
-        VIR_FREE(argv[i]);
-    VIR_FREE(argv);
-
-    for (i = 0 ; progenv[i] ; i++)
-        VIR_FREE(progenv[i]);
-    VIR_FREE(progenv);
+    virCommandFree(cmd);

     if (ret < 0) {
         virDomainConfVMNWFilterTeardown(vm);
diff --git a/src/util/command.c b/src/util/command.c
index 71db2a1..948a957 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -33,6 +33,7 @@
 #include "util.h"
 #include "logging.h"
 #include "files.h"
+#include "buf.h"

 #define VIR_FROM_THIS VIR_FROM_NONE

@@ -54,7 +55,9 @@ struct _virCommand {
     char *pwd;

     /* XXX Use int[] if we ever need to support more than FD_SETSIZE fd's.  */
-    fd_set preserve;
+    fd_set preserve; /* FDs to pass to child. */
+    fd_set transfer; /* FDs to close in parent. */
+
     unsigned int flags;

     char *inbuf;
@@ -99,6 +102,7 @@ virCommandNewArgs(const char *const*args)
         return NULL;

     FD_ZERO(&cmd->preserve);
+    FD_ZERO(&cmd->transfer);
     cmd->infd = cmd->outfd = cmd->errfd = -1;
     cmd->inpipe = -1;
     cmd->pid = -1;
@@ -113,27 +117,76 @@ virCommandNewArgs(const char *const*args)
     return cmd;
 }

+/*
+ * Create a new command with a NULL terminated
+ * list of args, starting with the binary to run
+ */
+virCommandPtr
+virCommandNewArgList(const char *binary, ...)
+{
+    virCommandPtr cmd = virCommandNew(binary);
+    va_list list;
+    const char *arg;
+
+    if (!cmd || cmd->has_error)
+        return NULL;
+
+    va_start(list, binary);
+    while ((arg = va_arg(list, const char *)) != NULL)
+        virCommandAddArg(cmd, arg);
+    va_end(list);
+    return cmd;
+}
+

 /*
  * Preserve the specified file descriptor in the child, instead of
- * closing it.  FD must not be one of the three standard streams.
+ * closing it.  FD must not be one of the three standard streams.  If
+ * transfer is true, then fd will be closed in the parent after a call
+ * to Run/RunAsync/Free, otherwise caller is still responsible for fd.
  */
-void
-virCommandPreserveFD(virCommandPtr cmd, int fd)
+static void
+virCommandKeepFD(virCommandPtr cmd, int fd, bool transfer)
 {
-    if (!cmd || cmd->has_error)
+    if (!cmd)
         return;

     if (fd <= STDERR_FILENO || FD_SETSIZE <= fd) {
-        cmd->has_error = -1;
+        if (!cmd->has_error)
+            cmd->has_error = -1;
         VIR_DEBUG("cannot preserve %d", fd);
         return;
     }

     FD_SET(fd, &cmd->preserve);
+    if (transfer)
+        FD_SET(fd, &cmd->transfer);
 }

 /*
+ * Preserve the specified file descriptor
+ * in the child, instead of closing it.
+ * The parent is still responsible for managing fd.
+ */
+void
+virCommandPreserveFD(virCommandPtr cmd, int fd)
+{
+    return virCommandKeepFD(cmd, fd, false);
+}
+
+/*
+ * Transfer the specified file descriptor
+ * to the child, instead of closing it.
+ * Close the fd in the parent during Run/RunAsync/Free.
+ */
+void
+virCommandTransferFD(virCommandPtr cmd, int fd)
+{
+    return virCommandKeepFD(cmd, fd, true);
+}
+
+
+/*
  * Save the child PID in a pidfile
  */
 void
@@ -192,6 +245,19 @@ virCommandDaemonize(virCommandPtr cmd)
 }

 /*
+ * Set FDs created by virCommandSetOutputFD and virCommandSetErrorFD
+ * as non-blocking in the parent.
+ */
+void
+virCommandNonblockingFDs(virCommandPtr cmd)
+{
+    if (!cmd || cmd->has_error)
+        return;
+
+    cmd->flags |= VIR_EXEC_NONBLOCK;
+}
+
+/*
  * Add an environment variable to the child
  * using separate name & value strings
  */
@@ -317,20 +383,24 @@ virCommandAddArg(virCommandPtr cmd, const char *val)


 /*
- * Add "NAME=VAL" as a single command line argument to the child
+ * Add a command line argument created by a printf-style format
  */
 void
-virCommandAddArgPair(virCommandPtr cmd, const char *name, const char *val)
+virCommandAddArgFormat(virCommandPtr cmd, const char *format, ...)
 {
     char *arg;
+    va_list list;

     if (!cmd || cmd->has_error)
         return;

-    if (virAsprintf(&arg, "%s=%s", name, val) < 0) {
+    va_start(list, format);
+    if (virVasprintf(&arg, format, list) < 0) {
         cmd->has_error = ENOMEM;
+        va_end(list);
         return;
     }
+    va_end(list);

     /* Arg plus trailing NULL. */
     if (VIR_RESIZE_N(cmd->args, cmd->maxargs, cmd->nargs, 1 + 1) < 0) {
@@ -343,6 +413,15 @@ virCommandAddArgPair(virCommandPtr cmd, const char *name, const char *val)
 }

 /*
+ * Add "NAME=VAL" as a single command line argument to the child
+ */
+void
+virCommandAddArgPair(virCommandPtr cmd, const char *name, const char *val)
+{
+    virCommandAddArgFormat(cmd, "%s=%s", name, val);
+}
+
+/*
  * Add a NULL terminated list of args
  */
 void
@@ -569,6 +648,89 @@ virCommandSetPreExecHook(virCommandPtr cmd, virExecHook hook, void *opaque)


 /*
+ * Call after adding all arguments and environment settings, but before
+ * Run/RunAsync, to immediately output the environment and arguments of
+ * cmd to logfd.  If virCommandRun cannot succeed (because of an
+ * out-of-memory condition while building cmd), nothing will be logged.
+ */
+void
+virCommandWriteArgLog(virCommandPtr cmd, int logfd)
+{
+    int ioError = 0;
+    size_t i;
+
+    /* Any errors will be reported later by virCommandRun, which means
+     * no command will be run, so there is nothing to log. */
+    if (!cmd || cmd->has_error)
+        return;
+
+    for (i = 0 ; i < cmd->nenv ; i++) {
+        if (safewrite(logfd, cmd->env[i], strlen(cmd->env[i])) < 0)
+            ioError = errno;
+        if (safewrite(logfd, " ", 1) < 0)
+            ioError = errno;
+    }
+    for (i = 0 ; i < cmd->nargs ; i++) {
+        if (safewrite(logfd, cmd->args[i], strlen(cmd->args[i])) < 0)
+            ioError = errno;
+        if (safewrite(logfd, i == cmd->nargs - 1 ? "\n" : " ", 1) < 0)
+            ioError = errno;
+    }
+
+    if (ioError) {
+        char ebuf[1024];
+        VIR_WARN("Unable to write command %s args to logfile: %s",
+                 cmd->args[0], virStrerror(ioError, ebuf, sizeof ebuf));
+    }
+}
+
+
+/*
+ * Call after adding all arguments and environment settings, but before
+ * Run/RunAsync, to return a string representation of the environment and
+ * arguments of cmd.  If virCommandRun cannot succeed (because of an
+ * out-of-memory condition while building cmd), NULL will be returned.
+ * Caller is responsible for freeing the resulting string.
+ */
+char *
+virCommandToString(virCommandPtr cmd)
+{
+    size_t i;
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+    /* Cannot assume virCommandRun will be called; so report the error
+     * now.  If virCommandRun is called, it will report the same error. */
+    if (!cmd || cmd->has_error == -1) {
+        virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
+                        _("invalid use of command API"));
+        return NULL;
+    }
+    if (cmd->has_error == ENOMEM) {
+        virReportOOMError();
+        return NULL;
+    }
+
+    for (i = 0; i < cmd->nenv; i++) {
+        virBufferAdd(&buf, cmd->env[i], strlen(cmd->env[i]));
+        virBufferAddChar(&buf, ' ');
+    }
+    virBufferAdd(&buf, cmd->args[0], strlen(cmd->args[0]));
+    for (i = 1; i < cmd->nargs; i++) {
+        virBufferAddChar(&buf, ' ');
+        virBufferAdd(&buf, cmd->args[i], strlen(cmd->args[i]));
+    }
+
+    if (virBufferError(&buf)) {
+        virBufferFreeAndReset(&buf);
+        virReportOOMError();
+        return NULL;
+    }
+
+    return virBufferContentAndReset(&buf);
+}
+
+
+/*
  * Manage input and output to the child process.
  */
 static int
@@ -823,6 +985,7 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
 {
     int ret;
     char *str;
+    int i;

     if (!cmd || cmd->has_error == -1) {
         virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -868,6 +1031,14 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
     VIR_DEBUG("Command result %d, with PID %d",
               ret, (int)cmd->pid);

+    for (i = STDERR_FILENO + 1; i < FD_SETSIZE; i++) {
+        if (FD_ISSET(i, &cmd->transfer)) {
+            int tmpfd = i;
+            VIR_FORCE_CLOSE(tmpfd);
+            FD_CLR(i, &cmd->transfer);
+        }
+    }
+
     if (ret == 0 && pid)
         *pid = cmd->pid;

@@ -941,6 +1112,13 @@ virCommandFree(virCommandPtr cmd)
     if (!cmd)
         return;

+    for (i = STDERR_FILENO + 1; i < FD_SETSIZE; i++) {
+        if (FD_ISSET(i, &cmd->transfer)) {
+            int tmpfd = i;
+            VIR_FORCE_CLOSE(tmpfd);
+        }
+    }
+
     VIR_FORCE_CLOSE(cmd->outfd);
     VIR_FORCE_CLOSE(cmd->errfd);

diff --git a/src/util/command.h b/src/util/command.h
index ca24869..9b04e68 100644
--- a/src/util/command.h
+++ b/src/util/command.h
@@ -39,18 +39,34 @@ virCommandPtr virCommandNew(const char *binary) ATTRIBUTE_NONNULL(1);
  */
 virCommandPtr virCommandNewArgs(const char *const*args) ATTRIBUTE_NONNULL(1);

+/*
+ * Create a new command with a NULL terminated
+ * list of args, starting with the binary to run
+ */
+virCommandPtr virCommandNewArgList(const char *binary, ...)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_SENTINEL;
+
 /* All error report from these setup APIs is
- * delayed until the Run/Exec/Wait methods
+ * delayed until the Run/RunAsync methods
  */

 /*
  * Preserve the specified file descriptor
- * in the child, instead of closing it
+ * in the child, instead of closing it.
+ * The parent is still responsible for managing fd.
  */
 void virCommandPreserveFD(virCommandPtr cmd,
                           int fd);

 /*
+ * Transfer the specified file descriptor
+ * to the child, instead of closing it.
+ * Close the fd in the parent during Run/RunAsync/Free.
+ */
+void virCommandTransferFD(virCommandPtr cmd,
+                          int fd);
+
+/*
  * Save the child PID in a pidfile
  */
 void virCommandSetPidFile(virCommandPtr cmd,
@@ -74,6 +90,11 @@ void virCommandAllowCap(virCommandPtr cmd,
  */
 void virCommandDaemonize(virCommandPtr cmd);

+/*
+ * Set FDs created by virCommandSetOutputFD and virCommandSetErrorFD
+ * as non-blocking in the parent.
+ */
+void virCommandNonblockingFDs(virCommandPtr cmd);

 /*
  * Add an environment variable to the child
@@ -106,6 +127,14 @@ void virCommandAddEnvPassCommon(virCommandPtr cmd);
  */
 void virCommandAddArg(virCommandPtr cmd,
                       const char *val) ATTRIBUTE_NONNULL(2);
+
+/*
+ * Add a command line argument created by a printf-style format
+ */
+void virCommandAddArgFormat(virCommandPtr cmd,
+                            const char *format, ...)
+    ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 3);
+
 /*
  * Add a command line argument to the child
  */
@@ -179,6 +208,24 @@ void virCommandSetPreExecHook(virCommandPtr cmd,
                               void *opaque) ATTRIBUTE_NONNULL(2);

 /*
+ * Call after adding all arguments and environment settings, but before
+ * Run/RunAsync, to immediately output the environment and arguments of
+ * cmd to logfd.  If virCommandRun cannot succeed (because of an
+ * out-of-memory condition while building cmd), nothing will be logged.
+ */
+void virCommandWriteArgLog(virCommandPtr cmd,
+                           int logfd);
+
+/*
+ * Call after adding all arguments and environment settings, but before
+ * Run/RunAsync, to return a string representation of the environment and
+ * arguments of cmd.  If virCommandRun cannot succeed (because of an
+ * out-of-memory condition while building cmd), NULL will be returned.
+ * Caller is responsible for freeing the resulting string.
+ */
+char *virCommandToString(virCommandPtr cmd) ATTRIBUTE_RETURN_CHECK;
+
+/*
  * Run the command and wait for completion.
  * Returns -1 on any error executing the
  * command. Returns 0 if the command executed,
diff --git a/src/util/hooks.c b/src/util/hooks.c
index 1139d88..5ba2036 100644
--- a/src/util/hooks.c
+++ b/src/util/hooks.c
@@ -250,12 +250,10 @@ virHookCall(int driver, const char *id, int op, int sub_op, const char *extra,
         return(-1);
     }

-    cmd = virCommandNew(path);
+    cmd = virCommandNewArgList(path, id, opstr, subopstr, extra, NULL);

     virCommandAddEnvPassCommon(cmd);

-    virCommandAddArgList(cmd, id, opstr, subopstr, extra, NULL);
-
     if (input)
         virCommandSetInputBuffer(cmd, input);

diff --git a/src/util/util.c b/src/util/util.c
index a2582aa..f6050de 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -88,42 +88,44 @@ verify(sizeof(gid_t) <= sizeof (unsigned int) &&
                              __FUNCTION__, __LINE__, __VA_ARGS__)

 /* Like read(), but restarts after EINTR */
-int saferead(int fd, void *buf, size_t count)
-{
-        size_t nread = 0;
-        while (count > 0) {
-                ssize_t r = read(fd, buf, count);
-                if (r < 0 && errno == EINTR)
-                        continue;
-                if (r < 0)
-                        return r;
-                if (r == 0)
-                        return nread;
-                buf = (char *)buf + r;
-                count -= r;
-                nread += r;
-        }
-        return nread;
+ssize_t
+saferead(int fd, void *buf, size_t count)
+{
+    size_t nread = 0;
+    while (count > 0) {
+        ssize_t r = read(fd, buf, count);
+        if (r < 0 && errno == EINTR)
+            continue;
+        if (r < 0)
+            return r;
+        if (r == 0)
+            return nread;
+        buf = (char *)buf + r;
+        count -= r;
+        nread += r;
+    }
+    return nread;
 }

 /* Like write(), but restarts after EINTR */
-ssize_t safewrite(int fd, const void *buf, size_t count)
-{
-        size_t nwritten = 0;
-        while (count > 0) {
-                ssize_t r = write(fd, buf, count);
-
-                if (r < 0 && errno == EINTR)
-                        continue;
-                if (r < 0)
-                        return r;
-                if (r == 0)
-                        return nwritten;
-                buf = (const char *)buf + r;
-                count -= r;
-                nwritten += r;
-        }
-        return nwritten;
+ssize_t
+safewrite(int fd, const void *buf, size_t count)
+{
+    size_t nwritten = 0;
+    while (count > 0) {
+        ssize_t r = write(fd, buf, count);
+
+        if (r < 0 && errno == EINTR)
+            continue;
+        if (r < 0)
+            return r;
+        if (r == 0)
+            return nwritten;
+        buf = (const char *)buf + r;
+        count -= r;
+        nwritten += r;
+    }
+    return nwritten;
 }

 #ifdef HAVE_POSIX_FALLOCATE
@@ -2197,6 +2199,22 @@ virParseVersionString(const char *str, unsigned long *version)
 }

 /**
+ * virVasprintf
+ *
+ * like glibc's vasprintf but makes sure *strp == NULL on failure
+ */
+int
+virVasprintf(char **strp, const char *fmt, va_list list)
+{
+    int ret;
+
+    if ((ret = vasprintf(strp, fmt, list)) == -1)
+        *strp = NULL;
+
+    return ret;
+}
+
+/**
  * virAsprintf
  *
  * like glibc's_asprintf but makes sure *strp == NULL on failure
@@ -2208,10 +2226,7 @@ virAsprintf(char **strp, const char *fmt, ...)
     int ret;

     va_start(ap, fmt);
-
-    if ((ret = vasprintf(strp, fmt, ap)) == -1)
-        *strp = NULL;
-
+    ret = virVasprintf(strp, fmt, ap);
     va_end(ap);
     return ret;
 }
diff --git a/src/util/util.h b/src/util/util.h
index a240d87..deaf8bb 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -31,12 +31,13 @@
 # include <unistd.h>
 # include <sys/select.h>
 # include <sys/types.h>
+# include <stdarg.h>

 # ifndef MIN
 #  define MIN(a, b) ((a) < (b) ? (a) : (b))
 # endif

-int saferead(int fd, void *buf, size_t count) ATTRIBUTE_RETURN_CHECK;
+ssize_t saferead(int fd, void *buf, size_t count) ATTRIBUTE_RETURN_CHECK;
 ssize_t safewrite(int fd, const void *buf, size_t count)
     ATTRIBUTE_RETURN_CHECK;
 int safezero(int fd, int flags, off_t offset, off_t len)
@@ -202,7 +203,10 @@ int virMacAddrCompare (const char *mac1, const char *mac2);
 void virSkipSpaces(const char **str);
 int virParseNumber(const char **str);
 int virParseVersionString(const char *str, unsigned long *version);
-int virAsprintf(char **strp, const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(2, 3);
+int virAsprintf(char **strp, const char *fmt, ...)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 3);
+int virVasprintf(char **strp, const char *fmt, va_list list)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 0);
 char *virStrncpy(char *dest, const char *src, size_t n, size_t destbytes)
     ATTRIBUTE_RETURN_CHECK;
 char *virStrcpy(char *dest, const char *src, size_t destbytes)
diff --git a/src/util/virtaudit.c b/src/util/virtaudit.c
index b630fce..e6bd07f 100644
--- a/src/util/virtaudit.c
+++ b/src/util/virtaudit.c
@@ -94,7 +94,7 @@ void virAuditSend(const char *file ATTRIBUTE_UNUSED, const char *func,
 #endif

     va_start(args, fmt);
-    if (vasprintf(&str, fmt, args) < 0) {
+    if (virVasprintf(&str, fmt, args) < 0) {
         VIR_WARN0("Out of memory while formatting audit message");
         str = NULL;
     }
diff --git a/tests/commanddata/test16.log b/tests/commanddata/test16.log
new file mode 100644
index 0000000..2433a4d
--- /dev/null
+++ b/tests/commanddata/test16.log
@@ -0,0 +1 @@
+A=B /bin/true C
diff --git a/tests/commandhelper.c b/tests/commandhelper.c
index dc8998f..2ee9153 100644
--- a/tests/commandhelper.c
+++ b/tests/commandhelper.c
@@ -29,6 +29,7 @@
 #include "internal.h"
 #include "util.h"
 #include "memory.h"
+#include "files.h"


 static int envsort(const void *a, const void *b) {
@@ -102,7 +103,7 @@ int main(int argc, char **argv) {
         strcpy(cwd, ".../commanddata");
     fprintf(log, "CWD:%s\n", cwd);

-    fclose(log);
+    VIR_FORCE_FCLOSE(log);

     char buf[1024];
     ssize_t got;
diff --git a/tests/commandtest.c b/tests/commandtest.c
index 5479bfc..46d6ae5 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -25,6 +25,8 @@
 #include <string.h>
 #include <unistd.h>
 #include <signal.h>
+#include <sys/stat.h>
+#include <fcntl.h>

 #include "testutils.h"
 #include "internal.h"
@@ -32,6 +34,7 @@
 #include "util.h"
 #include "memory.h"
 #include "command.h"
+#include "files.h"

 #ifndef __linux__

@@ -166,10 +169,9 @@ static int test3(const void *unused ATTRIBUTE_UNUSED) {
     int newfd1 = dup(STDERR_FILENO);
     int newfd2 = dup(STDERR_FILENO);
     int newfd3 = dup(STDERR_FILENO);
-    close(newfd2);

     virCommandPreserveFD(cmd, newfd1);
-    virCommandPreserveFD(cmd, newfd3);
+    virCommandTransferFD(cmd, newfd3);

     if (virCommandRun(cmd, NULL) < 0) {
         virErrorPtr err = virGetLastError();
@@ -177,7 +179,16 @@ static int test3(const void *unused ATTRIBUTE_UNUSED) {
         return -1;
     }

+    if (fcntl(newfd1, F_GETFL) < 0 ||
+        fcntl(newfd2, F_GETFL) < 0 ||
+        fcntl(newfd3, F_GETFL) >= 0) {
+        puts("fds in wrong state");
+        return -1;
+    }
+
     virCommandFree(cmd);
+    VIR_FORCE_CLOSE(newfd1);
+    VIR_FORCE_CLOSE(newfd2);

     return checkoutput("test3");
 }
@@ -501,6 +512,52 @@ static int test15(const void *unused ATTRIBUTE_UNUSED) {
     return checkoutput("test15");
 }

+/*
+ * Don't run program; rather, log what would be run.
+ */
+static int test16(const void *unused ATTRIBUTE_UNUSED) {
+    virCommandPtr cmd = virCommandNew("/bin/true");
+    char *outactual = NULL;
+    const char *outexpect = "A=B /bin/true C";
+    int ret = -1;
+    int fd = -1;
+
+    virCommandAddEnvPair(cmd, "A", "B");
+    virCommandAddArg(cmd, "C");
+
+    if ((outactual = virCommandToString(cmd)) == NULL) {
+        virErrorPtr err = virGetLastError();
+        printf("Cannot convert to string: %s\n", err->message);
+        return -1;
+    }
+    if ((fd = open(abs_builddir "/commandhelper.log",
+                   O_CREAT | O_TRUNC | O_WRONLY, 0600)) < 0) {
+        printf("Cannot open log file: %s\n", strerror (errno));
+        goto cleanup;
+    }
+    virCommandWriteArgLog(cmd, fd);
+    if (VIR_CLOSE(fd) < 0) {
+        printf("Cannot close log file: %s\n", strerror (errno));
+        goto cleanup;
+    }
+
+    virCommandFree(cmd);
+
+    if (checkoutput("test16") < 0)
+        goto cleanup;
+
+    if (!STREQ(outactual, outexpect)) {
+        virtTestDifference(stderr, outactual, outexpect);
+        goto cleanup;
+    }
+    ret = 0;
+
+cleanup:
+    VIR_FORCE_CLOSE(fd);
+    VIR_FREE(outactual);
+    return ret;
+}
+
 static int
 mymain(int argc, char **argv)
 {
@@ -563,6 +620,7 @@ mymain(int argc, char **argv)
     DO_TEST(test13);
     DO_TEST(test14);
     DO_TEST(test15);
+    DO_TEST(test16);

     return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
 }
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index b149ef4..07dde54 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -25,28 +25,30 @@ static struct qemud_driver driver;
 # define MAX_FILE 4096

 static int testCompareXMLToArgvFiles(const char *xml,
-                                     const char *cmd,
+                                     const char *cmdline,
                                      unsigned long long extraFlags,
                                      const char *migrateFrom,
                                      bool expectError) {
     char argvData[MAX_FILE];
     char *expectargv = &(argvData[0]);
+    int len;
     char *actualargv = NULL;
-    const char **argv = NULL;
-    const char **qenv = NULL;
-    const char **tmp = NULL;
-    int ret = -1, len;
+    int ret = -1;
     unsigned long long flags;
     virDomainDefPtr vmdef = NULL;
     virDomainChrDef monitor_chr;
     virConnectPtr conn;
     char *log = NULL;
+    virCommandPtr cmd = NULL;

     if (!(conn = virGetConnect()))
         goto fail;

-    if (virtTestLoadFile(cmd, &expectargv, MAX_FILE) < 0)
+    len = virtTestLoadFile(cmdline, &expectargv, MAX_FILE);
+    if (len < 0)
         goto fail;
+    if (len && expectargv[len - 1] == '\n')
+        expectargv[len - 1] = '\0';

     if (!(vmdef = virDomainDefParseFile(driver.caps, xml,
                                         VIR_DOMAIN_XML_INACTIVE)))
@@ -85,10 +87,9 @@ static int testCompareXMLToArgvFiles(const char *xml,

     free(virtTestLogContentAndReset());

-    if (qemudBuildCommandLine(conn, &driver,
-                              vmdef, &monitor_chr, 0, flags,
-                              &argv, &qenv,
-                              NULL, NULL, migrateFrom, NULL) < 0)
+    if (!(cmd = qemudBuildCommandLine(conn, &driver,
+                                      vmdef, &monitor_chr, false, flags,
+                                      migrateFrom, NULL)))
         goto fail;

     if ((log = virtTestLogContentAndReset()) == NULL)
@@ -105,36 +106,8 @@ static int testCompareXMLToArgvFiles(const char *xml,
         virResetLastError();
     }

-    len = 1; /* for trailing newline */
-    tmp = qenv;
-    while (*tmp) {
-        len += strlen(*tmp) + 1;
-        tmp++;
-    }
-
-    tmp = argv;
-    while (*tmp) {
-        len += strlen(*tmp) + 1;
-        tmp++;
-    }
-    if ((actualargv = malloc(sizeof(*actualargv)*len)) == NULL)
+    if (!(actualargv = virCommandToString(cmd)))
         goto fail;
-    actualargv[0] = '\0';
-    tmp = qenv;
-    while (*tmp) {
-        if (actualargv[0])
-            strcat(actualargv, " ");
-        strcat(actualargv, *tmp);
-        tmp++;
-    }
-    tmp = argv;
-    while (*tmp) {
-        if (actualargv[0])
-            strcat(actualargv, " ");
-        strcat(actualargv, *tmp);
-        tmp++;
-    }
-    strcat(actualargv, "\n");

     if (STRNEQ(expectargv, actualargv)) {
         virtTestDifference(stderr, expectargv, actualargv);
@@ -146,22 +119,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
  fail:
     free(log);
     free(actualargv);
-    if (argv) {
-        tmp = argv;
-        while (*tmp) {
-            free(*(char**)tmp);
-            tmp++;
-        }
-        free(argv);
-    }
-    if (qenv) {
-        tmp = qenv;
-        while (*tmp) {
-            free(*(char**)tmp);
-            tmp++;
-        }
-        free(qenv);
-    }
+    virCommandFree(cmd);
     virDomainDefFree(vmdef);
     virUnrefConnect(conn);
     return ret;

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]