Re: [libvirt] [PATCH] Convert dhcpStartDhcpDaemon from virRun to virCommand
On 12/10/2010 12:02 PM, Laine Stump wrote:
This is pretty straightforward - even though dnsmasq gets daemonized
and uses a pid file, those things are both handled by the dnsmasq
binary itself. And libvirt doesn't need any of the output of the
dnsmasq command either, so we just setup the args and call
virRun(). Mainly it was just a (mostly) mechanical job of replacing
the APPEND_ARG() macro (and some other *printfs()) with
virCommandAddArg*().
---
src/network/bridge_driver.c | 238 +++++++++++++++----------------------------
1 files changed, 80 insertions(+), 158 deletions(-)
- if (virAsprintf(&pidfileArg, "--pid-file=%s",
pidfile) < 0)
- goto no_memory;
- APPEND_ARG_LIT(*argv, i++, pidfileArg);
+ virCommandAddArgPair(cmd, "--pid-file", pidfile);
This technically changes from one arg to two, but that should be a safe
change.
- APPEND_ARG(*argv, i++, "--conf-file=");
- APPEND_ARG(*argv, i++, "");
+ /* *no* conf file */
+ virCommandAddArg(cmd, "--conf-file=");
+ virCommandAddArg(cmd, "");
dnsmasq really requires "--conf-file=" "" as two arguments? Yuck.
Generally, it's either "--conf-file" "" with no equal, or just
"--conf-file=" as a single argument, when one is being explicit that the
argument to --conf-file is the empty string. But this is an identity
transformation of the old code, so if it's a bug, it is pre-existing and
worth fixing in a separate patch than this mechanical transformation.
if (network->def->tftproot) {
- APPEND_ARG(*argv, i++, "--enable-tftp");
- APPEND_ARG(*argv, i++, "--tftp-root");
- APPEND_ARG(*argv, i++, network->def->tftproot);
+ virCommandAddArg(cmd, "--enable-tftp");
+ virCommandAddArg(cmd, "--tftp-root");
+ virCommandAddArg(cmd, network->def->tftproot);
If you want to compress the code even further, you can do things like:
virCommandAddArgList(cmd, "--enable-tftp", "--tftp-root",
network->def->tftproot, NULL);
in a few places where you have back-to-back simple string additions.
But it doesn't affect correctness, so you don't have to make that change.
ACK.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)