Re: [RFC PATCH 0/1] Enable config.d directory to be processed.

Steve Dickson
2020-11-02T16:37:44Z

Hello,

On 11/2/20 10:16 AM, Chuck Lever wrote:
>
>
>> On Nov 2, 2020, at 10:05 AM, Alice Mitchell <ajmitchell@redhat.com> wrote:
>>
>> Hi Steve,
>> That should work yes, although I am still dubious of the merits of
>> replacing the single config file with multiple ones rather than reading
>> them in addition to it. Surely this is going to lead to queries of why
>> the main config file is being ignored just because the directory also
>> existed.
>
> I would follow the pattern used in other tools. How does /etc/exports.d/
> work, for example?
I pattern my patch on how exports.d works today. Scandir() all the
dir-entries then process each file.

>
>
>> I also have concerns that blindly loading -every- file in the directory
>> is also going to lead to problems, such as foo.conf.rpmorig files and
>> the like. This is why i suggested a glob would be a better solution
>
> The usual behavior used by other tools is to load only files that match
> a particular file extension. That way, files can be left in place in the
> .d directory, but disabled, by simply renaming them.
Interesting... So only process *.conf files? Ex. 001-nfs.conf, 002-nfs.conf??

steved.

>
>
>> -Alice
>>
>>
>> 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.
>>>> I kinda do think we should be able to read in multiple files...
>>>> If that free was not done until all the files are read in, would
>>>> something
>>>> like that work? I guess I'm ask how difficult would be to re-work
>>>> the code to do something like this.
>>>>
>>> Something similar to this... load all the files under the same trans
>>> id:
>>> (Compiled tested):
>>> diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
>>> index c60e511..f003fe1 100644
>>> --- a/support/nfs/conffile.c
>>> +++ b/support/nfs/conffile.c
>>> @@ -578,6 +578,30 @@ static void conf_free_bindings(void)
>>> }
>>> }
>>>
>>> +static int
>>> +conf_load_files(int trans, const char *conf_file)
>>> +{
>>> + char *conf_data;
>>> + char *section = NULL;
>>> + char *subsection = NULL;
>>> +
>>> + conf_data = conf_readfile(conf_file);
>>> + if (conf_data == NULL)
>>> + return 1;
>>> +
>>> + /* Load default configuration values. */
>>> + conf_load_defaults();
>>> +
>>> + /* Parse config contents into the transaction queue */
>>> + conf_parse(trans, conf_data, &section, &subsection, conf_file);
>>> + if (section)
>>> + free(section);
>>> + if (subsection)
>>> + free(subsection);
>>> + free(conf_data);
>>> +
>>> + return 0;
>>> +}
>>> /* Open the config file and map it into our address space, then
>>> parse it. */
>>> static int
>>> conf_load_file(const char *conf_file)
>>> @@ -616,6 +640,7 @@ conf_init_dir(const char *conf_file)
>>> struct dirent **namelist = NULL;
>>> char *dname, fname[PATH_MAX + 1];
>>> int n = 0, nfiles = 0, i, fname_len, dname_len;
>>> + int trans;
>>>
>>> dname = malloc(strlen(conf_file) + 3);
>>> if (dname == NULL) {
>>> @@ -637,6 +662,7 @@ conf_init_dir(const char *conf_file)
>>> return nfiles;
>>> }
>>>
>>> + trans = conf_begin();
>>> dname_len = strlen(dname);
>>> for (i = 0; i < n; i++ ) {
>>> struct dirent *d = namelist[i];
>>> @@ -660,11 +686,17 @@ conf_init_dir(const char *conf_file)
>>> }
>>> sprintf(fname, "%s/%s", dname, d->d_name);
>>>
>>> - if (conf_load_file(fname))
>>> + if (conf_load_files(trans, fname))
>>> continue;
>>> nfiles++;
>>> }
>>>
>>> + /* Free potential existing configuration. */
>>> + conf_free_bindings();
>>> +
>>> + /* Apply the new configuration values */
>>> + conf_end(trans, 1);
>>> +
>>> for (i = 0; i < n; i++)
>>> free(namelist[i]);
>>> free(namelist);
>>>
>>
>
> --
> Chuck Lever
>
>
>