[libvirt] patch option needs clarification

[Gene Czarcinski]

On 10/22/2012 03:59 PM, Gene Czarcinski wrote:
> On 10/22/2012 03:17 PM, Laine Stump wrote:
>> On 10/22/2012 09:26 AM, Gene Czarcinski wrote:
>>> I am pretty much complete creating a patch which changes how dnsmasq
>>> is started by moving the command line parameters into a conf file.
>>> This new file is placed into the same directory and the lease file.
>>>
>>> The test for the command line arguments now checks the contents of the
>>> conf-file and there is no longer any tests for the command line
>>> parameters which are now two.
>> This will also change the requirements of whether or not a dnsmasq
>> restart or "refresh" (SIGHUP) is required when certain pieces of the
>> network definition are changed with virNetworkUpdate(), but that can be
>> submitted as a separate patch. (basically, things that used to require a
>> change on the commandline, but now only require a change in the conf
>> file, will no longer require a restart).
> SIGHUP will not do it ... I check into it.  By design, configuration 
> files are not revisited once started.  Some other files are but not 
> the configuration files.  It will take a restart whether is is on the 
> command line or in a conf-file.  I believe I understand why it is done 
> this way ... just too much could change ... better to start off clean 
> with a restart (that is from the dnsmasq perspective).
>>
>>>
>>> The first command line parameter is (naturally) --conf-file=<filename>.
>>>
>>> The second parameter adds new functionality and is
>>> --conf-dir=<directory>.  This directory is placed into the same
>>> directory as the conf-file and the lease file.  The name of this
>>> directory is "<net-name>.d". This was added to make testing/debugging
>>> of new dnsmasq options easier since it no longer requires rebuilding
>>> the binaries.  This is also useful for adding log-dhcp and/or
>>> log-queries for a specific network.
>>>
>>> Now the option question.  I can submit the patch assuming the my
>>> previously submitted patch to add --interface to the command line has
>>> been applied or I can assume that it has not been applied.  In either
>>> case, the new code adds a interface=<dev-name> to the conf-file.
>> Submit it assuming that the patch *has* been applied. I think we do need
>> to make that change (adding "--interface"), but it needs to be
>> explicitly visible in a separate patch (and we need to do some testing
>> before pushing it).
> Oops.  But then I am not sure how to do it.  The new code changes 
> "everything."
>
> If you want, I can go back and remove it ... make yet-another-patch on 
> top of the one one to way to the list which removes interface= from 
> the conf-file and then submit a third patch which puts it back in.
>
> I am not sure I understand the reluctance about putting the 
> --interface patch in.  This problem exists in using any dnsmasq => 
> 2.61.  I believe I have seen it happen although I did not realize it 
> at the time.  Simon says that there is logging of the event in 
> dnsmasq's rfc3315.c but I have not gone back to prove that the problem 
> exists if --interface is not specified.  I am not talking about adding 
> this to older versions of libvirt ... just the current one and the 
> version that is on Fedora 17 since dnsmasq was so quickly updated to 
> 2.63.
>
> OK, over to you ... what do you want me to do?
>
I just noticed something "strange" about the patch I submitted. Wait a 
bit while I look into what is going on.

Gene