Re: [PATCH/RFC nfs-utils] nfsdcltrack: read configuration from a file

[NeilBrown <neilb@suse.com>]

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

On Fri, Nov 11 2016, Jeff Layton wrote:

> On Thu, 2016-11-10 at 15:58 +1100, NeilBrown wrote:
>> As nfsdcltrack is normally run directly from the kernel
>> there is no opportunity to change the default
>> storage directory.  This can be useful in a cluster to
>> locate the "storage directory" on shared storage.
>> 
>> The easiest alternative is to allow configuration to be read from a
>> file, particularly as nfs-utils already has code for parsing a config file.
>> 
>> So read the config file "/etc/nfs.conf" (or as set by ./configure) and
>> look for "storagedir" and "debug" in the "nfsdcltrack" section.
>> These values can still be over-ridden by command line options.
>> 
>> A generic name (nfs.conf) was changes for the config file so that
>> other daemons can be enhanced to read configuration from there.
>> This may be easier than passing command line arguments through systemd.
>> 
> I like the basic idea, but I'm not so sure we want to use a generic
> config file like this. What else do you envision using this file?

See https://lwn.net/Articles/584373/ and surrounds (included where I
said that I wouldn't be providing patches:-).

I'm not happy with current mechanisms for passing configuration from a
configurator gui, through systemd, to various daemons.  Having a common
config file, rather having to stitch together command-line args, feels
like it might be a step in the right direction.  Given that I was adding
a config file, I thought that leaving it open-ended might be a good
idea.

We already have /etc/nfsmount.conf and /etc/idmapd.conf.
How many more do we want?
I note that that idmapd.conf contains Pipefs-Directory.  Various
other daemons need to know where that is.  Wouldn't it be nice if they
all read the one config file?

I haven't resolved in my mind where the "impedance matching" should
happen.  To explain:
Different distros put their configuration in different places
(/etc/sysconfig/nfs /etc/defaults/nfs) and use different names for the
same value.  These files are all "name=val" files, without the [section]
headings of conf files.
What is the best way to get the config from there to the local variables
inside the various programs?

Currently a systemd service runs a script which reads the configuration
file and writes out an environment file for systemd to read, which
provides command-line args for each daemon.  I'd rather something more
direct.

If the parsing of /etc/nfs.conf allowed
  name=$var
to extract 'var' from the environment, then (almost) each distro
could have a static /etc/nfs.conf which listed which configuration
variables affected which settings.  Then systemd could read the
original configuration file to set up the environment, then each tool
would read /etc/nfs.conf to extract the desired parts of the environment.

Except that wouldn't work for nfsdcltrack as we cannot easily control
it's environment.  And there would probably be other things that didn't
quite work right.

Maybe the best thing is for the configurator-gui to be required to run
some post-processing thing which creates /etc/nfs.conf.
Then of course, it could just create systemd drop-in files which
created all the required arguments - then tells systemd to re-read those
files.  So maybe this is only useful for programs that aren't run via
systemd.

I'm as yet far from certain as to what I want, but keeping things
extensible seems like a generally good principle.

>
> That said, if we are going to do this, we should probably make it clear
> that it's for server-side configuration. Maybe "nfsd.conf" or
> "nfs-server.conf" would be a better name?

Why only server-side?  rpc-gssd needs configuration too.  It and
svcgssd (where used) are needed on both server and client (for
NFSv4.0).

Thanks,
NeilBrown

