[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Add support for DNS TXT records



On 03/29/2011 01:16 PM, Paolo Bonzini wrote:
> On 03/29/2011 12:52 PM, Michal Novotny wrote:
>> [snip]
>>> It would be great to:
>>>
>>> 1) add<user-class>  and<vendor-class>  tags inside<dhcp>  that allow
>>> filtering according to user/vendor classes
>> Well, I didn't know this is supported by DNSMasq but it seems to be
> Yes, I am using it. :)


Great. Good to implement this then however I'm not sure about the
<network-id> parameter for DNSMasq. What should network-id tag mean and
what should it match?


>>         -4, --dhcp-mac=<network-id>,<MAC address>
>>                Map from a MAC address to a network-id tag. The MAC
>> address may include wildcards. For example
>> --dhcp-mac=3com,01:34:23:*:*:* will set the  tag  "3com"  for  any  host
>>                whose MAC address matches the pattern.
> Interesting.
>


Well, should we support this as well?


>>> 2) allow to specify<bootp>  inside those as well as inside<range>
>>> or<host>  elements.
>> Right, there's bootp option:
> It's already supported by libvirt.


Good. So the patch won't implement this.


>>> 3) add support for DHCP options besides bootp, with a tag like<option
>>> force="yes|no" name="..." value="...">.
>>>
>>> For example, my router's DHCP configuration would look like this:
>>>
>>> <dhcp>
>>>    <range ...>
>>>    <user-class prefix="iPXE">
>>>      <bootp file="http://playground.usersys.redhat.com/pxe/boot.ipxe";>
>>>    </user-class>
>>>    <bootp file="undionly.kpxe">
>>> </dhcp>
>>>


Basically this is using boot.ipxe file for the prefix of iPXE and
falling back to default undionly.kpxe... right? Is this what you mean by
your definition?


>> That's not a bad idea at all and I think it's worth it however
>> originally my patch was about DNS and not DHCP.
> Yes, of course.
>
> Thinking more about it, <range ...> could also be (optionally) placed 
> inside <user-class> and <vendor-class> ("serve this range only to this 
> user class or vendor class").


Well, this could be a good thing as well since for various
vendor/user-classes we should be having different ranges.


>> I have to admit that DNS
>> TXT record only patch was not the right thing to be implemented since I
>> should have implemented all the DNS records supported
> Should you?  I am not sure of that.  Are they really so useful, except 
> for CNAME (whose functionality dnsmasq more or less supports using A and 
> PTR records) and maybe SRV?  Remember that A and PTR records are added 
> automatically by dnsmasq based on /etc/hosts.
>
> Perhaps, you could implement (instead of tags for PTR, CNAME, etc.)
>
>    <dns>
>      <host ip="192.168.122.1">
>        <hostname>host1</hostname>
>        <hostname>host2</hostname>
>        <hostname>host3</hostname>
>      </host>
>    </dns>
>
> instead, which would write a file
>
>    192.168.122.1 host1 host2 host3
>
> and pass it to dnsmasq via --addn-hosts.  But every feature should be 
> added as a separate patch.


Well, that's a good idea IMHO. If we write a file and pass it directly
to dnsmasq using -addn-hosts then the implementation could be much
better since you'll still be able to define it in the XML file and the
libvirt network configuration and the file will be generate for you on
network-start only.


>> I tried following invocations of dnsmasq (I tried it on port 52 instead
>> not to mess up with my current networking):
>>
>> first-term# dnsmasq --strict-order --bind-interfaces
>> --pid-file=/var/run/libvirt/network/default.pid --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/default.leases
>> --dhcp-lease-max=253 --dhcp-no-override --no-daemon -p 52
>> --txt-record="some name","some value"
>>
>> second-term$ dig TXT some name @192.168.122.1 -p 52
>> connection timed out; no servers could be reached
>>
>> second-term$ dig TXT "some name" @192.168.122.1 -p 52
>> ;; ANSWER SECTION:
>> some\032name.           0       IN      TXT     "some value"
> This is just how dnsmasq prints the request.  You can see with wireshark 
> that the request is really for "some name".  BTW, please test your patch 
> with commas in the name.  Those should be forbidden probably (not sure 
> about the value).

You're right. It's really "some name" and it's matter of how dnsmasq
prints the request. Also, for the commas, I did try in both name and
value and it was not working at all in the name and for the value is was
showing 2 values basically:

;; ANSWER SECTION:
some\032name.           0       IN      TXT     "some" "value"

Value in wireshark is presented separated by a NULL byte, i.e.
"some\0value" (although wireshark shows comma character instead of \0
since it doesn't escape that way) so I guess we should disallow usage of
commas as well.

To sum this up, we should disallow usage of commas and quotes (or we
should at least escape the quotes).

>> So what do you think about this? Also, do you think we should implement
>> everything connected to DNSMasq mentioned there (i.e. both DNS and DHCP
>> stuff) in one commit, just few separate patches (e.g. one for DNS and
>> second for DHCP/BOOTP) ?
> Many many separate patches.
>
> BTW, regarding this particular series, you should update the XML schema, 
> and add many many testcases.  Do this for TXT, then you can start 
> thinking about everything else.
>

Ok. Good. Thanks!

Michal

-- 
Michal Novotny <minovotn redhat com>, RHCE
Virtualization Team (xen userspace), Red Hat


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]