Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <338aeb795a31c2233016d225dc114e33d02eb0cb.camel@redhat.com>
Subject: Re: [RFC PATCH 0/1] Enable config.d directory to be processed.
From:   Alice Mitchell <ajmitchell@redhat.com>
To:     Steve Dickson <steved@redhat.com>,
        Linux NFS Mailing list <linux-nfs@vger.kernel.org>
Date:   Mon, 02 Nov 2020 13:03:23 +0000
In-Reply-To: <20201029210401.446244-1-steved@redhat.com>
References: <20201029210401.446244-1-steved@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 2020-10-29 at 17:04 -0400, Steve Dickson wrote:
> The following patch looks for config.d directories
> and configuration file in those directories will
> be loaded. 
> 
> For example if /etc/nfs.conf.d or /etc/nfsmount.conf.d 
> exists and there are config files in those directories 
> will be loaded, but not the actual /etc/nfs.conf or 
> the /etc/nfsmount.conf files will not be.
> 
> I do have a couple questions/concerns 
> 
> 1) Is calling conf_load_file() more than once
>    kosher... It appears variable will just be 
>    over written. That does appear to happen
>    with my testing. 
> 
> 2) If conf.d file(s) do exist, should the give
>    flat conf file also be loaded. At this point if 
>    the conf.d file(s) do exist, then the given
>    flat config file is not loaded. 
> 
> 3) How to document this new feature.
> 
> Steve Dickson (1):
>   conffile: process config.d directory config files.
> 
>  support/nfs/conffile.c | 78
> +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 77 insertions(+), 1 deletion(-)

NAK.

Each call to conf_load_file() erases all existing config values from
memory via a call to conf_free_bindings() so this wont work as you
expect.

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 can't help but wonder if this would be better handled as an
improvement to the 'include' directive to support file globbing, we
could then retain the functionality of the master nfs.conf file but
tack on an 'include nfs.conf.d/*.conf' at the end which would go off
and load any files dropped in there by package management.

-Alice