On 05/05/2011 02:44 PM, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso@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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org