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

Re: [libvirt] [PATCH 2/6] lxc: driver: Convert emulator launching to virCommand



On Fri, May 06, 2011 at 01:26:07PM -0400, Cole Robinson wrote:
> v2:
>     Shorten a few virCommand calls
>     s/remain/retain/
> 
> Signed-off-by: Cole Robinson <crobinso redhat com>
> ---
>  src/lxc/lxc_driver.c |  185 ++++++++++----------------------------------------
>  1 files changed, 36 insertions(+), 149 deletions(-)
> 
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index b94941d..2537f5d 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -1247,134 +1247,58 @@ static int lxcControllerStart(lxc_driver_t *driver,
>                                int nveths,
>                                char **veths,
>                                int appPty,
> -                              int logfd)
> +                              int logfile)
>  {
>      int i;
> -    int rc;
> -    int largc = 0, larga = 0;
> -    const char **largv = NULL;
> -    int lenvc = 0, lenva = 0;
> -    const char **lenv = NULL;
> +    int ret = -1;
>      char *filterstr;
>      char *outputstr;
> -    char *tmp;
> -    int log_level;
> -    pid_t child;
> -    int status;
> -    fd_set keepfd;
> -    char appPtyStr[30];
> -    const char *emulator;
> -
> -    FD_ZERO(&keepfd);
> -
> -#define ADD_ARG_SPACE                                                   \
> -    do { \
> -        if (largc == larga) {                                           \
> -            larga += 10;                                                \
> -            if (VIR_REALLOC_N(largv, larga) < 0)                        \
> -                goto no_memory;                                         \
> -        }                                                               \
> -    } while (0)
> -
> -#define ADD_ARG(thisarg)                                                \
> -    do {                                                                \
> -        ADD_ARG_SPACE;                                                  \
> -        largv[largc++] = thisarg;                                       \
> -    } while (0)
> -
> -#define ADD_ARG_LIT(thisarg)                                            \
> -    do {                                                                \
> -        ADD_ARG_SPACE;                                                  \
> -        if ((largv[largc++] = strdup(thisarg)) == NULL)                 \
> -            goto no_memory;                                             \
> -    } while (0)
> -
> -#define ADD_ENV_SPACE                                                   \
> -    do {                                                                \
> -        if (lenvc == lenva) {                                           \
> -            lenva += 10;                                                \
> -            if (VIR_REALLOC_N(lenv, lenva) < 0)                         \
> -                goto no_memory;                                         \
> -        }                                                               \
> -    } while (0)
> -
> -#define ADD_ENV(thisarg)                                                \
> -    do {                                                                \
> -        ADD_ENV_SPACE;                                                  \
> -        lenv[lenvc++] = thisarg;                                        \
> -    } 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;                                             \
> -        lenv[lenvc++] = envval;                                         \
> -    } while (0)
> -
> -#define ADD_ENV_COPY(envname)                                           \
> -    do {                                                                \
> -        char *val = getenv(envname);                                    \
> -        if (val != NULL) {                                              \
> -            ADD_ENV_PAIR(envname, val);                                 \
> -        }                                                               \
> -    } while (0)
> +    virCommandPtr cmd;
>  
> -    /*
> -     * The controller may call ip command, so we have to remain PATH.
> -     */
> -    ADD_ENV_COPY("PATH");
> +    cmd = virCommandNew(vm->def->emulator);
> +
> +    /* The controller may call ip command, so we have to retain PATH. */
> +    virCommandAddEnvPass(cmd, "PATH");
>  
> -    log_level = virLogGetDefaultPriority();
> -    if (virAsprintf(&tmp, "LIBVIRT_DEBUG=%d", log_level) < 0)
> -        goto no_memory;
> -    ADD_ENV(tmp);
> +    virCommandAddEnvFormat(cmd, "LIBVIRT_DEBUG=%d",
> +                           virLogGetDefaultPriority());
>  
>      if (virLogGetNbFilters() > 0) {
>          filterstr = virLogGetFilters();
> -        if (!filterstr)
> -            goto no_memory;
> -        ADD_ENV_PAIR("LIBVIRT_LOG_FILTERS", filterstr);
> +        if (!filterstr) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
> +
> +        virCommandAddEnvPair(cmd, "LIBVIRT_LOG_FILTERS", filterstr);
>          VIR_FREE(filterstr);
>      }
>  
>      if (driver->log_libvirtd) {
>          if (virLogGetNbOutputs() > 0) {
>              outputstr = virLogGetOutputs();
> -            if (!outputstr)
> -                goto no_memory;
> -            ADD_ENV_PAIR("LIBVIRT_LOG_OUTPUTS", outputstr);
> +            if (!outputstr) {
> +                virReportOOMError();
> +                goto cleanup;
> +            }
> +
> +            virCommandAddEnvPair(cmd, "LIBVIRT_LOG_OUTPUTS", outputstr);
>              VIR_FREE(outputstr);
>          }
>      } else {
> -        if (virAsprintf(&tmp, "LIBVIRT_LOG_OUTPUTS=%d:stderr", log_level) < 0)
> -            goto no_memory;
> -        ADD_ENV(tmp);
> +        virCommandAddEnvFormat(cmd,
> +                               "LIBVIRT_LOG_OUTPUTS=%d:stderr",
> +                               virLogGetDefaultPriority());
>      }
>  
> -    ADD_ENV(NULL);
> -
> -    snprintf(appPtyStr, sizeof(appPtyStr), "%d", appPty);
> -
> -    emulator = vm->def->emulator;
> -
> -    ADD_ARG_LIT(emulator);
> -    ADD_ARG_LIT("--name");
> -    ADD_ARG_LIT(vm->def->name);
> -    ADD_ARG_LIT("--console");
> -    ADD_ARG_LIT(appPtyStr);
> -    ADD_ARG_LIT("--background");
> +    virCommandAddArgList(cmd, "--name", vm->def->name, "--console", NULL);
> +    virCommandAddArgFormat(cmd, "%d", appPty);
> +    virCommandAddArg(cmd, "--background");
>  
>      for (i = 0 ; i < nveths ; i++) {
> -        ADD_ARG_LIT("--veth");
> -        ADD_ARG_LIT(veths[i]);
> +        virCommandAddArgList(cmd, "--veth", veths[i], NULL);
>      }
>  
> -    ADD_ARG(NULL);
> -
> -    FD_SET(appPty, &keepfd);
> -
>      /* now that we know it is about to start call the hook if present */
>      if (virHookPresent(VIR_HOOK_DRIVER_LXC)) {
>          char *xml = virDomainDefFormat(vm->def, 0);
> @@ -1391,52 +1315,15 @@ static int lxcControllerStart(lxc_driver_t *driver,
>              goto cleanup;
>      }
>  
> -    if (virExec(largv, lenv, &keepfd, &child,
> -                -1, &logfd, &logfd,
> -                VIR_EXEC_NONE) < 0)
> -        goto cleanup;
> -
> -    /* We now wait for the process to exit - the controller
> -     * will fork() itself into the background - waiting for
> -     * it to exit thus guarentees it has written its pidfile
> -     */
> -    while ((rc = waitpid(child, &status, 0) == -1) && errno == EINTR);
> -    if (rc == -1) {
> -        virReportSystemError(errno,
> -                             _("Cannot wait for '%s'"),
> -                             largv[0]);
> -        goto cleanup;
> -    }
> -
> -    if (!(WIFEXITED(status) && WEXITSTATUS(status) == 0)) {
> -        lxcError(VIR_ERR_INTERNAL_ERROR,
> -                 _("Container '%s' unexpectedly shutdown during startup"),
> -                 largv[0]);
> -        goto cleanup;
> -    }
> +    virCommandPreserveFD(cmd, appPty);
> +    virCommandSetOutputFD(cmd, &logfile);
> +    virCommandSetErrorFD(cmd, &logfile);
>  
> -#undef ADD_ARG
> -#undef ADD_ARG_LIT
> -#undef ADD_ARG_SPACE
> -#undef ADD_ENV_SPACE
> -#undef ADD_ENV_PAIR
> -
> -    return 0;
> +    ret = virCommandRun(cmd, NULL);
>  
> -no_memory:
> -    virReportOOMError();
>  cleanup:
> -    if (largv) {
> -        for (i = 0 ; i < largc ; i++)
> -            VIR_FREE(largv[i]);
> -        VIR_FREE(largv);
> -    }
> -    if (lenv) {
> -        for (i=0 ; i < lenvc ; i++)
> -            VIR_FREE(lenv[i]);
> -        VIR_FREE(lenv);
> -    }
> -    return -1;
> +    virCommandFree(cmd);
> +    return ret;
>  }

ACK


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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