[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Convert dhcpStartDhcpDaemon from virRun to virCommand



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]