Hello, On 11/2/20 10:16 AM, Chuck Lever wrote: > > >> On Nov 2, 2020, at 10:05 AM, Alice Mitchell <ajmitchell@xxxxxxxxxx> 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, §ion, &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 > > >