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

[Eric Blake]

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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20101210/592890a9/attachment-0001.sig>