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

[Eric Blake]

On 05/05/2011 02:44 PM, Cole Robinson wrote:
> 
> Signed-off-by: Cole Robinson <crobinso at redhat.com>
> ---
>  src/lxc/lxc_driver.c |  186 ++++++++++----------------------------------------
>  1 files changed, 37 insertions(+), 149 deletions(-)

I love diffstats like this.  Isn't it fun how much cruft virCommand removes?

> 
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index b94941d..930e445 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -1247,134 +1247,59 @@ static int lxcControllerStart(lxc_driver_t *driver,
>                                int nveths,
>                                char **veths,
>                                int appPty,
> -                              int logfd)
> +                              int logfile)

Why the rename here?  But not a show-stopper.

>  
> -    /*
> -     * The controller may call ip command, so we have to remain PATH.
> -     */
> -    ADD_ENV_COPY("PATH");
> +    cmd = virCommandNewArgList(vm->def->emulator, NULL);

Why not just:

cmd = virCommandNew(vm->def->emulator);

> +
> +    /* The controller may call ip command, so we have to remain PATH. */

Pre-existing typo, but s/remain/retain/

> +    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());

Aha - the new API from the last patch now in use.

> -
> -    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, NULL);
> +    virCommandAddArg(cmd, "--console");

Why not merge these two lines:

virCommandAddArgList(cmd, "--name", vm->def->name, "--console", NULL);

ACK with those nits fixed.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110505/e4fca7d0/attachment-0001.sig>