[libvirt] [PATCH v5 3/5] Network: Move dnsmasqContext creation to networkSaveDnsmasqHostsfile()

[Laine Stump]

On 06/14/2011 09:02 AM, Michal Novotny wrote:
> This patch moves the dnsmasqContext creation to the networkSaveDnsmasqHostsfile()
> and saves and passes the hostsfile to the dnsmasq only if applicable.
AS I mentioned in the review of 2/5, the bridge_driver.c changes in this 
patch should be combined with the bridge_driver.c changes in PATCH 2/5, 
becoming the new 2/5, and the changes to the test data in this patch 
should be combined with the remainder of PATCH 2/5, becoming PATCH 3/5. 
This will ensure that make check succeeds at each step along the git 
history (which is very important if we want git bisect to be useful). 
Otherwise I don't see any issues in this patch.


> Signed-off-by: Michal Novotny<minovotn at redhat.com>
> ---
>   src/network/bridge_driver.c                        |   38 +++++++++++---------
>   .../nat-network-dns-txt-record.argv                |    2 +-
>   tests/networkxml2argvdata/nat-network.argv         |    2 +-
>   3 files changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 1fad160..10bb928 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -434,13 +434,20 @@ networkShutdown(void) {
>   }
>
>
> -static int
> +static dnsmasqContext*
>   networkSaveDnsmasqHostsfile(virNetworkIpDefPtr ipdef,
> -                            dnsmasqContext *dctx,
> +                            char *name,
>                               bool force)
>   {
>       unsigned int i;
>
> +    dnsmasqContext *dctx = dnsmasqContextNew(name,
> +                                             DNSMASQ_STATE_DIR);
> +    if (dctx == NULL) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
> +
>       if (! force&&  virFileExists(dctx->hostsfile->path))
>           return 0;
>
> @@ -451,9 +458,14 @@ networkSaveDnsmasqHostsfile(virNetworkIpDefPtr ipdef,
>       }
>
>       if (dnsmasqSave(dctx)<  0)
> -        return -1;
> +        goto cleanup;
>
> -    return 0;
> +    return dctx;
> +
> +cleanup:
> +    dnsmasqContextFree(dctx);
> +
> +    return NULL;
>   }
>
>   static int
> @@ -465,6 +477,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
>       int nbleases = 0;
>       int ii;
>       virNetworkIpDefPtr tmpipdef;
> +    dnsmasqContext *dctx = NULL;
>
>       /*
>        * NB, be careful about syntax for dnsmasq options in long format.
> @@ -590,18 +603,11 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
>           if (ipdef->nranges || ipdef->nhosts)
>               virCommandAddArg(cmd, "--dhcp-no-override");
>
> -        if (ipdef->nhosts>  0) {
> -            dnsmasqContext *dctx = dnsmasqContextNew(network->def->name,
> -                                                     DNSMASQ_STATE_DIR);
> -            if (dctx == NULL) {
> -                virReportOOMError();
> -                goto cleanup;
> -            }
> -
> -            if (networkSaveDnsmasqHostsfile(ipdef, dctx, false) == 0) {
> +        if ((dctx = networkSaveDnsmasqHostsfile(ipdef, network->def->name, false))) {
> +            if (dctx->hostsfile->nhosts)
>                   virCommandAddArgPair(cmd, "--dhcp-hostsfile",
>                                        dctx->hostsfile->path);
> -            }
> +
>               dnsmasqContextFree(dctx);
>           }
>
> @@ -2230,11 +2236,9 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) {
>           }
>       }
>       if (ipv4def) {
> -        dnsmasqContext *dctx = dnsmasqContextNew(network->def->name, DNSMASQ_STATE_DIR);
> +        dnsmasqContext* dctx = networkSaveDnsmasqHostsfile(ipv4def, network->def->name, true);
>           if (dctx == NULL)
>               goto cleanup;
> -
> -        networkSaveDnsmasqHostsfile(ipv4def, dctx, true);
>           dnsmasqContextFree(dctx);
>       }
>
> diff --git a/tests/networkxml2argvdata/nat-network-dns-txt-record.argv b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
> index 96fde34..90cd578 100644
> --- a/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
> +++ b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
> @@ -1 +1 @@
> -/usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=(null) --conf-file= --except-interface lo --txt-record=example,example value --listen-address 192.168.122.1 --listen-address 192.168.123.1 --listen-address 2001:db8:ac10:fe01::1 --listen-address 2001:db8:ac10:fd01::1 --listen-address 10.24.10.1 --dhcp-range 192.168.122.2,192.168.122.254 --dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases --dhcp-lease-max=253 --dhcp-no-override --dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile
> \ No newline at end of file
> +/usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=(null) --conf-file= --except-interface lo --txt-record=example,example value --listen-address 192.168.122.1 --listen-address 192.168.123.1 --listen-address 2001:db8:ac10:fe01::1 --listen-address 2001:db8:ac10:fd01::1 --listen-address 10.24.10.1 --dhcp-range 192.168.122.2,192.168.122.254 --dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases --dhcp-lease-max=253 --dhcp-no-override
> \ No newline at end of file
> diff --git a/tests/networkxml2argvdata/nat-network.argv b/tests/networkxml2argvdata/nat-network.argv
> index ccffa67..54a34e9 100644
> --- a/tests/networkxml2argvdata/nat-network.argv
> +++ b/tests/networkxml2argvdata/nat-network.argv
> @@ -1 +1 @@
> -/usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=(null) --conf-file= --except-interface lo --listen-address 192.168.122.1 --listen-address 192.168.123.1 --listen-address 2001:db8:ac10:fe01::1 --listen-address 2001:db8:ac10:fd01::1 --listen-address 10.24.10.1 --dhcp-range 192.168.122.2,192.168.122.254 --dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases --dhcp-lease-max=253 --dhcp-no-override --dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile
> \ No newline at end of file
> +/usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=(null) --conf-file= --except-interface lo --listen-address 192.168.122.1 --listen-address 192.168.123.1 --listen-address 2001:db8:ac10:fe01::1 --listen-address 2001:db8:ac10:fd01::1 --listen-address 10.24.10.1 --dhcp-range 192.168.122.2,192.168.122.254 --dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases --dhcp-lease-max=253 --dhcp-no-override
> \ No newline at end of file