Re: [libvirt] Plan A or Plan B?

On 11/19/2012 08:31 AM, Gene Czarcinski wrote: At least on my part, you're misinterpreting the lack of response as reluctance to change, when it really is just due to a lack of time to draft a response. I have no problems with switching to using a conf file (although, as I've said a couple of times, pointing to a conf *directory* which could contain admin-supplied additions to the configuration is unlikely to be accepted - we do need an alternative to that, though; I think the method most likely to succeed would be to add a private dnsmasq:commandline namespace, as we've already done for qemu). http://berrange.com/posts/2011/12/19/using-command-line-arg-monitor-comma... Here is an open BZ that mentions the idea of adding similar functionality for dnsmasq (see comment 8, in particular): https://bugzilla.redhat.com/show_bug.cgi?id=824573 This is "the right way" to do this, since it makes sure that all configuration information (including for knobs that we don't support) is available in one place. One of the things that's actually taken up some of my time in the last few days is that I'm looking at an open CVE about dnsmasq and its use in libvirt: https://bugzilla.redhat.com/show_bug.cgi?id=833033 https://bugzilla.redhat.com/show_bug.cgi?id=874702 Fixing this also requires changing the dnsmasq commandline dependent upon the version of dnsmasq running on the host. In this case, though, we can't rely on the version number, but need to check for presence of a particular option in the output of "dnsmasq --help" (this is because there will be backports of the new dnsmasq option to older versions of dnsmasq on various distros that can't/won't upgrade to a new release, but still require the CVE fix, and yet we can't completely refuse to run if someone hasn't upgraded their dnsmasq, because the problem doesn't occur in the majority of configurations). Since there will be conflicts with your changes, and the libvirt fixes also will need to be backported to older versions of libvirt, the CVE fix will need to be pushed before your patches (in order to make the backport patch as similar to the upstream patch as possible) In the meantime, this bug has shown that we really need a general-purpose "capabilities" mechanism for dnsmasq similar to what we have for qemu, so I'm making a trimmed down version of that. This is my top priority right now, so hang on for a couple days and I'll see how much conflict it creates with your patches (one thing is that the mechanism for getting the dnsmasq version will be different). So before you go re-arranging all your patches, give me a couple days to work out the patch for the CVE and see how it sits with your current patches. This discussion was left open at the end - Dan, do you see any problem with adding the rules permitting IPv6 traffic between the guests as long as the host has disable_ipv6 set? Or will we still need to add an "ipv6='yes'" attribute to the toplevel <network> element? If you can find a good place in the wiki to expand on what's in formatnetwork, that would be good, but it's a separate process (since the wiki isn't stored in git). No. It must log an UNSUPPORTED error and fail. This should be done at the time the network is started/created though, not when it is defined, since the dnsmasq version may change in the interim. Right. Yes. Although I'm creating a small "dnsmasqCapabilities" API where you will be able to get this information (btw, I don't think the version should be stored in the network object, but instead be retrieved directly from the API (we may want to cache the info somewhere globally in situations such as a mass-restart of all networks). No. If something requires support in libvirt itself that is lacking (e.g. different code needs to be compiled in, or required code isn't yet written), you can fail during network define, but if it requires support in an external binary that may or may not have the support, you need to wait until you're actually going to start it, since that support may be added or removed at any time. Again, I would say wait for a few days. Hopefully before we take off for the holiday on Thursday I'll have a patch that adds dnsmasqCapabilities functions and uses that to conditionally enable --bind-dynamic (as required for the CVE mentioned above) and we can see how disruptive that is to your current patches.