uml: convert to virCommand

* src/uml/uml_conf.c (umlBuildCommandLineChr)
(umlBuildCommandLine): Rewrite with virCommand.
* src/uml/uml_conf.h (umlBuildCommandLine): Update signature.
* src/uml/uml_driver.c (umlStartVMDaemon): Adjust caller.

src/uml/uml_conf.c

@@ -48,6 +48,7 @@
 #include "logging.h"
 #include "domain_nwfilter.h"
 #include "files.h"
+#include "command.h"
 
 #define VIR_FROM_THIS VIR_FROM_UML
 
@@ -307,7 +308,7 @@ error:
 static char *
 umlBuildCommandLineChr(virDomainChrDefPtr def,
                        const char *dev,
-                       fd_set *keepfd)
+                       virCommandPtr cmd)
 {
     char *ret = NULL;
 
@@ -371,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:
@@ -419,109 +420,27 @@ static char *umlNextArg(char *args)
  * Constructs a argv suitable for launching uml with config defined
  * for a given virtual machine.
  */
-int umlBuildCommandLine(virConnectPtr conn,
+virCommandPtr umlBuildCommandLine(virConnectPtr conn,
                                   struct uml_driver *driver,
-                        virDomainObjPtr vm,
+                                  virDomainObjPtr vm)
-                        fd_set *keepfd,
-                        const char ***retargv,
-                        const char ***retenv)
 {
     int i, j;
-    char memory[50];
     struct utsname ut;
-    int qargc = 0, qarga = 0;
+    virCommandPtr cmd;
-    const char **qargv = NULL;
-    int qenvc = 0, qenva = 0;
-    const char **qenv = NULL;
-    char *cmdline = NULL;
 
     uname(&ut);
 
-#define ADD_ARG_SPACE                                                   \
+    cmd = virCommandNew(vm->def->os.kernel);
-    do {                                                                \
-        if (qargc == qarga) {                                           \
-            qarga += 10;                                                \
-            if (VIR_REALLOC_N(qargv, qarga) < 0)                        \
-                goto no_memory;                                         \
-        }                                                               \
-    } while (0)
 
-#define ADD_ARG(thisarg)                                                \
+    virCommandAddEnvPassCommon(cmd);
-    do {                                                                \
-        ADD_ARG_SPACE;                                                  \
-        qargv[qargc++] = thisarg;                                       \
-    } while (0)
 
-#define ADD_ARG_LIT(thisarg)                                            \
+    //virCommandAddArgPair(cmd, "con0", "fd:0,fd:1");
-    do {                                                                \
+    virCommandAddArgFormat(cmd, "mem=%luK", vm->def->mem.cur_balloon);
-        ADD_ARG_SPACE;                                                  \
+    virCommandAddArgPair(cmd, "umid", vm->def->name);
-        if ((qargv[qargc++] = strdup(thisarg)) == NULL)                 \
+    virCommandAddArgPair(cmd, "uml_dir", driver->monitorDir);
-            goto no_memory;                                             \
-    } while (0)
-
-#define ADD_ARG_PAIR(key,val)                                           \
-    do {                                                                \
-        char *arg;                                                      \
-        ADD_ARG_SPACE;                                                  \
-        if (virAsprintf(&arg, "%s=%s", key, val) < 0)                   \
-            goto no_memory;                                             \
-        qargv[qargc++] = arg;                                           \
-    } 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_COPY(envname)                                           \
-    do {                                                                \
-        char *val = getenv(envname);                                    \
-        char *envval;                                                   \
-        ADD_ENV_SPACE;                                                  \
-        if (val != NULL) {                                              \
-            if (virAsprintf(&envval, "%s=%s", envname, val) < 0)        \
-                goto no_memory;                                         \
-            qenv[qenvc++] = envval;                                     \
-        }                                                               \
-    } while (0)
-
-    snprintf(memory, sizeof(memory), "%luK", vm->def->mem.cur_balloon);
-
-    ADD_ENV_LIT("LC_ALL=C");
-
-    ADD_ENV_COPY("LD_PRELOAD");
-    ADD_ENV_COPY("LD_LIBRARY_PATH");
-    ADD_ENV_COPY("PATH");
-    ADD_ENV_COPY("USER");
-    ADD_ENV_COPY("LOGNAME");
-    ADD_ENV_COPY("TMPDIR");
-
-    ADD_ARG_LIT(vm->def->os.kernel);
-    //ADD_ARG_PAIR("con0", "fd:0,fd:1");
-    ADD_ARG_PAIR("mem", memory);
-    ADD_ARG_PAIR("umid", vm->def->name);
-    ADD_ARG_PAIR("uml_dir", driver->monitorDir);
 
     if (vm->def->os.root)
-        ADD_ARG_PAIR("root", vm->def->os.root);
+        virCommandAddArgPair(cmd, "root", vm->def->os.root);
 
     for (i = 0 ; i < vm->def->ndisks ; i++) {
         virDomainDiskDefPtr disk = vm->def->disks[i];
@@ -532,24 +451,26 @@ int umlBuildCommandLine(virConnectPtr conn,
             goto error;
         }
 
-        ADD_ARG_PAIR(disk->dst, disk->src);
+        virCommandAddArgPair(cmd, disk->dst, disk->src);
     }
 
     for (i = 0 ; i < vm->def->nnets ; i++) {
         char *ret = umlBuildCommandLineNet(conn, vm->def->nets[i], i);
         if (!ret)
             goto error;
-        ADD_ARG(ret);
+        virCommandAddArg(cmd, ret);
+        VIR_FREE(ret);
     }
 
     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;
-        ADD_ARG(ret);
+        virCommandAddArg(cmd, ret);
+        VIR_FREE(ret);
     }
 
     for (i = 0 ; i < UML_MAX_CHAR_DEVICE ; i++) {
@@ -559,15 +480,18 @@ int 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;
-        ADD_ARG(ret);
+
+        virCommandAddArg(cmd, ret);
+        VIR_FREE(ret);
     }
 
     if (vm->def->os.cmdline) {
         char *args, *next_arg;
+        char *cmdline;
         if ((cmdline = strdup(vm->def->os.cmdline)) == NULL)
             goto no_memory;
 
@@ -577,41 +501,18 @@ int umlBuildCommandLine(virConnectPtr conn,
 
         while (*args) {
             next_arg = umlNextArg(args);
-            ADD_ARG_LIT(args);
+            virCommandAddArg(cmd, args);
             args = next_arg;
         }
+        VIR_FREE(cmdline);
     }
 
-    ADD_ARG(NULL);
+    return cmd;
-    ADD_ENV(NULL);
-
-    *retargv = qargv;
-    *retenv = qenv;
-    return 0;
 
  no_memory:
     virReportOOMError();
  error:
 
-    if (qargv) {
+    virCommandFree(cmd);
-        for (i = 0 ; i < qargc ; i++)
+    return NULL;
-            VIR_FREE((qargv)[i]);
-        VIR_FREE(qargv);
-    }
-    if (qenv) {
-        for (i = 0 ; i < qenvc ; i++)
-            VIR_FREE((qenv)[i]);
-        VIR_FREE(qenv);
-    }
-    VIR_FREE(cmdline);
-    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
 }

src/uml/uml_conf.h

@@ -31,6 +31,7 @@
 # include "domain_conf.h"
 # include "virterror_internal.h"
 # include "threads.h"
+# include "command.h"
 
 # define umlDebug(fmt, ...) do {} while(0)
 
@@ -68,11 +69,8 @@ struct uml_driver {
 
 virCapsPtr  umlCapsInit               (void);
 
-int         umlBuildCommandLine       (virConnectPtr conn,
+virCommandPtr umlBuildCommandLine(virConnectPtr conn,
                                   struct uml_driver *driver,
-                                       virDomainObjPtr dom,
+                                  virDomainObjPtr dom);
-                                       fd_set *keepfd,
-                                       const char ***retargv,
-                                       const char ***retenv);
 
 #endif /* __UML_CONF_H */

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;
+    int ret;
-    const char **progenv = NULL;
-    int i, ret;
-    pid_t pid;
     char *logfile;
     int logfd = -1;
-    struct stat sb;
-    fd_set keepfd;
-    char ebuf[1024];
     umlDomainObjPrivatePtr priv = vm->privateData;
-
+    virCommandPtr cmd = NULL;
-    FD_ZERO(&keepfd);
 
     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,
+    if (!(cmd = umlBuildCommandLine(conn, driver, vm))) {
-                            &argv, &progenv) < 0) {
         VIR_FORCE_CLOSE(logfd);
         virDomainConfVMNWFilterTeardown(vm);
         umlCleanupTapDevices(conn, vm);
         return -1;
     }
 
-    tmp = progenv;
+    virCommandWriteArgLog(cmd, logfd);
-    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));
 
     priv->monitor = -1;
 
-    ret = virExecDaemonize(argv, progenv, &keepfd, &pid,
+    virCommandClearCaps(cmd);
-                           -1, &logfd, &logfd,
+    virCommandSetOutputFD(cmd, &logfd);
-                           VIR_EXEC_CLEAR_CAPS,
+    virCommandSetErrorFD(cmd, &logfd);
-                           NULL, NULL, NULL);
+    virCommandDaemonize(cmd);
+
+    ret = virCommandRun(cmd, NULL);
     VIR_FORCE_CLOSE(logfd);
     if (ret < 0)
         goto cleanup;
 
     ret = virDomainObjSetDefTransient(driver->caps, vm);
 cleanup:
-    /*
+    virCommandFree(cmd);
-     * 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);
 
     if (ret < 0) {
         virDomainConfVMNWFilterTeardown(vm);