Question about nfsdcltrack --storagedir

Question about nfsdcltrack --storagedir

[NeilBrown]

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


Hi,
 I notice that nfsdcltrack has a "--storagedir" option.
 I wonder how this can be used, given the nfsdcltrack is only(?) called
 from the kernel and there is no(?) mechanism to pass extra options.

 In a clustered-server context it would make sense(?) to share the
 database between cluster nodes and it is easiest to do this if the
 file in a separate filesystem (mounted as part of fail-over) rather
 than in /var.
 This can(?) be achieved using a symlink, but rpm likes to remove
 symlinks to non-existent locations.

 With NFSv3 the equivalent is the state files maintained by statd, and
 these can be relocated by passing the -P option to rpc.statd.
 How does one do a similar thing for NFSv4???

Thanks,
NeilBrown

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

Re: Question about nfsdcltrack --storagedir

[Jeff Layton]

On Wed, 2016-11-09 at 14:46 +1100, NeilBrown wrote:
> Hi,
>  I notice that nfsdcltrack has a "--storagedir" option.
>  I wonder how this can be used, given the nfsdcltrack is only(?) called
>  from the kernel and there is no(?) mechanism to pass extra options.
> 
>  In a clustered-server context it would make sense(?) to share the
>  database between cluster nodes and it is easiest to do this if the
>  file in a separate filesystem (mounted as part of fail-over) rather
>  than in /var.
>  This can(?) be achieved using a symlink, but rpm likes to remove
>  symlinks to non-existent locations.
> 
>  With NFSv3 the equivalent is the state files maintained by statd, and
>  these can be relocated by passing the -P option to rpc.statd.
>  How does one do a similar thing for NFSv4???
> 
> 

Ahh, I added that option mostly for when I was testing it. I did a lot
of the earlier testing running it by hand, and --storagedir let me use a
different directory for the db. I did have a vague idea that we might
use it in the situation you describe, but I never wired that up as I
didn't have a real need for it.

We could add a new module parm that would set that option when the
kernel does its callout, or allow passing the storagedir by environment
variable.

What would make the most sense from a usability standpoint?

-- 
Jeff Layton <jlayton@redhat.com>

Re: Question about nfsdcltrack --storagedir

[NeilBrown]

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

On Wed, Nov 09 2016, Jeff Layton wrote:

> On Wed, 2016-11-09 at 14:46 +1100, NeilBrown wrote:
>> Hi,
>>  I notice that nfsdcltrack has a "--storagedir" option.
>>  I wonder how this can be used, given the nfsdcltrack is only(?) called
>>  from the kernel and there is no(?) mechanism to pass extra options.
>> 
>>  In a clustered-server context it would make sense(?) to share the
>>  database between cluster nodes and it is easiest to do this if the
>>  file in a separate filesystem (mounted as part of fail-over) rather
>>  than in /var.
>>  This can(?) be achieved using a symlink, but rpm likes to remove
>>  symlinks to non-existent locations.
>> 
>>  With NFSv3 the equivalent is the state files maintained by statd, and
>>  these can be relocated by passing the -P option to rpc.statd.
>>  How does one do a similar thing for NFSv4???
>> 
>> 
>
> Ahh, I added that option mostly for when I was testing it. I did a lot
> of the earlier testing running it by hand, and --storagedir let me use a
> different directory for the db. I did have a vague idea that we might
> use it in the situation you describe, but I never wired that up as I
> didn't have a real need for it.
>
> We could add a new module parm that would set that option when the
> kernel does its callout, or allow passing the storagedir by environment
> variable.
>
> What would make the most sense from a usability standpoint?

Maybe a config file in /etc/ which nfsdcltrack reads on start-up?
Though in some ways I'd rather that instead of running a program, the
kernel sent a message to user-space.  Possibly a u-event?
Then existing configuration mechanisms could be used to choose a program
and a context for it to run in.
I wonder if u-events handle namespaces at all.

This came up because a customer was symlinking all of /var/lib/nfs to
shared storage (and lost their symlink thanks to rpm).  That isn't a
solution that I really like, and it led me to reflect on other things in
/var/lib/nfs.

etab - holds a normalized copy of /etc/exports, plus ad hoc changes.
       It would like in /run/nfs if we built this today
