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: <1ac387a1ef608258b2e23e7923a1c4e2ec6b25b3.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 15:57:02 +0000
In-Reply-To: <4836616f-3aa6-d0bd-22db-cd7fecf4dce9@RedHat.com>
References: <20201029210401.446244-1-steved@redhat.com>
         <338aeb795a31c2233016d225dc114e33d02eb0cb.camel@redhat.com>
         <6f3caf91-296c-0aa8-ba41-bc35d500adaa@RedHat.com>
         <4836616f-3aa6-d0bd-22db-cd7fecf4dce9@RedHat.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

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


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