On 05/05/2011 02:44 PM, Cole Robinson wrote:
>
> Signed-off-by: Cole Robinson <crobinso@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list