export-lock - lockfile to protect changes to above. Would also be
       in /run if we built it today. (I wonder why that doesn't
       use .etab.lock, which is already used for locking)
state, sm, sm.bak - statd state files.  These belong in /var/lib/nfs
       but are easily relocated with args to rpc.statd and sm-notify.
v4recovery - the NFSv4 version of above
xtab - this hasn't been needed since we gained /proc/fs/nfs/exports
       It is just a record of what should be in the kernel
       We should remove this.  I'll make a patch.
rmtab - this hasn't been needed since the "new cache" and the
       up-call mechanism were created.  It might be still used
       to respond to "showmount" commands, but that was never reliable.
       If we keep it, it should probably move to /run.
       But what do people think if finally discarding the old
       (non-new_cache) code and using that as an excuse to increment
       the major version number of nfs-utils?

rpc_pipefs - mountpoint of NFS upcall filesystem.  This was another
       source of problems when /var/lib/nfs is a symlink elsewhere.
       It isn't nice to mount this filesystem on that shared storage.
       While programs that access this can be told to use an alternate
       directory, it is hard to tell systemd's unit files to mount
       it somewhere special (previously an init script would just
       mount it wherever the config file said)
       I note that Debian mounts this at /run/rpc_pipefs.
       That seems like a really good idea. What do people think of
       making this the "official" mount point?

If we moved some things to /run and removed others, it would just leave
state,sm,sm.bak and v4recovery in /var/lib/nfs.  That is all the same
type of data, which is nice.

So there are lots of things we could do, but at a minimum -
/etc/nfsdcltrack.conf ??

Thanks,
NeilBrown

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

Re: Question about nfsdcltrack --storagedir

[Jeff Layton]

On Thu, 2016-11-10 at 10:54 +1100, NeilBrown wrote:
> On Wed, Nov 09 2016, Jeff Layton wrote:
> 
> > On Wed, 2016-11-09 at 14:46 +1100, NeilBrown wrote:
> > > Hi,
> > >  I notice that nfsdcltrack has a "--storagedir" option.
> > >  I wonder how this can be used, given the nfsdcltrack is only(?) called
> > >  from the kernel and there is no(?) mechanism to pass extra options.
> > > 
> > >  In a clustered-server context it would make sense(?) to share the
> > >  database between cluster nodes and it is easiest to do this if the
> > >  file in a separate filesystem (mounted as part of fail-over) rather
> > >  than in /var.
> > >  This can(?) be achieved using a symlink, but rpm likes to remove
> > >  symlinks to non-existent locations.
> > > 
> > >  With NFSv3 the equivalent is the state files maintained by statd, and
> > >  these can be relocated by passing the -P option to rpc.statd.
> > >  How does one do a similar thing for NFSv4???
> > > 
> > > 
> > 
> > Ahh, I added that option mostly for when I was testing it. I did a lot
> > of the earlier testing running it by hand, and --storagedir let me use a
> > different directory for the db. I did have a vague idea that we might
> > use it in the situation you describe, but I never wired that up as I
> > didn't have a real need for it.
> > 
> > We could add a new module parm that would set that option when the
> > kernel does its callout, or allow passing the storagedir by environment
> > variable.
> > 
> > What would make the most sense from a usability standpoint?
> 
> Maybe a config file in /etc/ which nfsdcltrack reads on start-up?
> Though in some ways I'd rather that instead of running a program, the
> kernel sent a message to user-space.  Possibly a u-event?
> Then existing configuration mechanisms could be used to choose a program
> and a context for it to run in.
> I wonder if u-events handle namespaces at all.
> 
> This came up because a customer was symlinking all of /var/lib/nfs to
> shared storage (and lost their symlink thanks to rpm).  That isn't a
> solution that I really like, and it led me to reflect on other things in
> /var/lib/nfs.
> 
> etab - holds a normalized copy of /etc/exports, plus ad hoc changes.
>        It would like in /run/nfs if we built this today
> export-lock - lockfile to protect changes to above. Would also be
>        in /run if we built it today. (I wonder why that doesn't
>        use .etab.lock, which is already used for locking)
> state, sm, sm.bak - statd state files.  These belong in /var/lib/nfs
>        but are easily relocated with args to rpc.statd and sm-notify.
> v4recovery - the NFSv4 version of above
> xtab - this hasn't been needed since we gained /proc/fs/nfs/exports
>        It is just a record of what should be in the kernel
>        We should remove this.  I'll make a patch.
> rmtab - this hasn't been needed since the "new cache" and the
>        up-call mechanism were created.  It might be still used
>        to respond to "showmount" commands, but that was never reliable.
>        If we keep it, it should probably move to /run.
>        But what do people think if finally discarding the old
>        (non-new_cache) code and using that as an excuse to increment
>        the major version number of nfs-utils?
> 
> rpc_pipefs - mountpoint of NFS upcall filesystem.  This was another
>        source of problems when /var/lib/nfs is a symlink elsewhere.
>        It isn't nice to mount this filesystem on that shared storage.
>        While programs that access this can be told to use an alternate
>        directory, it is hard to tell systemd's unit files to mount
>        it somewhere special (previously an init script would just
>        mount it wherever the config file said)
>        I note that Debian mounts this at /run/rpc_pipefs.
>        That seems like a really good idea. What do people think of
>        making this the "official" mount point?
> 
> If we moved some things to /run and removed others, it would just leave
> state,sm,sm.bak and v4recovery in /var/lib/nfs.  That is all the same
> type of data, which is nice.
> 
> So there are lots of things we could do, but at a minimum -
> /etc/nfsdcltrack.conf ??
> 
> Thanks,
> NeilBrown

