Re: [libvirt] dhcp6, radvd, ip6tables, etc. (update)

On 10/29/2012 08:26 AM, Gene Czarcinski wrote: I think that is the proper thing to do for now. As discussed earlier, before supporting dhcp on multiple subnets of the same protocol (ipv6 vs ipv4) we would need to decide why and how we want to do that - IPs assigned from different subnets need to be matched with the IP address of that subnet, and it will take a more complicated dnsmasq commandline to do that, iirc. I haven't looked at how dhcp6 works, but if its anything like dhcp4, the IP address is irrelevant and shouldn't be included in the rule. As long as your rule specifies both the interface and port, that should be fine (take a look at the rules already being added for dhcp4) (and no, I have absolutely no idea why we add a rule to allow *tcp* on the dhcp port. It's just been that way since the first day I set eyes on the code). Interesting - so both radvd and dnsmasq are involved, correct? I have no problem with the "convert from long commandline to conf file" patch except for the bit that points to a "conf directory" where user supplied conf files can be added. Aside from that part needing to be in a spearate patch, if we're going to add that kind of configurability, we need to do it in a way that will allow us to easily see that the user is playing outside the fence (otherwise we spend a lot of time chasing "bugs" that end up being caused by user-supplied options). Because we're in freeze right now I haven't spent a lot of time discussing that, but planned to send a message about it when I get a minute. "what network_conf.c does"? Well, it of course doesn't deal directly with those options, but the config that feeds into some of those options is parsed in virNetworkIPParseXML(), and is only done if the <ip> element is ipv4. But then you've already seen that code if you have dhcp working for ipv6 - the <dhcp> element is also only parsed for ipv6. The format-side code doesn't have that extra check; I guess I figured that if there was no way to configure ipv6 with dhcp or tftp, it was safe to assume any ip element with dhcp or tftp info was ipv4 anyway. I've actually never liked the "dnsmasqContext" concept, as it seems like overkill and has conceptual problems such as what you've described. I would be just as happy with replacements that were simpler and easier to deal with. I think part of what complicates dnsmasq.[hc] is that it's in the util directory, so it isn't allowed to understand the contents of virNetworkDef, and must instead be sent the list of hosts in a simpler format. If, instead, there was a file src/network/bridge_dnsmasq.[hc], that could have functions that took virNetworkDef as an arg, and just immediately return a string (or, in a separate function, write to a file). That should simplify calling, and writing tests. And existing dnsmasq-related functions in bridge_driver.c could be moved there as well, reducing clutter in bridge_driver.c. (Of course I'm saying all this without ever seriously considering it, just talking off the cuff, so I may be completely wrong :-) It is made that way because a single IP address may have multiple hostnames associated with it, and we want to avoid having multiple methods of describing the same thing. What you propose in the next couple sentences was already proposed, tried, and rejected when dns host support was originally added. There are a few places in libvirt's XML where the same thing can be expressed in two different ways, but that is only done when necessary because the existing XML is unable to completely describe the new functionality but backward compatibility is required. It creates all sorts of problems when formatting the config back into XML though (which of the two do you choose? Or do you do both? Either of these is a bad answer), therefore we definitely don't want to do that except in cases where it is absolutely necessary; this isn't one of those cases.