[libvirt] [PATCH 1/2] v3: put dnsmasq parameters into a file instead of the command line

[Laine Stump]

On 10/23/2012 12:57 PM, Gene Czarcinski wrote:
> On 10/23/2012 12:01 PM, Laine Stump wrote:
>> On 10/23/2012 11:07 AM, Gene Czarcinski wrote:
>>> This patch changes the way parameters are passed to dnsmasq.  They are
>>> put into a conf-file instead of being on the dnsmasq command line.
>> I was thinking about this last night after I learned from you that the
>> conf file *isn't* reread when dnsmasq received SIGHUP. That being the
>> case, what are the other reasons for switching from commandline to conf
>> file? I suppose one is that it eliminates clutter in the ps output (and
>> gets it away from prying eyes (since anyone can see the commandline by
>> looking in the output of ps -AlF). Another would be it avoids hitting
>> the commandline limit (which is rather large, but still exists) when
>> there are a lot of srv and txt entries. Anything else?
>
> Not really.  Maybe there is also the fact that I believe this is a
> "cleaner," but not necessarily better, approach.
>
> There is also the possibility that Simon could be convinced to add the
> capability of re-reading configuration files.  I will ask to see what
> his reaction is.  But, his reaction may be: "Sure, go ahead and do
> that; I look forward to your patches."  I am not sure I want to wander
> in that swamp right now.
>
> The most important thing to me is adding the --conf-dir=<directory>. 
> This will allow debugging different parameters without having to
> recompile/rebuild the entire libvirt set of packages.  Now, when I
> first did this, I thought that the configuration files in the
> directory would be re-read.  When it did not do that, and, upon doing
> a little research, found that it was working as designed.
>
> Oh, BTW, there is one other little thing about the command line
> parameters.  To support IPv6 completely, the should be
> local=/<reverse-ip6-addr>.ip6.arpa/ parameters and these are very
> long.  Putting things into a conf-file just makes it (IMHO) a lot
> simpler to understand.
>
> I do realize that this is open source and I can always "do my own
> thing" and continue using the patches myself.  However, that can mean
> a lot of work just keeping up.

Sure. I think it's a net gain. It's just that my main reason for wanting
to do this switch was to avoid restarting dnsmasq for every little
change (mainly because that's one of the things I've been working on
lately - virNetworkUpdate()), and when I saw that carrot go away, I
needed a sanity check to rationalize the change.

>
> One last observation/question:  Why not?

I often find myself falling into the hole of making change for change's
sake, which leads to cleaner code, but makes backporting fixes to
earlier releases more difficult. I need to constantly watch myself for
too much of this behavior (some amount is good, as it does lean to
cleaner code, but there is such a thing as too much of anything).

I tried your earlier --interface patch on F17 last night, and built it
on F16 and RHEL6 but didn't get a chance to test it. So instead I'm
going to try a build with both of these patches on the three platforms
today and (hopefully) push them both prior to the 1.0.0 freeze late
tonight. That way they'll have a reasonable amount of testing, and will
get even more (and especially on other distros) during the freeze, with
time enough to tweak (or even back out if necessary) before 1.0.0.