No objection here, especially if we make it so that we have existing
behavior when there is no config file. nfs-utils even has some config
file parsing routines now in support/nfs that should be sufficient.

-- 
Jeff Layton <jlayton@redhat.com>

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

[NeilBrown]

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


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.

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
-- 
2.10.1


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

Re: Question about nfsdcltrack --storagedir

[Chuck Lever]

> On Nov 9, 2016, at 7:55 PM, Jeff Layton <jlayton@redhat.com> wrote:
> 
> On Thu, 2016-11-10 at 10:54 +1100, NeilBrown wrote:
>> On Wed, Nov 09 2016, Jeff Layton wrote:
>> 
>>> On Wed, 2016-11-09 at 14:46 +1100, NeilBrown wrote:
>>>> Hi,
>>>> I notice that nfsdcltrack has a "--storagedir" option.
>>>> I wonder how this can be used, given the nfsdcltrack is only(?) called
>>>> from the kernel and there is no(?) mechanism to pass extra options.
>>>> 
>>>> In a clustered-server context it would make sense(?) to share the
>>>> database between cluster nodes and it is easiest to do this if the
>>>> file in a separate filesystem (mounted as part of fail-over) rather
>>>> than in /var.
>>>> This can(?) be achieved using a symlink, but rpm likes to remove
>>>> symlinks to non-existent locations.
>>>> 
>>>> With NFSv3 the equivalent is the state files maintained by statd, and
>>>> these can be relocated by passing the -P option to rpc.statd.
>>>> How does one do a similar thing for NFSv4???
>>>> 
>>>> 
>>> 
>>> Ahh, I added that option mostly for when I was testing it. I did a lot
>>> of the earlier testing running it by hand, and --storagedir let me use a
>>> different directory for the db. I did have a vague idea that we might
>>> use it in the situation you describe, but I never wired that up as I
>>> didn't have a real need for it.
>>> 
>>> We could add a new module parm that would set that option when the
>>> kernel does its callout, or allow passing the storagedir by environment
>>> variable.
>>> 
>>> What would make the most sense from a usability standpoint?
>> 
>> Maybe a config file in /etc/ which nfsdcltrack reads on start-up?
>> Though in some ways I'd rather that instead of running a program, the
>> kernel sent a message to user-space.  Possibly a u-event?
>> Then existing configuration mechanisms could be used to choose a program
>> and a context for it to run in.
>> I wonder if u-events handle namespaces at all.
>> 
>> This came up because a customer was symlinking all of /var/lib/nfs to
>> shared storage (and lost their symlink thanks to rpm).  That isn't a
>> solution that I really like, and it led me to reflect on other things in
>> /var/lib/nfs.
>> 
>> etab - holds a normalized copy of /etc/exports, plus ad hoc changes.
>>       It would like in /run/nfs if we built this today
>> export-lock - lockfile to protect changes to above. Would also be
>>       in /run if we built it today. (I wonder why that doesn't
>>       use .etab.lock, which is already used for locking)
>> state, sm, sm.bak - statd state files.  These belong in /var/lib/nfs
>>       but are easily relocated with args to rpc.statd and sm-notify.
>> v4recovery - the NFSv4 version of above
>> xtab - this hasn't been needed since we gained /proc/fs/nfs/exports
>>       It is just a record of what should be in the kernel
>>       We should remove this.  I'll make a patch.
>> rmtab - this hasn't been needed since the "new cache" and the
>>       up-call mechanism were created.  It might be still used
>>       to respond to "showmount" commands, but that was never reliable.
>>       If we keep it, it should probably move to /run.
>>       But what do people think if finally discarding the old
>>       (non-new_cache) code and using that as an excuse to increment
>>       the major version number of nfs-utils?
>> 
>> rpc_pipefs - mountpoint of NFS upcall filesystem.  This was another
>>       source of problems when /var/lib/nfs is a symlink elsewhere.
>>       It isn't nice to mount this filesystem on that shared storage.
>>       While programs that access this can be told to use an alternate
>>       directory, it is hard to tell systemd's unit files to mount
>>       it somewhere special (previously an init script would just
>>       mount it wherever the config file said)
>>       I note that Debian mounts this at /run/rpc_pipefs.
>>       That seems like a really good idea. What do people think of
>>       making this the "official" mount point?
>> 
>> If we moved some things to /run and removed others, it would just leave
>> state,sm,sm.bak and v4recovery in /var/lib/nfs.  That is all the same
>> type of data, which is nice.

