On 11/3/20 5:14 AM, Alice Mitchell wrote:
> I know the code doesn't look like very much but it does do exactly what
> I suggest, and i feel is quite a simple and elegant solution that is
> inline with what many of services do.
I'm not saying the include does not work...
Its just the example you gave does not work.
I mistakenly include the sub conf in the sub conf
which cause an infinite loop... but again that was
a pilot error on my part.
>
> relative_path() is just as it suggests only for building relative
> paths, whatever comes out of it, either the constructed relative path
> or the untouched absolute one gets handed to glob(3) which builds a
> list of files which match the wildcard pattern given, the for() loop
> around conf_readfile()/conf_parse() loads all of the contents of those
> files into the current transaction as if it was one big config file,
> the wildcard pattern will also do the file extension matching that
> Chuck suggested.
Here is what I'm seeing:
mount -v steved-fedora:/home/tmp /mnt/tmp
conf_readfile: stat(/etc/nfsmount.conf) 0 errno 2
relative_path: newfile '/etc/nfsmount.conf.d/*.conf'
conf_parse_line: relpath '/etc/nfsmount.conf.d/*.conf'
conf_readfile: stat(/etc/nfsmount.conf.d/*.conf) -1 errno 2
conf_parse_line: subconf '(null)'
^^^^ this NULL stop the processing so the vers=4 mount option
is never processed in the sub config file.
>
> Looking around many other services handle config directories in the
> same way, not all admittedly, but things like apache always handled
> their config this way and at Vincents suggestion I checked and this is
> also how autofs handles it, a directive in the main config file that
> loads a subdirectory.
>
> /etc/auto.master :
> # Include /etc/auto.master.d/*.autofs
> # The included files must conform to the format of this file.
> #
> +dir:/etc/auto.master.d
Question I have is... Do all the files under /etc/auto.master.d
get processed?
>
> So the patch i included adds comparable functionality to all of the NFS
> tools, you simply add the include line to the master config files that
> require directory configs as well.
Well not all of the tools... As said before, I pattern my patch
after what exportfs does with /etc/export.d.
steved.
>
> -Alice
>
>
>
> On Mon, 2020-11-02 at 14:42 -0500, Steve Dickson wrote:
>> Hello,
>>
>> On 11/2/20 10:57 AM, Alice Mitchell wrote:
>>> On Mon, 2020-11-02 at 09:23 -0500, Steve Dickson wrote:
>>>> Hello,
>>>>
>>>> On 11/2/20 8:24 AM, Steve Dickson wrote:
>>>>>> You would need to write an equivalent of conf_load_file()
>>>>>> that
>>>>>> created a new transaction id and read in all the files before
>>>>>> committing them to do it this way.
>>>
>>> How about the following as an alternative...
>>>
>>> It changes none of the past behaviour, but if you wanted to add an
>>> optional directory structure to a config file then simply add this
>>> to
>>> the default single config file that we ship.
>>>
>>> /etc/nfsmount.conf:
>>> [NFSMount_Global_Options]
>>> include=-/etc/nfsmount.conf.d/*.conf
>> If it was this simple I would go for it...
>> but it just not work... as expected. Here is why.
>>
>> In relative_path() looks at the new file
>> (/etc/nfsmount.conf.d/*.conf). If the path starts
>> with '/', the path is strdup-ed and returned.
>>
>> The '*' is never expanded. Even if it was... there
>> no code (that I see) that will process multiple
>> files. TBL... This works
>> include=-/etc/nfsmount.conf.d/nfsmount.conf
>>
>> This doesn't
>> include=-/etc/nfsmount.conf.d/*.conf
>>
>> Also I don't know if it is a good idea to have the sub configs
>> dependent on the main config file. If the main config is remove
>> the sub config will never be seen. Is that a good thing?
>>
>> I'm thinking we go with processing the sub configs separate
>> from the main config and allow multiple sub configs be processed
>> if they end with ".config" (mrchuck's suggestion).
>>
>> Then document all this in the man pages.
>>
>> The last question should the main config be process,
>> not at all or after or before the sub configs?
>>
>> steved.
>>
>>>
>>> The leading minus tells it that it isnt an error if its empty, and
>>> it
>>> will load all of the fragments it finds in there as well as the
>>> existing single file. Apply the same structure to any existing
>>> config
>>> file that you want to also have a directory for.
>>>
>>> -Alice
>>>
>>>
>>>
>>> diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
>>> index 58c03911..aade50c8 100644
>>> --- a/support/nfs/conffile.c
>>> +++ b/support/nfs/conffile.c
>>> @@ -53,6 +53,7 @@
>>> #include <libgen.h>
>>> #include <sys/file.h>
>>> #include <time.h>
>>> +#include <glob.h>
>>>
>>> #include "conffile.h"
>>> #include "xlog.h"
>>> @@ -690,6 +691,7 @@ conf_parse_line(int trans, char *line, const
>>> char *filename, int lineno, char **
>>> if (strcasecmp(line, "include")==0) {
>>> /* load and parse subordinate config files */
>>> _Bool optional = false;
>>> + glob_t globdata;
>>>
>>> if (val && *val == '-') {
>>> optional = true;
>>> @@ -704,33 +706,46 @@ conf_parse_line(int trans, char *line, const
>>> char *filename, int lineno, char **
>>> return;
>>> }
>>>
>>> - subconf = conf_readfile(relpath);
>>> - if (subconf == NULL) {
>>> - if (!optional)
>>> - xlog_warn("config error at %s:%d: error
>>> loading included config",
>>> - filename, lineno);
>>> - if (relpath)
>>> - free(relpath);
>>> - return;
>>> - }
>>> + if (glob(relpath, GLOB_MARK|GLOB_NOCHECK, NULL,
>>> &globdata)) {
>>> + xlog_warn("config error at %s:%d: error with
>>> matching pattern", filename, lineno);
>>> + } else
>>> + {
>>> + for (size_t g=0; g<globdata.gl_pathc; g++) {
>>> + const char * subpath =
>>> globdata.gl_pathv[g];
>>>
>>> - /* copy the section data so the included file can
>>> inherit it
>>> - * without accidentally changing it for us */
>>> - if (*section != NULL) {
>>> - inc_section = strdup(*section);
>>> - if (*subsection != NULL)
>>> - inc_subsection = strdup(*subsection);
>>> - }
>>> + if (subpath[strlen(subpath)-1] == '/')
>>> {
>>> + continue;
>>> + }
>>> + subconf = conf_readfile(subpath);
>>> + if (subconf == NULL) {
>>> + if (!optional)
>>> + xlog_warn("config error
>>> at %s:%d: error loading included config",
>>> + filename,
>>> lineno);
>>> + if (relpath)
>>> + free(relpath);
>>> + return;
>>> + }
>>> +
>>> + /* copy the section data so the
>>> included file can inherit it
>>> + * without accidentally changing it for
>>> us */
>>> + if (*section != NULL) {
>>> + inc_section = strdup(*section);
>>> + if (*subsection != NULL)
>>> + inc_subsection =
>>> strdup(*subsection);
>>> + }
>>>
>>> - conf_parse(trans, subconf, &inc_section,
>>> &inc_subsection, relpath);
>>> + conf_parse(trans, subconf,
>>> &inc_section, &inc_subsection, relpath);
>>>
>>> - if (inc_section)
>>> - free(inc_section);
>>> - if (inc_subsection)
>>> - free(inc_subsection);
>>> + if (inc_section)
>>> + free(inc_section);
>>> + if (inc_subsection)
>>> + free(inc_subsection);
>>> + free(subconf);
>>> + }
>>> + }
>>> if (relpath)
>>> free(relpath);
>>> - free(subconf);
>>> + globfree(&globdata);
>>> } else {
>>> /* XXX Perhaps should we not ignore errors? */
>>> conf_set(trans, *section, *subsection, line, val, 0,
>>> 0);
>>>
>>>
>
