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@xxxxxxxxxx>
> ---
> 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list