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

Chuck Lever
2020-11-02T15:17:01Z

> 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 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.

> -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