I'm in favor of the house cleaning above, as long as it doesn't
affect operation on recent kernels. Where do we draw that line?
RHEL 5, perhaps?


>> So there are lots of things we could do, but at a minimum -
>> /etc/nfsdcltrack.conf ??
>> 
>> Thanks,
>> NeilBrown
> 
> No objection here, especially if we make it so that we have existing
> behavior when there is no config file. nfs-utils even has some config
> file parsing routines now in support/nfs that should be sufficient.

Just a general thought:

It's fine to store the configuration data under /etc, but for new
administrative interfaces, please create a command line UI which
is used to edit said configuration data. This makes it much easier
for users to script administrative changes, to build GUIs around,
and for our Q/A teams to construct automated tests.


--
Chuck Lever

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

[Jeff Layton]

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?

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?

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

Re: Question about nfsdcltrack --storagedir

[J. Bruce Fields]

On Thu, Nov 10, 2016 at 10:54:41AM +1100, NeilBrown wrote:
> On Wed, Nov 09 2016, Jeff Layton wrote:
> 
> > On Wed, 2016-11-09 at 14:46 +1100, NeilBrown wrote:
> >> Hi,
> >>  I notice that nfsdcltrack has a "--storagedir" option.
> >>  I wonder how this can be used, given the nfsdcltrack is only(?) called
> >>  from the kernel and there is no(?) mechanism to pass extra options.
> >> 
> >>  In a clustered-server context it would make sense(?) to share the
> >>  database between cluster nodes and it is easiest to do this if the
> >>  file in a separate filesystem (mounted as part of fail-over) rather
> >>  than in /var.
> >>  This can(?) be achieved using a symlink, but rpm likes to remove
> >>  symlinks to non-existent locations.
> >> 
> >>  With NFSv3 the equivalent is the state files maintained by statd, and
> >>  these can be relocated by passing the -P option to rpc.statd.
> >>  How does one do a similar thing for NFSv4???
> >> 
> >> 
> >
> > Ahh, I added that option mostly for when I was testing it. I did a lot
> > of the earlier testing running it by hand, and --storagedir let me use a
> > different directory for the db. I did have a vague idea that we might
> > use it in the situation you describe, but I never wired that up as I
> > didn't have a real need for it.
> >
> > We could add a new module parm that would set that option when the
> > kernel does its callout, or allow passing the storagedir by environment
> > variable.
> >
> > What would make the most sense from a usability standpoint?
> 
> Maybe a config file in /etc/ which nfsdcltrack reads on start-up?
> Though in some ways I'd rather that instead of running a program, the
> kernel sent a message to user-space.  Possibly a u-event?
> Then existing configuration mechanisms could be used to choose a program
> and a context for it to run in.
> I wonder if u-events handle namespaces at all.
> 
> This came up because a customer was symlinking all of /var/lib/nfs to
> shared storage (and lost their symlink thanks to rpm).  That isn't a
> solution that I really like, and it led me to reflect on other things in
> /var/lib/nfs.
> 
> etab - holds a normalized copy of /etc/exports, plus ad hoc changes.
>        It would like in /run/nfs if we built this today
> export-lock - lockfile to protect changes to above. Would also be
>        in /run if we built it today. (I wonder why that doesn't
>        use .etab.lock, which is already used for locking)
> state, sm, sm.bak - statd state files.  These belong in /var/lib/nfs
>        but are easily relocated with args to rpc.statd and sm-notify.
> v4recovery - the NFSv4 version of above
> xtab - this hasn't been needed since we gained /proc/fs/nfs/exports
>        It is just a record of what should be in the kernel
>        We should remove this.  I'll make a patch.
> rmtab - this hasn't been needed since the "new cache" and the
>        up-call mechanism were created.  It might be still used
>        to respond to "showmount" commands, but that was never reliable.
>        If we keep it, it should probably move to /run.
>        But what do people think if finally discarding the old
>        (non-new_cache) code and using that as an excuse to increment
>        the major version number of nfs-utils?

