On Fri, Dec 10, 2010 at 01:34:15PM -0700, Eric Blake wrote: > 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. I'm fairly sure that dnsmsq does require it the way I did it originally. I remember finding its arg handling a little odd when I wrote this :-) Daniel -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list