In commit f386825 we began adding the options --domain-needed --local=/$mydomain/ to all dnsmasq commandlines with the stated reason of preventing forwarding of DNS queries for names that weren't fully qualified domain names ("FQDN", i.e. a name that included some "."s and a domain name). This was later changed to domain-needed local=/$mydomain/ when we moved the options from the dnsmasq commandline to a conf file. The original patch on the list, and discussion about it, is here: https://www.redhat.com/archives/libvir-list/2012-August/msg01594.html When a domain name isn't specified (mydomain == ""), the addition of "domain-needed local=//" will prevent forwarding of domain-less requests to the virtualization host's DNS resolver, but if a domain *is* specified, the addition of "local=/domain/" will prevent forwarding of any requests for *qualified* names within that domain that aren't resolvable by libvirt's dnsmasq itself. An example of the problems this causes - let's say a network is defined with: <domain name='example.com'/> <dhcp> .. <host mac='52:54:00:11:22:33' ip='1.2.3.4' name='myguest'/> </dhcp> This results in "local=/example.com/" being added to the dnsmasq options. If a guest requests "myguest" or "myguest.example.com", that will be resolved by dnsmasq. If the guest asks for "www.example.com", dnsmasq will not know the answer, but instead of forwarding it to the host, it will return NOT FOUND to the guest. In most cases that isn't the behavior an admin is looking for. A later patch (commit 4f595ba) attempted to remedy this by adding a "forwardPlainNames" attribute to the <dns> element. The idea was that if forwardPlainNames='yes' (default is 'no'), we would allow unresolved names to be forwarded. However, that patch was botched, in that it only removed the "domain-needed" option when forwardPlainNames='yes', and left the "local=/mydomain/". Really we should have been just including the option "--domain-needed --local=//" (note the lack of domain name) regardless of the configured domain of the network, so that requests for names without a domain would be treated as "local to dnsmasq" and not forwarded, but all others (including those in the network's configured domain) would be forwarded. We also shouldn't include *either* of those options if forwardPlainNames='yes'. This patch makes those corrections. This patch doesn't remedy the fact that default behavior was changed by the addition of this feature. That will be handled in a subsequent patch. --- src/network/bridge_driver.c | 16 ++++++---------- tests/networkxml2confdata/dhcp6-network.conf | 4 ++-- .../networkxml2confdata/nat-network-dns-forwarders.conf | 2 +- tests/networkxml2confdata/nat-network-dns-hosts.conf | 4 ++-- tests/networkxml2confdata/netboot-network.conf | 4 ++-- tests/networkxml2confdata/netboot-proxy-network.conf | 4 ++-- 6 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 3b9b58d..f50795b 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -710,9 +710,6 @@ networkDnsmasqConfContents(virNetworkObjPtr network, "strict-order\n", network->def->name); - if (!network->def->dns.forwardPlainNames) - virBufferAddLit(&configbuf, "domain-needed\n"); - if (network->def->dns.forwarders) { virBufferAddLit(&configbuf, "no-resolv\n"); for (i = 0; i < network->def->dns.nfwds; i++) { @@ -728,14 +725,13 @@ networkDnsmasqConfContents(virNetworkObjPtr network, network->def->domain); } - if (network->def->domain || !network->def->dns.forwardPlainNames) { - /* need to specify local even if no domain specified, unless - * the config says we should forward "plain" names (i.e. not - * fully qualified, no '.' characters) + if (!network->def->dns.forwardPlainNames) { + virBufferAddLit(&configbuf, "domain-needed\n"); + /* need to specify local=// whether or not a domain is + * specified, unless the config says we should forward "plain" + * names (i.e. not fully qualified, no '.' characters) */ - virBufferAsprintf(&configbuf, - "local=/%s/\n", - network->def->domain ? network->def->domain : ""); + virBufferAddLit(&configbuf, "local=//\n"); } if (pidfile) diff --git a/tests/networkxml2confdata/dhcp6-network.conf b/tests/networkxml2confdata/dhcp6-network.conf index 5fde07f..92ea2a4 100644 --- a/tests/networkxml2confdata/dhcp6-network.conf +++ b/tests/networkxml2confdata/dhcp6-network.conf @@ -5,10 +5,10 @@ ## ## dnsmasq conf file created by libvirt strict-order -domain-needed domain=mynet expand-hosts -local=/mynet/ +domain-needed +local=// except-interface=lo bind-dynamic interface=virbr0 diff --git a/tests/networkxml2confdata/nat-network-dns-forwarders.conf b/tests/networkxml2confdata/nat-network-dns-forwarders.conf index ebca289..5f95f77 100644 --- a/tests/networkxml2confdata/nat-network-dns-forwarders.conf +++ b/tests/networkxml2confdata/nat-network-dns-forwarders.conf @@ -5,10 +5,10 @@ ## ## dnsmasq conf file created by libvirt strict-order -domain-needed no-resolv server=8.8.8.8 server=8.8.4.4 +domain-needed local=// except-interface=lo bind-dynamic diff --git a/tests/networkxml2confdata/nat-network-dns-hosts.conf b/tests/networkxml2confdata/nat-network-dns-hosts.conf index 2577882..021316f 100644 --- a/tests/networkxml2confdata/nat-network-dns-hosts.conf +++ b/tests/networkxml2confdata/nat-network-dns-hosts.conf @@ -5,10 +5,10 @@ ## ## dnsmasq conf file created by libvirt strict-order -domain-needed domain=example.com expand-hosts -local=/example.com/ +domain-needed +local=// except-interface=lo bind-dynamic interface=virbr0 diff --git a/tests/networkxml2confdata/netboot-network.conf b/tests/networkxml2confdata/netboot-network.conf index b6f3c23..ce33176 100644 --- a/tests/networkxml2confdata/netboot-network.conf +++ b/tests/networkxml2confdata/netboot-network.conf @@ -5,10 +5,10 @@ ## ## dnsmasq conf file created by libvirt strict-order -domain-needed domain=example.com expand-hosts -local=/example.com/ +domain-needed +local=// except-interface=lo bind-interfaces listen-address=192.168.122.1 diff --git a/tests/networkxml2confdata/netboot-proxy-network.conf b/tests/networkxml2confdata/netboot-proxy-network.conf index 1e969fa..f4d3880 100644 --- a/tests/networkxml2confdata/netboot-proxy-network.conf +++ b/tests/networkxml2confdata/netboot-proxy-network.conf @@ -5,10 +5,10 @@ ## ## dnsmasq conf file created by libvirt strict-order -domain-needed domain=example.com expand-hosts -local=/example.com/ +domain-needed +local=// except-interface=lo bind-interfaces listen-address=192.168.122.1 -- 1.8.5.3

The previous patch fixed "forwardPlainNames" so that it really is doing only what is intended, but left the default to be "forwardPlainNames='no'". Discussion around the initial version of that patch led to the decision that the default should instead be "forwardPlainNames='yes'" (i.e. the original behavior before commit f3886825). This patch makes that change to the default. --- src/conf/network_conf.c | 32 +++++++++++++++------- src/conf/network_conf.h | 17 ++++++++++-- src/network/bridge_driver.c | 3 +- tests/networkxml2confdata/dhcp6-nat-network.conf | 2 -- tests/networkxml2confdata/dhcp6-network.conf | 2 -- .../dhcp6host-routed-network.conf | 2 -- tests/networkxml2confdata/isolated-network.conf | 2 -- .../nat-network-dns-forwarders.conf | 2 -- .../nat-network-dns-srv-record-minimal.conf | 2 -- .../nat-network-dns-srv-record.conf | 2 -- .../nat-network-dns-txt-record.conf | 2 -- tests/networkxml2confdata/nat-network.conf | 2 -- tests/networkxml2confdata/netboot-network.conf | 2 -- .../networkxml2confdata/netboot-proxy-network.conf | 2 -- tests/networkxml2confdata/routed-network.conf | 2 -- tests/networkxml2xmlout/nat-network-dns-hosts.xml | 2 +- .../nat-network-dns-more-hosts.xml | 2 +- .../nat-network-no-hosts.xml | 1 + 18 files changed, 42 insertions(+), 39 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 8803143..e59938c 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1,7 +1,7 @@ /* * network_conf.c: network XML handling * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -66,6 +66,12 @@ VIR_ENUM_IMPL(virNetworkForwardDriverName, "kvm", "vfio") +VIR_ENUM_IMPL(virNetworkDNSForwardPlainNames, + VIR_NETWORK_DNS_FORWARD_PLAIN_NAMES_LAST, + "default", + "yes", + "no") + virNetworkObjPtr virNetworkFindByUUID(virNetworkObjListPtr nets, const unsigned char *uuid) { @@ -1053,9 +1059,9 @@ virNetworkDNSDefParseXML(const char *networkName, forwardPlainNames = virXPathString("string(./@forwardPlainNames)", ctxt); if (forwardPlainNames) { - if (STREQ(forwardPlainNames, "yes")) { - def->forwardPlainNames = true; - } else if (STRNEQ(forwardPlainNames, "no")) { + def->forwardPlainNames + = virNetworkDNSForwardPlainNamesTypeFromString(forwardPlainNames); + if (def->forwardPlainNames <= 0) { virReportError(VIR_ERR_XML_ERROR, _("Invalid dns forwardPlainNames setting '%s' " "in network '%s'"), @@ -2295,19 +2301,26 @@ static int virNetworkDNSDefFormat(virBufferPtr buf, const virNetworkDNSDef *def) { - int result = 0; size_t i, j; if (!(def->forwardPlainNames || def->forwarders || def->nhosts || def->nsrvs || def->ntxts)) - goto out; + return 0; virBufferAddLit(buf, "<dns"); if (def->forwardPlainNames) { - virBufferAddLit(buf, " forwardPlainNames='yes'"); + const char *fwd = virNetworkDNSForwardPlainNamesTypeToString(def->forwardPlainNames); + + if (!fwd) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown forwardPlainNames type %d in network"), + def->forwardPlainNames); + return -1; + } + virBufferAsprintf(buf, " forwardPlainNames='%s'", fwd); if (!(def->forwarders || def->nhosts || def->nsrvs || def->ntxts)) { virBufferAddLit(buf, "/>\n"); - goto out; + return 0; } } @@ -2363,8 +2376,7 @@ virNetworkDNSDefFormat(virBufferPtr buf, } virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</dns>\n"); -out: - return result; + return 0; } static int diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index e3ef285..b84762a 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -1,7 +1,7 @@ /* * network_conf.h: network XML handling * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -112,10 +112,23 @@ struct _virNetworkDNSHostDef { char **names; }; +/* If forwardPlainNames is 0 (default), that is equivalent to "yes", + * but won't be encoded in newly formatted XML. + */ +typedef enum { + VIR_NETWORK_DNS_FORWARD_PLAIN_NAMES_DEFAULT = 0, /* silent "yes" */ + VIR_NETWORK_DNS_FORWARD_PLAIN_NAMES_YES, + VIR_NETWORK_DNS_FORWARD_PLAIN_NAMES_NO, + + VIR_NETWORK_DNS_FORWARD_PLAIN_NAMES_LAST, +} virNetworkDNSForwardPlainNamesType; + +VIR_ENUM_DECL(virNetworkDNSForwardPlainNames) + typedef struct _virNetworkDNSDef virNetworkDNSDef; typedef virNetworkDNSDef *virNetworkDNSDefPtr; struct _virNetworkDNSDef { - bool forwardPlainNames; + int forwardPlainNames; /* enum virNetworkDNSForwardPlainNamesType */ size_t ntxts; virNetworkDNSTxtDefPtr txts; size_t nhosts; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index f50795b..6796ee8 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -725,7 +725,8 @@ networkDnsmasqConfContents(virNetworkObjPtr network, network->def->domain); } - if (!network->def->dns.forwardPlainNames) { + if (network->def->dns.forwardPlainNames + == VIR_NETWORK_DNS_FORWARD_PLAIN_NAMES_NO) { virBufferAddLit(&configbuf, "domain-needed\n"); /* need to specify local=// whether or not a domain is * specified, unless the config says we should forward "plain" diff --git a/tests/networkxml2confdata/dhcp6-nat-network.conf b/tests/networkxml2confdata/dhcp6-nat-network.conf index 050f3db..f270a43 100644 --- a/tests/networkxml2confdata/dhcp6-nat-network.conf +++ b/tests/networkxml2confdata/dhcp6-nat-network.conf @@ -5,8 +5,6 @@ ## ## dnsmasq conf file created by libvirt strict-order -domain-needed -local=// except-interface=lo bind-dynamic interface=virbr0 diff --git a/tests/networkxml2confdata/dhcp6-network.conf b/tests/networkxml2confdata/dhcp6-network.conf index 92ea2a4..f0a9660 100644 --- a/tests/networkxml2confdata/dhcp6-network.conf +++ b/tests/networkxml2confdata/dhcp6-network.conf @@ -7,8 +7,6 @@ strict-order domain=mynet expand-hosts -domain-needed -local=// except-interface=lo bind-dynamic interface=virbr0 diff --git a/tests/networkxml2confdata/dhcp6host-routed-network.conf b/tests/networkxml2confdata/dhcp6host-routed-network.conf index f8f05c2..ad6db36 100644 --- a/tests/networkxml2confdata/dhcp6host-routed-network.conf +++ b/tests/networkxml2confdata/dhcp6host-routed-network.conf @@ -5,8 +5,6 @@ ## ## dnsmasq conf file created by libvirt strict-order -domain-needed -local=// except-interface=lo bind-dynamic interface=virbr1 diff --git a/tests/networkxml2confdata/isolated-network.conf b/tests/networkxml2confdata/isolated-network.conf index f8997bd..6ba34ae 100644 --- a/tests/networkxml2confdata/isolated-network.conf +++ b/tests/networkxml2confdata/isolated-network.conf @@ -5,8 +5,6 @@ ## ## dnsmasq conf file created by libvirt strict-order -domain-needed -local=// except-interface=lo bind-interfaces listen-address=192.168.152.1 diff --git a/tests/networkxml2confdata/nat-network-dns-forwarders.conf b/tests/networkxml2confdata/nat-network-dns-forwarders.conf index 5f95f77..8bf3b9c 100644 --- a/tests/networkxml2confdata/nat-network-dns-forwarders.conf +++ b/tests/networkxml2confdata/nat-network-dns-forwarders.conf @@ -8,8 +8,6 @@ strict-order no-resolv server=8.8.8.8 server=8.8.4.4 -domain-needed -local=// except-interface=lo bind-dynamic interface=virbr0 diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf b/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf index 1e9b59c..ce4dd6f 100644 --- a/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf +++ b/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf @@ -5,8 +5,6 @@ ## ## dnsmasq conf file created by libvirt strict-order -domain-needed -local=// except-interface=lo bind-interfaces listen-address=192.168.122.1 diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record.conf b/tests/networkxml2confdata/nat-network-dns-srv-record.conf index 53d044a..b47cbe7 100644 --- a/tests/networkxml2confdata/nat-network-dns-srv-record.conf +++ b/tests/networkxml2confdata/nat-network-dns-srv-record.conf @@ -5,8 +5,6 @@ ## ## dnsmasq conf file created by libvirt strict-order -domain-needed -local=// except-interface=lo bind-dynamic interface=virbr0 diff --git a/tests/networkxml2confdata/nat-network-dns-txt-record.conf b/tests/networkxml2confdata/nat-network-dns-txt-record.conf index 921cae1..ff53f4e 100644 --- a/tests/networkxml2confdata/nat-network-dns-txt-record.conf +++ b/tests/networkxml2confdata/nat-network-dns-txt-record.conf @@ -5,8 +5,6 @@ ## ## dnsmasq conf file created by libvirt strict-order -domain-needed -local=// except-interface=lo bind-dynamic interface=virbr0 diff --git a/tests/networkxml2confdata/nat-network.conf b/tests/networkxml2confdata/nat-network.conf index beb714b..ced4123 100644 --- a/tests/networkxml2confdata/nat-network.conf +++ b/tests/networkxml2confdata/nat-network.conf @@ -5,8 +5,6 @@ ## ## dnsmasq conf file created by libvirt strict-order -domain-needed -local=// except-interface=lo bind-dynamic interface=virbr0 diff --git a/tests/networkxml2confdata/netboot-network.conf b/tests/networkxml2confdata/netboot-network.conf index ce33176..8ea1f67 100644 --- a/tests/networkxml2confdata/netboot-network.conf +++ b/tests/networkxml2confdata/netboot-network.conf @@ -7,8 +7,6 @@ strict-order domain=example.com expand-hosts -domain-needed -local=// except-interface=lo bind-interfaces listen-address=192.168.122.1 diff --git a/tests/networkxml2confdata/netboot-proxy-network.conf b/tests/networkxml2confdata/netboot-proxy-network.conf index f4d3880..4774a92 100644 --- a/tests/networkxml2confdata/netboot-proxy-network.conf +++ b/tests/networkxml2confdata/netboot-proxy-network.conf @@ -7,8 +7,6 @@ strict-order domain=example.com expand-hosts -domain-needed -local=// except-interface=lo bind-interfaces listen-address=192.168.122.1 diff --git a/tests/networkxml2confdata/routed-network.conf b/tests/networkxml2confdata/routed-network.conf index 62ffd7a..970aa3c 100644 --- a/tests/networkxml2confdata/routed-network.conf +++ b/tests/networkxml2confdata/routed-network.conf @@ -5,8 +5,6 @@ ## ## dnsmasq conf file created by libvirt strict-order -domain-needed -local=// except-interface=lo bind-dynamic interface=virbr1 diff --git a/tests/networkxml2xmlout/nat-network-dns-hosts.xml b/tests/networkxml2xmlout/nat-network-dns-hosts.xml index a2f46da..17e2697 100644 --- a/tests/networkxml2xmlout/nat-network-dns-hosts.xml +++ b/tests/networkxml2xmlout/nat-network-dns-hosts.xml @@ -5,7 +5,7 @@ <interface dev='eth0'/> </forward> <bridge name='virbr0' stp='on' delay='0'/> - <dns> + <dns forwardPlainNames='no'> <host ip='192.168.122.1'> <hostname>host</hostname> <hostname>gateway</hostname> diff --git a/tests/networkxml2xmlupdateout/nat-network-dns-more-hosts.xml b/tests/networkxml2xmlupdateout/nat-network-dns-more-hosts.xml index 6554c07..5464d90 100644 --- a/tests/networkxml2xmlupdateout/nat-network-dns-more-hosts.xml +++ b/tests/networkxml2xmlupdateout/nat-network-dns-more-hosts.xml @@ -5,7 +5,7 @@ <interface dev='eth0'/> </forward> <bridge name='virbr0' stp='on' delay='0'/> - <dns> + <dns forwardPlainNames='no'> <host ip='f0:d::f0:d'> <hostname>pudding</hostname> </host> diff --git a/tests/networkxml2xmlupdateout/nat-network-no-hosts.xml b/tests/networkxml2xmlupdateout/nat-network-no-hosts.xml index bc9e419..ee2d773 100644 --- a/tests/networkxml2xmlupdateout/nat-network-no-hosts.xml +++ b/tests/networkxml2xmlupdateout/nat-network-no-hosts.xml @@ -5,6 +5,7 @@ <interface dev='eth0'/> </forward> <bridge name='virbr0' stp='on' delay='0'/> + <dns forwardPlainNames='no'/> <ip address='192.168.122.1' netmask='255.255.255.0'> </ip> </network> -- 1.8.5.3