>
>> Signed-off-by: NeilBrown <neilb@suse.com>
>> ---
>>  configure.ac                      |  7 +++++++
>>  utils/nfsdcltrack/nfsdcltrack.c   | 12 ++++++++++++
>>  utils/nfsdcltrack/nfsdcltrack.man | 14 ++++++++++++++
>>  3 files changed, 33 insertions(+)
>> 
>> diff --git a/configure.ac b/configure.ac
>> index d60f3a2f7efa..8a5aa2e5203b 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -24,6 +24,12 @@ AC_ARG_WITH(statedir,
>>  	statedir=$withval,
>>  	statedir=/var/lib/nfs)
>>  	AC_SUBST(statedir)
>> +AC_ARG_WITH(nfsconfig,
>> +	[AC_HELP_STRING([--with-nfsconfig=/config/file],
>> +			[use general config file /config/file @<:@default=/etc/nfs.conf@:>@])],
>> +	nfsconfig=$withval,
>> +	nfsconfig=/etc/nfs.conf)
>> +	AC_SUBST(nfsconfig)
>>  AC_ARG_WITH(statdpath,
>>  	[AC_HELP_STRING([--with-statdpath=/foo],
>>  			[define the statd state dir as /foo instead of the NFS statedir @<:@default=/var/lib/nfs@:>@])],
>> @@ -468,6 +474,7 @@ dnl Export some path names to config.h
>>  dnl *************************************************************
>>  AC_DEFINE_UNQUOTED(NFS_STATEDIR, "$statedir", [This defines the location of the NFS state files. Warning: this must match definitions in config.mk!])
>>  AC_DEFINE_UNQUOTED(NSM_DEFAULT_STATEDIR, "$statdpath", [Define this to the pathname where statd keeps its state file])
>> +AC_DEFINE_UNQUOTED(NFS_CONFFILE, "$nfsconfig", [This defines the location of NFS daemon config file])
>>  
>>  if test "x$cross_compiling" = "xno"; then
>>  	CFLAGS_FOR_BUILD=${CFLAGS_FOR_BUILD-"$CFLAGS"}
>> diff --git a/utils/nfsdcltrack/nfsdcltrack.c b/utils/nfsdcltrack/nfsdcltrack.c
>> index fcdda7f66b8b..e6e514b78316 100644
>> --- a/utils/nfsdcltrack/nfsdcltrack.c
>> +++ b/utils/nfsdcltrack/nfsdcltrack.c
>> @@ -43,6 +43,7 @@
>>  #include <sys/capability.h>
>>  #endif
>>  
>> +#include "conffile.h"
>>  #include "xlog.h"
>>  #include "sqlite.h"
>>  
>> @@ -55,6 +56,8 @@
>>  /* defined by RFC 3530 */
>>  #define NFS4_OPAQUE_LIMIT	1024
>>  
>> +char *conf_path = NFS_CONFFILE;
>> +
>>  /* private data structures */
>>  struct cltrack_cmd {
>>  	char *name;
>> @@ -553,6 +556,7 @@ int
>>  main(int argc, char **argv)
>>  {
>>  	char arg;
>> +	char *val;
>>  	int rc = 0;
>>  	char *progname, *cmdarg = NULL;
>>  	struct cltrack_cmd *cmd;
>> @@ -562,6 +566,14 @@ main(int argc, char **argv)
>>  	xlog_syslog(1);
>>  	xlog_stderr(0);
>>  
>> +	conf_init();
>> +	val = conf_get_str("nfsdcltrack", "storagedir");
>> +	if (val)
>> +		storagedir = val;
>> +	rc = conf_get_num("nfsdcltrack", "debug", 0);
>> +	if (rc > 0)
>> +		xlog_config(D_ALL, 1);
>> +
>>  	/* process command-line options */
>>  	while ((arg = getopt_long(argc, argv, "hdfs:", longopts,
>>  				  NULL)) != EOF) {
>> diff --git a/utils/nfsdcltrack/nfsdcltrack.man b/utils/nfsdcltrack/nfsdcltrack.man
>> index 4b8f4d762a00..cc24b7a2c32e 100644
>> --- a/utils/nfsdcltrack/nfsdcltrack.man
>> +++ b/utils/nfsdcltrack/nfsdcltrack.man
>> @@ -67,6 +67,20 @@ Check to see if a nfs_client_id4 is allowed to reclaim. This command requires a
>>  .IP "\fBgracedone\fR" 4
>>  .IX Item "gracedone"
>>  Remove any unreclaimed client records from the database. This command requires a epoch boot time as an argument.
>> +.SH "EXTERNAL CONFIGURATION"
>> +The directory for stable storage information can be set via the file
>> +.B /etc/nfs.conf
>> +by setting the
>> +.B storagedir
>> +value in the
>> +.B nfsdcltrack
>> +section.  For example:
>> +.in +5
>> +[nfsdcltrack]
>> +.br
>> +  storagedir = /shared/nfs/nfsdcltrack
>> +.in -5
>> +Debuging to syslog can also be enabled by setting "debug = 1" in this file.
>>  .SH "LEGACY TRANSITION MECHANISM"
>>  .IX Header "LEGACY TRANSITION MECHANISM"
>>  The Linux kernel NFSv4 server has historically tracked this information
>
> -- 
> Jeff Layton <jlayton@redhat.com>

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