[Dnsmasq-discuss] feature: dictionary order import of addn-hosts dirs?

Simon Kelley simon at thekelleys.org.uk
Thu Aug 12 15:55:51 UTC 2021


On 12/08/2021 16:04, Simon Kelley wrote:
> On 11/08/2021 00:25, Ed W wrote:
>> On 10/08/2021 23:12, Simon Kelley wrote:
>>> On 08/08/2021 14:02, Ed W wrote:
>>>> On 19/07/2021 18:52, Ed W wrote:
>>>>> Hi, around 2.82 someone posted a little patch to import the config files in dictionary order, which
>>>>> is very useful for situations where you have overlapping definitions. I'm using an addn-hosts stanza
>>>>> pointing to a directory and files currently import in a somewhat random order (suppose inode
>>>>> order?), which can lead to unexpected reverse host definitions in some cases
>>>>>
>>>>> Could we have a dictionary order import for add-hosts files please?
>>>>>
>>>>> Ed W
>>>>
>>>> Hi, I have developed the attached patch without really being sure that this is the best approach. I
>>>> would be grateful for some feedback. I have used scandir without understanding if this is portable
>>>> across systems that we support with dnsmasq. Also I am trying to copy the existing coding style, but
>>>> surely I have failed.
>>>>
>>> I'll look in more detail soon, but this certainly looks like the right
>>> way to go.
>>>
>>>> I'm also unclear that it still works as advertised in the case that I don't have inotify enabled?
>>>> Any help?
>>> Since the whole point of the inotify stuff is that individual files get
>>> read as they change, imposing or relying on a particular order doesn't
>>> make much sense. I'd therefore not make any changes to inotify.c.
>>>
>>> The man page then needs to note that --dhcp-hostsdir --dhcp-optsdir and
>>> --hostsdir DON'T offer any ordering of files read, but
>>> --dhcp-hostsfile --dhcp-optsfile and --addn-hosts with directory
>>> arguments DO.
>>
>>
>> Aha, then I might have gone off the rails here then...
>>
>> It's the changes in inotify.c which affect my --addn-hosts directives? I left the top part of the
>> patch to option.c in there as it seemed like it probably affected (hostsdir?), and seemed useful yet
>> without any real cost?
> 
> 
> That's not my understanding.
> 
> --dhcp-hostsfile --dhcp-optsfile and --addn-hosts have nothing to do
> with inotify. The argument to any of them can be a directory, rather a
> file, and then all the files in the directory get read. Reading these
> files ONLY happens at start-up and when SIGHUP is received, at which
> times _all_ the files get read. It makes sense to define the order of
> reading these files within a directory. When SIGHUP is received, the old
> data from the files in discarded, and they are all re-read.
> 
> --dhcp-hostsdir --dhcp-optsdir and --hostsdir work in much the same way
> as when the first three options are given a directory; the files are
> read at start-up and when SIGHUP is received. BUT any file which is
> modified or created gets read asynchronously, without needing SIGHUP to
> be sent. Note that the old data from the files is not discarded when
> this is done: if a file gets modified, then it gets re-read but data
> from the previous version of the file is not deleted. This means that
> this facility is useful for adding hosts to the configuration without
> the upheaval of a full SIGHUP re-read. Anytime you want to delete stuff,
> SIGHUP is still needed.
> 
> Given the above, defining the order in which files in those directories
> are read is a bit pointless: sure, you can define and order for startup
> and SIGHUP, but then as soon as file is touched or added, the order
> depends on that.
> 
> If you're using --addn-hosts, then the inotify code isn't affecting what
> you're doing: you must be sending SIGHUP to re-read any changes to the
> files.
> 
> The inotify code can be omitted entirely and --addn-hosts will still
> work, but --dhcp-hostsdir --dhcp-optsdir and --hostsdir options will be
> errored.
> 
> Given that, I propose to take your patch to expand_filelist(), which
> will define the order for --dhcp-hostsfile --dhcp-optsfile and
> --addn-hosts when they take directories, but not the changes in
> inotify.c, which are not needed for that.
> 
> 

Change now committed. I started with your patch to expand_filelist() and
made the following changes.

1) Moved the editor-file filter code into a filter callback for scandir,
just to avoid allocating memory for then-ignored files.

2) Linked-list horribleness to avoid inverting the order of files
retrieved from scandir.

3) Made things clear in the manpage for the options which are affected
by this, and the inotify ones which aren't.

https://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=2f2d59b35c3338ffa20361d409f07ef340987d1b

Cheers,

Simon.




More information about the Dnsmasq-discuss mailing list