Looking at the history....  Looks like that went into 2.5.42 in late
2002.  In distro terms, first distros to support the new interface
include RHEL4, Debian 3.1/Sarge, Suse 9.

No comment on the other stuff, just wanted to check that as I'm inclined
to extreme conservatism on backwards compatibility.  But, yeah, I guess
that'd be OK.

--b.

> 
> rpc_pipefs - mountpoint of NFS upcall filesystem.  This was another
>        source of problems when /var/lib/nfs is a symlink elsewhere.
>        It isn't nice to mount this filesystem on that shared storage.
>        While programs that access this can be told to use an alternate
>        directory, it is hard to tell systemd's unit files to mount
>        it somewhere special (previously an init script would just
>        mount it wherever the config file said)
>        I note that Debian mounts this at /run/rpc_pipefs.
>        That seems like a really good idea. What do people think of
>        making this the "official" mount point?
> 
> If we moved some things to /run and removed others, it would just leave
> state,sm,sm.bak and v4recovery in /var/lib/nfs.  That is all the same
> type of data, which is nice.
> 
> So there are lots of things we could do, but at a minimum -
> /etc/nfsdcltrack.conf ??
> 
> Thanks,
> NeilBrown

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

[NeilBrown]

[-- 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 --]

Re: Question about nfsdcltrack --storagedir

[NeilBrown]

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

On Fri, Nov 11 2016, Chuck Lever wrote:

>> 
>> No objection here, especially if we make it so that we have existing
>> behavior when there is no config file. nfs-utils even has some config
>> file parsing routines now in support/nfs that should be sufficient.
>
> Just a general thought:
>
> It's fine to store the configuration data under /etc, but for new
> administrative interfaces, please create a command line UI which
> is used to edit said configuration data. This makes it much easier
> for users to script administrative changes, to build GUIs around,
> and for our Q/A teams to construct automated tests.

If that is an important concern (and I don't disagree) then I would
suggest dropping the idea of a config file with sections and just have a
file with simple (case sensitive) "name=value" assignments.  Then sed
can be used to modify the file.

Or.... just use git to modify the config file.

  sudo git config --file /etc/idmapd.conf mapping.nobody-user neilb

did what I expected it to do :-)

I would lean towards a flat config file myself, but nfs-utils already
has code for structured config files, and that code ignores settings
before the first section.

Thanks,
NeilBrown

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

Re: Question about nfsdcltrack --storagedir

[NeilBrown]

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

On Fri, Nov 11 2016, J. Bruce Fields wrote:

