On 12/02/2014 08:06 AM, Laine Stump wrote: > On 12/01/2014 01:50 PM, Josh Stone wrote: >> On 12/01/2014 05:18 AM, Martin Kletzander wrote: >>> On Mon, Dec 01, 2014 at 07:49:32AM -0500, Laine Stump wrote: >>>> On 11/30/2014 04:06 PM, Martin Kletzander wrote: >>>>> On Thu, Nov 20, 2014 at 06:46:41PM -0800, Josh Stone wrote: >>>>>> This adds a new "localOnly" attribute on the domain element of the >>>>>> network xml. With this set to "yes", DNS requests under that domain >>>>>> will only be resolved by libvirt's dnsmasq, never forwarded upstream. >>>>>> >>>>>> This was how it worked before commit f69a6b987d616, and I found that >>>>>> functionality useful. For example, I have my host's NetworkManager >>>>>> dnsmasq configured to forward that domain to libvirt's dnsmasq, so I can >>>>>> easily resolve guest names from outside. But if libvirt's dnsmasq >>>>>> doesn't know a name and forwards it to the host, I'd get an endless >>>>>> forwarding loop. Now I can set localOnly="yes" to prevent the loop. >>>>>> >>>>> I've found 2 things in this patch I'm not sure about: >>>>> >>>>> 1) According to the documentation about --local= parameter, it says: >>>>> "Setting this flag does not suppress reading of /etc/resolv.conf, >>>>> use -R to do that." Shouldn't "no-resolv" be added as the option >>>>> you want in the config file? >>>> No, "-R" (aka "no-resolv") tells dnsmasq to not use the servers listed >>>> in /etc/resolv.conf for *anything*. In this case, all that is wanted is >>>> to resolve requests _within the configured domain_ only locally, and not >>>> forward those requests upstream. Requests for any other domain _should_ >>>> be forwarded to whatever server is listed in /etc/resolv.conf. Based on >>>> a discussion with Josh, I'm certain that this is the behavior he wants >>>> (and it makes sense to have it available). >>>> >>> OK, I just misunderstood this part, sorry for that. >> Right, I want to keep its own domain local, but go out otherwise. >> >>>>> 2) If your answer to the previous question is "NO", which might very >>>>> well be the case, for example if you don't want to forward that >>>>> one particular domain only (and I misunderstood the commit >>>>> message), I think you should just use the ",local" subparameter of >>>>> the domain parameter (domain=example.com,local). >>>> Interesting idea - this requires specifying the IP/prefix of the network >>>> in the option along with domain (which I suppose isn't a problem), and >>>> has the same effect as adding local=/domain.com/ as well as the reverse >>>> resolution, which is a nice plus. However, the prefix for the network >>>> must be 8, 16, or 24 for this to work (because of the way that >>>> *.in-addr.arpa records are defined), so we can't do this all the time. >>>> The question is then - do we write in a special case to do it this way >>>> when the prefix is one of the right lengths, or just not do it at all? >>>> Having it "magically happen" for some configs and not others could lead >>>> to confusion, but having reverse resolution when possible would be very >>>> useful (for example, some sshd setups have a long delay if the >>>> reverse-resolve of the client's IP doesn't return something other than >>>> "not found"; a pointless piece of security theater, but still fairly >>>> common). >>>> >>> Looking at the documentation again, I understand it a bit more yet >>> again. Let's assume people use normal domain names. Libvirt can use >>> the IP address and netmask from the 'ip' element (if known) and >>> construct the domain parameter for the config file: >>> >>> domain=virt,192.168.13.0/24,local >>> >>> If there's no IP address known, then it could be constructed as >>> follows: >>> >>> domain=virt >>> local=/virt/ >>> >>> What do you say? > > Yeah, that's what I was suggesting as a possibility above (but wondering > if it was worth the ambiguity), but according to Josh's testing, there > is at least one version of dnsmasq that implements > "domain=blah,x.x.x.x/y,local" incorrectly, so I guess we can't use it, > at least not by itself. > >>> >>> >>> Currently this is possible to achieve with the following in the XML: >>> >>> <domain name='virt,192.168.13.0/24,local'/> >>> >>> But that's just plain ugly ;-) >> Interesting. If this works, and libvirt promises not to take it away >> from me later, then it would be enough to satisfy my itch. :) > > I'm actually glad that it didn't work, as even the presence of this > email could create a precedent that we don't want to exist. > > As a rule, libvirt does not like multiple attributes in a single string; > if multiple attributes are needed, we try to represent them separately > in the XML. > > In other words, don't count on the above XML working. As a matter of > fact, assume that it will be eliminated. Fair enough, I'll ignore this possibility. >> However, it doesn't actually seem to do the right thing. The option >> does survive from XML to the dnsmasq conf, but it doesn't seem to >> actually behave as local. When I manually tweak the conf and restart >> libvirt's dnsmasq myself, it seems that having separate "domain=foo" and >> "local=/foo/" terminates properly, but having a oneliner >> "domain=foo,192.168.122.0/24,local" is still forwarding to the host. >> Which then forwards back to libvirt, and I get my loop. :( >> >> This is dnsmasq-2.72-3.fc21.x86_64. Maybe dnsmasq only only treats it >> local when coming from that subnet? But it's documented as equivalent >> to local=/foo/, so maybe dnsmasq has a bug here. Did you try it? > > From the other mail it sounds like you discovered a bug in dnsmasq. But > libvirt will anyway want to work even with buggy versions of dnsmasq, so > perhaps localOnly should continue to add local=/foo/, but also add > ",${ip}/$prefix,local" to the domain option *if* the prefix is 8, 16, or > 24. I would say this can/should be done in a separate patch (which would > update the test cases and the docs accordingly). Once you have to have the local=/domain/ option anyway, the only other thing you really get is rDNS. Maybe <ip> or its <dhcp> should have their own localOnly attribute to restrict the in-addr.arpa separately? > Back to the code of your patch - the only issue I see with it is what > John caught (missing the addition to virNetworkDefFormatBuf(), and using > virTriStateBool(To|From)String rather than comparing to "yes"). Other > than that, it would have been nice if this config could be in the <dns> > element, but <domain> is at the top level of network because it is used > by both dns and dhcp, and this new attribute is most closely associated > with the domain name, so that's not really possible. I'll send an update for tristate comparisons and formatting. Thanks, Josh -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list