Re: [libvirt] [PATCH 2/3] lxc: driver: Convert emulator launching to virCommand
On 05/05/2011 02:44 PM, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso(a)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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)