> On Thu, Nov 10, 2016 at 10:54:41AM +1100, NeilBrown wrote:
>> On Wed, Nov 09 2016, Jeff Layton wrote:
>> 
>> > On Wed, 2016-11-09 at 14:46 +1100, NeilBrown wrote:
>> >> Hi,
>> >>  I notice that nfsdcltrack has a "--storagedir" option.
>> >>  I wonder how this can be used, given the nfsdcltrack is only(?) called
>> >>  from the kernel and there is no(?) mechanism to pass extra options.
>> >> 
>> >>  In a clustered-server context it would make sense(?) to share the
>> >>  database between cluster nodes and it is easiest to do this if the
>> >>  file in a separate filesystem (mounted as part of fail-over) rather
>> >>  than in /var.
>> >>  This can(?) be achieved using a symlink, but rpm likes to remove
>> >>  symlinks to non-existent locations.
>> >> 
>> >>  With NFSv3 the equivalent is the state files maintained by statd, and
>> >>  these can be relocated by passing the -P option to rpc.statd.
>> >>  How does one do a similar thing for NFSv4???
>> >> 
>> >> 
>> >
>> > Ahh, I added that option mostly for when I was testing it. I did a lot
>> > of the earlier testing running it by hand, and --storagedir let me use a
>> > different directory for the db. I did have a vague idea that we might
>> > use it in the situation you describe, but I never wired that up as I
>> > didn't have a real need for it.
>> >
>> > We could add a new module parm that would set that option when the
>> > kernel does its callout, or allow passing the storagedir by environment
>> > variable.
>> >
>> > What would make the most sense from a usability standpoint?
>> 
>> Maybe a config file in /etc/ which nfsdcltrack reads on start-up?
>> Though in some ways I'd rather that instead of running a program, the
>> kernel sent a message to user-space.  Possibly a u-event?
>> Then existing configuration mechanisms could be used to choose a program
>> and a context for it to run in.
>> I wonder if u-events handle namespaces at all.
>> 
>> This came up because a customer was symlinking all of /var/lib/nfs to
>> shared storage (and lost their symlink thanks to rpm).  That isn't a
>> solution that I really like, and it led me to reflect on other things in
>> /var/lib/nfs.
>> 
>> etab - holds a normalized copy of /etc/exports, plus ad hoc changes.
>>        It would like in /run/nfs if we built this today
>> export-lock - lockfile to protect changes to above. Would also be
>>        in /run if we built it today. (I wonder why that doesn't
>>        use .etab.lock, which is already used for locking)
>> state, sm, sm.bak - statd state files.  These belong in /var/lib/nfs
>>        but are easily relocated with args to rpc.statd and sm-notify.
>> v4recovery - the NFSv4 version of above
>> xtab - this hasn't been needed since we gained /proc/fs/nfs/exports
>>        It is just a record of what should be in the kernel
>>        We should remove this.  I'll make a patch.
>> rmtab - this hasn't been needed since the "new cache" and the
>>        up-call mechanism were created.  It might be still used
>>        to respond to "showmount" commands, but that was never reliable.
>>        If we keep it, it should probably move to /run.
>>        But what do people think if finally discarding the old
>>        (non-new_cache) code and using that as an excuse to increment
>>        the major version number of nfs-utils?
>
> Looking at the history....  Looks like that went into 2.5.42 in late
> 2002.  In distro terms, first distros to support the new interface
> include RHEL4, Debian 3.1/Sarge, Suse 9.
>
> No comment on the other stuff, just wanted to check that as I'm inclined
> to extreme conservatism on backwards compatibility.  But, yeah, I guess
> that'd be OK.

Thanks.
On the flip side, kernels since 3.1 (4.5 years ago) cannot make use of
the code I'm suggesting be removed.  So it does feel like it might be time.

NeilBrown

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

Re: Question about nfsdcltrack --storagedir

[Chuck Lever]

Hiya Neil-

> On Nov 10, 2016, at 5:32 PM, NeilBrown <neilb@suse.com> wrote:
> 
> On Fri, Nov 11 2016, Chuck Lever wrote:
> 
>>> 
>>> No objection here, especially if we make it so that we have existing
>>> behavior when there is no config file. nfs-utils even has some config
>>> file parsing routines now in support/nfs that should be sufficient.
>> 
>> Just a general thought:
>> 
>> It's fine to store the configuration data under /etc, but for new
>> administrative interfaces, please create a command line UI which
>> is used to edit said configuration data. This makes it much easier
>> for users to script administrative changes, to build GUIs around,
>> and for our Q/A teams to construct automated tests.
> 
> If that is an important concern (and I don't disagree) then I would
> suggest dropping the idea of a config file with sections and just have a
> file with simple (case sensitive) "name=value" assignments.

I'm not necessarily advocating this, but we already have a single
central key-value file that stores NFS configuration information
(at least on RH-based distros), and that's /etc/sysconfig/nfs.


> Then sed
> can be used to modify the file.
> 
> Or.... just use git to modify the config file.
> 
>  sudo git config --file /etc/idmapd.conf mapping.nobody-user neilb
> 
> did what I expected it to do :-)

