11:59 a.m.
As I said in a previous message, dnsmasq is forwarding a number of
queries upstream that should not be done. There still remains an MX
query for a plain name with no domain specified that will be forwarded
is dnsmasq has --domain=xxx --local=/xxx/ specified. This does not
happen with no domain name and --local=// ... not a libvirt problem.
BTW, thanks again to Claudio Bley!
====================================================
diff -uNr libvirt-0.9.11.4.orig/src/network/bridge_driver.c
libvirt-0.9.11.4/src/network/bridge_driver.c
--- libvirt-0.9.11.4.orig/src/network/bridge_driver.c 2012-06-15
14:23:21.000000000 -0400
+++ libvirt-0.9.11.4/src/network/bridge_driver.c 2012-08-22
12:16:45.263488789 -0400
@@ -490,8 +490,15 @@
*/
virCommandAddArgList(cmd, "--strict-order", "--bind-interfaces",
NULL);
- if (network->def->domain)
+ if (network->def->domain) {
virCommandAddArgList(cmd, "--domain", network->def->domain,
NULL);
+ virCommandAddArgFormat(cmd, "--local=/%s/",
network->def->domain);
+ virCommandAddArgList(cmd, "--domain-needed", "--filterwin2k",
NULL);
+ }
+ else { /* need to specify local even if no domain specified */
+ virCommandAddArg(cmd, "--local=//");
+ virCommandAddArgList(cmd, "--domain-needed", "--filterwin2k",
NULL);
+ }
if (pidfile)
virCommandAddArgPair(cmd, "--pid-file", pidfile);
diff -uNr
libvirt-0.9.11.4.orig/tests/networkxml2argvdata/isolated-network.argv
libvirt-0.9.11.4/tests/networkxml2argvdata/isolated-network.argv
---
libvirt-0.9.11.4.orig/tests/networkxml2argvdata/isolated-network.argv
2012-06-15 14:21:54.000000000 -0400
+++ libvirt-0.9.11.4/tests/networkxml2argvdata/isolated-network.argv
2012-08-22 12:20:37.700995728 -0400
@@ -1,4 +1,5 @@
-@DNSMASQ@ --strict-order --bind-interfaces --conf-file= \
+@DNSMASQ@ --strict-order --bind-interfaces \
+--local=// --domain-needed --filterwin2k --conf-file= \
--except-interface lo --dhcp-option=3 --no-resolv \
--listen-address 192.168.152.1 \
--dhcp-range 192.168.152.2,192.168.152.254 \
diff -uNr
libvirt-0.9.11.4.orig/tests/networkxml2argvdata/nat-network.argv
libvirt-0.9.11.4/tests/networkxml2argvdata/nat-network.argv
--- libvirt-0.9.11.4.orig/tests/networkxml2argvdata/nat-network.argv
2012-06-15 14:21:54.000000000 -0400
+++ libvirt-0.9.11.4/tests/networkxml2argvdata/nat-network.argv
2012-08-22 12:21:24.481703184 -0400
@@ -1,4 +1,5 @@
-@DNSMASQ@ --strict-order --bind-interfaces --conf-file= \
+@DNSMASQ@ --strict-order --bind-interfaces \
+--local=// --domain-needed --filterwin2k --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 \
diff -uNr
libvirt-0.9.11.4.orig/tests/networkxml2argvdata/nat-network-dns-hosts.argv
libvirt-0.9.11.4/tests/networkxml2argvdata/nat-network-dns-hosts.argv
---
libvirt-0.9.11.4.orig/tests/networkxml2argvdata/nat-network-dns-hosts.argv
2012-06-15 14:21:54.000000000 -0400
+++
libvirt-0.9.11.4/tests/networkxml2argvdata/nat-network-dns-hosts.argv
2012-08-22 12:25:30.378218203 -0400
@@ -1,3 +1,4 @@
@DNSMASQ@ --strict-order --bind-interfaces --domain example.com \
+--local=/example.com/ --domain-needed --filterwin2k \
--conf-file= --except-interface lo --listen-address 192.168.122.1 \
--expand-hosts --addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts\
diff -uNr
libvirt-0.9.11.4.orig/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
libvirt-0.9.11.4/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
---
libvirt-0.9.11.4.orig/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
2012-06-15 14:21:54.000000000 -0400
+++
libvirt-0.9.11.4/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
2012-08-22 12:22:35.471268869 -0400
@@ -1,7 +1,7 @@
@DNSMASQ@ \
--strict-order \
--bind-interfaces \
---conf-file= \
+--local=// --domain-needed --filterwin2k --conf-file= \
--except-interface lo \
--srv-host=name.tcp.test-domain-name,.,1024,10,10 \
--listen-address 192.168.122.1 \
diff -uNr
libvirt-0.9.11.4.orig/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
libvirt-0.9.11.4/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
---
libvirt-0.9.11.4.orig/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
2012-06-15 14:21:54.000000000 -0400
+++
libvirt-0.9.11.4/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
2012-08-22 12:22:48.422191468 -0400
@@ -1,7 +1,7 @@
@DNSMASQ@ \
--strict-order \
--bind-interfaces \
---conf-file= \
+--local=// --domain-needed --filterwin2k --conf-file= \
--except-interface lo \
--srv-host=name.tcp.,,,, \
--listen-address 192.168.122.1 \
diff -uNr
libvirt-0.9.11.4.orig/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
libvirt-0.9.11.4/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
---
libvirt-0.9.11.4.orig/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
2012-06-15 14:21:54.000000000 -0400
+++
libvirt-0.9.11.4/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
2012-08-22 12:23:47.642834970 -0400
@@ -1,4 +1,5 @@
-@DNSMASQ@ --strict-order --bind-interfaces --conf-file= \
+@DNSMASQ@ --strict-order --bind-interfaces \
+--local=// --domain-needed --filterwin2k --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 \
diff -uNr
libvirt-0.9.11.4.orig/tests/networkxml2argvdata/netboot-network.argv
libvirt-0.9.11.4/tests/networkxml2argvdata/netboot-network.argv
--- libvirt-0.9.11.4.orig/tests/networkxml2argvdata/netboot-network.argv
2012-06-15 14:21:54.000000000 -0400
+++ libvirt-0.9.11.4/tests/networkxml2argvdata/netboot-network.argv
2012-08-22 12:24:31.838569611 -0400
@@ -1,5 +1,6 @@
@DNSMASQ@ --strict-order --bind-interfaces --domain example.com \
---conf-file= --except-interface lo --listen-address 192.168.122.1 \
+--local=/example.com/ --domain-needed --filterwin2k --conf-file= \
+--except-interface lo --listen-address 192.168.122.1 \
--dhcp-range 192.168.122.2,192.168.122.254 \
--dhcp-leasefile=/var/lib/libvirt/dnsmasq/netboot.leases \
--dhcp-lease-max=253 --dhcp-no-override --expand-hosts --enable-tftp \
diff -uNr
libvirt-0.9.11.4.orig/tests/networkxml2argvdata/netboot-proxy-network.argv
libvirt-0.9.11.4/tests/networkxml2argvdata/netboot-proxy-network.argv
---
libvirt-0.9.11.4.orig/tests/networkxml2argvdata/netboot-proxy-network.argv
2012-06-15 14:21:54.000000000 -0400
+++
libvirt-0.9.11.4/tests/networkxml2argvdata/netboot-proxy-network.argv
2012-08-22 12:27:24.586499884 -0400
@@ -1,5 +1,6 @@
@DNSMASQ@ --strict-order --bind-interfaces --domain example.com \
---conf-file= --except-interface lo --listen-address 192.168.122.1 \
+--local=/example.com/ --domain-needed --filterwin2k --conf-file= \
+--except-interface lo --listen-address 192.168.122.1 \
--dhcp-range 192.168.122.2,192.168.122.254 \
--dhcp-leasefile=/var/lib/libvirt/dnsmasq/netboot.leases \
--dhcp-lease-max=253 --dhcp-no-override --expand-hosts \
diff -uNr
libvirt-0.9.11.4.orig/tests/networkxml2argvdata/routed-network.argv
libvirt-0.9.11.4/tests/networkxml2argvdata/routed-network.argv
--- libvirt-0.9.11.4.orig/tests/networkxml2argvdata/routed-network.argv
2012-06-15 14:21:54.000000000 -0400
+++ libvirt-0.9.11.4/tests/networkxml2argvdata/routed-network.argv
2012-08-22 12:28:06.111841912 -0400
@@ -1,2 +1,3 @@
-@DNSMASQ@ --strict-order --bind-interfaces --conf-file= \
+@DNSMASQ@ --strict-order --bind-interfaces \
+--local=// --domain-needed --filterwin2k --conf-file= \
--except-interface lo --listen-address 192.168.122.1\
==========================================
Just in case, I also attached the patch.
Gene
12:39 p.m.
On 08/22/2012 10:59 AM, Gene Czarcinski wrote:
As I said in a previous message, dnsmasq is forwarding a number of
queries upstream that should not be done. There still remains an MX
query for a plain name with no domain specified that will be forwarded
is dnsmasq has --domain=xxx --local=/xxx/ specified. This does not
happen with no domain name and --local=// ... not a libvirt problem.
This message needs adjustment before it is appropriate for libvirt.git
(a year from now, someone reading 'git log' will have no idea what it
was '(updated)' from, nor know the URL to the 'previous message').
- if (network->def->domain)
+ if (network->def->domain) {
virCommandAddArgList(cmd, "--domain", network->def->domain,
NULL);
+ virCommandAddArgFormat(cmd, "--local=/%s/",
network->def->domain);
+ virCommandAddArgList(cmd, "--domain-needed",
"--filterwin2k",
NULL);
+ }
+ else { /* need to specify local even if no domain specified */
+ virCommandAddArg(cmd, "--local=//");
+ virCommandAddArgList(cmd, "--domain-needed",
"--filterwin2k",
NULL);
+ }
Simpler as:
if (network->def->domain)
virCommandArgPair(cmd, "--domain", network->def->domain);
virCommandAddArgFormat(cmd, "--local=/%s/",
network->def->domain ? network->def->domain : "");
virCommandAddArgList(cmd, "--domain-needed", "--filterwin2k", NULL);
with a corresponding tweak in the testsuite to recognize
'--domain=example.com' as a result.
Just in case, I also attached the patch.
Thanks, that helped. Your mail failed to make it through 'git am', but
your attachment made it through 'git apply' so I was still able to piece
the two together to form the commit message using the headers from your
email for correct attribution. But using 'git send-email' in the future
would make it easier to apply your patches; we have some hints on that
in our HACKING file.
ACK and pushed with the above tweak, and with adding you to AUTHORS (let
us know if you prefer any other spelling or email address; the file is
in UTF-8).
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:47 p.m.
On 08/22/2012 11:39 AM, Eric Blake wrote:
On 08/22/2012 10:59 AM, Gene Czarcinski wrote:
> As I said in a previous message, dnsmasq is forwarding a number of
> queries upstream that should not be done. There still remains an MX
> query for a plain name with no domain specified that will be forwarded
> is dnsmasq has --domain=xxx --local=/xxx/ specified. This does not
> happen with no domain name and --local=// ... not a libvirt problem.
>
ACK and pushed with the above tweak, and with adding you to AUTHORS (let
us know if you prefer any other spelling or email address; the file is
in UTF-8).
Oh, and now that I've already pushed, I have a high-level question: what
is the minimum version of 'dnsmasq' that supports the command-line
syntax that this patch introduces?
+--local=// --domain-needed --filterwin2k \
If older dnsmasq doesn't recognize --local=// or the new --domain-needed
or --filterwin2k options, then we either need to make this code
conditional based on probing 'dnsmasq --help' at startup, or else change
the spec file to require a larger minimum version of dnsmasq (we already
require 2.41 for other reasons).
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:36 p.m.
On 08/22/2012 01:47 PM, Eric Blake wrote:
On 08/22/2012 11:39 AM, Eric Blake wrote:
> On 08/22/2012 10:59 AM, Gene Czarcinski wrote:
>> As I said in a previous message, dnsmasq is forwarding a number of
>> queries upstream that should not be done. There still remains an MX
>> query for a plain name with no domain specified that will be forwarded
>> is dnsmasq has --domain=xxx --local=/xxx/ specified. This does not
>> happen with no domain name and --local=// ... not a libvirt problem.
>>
> ACK and pushed with the above tweak, and with adding you to AUTHORS (let
> us know if you prefer any other spelling or email address; the file is
> in UTF-8).
Oh, and now that I've already pushed, I have a high-level question: what
is the minimum version of 'dnsmasq' that supports the command-line
syntax that this patch introduces?
+--local=// --domain-needed --filterwin2k \
I honestly do not know for sure ... I
am running dnsmasq-2.59 and
checking the dnsmasq website shows that the latest is 2.63. The oldest
"active" tarball is 2.45 and the example dnsmasq.conf it it has local=
domain-needed and filterwin2k
I believe these have been around for some time and no version that
libvirt would support will not support them.
BTW, I believe that you folks are making the right choice in using the
long-names form of the options ... the single character versions can
just be too confusing: "-s" and "-S" mean two very different things.
"-s" is the same as "--domain" and "-S" is the same as
"--server" which
is also the same as "--local".
I am not absolutely sure that "--filterwin2k" is needed to get rid of
the bad/undesired forwarding but it sounded like an option that should
be used in any case.
If older dnsmasq doesn't recognize --local=// or the new --domain-needed
or --filterwin2k options, then we either need to make this code
conditional based on probing 'dnsmasq --help' at startup, or else change
the spec file to require a larger minimum version of dnsmasq (we already
require 2.41 for other reasons).
I went back and checked some of the older versions of dnsmasq (2.41 and
2.37). local=//, --domain-needed, and --filterwin2k are all present in
the example dnsmasq.conf files. This is not to say the the dnsmasq
software does exactly the same thing in all versions. According the the
CHANGELOG, there was some code "tweaking" for domain-needed in 2.58.
I am truly sorry that my patch and email made life a bit difficult for
you. I have not delved into any of the virtualization code and your
setup & convensions are all reasonable but I just did not know. The
last package I hacked around with was NetworkManager and libvirt is a
lot different.
As far as the code change you made to my patch, both versions work but
yours is a bit more eligant.
Gene
4:17 p.m.
On 08/22/2012 01:36 PM, Gene Czarcinski wrote:
> Oh, and now that I've already pushed, I have a high-level
question: what
> is the minimum version of 'dnsmasq' that supports the command-line
> syntax that this patch introduces?
>
I went back and checked some of the older versions of dnsmasq (2.41 and
2.37). local=//, --domain-needed, and --filterwin2k are all present in
That should be good enough. Thanks for the research!
the example dnsmasq.conf files. This is not to say the the dnsmasq
software does exactly the same thing in all versions. According the the
CHANGELOG, there was some code "tweaking" for domain-needed in 2.58.
May be true, but hopefully we'll get a decent report if someone runs
into subtleties caused by this; but at least we know we won't hit anyone
complaining that dnsmasq no longer starts due to unknown options.
I am truly sorry that my patch and email made life a bit difficult for
you. I have not delved into any of the virtualization code and your
setup & convensions are all reasonable but I just did not know. The
last package I hacked around with was NetworkManager and libvirt is a
lot different.
No problem - we're used to helping out first-time contributors. Open
source is successful when you give people the benefit of a doubt, and
encourage their contribution in spite of difficulties. On the converse
side, touching up every single patch doesn't scale well, which is why we
write HACKING documents, and why we try to be friendly even when asking
for a re-submission, and why we aren't quite so lenient on HACKING
violations from repeated contributors.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:12 a.m.
On 08/22/2012 11:47 AM, Eric Blake wrote:
On 08/22/2012 11:39 AM, Eric Blake wrote:
> On 08/22/2012 10:59 AM, Gene Czarcinski wrote:
>> As I said in a previous message, dnsmasq is forwarding a number of
>> queries upstream that should not be done. There still remains an MX
>> query for a plain name with no domain specified that will be forwarded
>> is dnsmasq has --domain=xxx --local=/xxx/ specified. This does not
>> happen with no domain name and --local=// ... not a libvirt problem.
>>
>
> ACK and pushed with the above tweak, and with adding you to AUTHORS (let
> us know if you prefer any other spelling or email address; the file is
> in UTF-8).
Oh, and now that I've already pushed, I have a high-level question: what
is the minimum version of 'dnsmasq' that supports the command-line
syntax that this patch introduces?
+--local=// --domain-needed --filterwin2k \
If older dnsmasq doesn't recognize --local=// or the new --domain-needed
or --filterwin2k options, then we either need to make this code
conditional based on probing 'dnsmasq --help' at startup, or else change
the spec file to require a larger minimum version of dnsmasq (we already
require 2.41 for other reasons).
Just as I feared, we introduced a regression:
https://bugzilla.redhat.com/show_bug.cgi?id=854137
Apparently, --filterwin2k disables features needed by Windows guests.
Gene, what is the benefit vs. cost of adding this flag? I'm trying to
figure out whether we need to expose it as something user-configurable,
or whether we should just revert back to the pre-patch version that did
not supply that option.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:55 a.m.
On 09/04/2012 11:12 AM, Eric Blake wrote:
On 08/22/2012 11:47 AM, Eric Blake wrote:
> On 08/22/2012 11:39 AM, Eric Blake wrote:
>> On 08/22/2012 10:59 AM, Gene Czarcinski wrote:
>>> As I said in a previous message, dnsmasq is forwarding a number of
>>> queries upstream that should not be done. There still remains an MX
>>> query for a plain name with no domain specified that will be forwarded
>>> is dnsmasq has --domain=xxx --local=/xxx/ specified. This does not
>>> happen with no domain name and --local=// ... not a libvirt problem.
>>>
>> ACK and pushed with the above tweak, and with adding you to AUTHORS (let
>> us know if you prefer any other spelling or email address; the file is
>> in UTF-8).
> Oh, and now that I've already pushed, I have a high-level question: what
> is the minimum version of 'dnsmasq' that supports the command-line
> syntax that this patch introduces?
>
> +--local=// --domain-needed --filterwin2k \
>
> If older dnsmasq doesn't recognize --local=// or the new --domain-needed
> or --filterwin2k options, then we either need to make this code
> conditional based on probing 'dnsmasq --help' at startup, or else change
> the spec file to require a larger minimum version of dnsmasq (we already
> require 2.41 for other reasons).
Just as I feared, we introduced a regression:
https://bugzilla.redhat.com/show_bug.cgi?id=854137
Apparently, --filterwin2k disables features needed by Windows guests.
Gene, what is the benefit vs. cost of adding this flag? I'm trying to
figure out whether we need to expose it as something user-configurable,
or whether we should just revert back to the pre-patch version that did
not supply that option.
I already had some second thoughts about --filterwin2k but you had
pushed it. "--filterwin2k" should be removed.
Unfortunately, as I said in another message, this "--local=" and
"--domain-needed" are required but are not complete. dnsmasq will
forward PTR queries which fall in the dnsmasq domain. I need to figure
out how to create something of the form
"--local=/xxx.yyy.zzz.in-addr.arpa/". I have tested what the effect of
this is and it works.
Gene
10:41 a.m.
On 09/05/2012 07:55 AM, Gene Czarcinski wrote:
On 09/04/2012 11:12 AM, Eric Blake wrote:
> On 08/22/2012 11:47 AM, Eric Blake wrote:
>> On 08/22/2012 11:39 AM, Eric Blake wrote:
>>> On 08/22/2012 10:59 AM, Gene Czarcinski wrote:
>>>> As I said in a previous message, dnsmasq is forwarding a number of
>>>> queries upstream that should not be done. There still remains an MX
>>>> query for a plain name with no domain specified that will be
>>>> forwarded
>>>> is dnsmasq has --domain=xxx --local=/xxx/ specified. This does not
>>>> happen with no domain name and --local=// ... not a libvirt problem.
>>>>
>>> ACK and pushed with the above tweak, and with adding you to AUTHORS
>>> (let
>>> us know if you prefer any other spelling or email address; the file is
>>> in UTF-8).
>> Oh, and now that I've already pushed, I have a high-level question:
>> what
>> is the minimum version of 'dnsmasq' that supports the command-line
>> syntax that this patch introduces?
>>
>> +--local=// --domain-needed --filterwin2k \
>>
>> If older dnsmasq doesn't recognize --local=// or the new
>> --domain-needed
>> or --filterwin2k options, then we either need to make this code
>> conditional based on probing 'dnsmasq --help' at startup, or else
>> change
>> the spec file to require a larger minimum version of dnsmasq (we
>> already
>> require 2.41 for other reasons).
> Just as I feared, we introduced a regression:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=854137
>
> Apparently, --filterwin2k disables features needed by Windows guests.
> Gene, what is the benefit vs. cost of adding this flag? I'm trying to
> figure out whether we need to expose it as something user-configurable,
> or whether we should just revert back to the pre-patch version that did
> not supply that option.
>
I already had some second thoughts about --filterwin2k but you had
pushed it. "--filterwin2k" should be removed.
Yes, as rare as dialup lines are these days, I think it's highly
unlikely that anyone running a virt host will be connected to the rest
of the network in a way which will require bringing up a dialup network
connection in order to send a packet to a domain controller. So, I don't
think we should clutter the XML with such a specific option that will in
all likelyhood never be used.
4497
days inactive
4511
days old
7 comments
3 participants
participants (3)
-
Eric Blake -
Gene Czarcinski -
Laine Stump