Re: [RFC nfs-utils PATCH 1/2] idmapd: move the pipefs-directory config option to nfs.conf

[NeilBrown <neilb@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 5683 bytes --]

On Fri, Mar 31 2017, Scott Mayhew wrote:

> Changed idmapd to read its value for the pipefs-directory from
> /etc/nfs.conf rather than /etc/idmapd.conf.  All other configurations
> related to id mapping still reside in /etc/idmapd.conf for now.
>
> Removed the -c option, since it would be confusing as whether it should
> override nfs.conf, idmapd.conf, or both.

You didn't remove mention of the -c option from idmapd.man.
Otherwise:
  Reviewed-by: NeilBrown <neilb@suse.com>


Thanks,
NeilBrown


>
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> ---
>  nfs.conf                |  3 +++
>  systemd/nfs.conf.man    |  9 +++++++++
>  utils/idmapd/idmapd.c   | 35 ++++++++++++++---------------------
>  utils/idmapd/idmapd.man | 19 ++++++++++++++++++-
>  4 files changed, 44 insertions(+), 22 deletions(-)
>
> diff --git a/nfs.conf b/nfs.conf
> index 81ece06..4359904 100644
> --- a/nfs.conf
> +++ b/nfs.conf
> @@ -17,6 +17,9 @@
>  # cred-cache-directory=
>  # preferred-realm=
>  #
> +#[idmapd]
> +# pipefs-directory=/var/lib/nfs/rpc_pipefs
> +#
>  #[lockd]
>  # port=0
>  # udp-port=0
> diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
> index bdc0988..83cf84a 100644
> --- a/systemd/nfs.conf.man
> +++ b/systemd/nfs.conf.man
> @@ -215,6 +215,15 @@ See
>  for details.
>  
>  .TP
> +.B idmapd
> +Recognized values:
> +.BR pipefs-directory .
> +
> +See
> +.BR rpc.idmapd (8)
> +for details.
> +
> +.TP
>  .B svcgssd
>  Recognized values:
>  .BR principal .
> diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
> index f4e083a..561a4b8 100644
> --- a/utils/idmapd/idmapd.c
> +++ b/utils/idmapd/idmapd.c
> @@ -214,13 +214,11 @@ main(int argc, char **argv)
>  	struct event initialize;
>  	struct passwd *pw;
>  	struct group *gr;
> -	struct stat sb;
>  	char *xpipefsdir = NULL;
>  	int serverstart = 1, clientstart = 1;
>  	int ret;
>  	char *progname;
>  
> -	conf_path = _PATH_IDMAPDCONF;
>  	nobodyuser = NFS4NOBODY_USER;
>  	nobodygroup = NFS4NOBODY_GROUP;
>  	strlcpy(pipefsdir, PIPEFS_DIR, sizeof(pipefsdir));
> @@ -231,11 +229,9 @@ main(int argc, char **argv)
>  		progname = argv[0];
>  	xlog_open(progname);
>  
> -#define GETOPTSTR "hvfd:p:U:G:c:CS"
> +#define GETOPTSTR "hvfd:p:U:G:CS"
>  	opterr=0; /* Turn off error messages */
>  	while ((opt = getopt(argc, argv, GETOPTSTR)) != -1) {
> -		if (opt == 'c')
> -			conf_path = optarg;
>  		if (opt == '?') {
>  			if (strchr(GETOPTSTR, optopt))
>  				warnx("'-%c' option requires an argument.", optopt);
> @@ -247,20 +243,19 @@ main(int argc, char **argv)
>  	}
>  	optind = 1;
>  
> -	if (stat(conf_path, &sb) == -1 && (errno == ENOENT || errno == EACCES)) {
> -		warn("Skipping configuration file \"%s\"", conf_path);
> -		conf_path = NULL;
> -	} else {
> -		conf_init();
> -		verbose = conf_get_num("General", "Verbosity", 0);
> -		cache_entry_expiration = conf_get_num("General",
> -				"Cache-Expiration", DEFAULT_IDMAP_CACHE_EXPIRY);
> -		CONF_SAVE(xpipefsdir, conf_get_str("General", "Pipefs-Directory"));
> -		if (xpipefsdir != NULL)
> -			strlcpy(pipefsdir, xpipefsdir, sizeof(pipefsdir));
> -		CONF_SAVE(nobodyuser, conf_get_str("Mapping", "Nobody-User"));
> -		CONF_SAVE(nobodygroup, conf_get_str("Mapping", "Nobody-Group"));
> -	}
> +	conf_path = NFS_CONFFILE;
> +	conf_init();
> +	CONF_SAVE(xpipefsdir, conf_get_str("idmapd", "pipefs-directory"));
> +	if (xpipefsdir != NULL)
> +		strlcpy(pipefsdir, xpipefsdir, sizeof(pipefsdir));
> +
> +	conf_path = _PATH_IDMAPDCONF;
> +	conf_init();
> +	verbose = conf_get_num("General", "Verbosity", 0);
> +	cache_entry_expiration = conf_get_num("General",
> +			"cache-expiration", DEFAULT_IDMAP_CACHE_EXPIRY);
> +	CONF_SAVE(nobodyuser, conf_get_str("Mapping", "Nobody-User"));
> +	CONF_SAVE(nobodygroup, conf_get_str("Mapping", "Nobody-Group"));
>  
>  	while ((opt = getopt(argc, argv, GETOPTSTR)) != -1)
>  		switch (opt) {
> @@ -307,8 +302,6 @@ main(int argc, char **argv)
>  #ifdef HAVE_NFS4_SET_DEBUG
>  	nfs4_set_debug(verbose, xlog_warn);
>  #endif
> -	if (conf_path == NULL)
> -		conf_path = _PATH_IDMAPDCONF;
>  	if (nfs4_init_name_mapping(conf_path))
>  		errx(1, "Unable to create name to user id mappings.");
>  
> diff --git a/utils/idmapd/idmapd.man b/utils/idmapd/idmapd.man
> index d4ab894..0fbc24c 100644
> --- a/utils/idmapd/idmapd.man
> +++ b/utils/idmapd/idmapd.man
> @@ -78,6 +78,21 @@ Client-only: perform no idmapping for any NFS server, even if one is detected.
>  .It Fl S
>  Server-only: perform no idmapping for any NFS client, even if one is detected.
>  .El
> +.Sh CONFIGURATION FILES
> +The
> +.Sy [idmapd]
> +section of the
> +.Pa /etc/nfs.conf
> +configuration file recognizes the following value:
> +.Bl -tag -width Ds_imagedir
> +.It Sy pipefs-directory
> +Equivalent to
> +.Sy -p .
> +.El
> +.Pp
> +All other settings related to id mapping are found in the
> +.Pa /etc/idmapd.conf
> +configuration file.
>  .Sh EXAMPLES
>  .Cm rpc.idmapd -f -vvv
>  .Pp
> @@ -94,9 +109,11 @@ messages to console, and with a verbosity level of 3.
>  .\" This next request is for sections 1, 6, 7 & 8 only.
>  .\" .Sh ENVIRONMENT
>  .Sh FILES
> -.Pa /etc/idmapd.conf
> +.Pa /etc/idmapd.conf ,
> +.Pa /etc/nfs.conf
>  .Sh SEE ALSO
>  .Xr idmapd.conf 5 ,
> +.Xr nfs.conf 5 ,
>  .Xr nfsidmap 8
>  .\".Sh SEE ALSO
>  .\".Xr nylon.conf 4
> -- 
> 2.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]