Clever! But to be a little more clear, this is not about the
technology used to modify a config file.

After an administrator finds and modifies the correct file, there's
a set of dodgy distribution-dependent instructions to figure out what
daemons need to be poked with a sharp stick to pick up the new config.

For idmapd.conf changes, for example, it's going to be a different
set of pokes depending on whether its server or client.

A kernel cache has to be invalidated in some important cases. This
is true for modifying things related to the FH or export cache, and
that is already wrapped in an administrative CLI: exportfs.

For gssd, Bruce recently showed me a series of (as far as I know,
undocumented) systemctl commands that are needed to pick up changes
to gssd command line options on RHEL 7.

If I had a lot of time to explore this, my strategy would be:

a) Provide one or a few well-known tools to edit NFS config files

b) Mechanize the acts of putting said edits into effect, and integrate
those mechanisms into the tools

c) Tie the tools directly to documentation (--help or a man page)

Among other benefits, this makes it so much easier for QA folks to
automate tests of configuration adjustments. Also it hides the grimy
details about how the NFS community or the Linux distributor has
implemented these settings, making it easier to modify these
mechanisms without unnecessarily exposing users to change.

I've been encouraging this kind of direction with the nfsidmap
command, for example.


> I would lean towards a flat config file myself, but nfs-utils already
> has code for structured config files, and that code ignores settings
> before the first section.

If there was a library that managed this for us, open-coded config
file parsing for nfsmount.conf and for idmapd.conf in nfs-utils
could be replaced.


--
Chuck Lever

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

[Jeff Layton]

On Fri, 2016-11-11 at 09:17 +1100, NeilBrown wrote:
> 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.

That could be remedied though it would take code changes in nfsdcltrack.

> 
> 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.
> 

Agreed.

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

I was thinking that we already had nfsmount.conf, so making this about
server-side configuration would be more intuitive for users. You do have
a good point about rpc.gssd though.

Regardless, I do applaud the idea making the setup of NFS clients and
servers less "fiddly". Once you get beyond a very basic setup,
administering NFS as a service (client or server) is rather difficult
today.

Transitioning to a more unified configuration scheme seems like it would
be good. Maybe we could even come up with a way to subsume nfsmount.conf
as well?

-- 
Jeff Layton <jlayton@redhat.com>

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

[Steve Dickson]

On 11/13/2016 07:40 AM, Jeff Layton wrote:
> On Fri, 2016-11-11 at 09:17 +1100, NeilBrown wrote:
>> 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:-).
Yeah the systemd folks have been asking for something like this for a while.

>>
>> 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.
I agree... A configuration file is better than command line arguments..
>>
>> 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?
Yes. Having one file to configure both sides wold be nice. And having
a common way to change/edit that file would be good too... IMHO..
>>
>> 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.
+1

>>
>> 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.
> That could be remedied though it would take code changes in nfsdcltrack.
A lot of daemons would need this kind of updates.

>
>> 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.
This first thing I thought of as well. Have GUI based configuration create the file
but i think the first step is have a static file for now...
>>
>> I'm as yet far from certain as to what I want, but keeping things
>> extensible seems like a generally good principle.
>>
> Agreed.
ditto
>
>>>
>>> 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).
>>
> I was thinking that we already had nfsmount.conf, so making this about
> server-side configuration would be more intuitive for users. You do have
> a good point about rpc.gssd though.
I think having a single configuration file for both side is the way we should go...

>
> Regardless, I do applaud the idea making the setup of NFS clients and
> servers less "fiddly". Once you get beyond a very basic setup,
> administering NFS as a service (client or server) is rather difficult
> today.
>
> Transitioning to a more unified configuration scheme seems like it would
> be good. Maybe we could even come up with a way to subsume nfsmount.conf
> as well?
>
Exactly... Or those files could be generated from the one configuration file...

A couple concerns:

- Precedence, will command argument still override what is in he configuration file?
  With nfsmount.conf the precedence is command line overrides config file which
  overrides the defaults

- Migration, how did we make it know that the variables in one file are no longer used
 or will be over written by another file?

- a common and safe way to edit the file?? 

steved.

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

[Steve Dickson]

On 11/09/2016 11:58 PM, 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.
Question... Is this config file (/etc/nfs.conf) going to be in the public git tree
or generated by the distros?

Also shouldn't there be some type of man page describing how to use it?

steved.
>
> 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

Re: Question about nfsdcltrack --storagedir

[NeilBrown]

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

On Sat, Nov 12 2016, Chuck Lever wrote:

> Hiya Neil-
>
>> On Nov 10, 2016, at 5:32 PM, NeilBrown <neilb@suse.com> wrote:
>> 
>> On Fri, Nov 11 2016, Chuck Lever wrote:
>> 
>>>> 
>>>> No objection here, especially if we make it so that we have existing
>>>> behavior when there is no config file. nfs-utils even has some config
>>>> file parsing routines now in support/nfs that should be sufficient.
>>> 
>>> Just a general thought:
>>> 
>>> It's fine to store the configuration data under /etc, but for new
>>> administrative interfaces, please create a command line UI which
>>> is used to edit said configuration data. This makes it much easier
>>> for users to script administrative changes, to build GUIs around,
>>> and for our Q/A teams to construct automated tests.
>> 
>> If that is an important concern (and I don't disagree) then I would
>> suggest dropping the idea of a config file with sections and just have a
>> file with simple (case sensitive) "name=value" assignments.
>
> I'm not necessarily advocating this, but we already have a single
> central key-value file that stores NFS configuration information
> (at least on RH-based distros), and that's /etc/sysconfig/nfs.

SUSE also has /etc/sysconfig/nfs, but with different names having
different meanings.

Debian has /etc/default/nfs_common, again with different names.

>
>
>> Then sed
>> can be used to modify the file.
>> 
>> Or.... just use git to modify the config file.
>> 
>>  sudo git config --file /etc/idmapd.conf mapping.nobody-user neilb
>> 
>> did what I expected it to do :-)
>
> Clever! But to be a little more clear, this is not about the
> technology used to modify a config file.
>
> After an administrator finds and modifies the correct file, there's
> a set of dodgy distribution-dependent instructions to figure out what
> daemons need to be poked with a sharp stick to pick up the new config.
>
> For idmapd.conf changes, for example, it's going to be a different
> set of pokes depending on whether its server or client.
>
> A kernel cache has to be invalidated in some important cases. This
> is true for modifying things related to the FH or export cache, and
> that is already wrapped in an administrative CLI: exportfs.
>
> For gssd, Bruce recently showed me a series of (as far as I know,
> undocumented) systemctl commands that are needed to pick up changes
> to gssd command line options on RHEL 7.

Ahh, I understand your concern better now I think.

I would hope that "systemctl restart nfs-utils" would do everything
needed to make sure all config changes are effective.  That was
certainly the intention.

This may be a little heavy-handed (killing and restarting, rather than
just reloading config) but all the state lost should be refillable
caches so I don't think it should be a problem.

The questions is: does this command really do all that is needed?
If not, it should be fixed.

>
> If I had a lot of time to explore this, my strategy would be:
>
> a) Provide one or a few well-known tools to edit NFS config files
>
> b) Mechanize the acts of putting said edits into effect, and integrate
> those mechanisms into the tools
>
> c) Tie the tools directly to documentation (--help or a man page)

I'm not at all convinced about the need for an editing tool but I do
agree that documentation would be a big help.  I imagine a "nfs.systemd"
man page (or maybe "systemd.nfs", but that would cause arguments about
ownership of the namespace) which set out which services should be
enabled or masked or restarted to provide which outcomes.
>
> If there was a library that managed this for us, open-coded config
> file parsing for nfsmount.conf and for idmapd.conf in nfs-utils
> could be replaced.

It already was.  support/nfs/conffile.c is used by both mount and
idmapd.
e.g.
		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"));

Thanks,
NeilBrown

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

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

[Steve Dickson]

On 11/09/2016 11:58 PM, 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.
>
> Signed-off-by: NeilBrown <neilb@suse.com>
Committed....

steved.

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