* [PATCH 1/1] statd: Decouple statd's state directory from the NFS state directory
@ 2011-09-20 15:03 Steve Dickson
2011-09-20 15:40 ` Christoph Hellwig
2011-09-20 15:43 ` Chuck Lever
0 siblings, 2 replies; 6+ messages in thread
From: Steve Dickson @ 2011-09-20 15:03 UTC (permalink / raw)
To: Linux NFS Mailing list
To allow greater flexibility to where statd's state is kept,
statd's state path can now be decoupled from the normal
NFS state directory.
In configure.ac, the NSM_STATD_PATH definition will now define
the path to where the state information is kept. The default
value, /var/lib/nfs, can be redefined with the --with-statdpath
flag.
Signed-off-by: Steve Dickson <steved@redhat.com>
---
configure.ac | 9 +++++++++
support/nsm/file.c | 7 ++-----
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/configure.ac b/configure.ac
index b5934c4..7819d98 100644
--- a/configure.ac
+++ b/configure.ac
@@ -23,6 +23,14 @@ AC_ARG_WITH(statedir,
statedir=$withval,
statedir=/var/lib/nfs)
AC_SUBST(statedir)
+AC_ARG_WITH(statdpath,
+ [AC_HELP_STRING([--with-statdpath=/foo @<:@default=/var/lib/nfs@:>@],
+ [define statd's state dir as /foo instead of the NFS statedir]
+ )],
+ statdpath=$withval,
+ statdpath=$statedir
+ )
+ AC_SUBST(statdpath)
AC_ARG_WITH(statduser,
[AC_HELP_STRING([--with-statduser=rpcuser],
[statd to run under @<:@rpcuser or nobody@:>@]
@@ -387,6 +395,7 @@ dnl *************************************************************
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_STATD_PATH, "$statdpath", [Define this if you what statd file placed in somewhere other than NFS_STATEDIR])
if test "x$cross_compiling" = "xno"; then
CFLAGS_FOR_BUILD=${CFLAGS_FOR_BUILD-"$CFLAGS"}
diff --git a/support/nsm/file.c b/support/nsm/file.c
index a12c753..d9da572 100644
--- a/support/nsm/file.c
+++ b/support/nsm/file.c
@@ -95,12 +95,9 @@
#define NSM_KERNEL_STATE_FILE "/proc/sys/fs/nfs/nsm_local_state"
/*
- * Some distributions place statd's files in a subdirectory
+ * Defines where statd's state files live.
*/
-#define NSM_PATH_EXTENSION
-/* #define NSM_PATH_EXTENSION "/statd" */
-
-#define NSM_DEFAULT_STATEDIR NFS_STATEDIR NSM_PATH_EXTENSION
+#define NSM_DEFAULT_STATEDIR NSM_STATD_PATH
static char nsm_base_dirname[PATH_MAX] = NSM_DEFAULT_STATEDIR;
--
1.7.6.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] statd: Decouple statd's state directory from the NFS state directory
2011-09-20 15:03 [PATCH 1/1] statd: Decouple statd's state directory from the NFS state directory Steve Dickson
@ 2011-09-20 15:40 ` Christoph Hellwig
2011-09-20 16:16 ` Steve Dickson
2011-09-20 15:43 ` Chuck Lever
1 sibling, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2011-09-20 15:40 UTC (permalink / raw)
To: Steve Dickson; +Cc: Linux NFS Mailing list
On Tue, Sep 20, 2011 at 11:03:40AM -0400, Steve Dickson wrote:
> To allow greater flexibility to where statd's state is kept,
> statd's state path can now be decoupled from the normal
> NFS state directory.
>
> In configure.ac, the NSM_STATD_PATH definition will now define
> the path to where the state information is kept. The default
> value, /var/lib/nfs, can be redefined with the --with-statdpath
> flag.
What is thje rationale for this? And who would want to move it at
compile time, not at run time?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] statd: Decouple statd's state directory from the NFS state directory
2011-09-20 15:03 [PATCH 1/1] statd: Decouple statd's state directory from the NFS state directory Steve Dickson
2011-09-20 15:40 ` Christoph Hellwig
@ 2011-09-20 15:43 ` Chuck Lever
1 sibling, 0 replies; 6+ messages in thread
From: Chuck Lever @ 2011-09-20 15:43 UTC (permalink / raw)
To: Steve Dickson; +Cc: Linux NFS Mailing list
On Sep 20, 2011, at 11:03 AM, Steve Dickson wrote:
> To allow greater flexibility to where statd's state is kept,
> statd's state path can now be decoupled from the normal
> NFS state directory.
statd has always allowed this via command line options. What the patch does is allow distributions to specify a _default_ statd state directory that is not under /var/lib/nfs, via a convenient configure option instead of by patching.
This actually corrects a limitation introduced in January 2010 by me. Previously the macro definitions allowed the default to be any directory, not just under /var/lib/nfs. Introduced by commit f16fb1cd.
> In configure.ac, the NSM_STATD_PATH definition will now define
> the path to where the state information is kept. The default
> value, /var/lib/nfs, can be redefined with the --with-statdpath
> flag.
>
> Signed-off-by: Steve Dickson <steved@redhat.com>
> ---
> configure.ac | 9 +++++++++
> support/nsm/file.c | 7 ++-----
> 2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index b5934c4..7819d98 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -23,6 +23,14 @@ AC_ARG_WITH(statedir,
> statedir=$withval,
> statedir=/var/lib/nfs)
> AC_SUBST(statedir)
> +AC_ARG_WITH(statdpath,
> + [AC_HELP_STRING([--with-statdpath=/foo @<:@default=/var/lib/nfs@:>@],
> + [define statd's state dir as /foo instead of the NFS statedir]
> + )],
> + statdpath=$withval,
> + statdpath=$statedir
> + )
> + AC_SUBST(statdpath)
Very clean!
> AC_ARG_WITH(statduser,
> [AC_HELP_STRING([--with-statduser=rpcuser],
> [statd to run under @<:@rpcuser or nobody@:>@]
> @@ -387,6 +395,7 @@ dnl *************************************************************
> 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_STATD_PATH, "$statdpath", [Define this if you what statd file placed in somewhere other than NFS_STATEDIR])
The help text isn't quite accurate, since $statdpath is always defined now. Something more like this:
"Define this to the pathname where statd keeps its state files"
> if test "x$cross_compiling" = "xno"; then
> CFLAGS_FOR_BUILD=${CFLAGS_FOR_BUILD-"$CFLAGS"}
> diff --git a/support/nsm/file.c b/support/nsm/file.c
> index a12c753..d9da572 100644
> --- a/support/nsm/file.c
> +++ b/support/nsm/file.c
> @@ -95,12 +95,9 @@
> #define NSM_KERNEL_STATE_FILE "/proc/sys/fs/nfs/nsm_local_state"
>
>
> - * Some distributions place statd's files in a subdirectory
> + * Defines where statd's state files live.
> */
> -#define NSM_PATH_EXTENSION
> -/* #define NSM_PATH_EXTENSION "/statd" */
> -
> -#define NSM_DEFAULT_STATEDIR NFS_STATEDIR NSM_PATH_EXTENSION
> +#define NSM_DEFAULT_STATEDIR NSM_STATD_PATH
IMO we don't need two macros that by definition always have the same value. That will either get fixed or tripped on eventually, so I think it should be corrected now.
The cleanest approach would be to set up NSM_DEFAULT_STATEDIR in configure.ac, and drop the use of NSM_STATD_PATH. Or you can replace the use of NSM_DEFAULT_STATEDIR everywhere in support/nsm/file.c. Up to you.
> static char nsm_base_dirname[PATH_MAX] = NSM_DEFAULT_STATEDIR;
>
> --
> 1.7.6.2
>
> --
> 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
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] statd: Decouple statd's state directory from the NFS state directory
2011-09-20 15:40 ` Christoph Hellwig
@ 2011-09-20 16:16 ` Steve Dickson
2011-09-20 16:19 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Steve Dickson @ 2011-09-20 16:16 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Linux NFS Mailing list
On 09/20/2011 11:40 AM, Christoph Hellwig wrote:
> On Tue, Sep 20, 2011 at 11:03:40AM -0400, Steve Dickson wrote:
>> To allow greater flexibility to where statd's state is kept,
>> statd's state path can now be decoupled from the normal
>> NFS state directory.
>>
>> In configure.ac, the NSM_STATD_PATH definition will now define
>> the path to where the state information is kept. The default
>> value, /var/lib/nfs, can be redefined with the --with-statdpath
>> flag.
>
> What is thje rationale for this? And who would want to move it at
> compile time, not at run time?
It all has to do with statd not running as root...
During the rpm installation, a state directory is created and
the uid/gid are set to rpcuser id. When statd fires up, those
uid/gids are obtained and used to set the process's uid/gid so
the daemon does not run as root..
The default NFS state directory is /var/lib/nfs. Since other
processes, like mountd and exportfs, read and write to that,
we don't want to muck around with its ownership. So a statd
directory is created and the ownership of that directory
is mucked with.
steved.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] statd: Decouple statd's state directory from the NFS state directory
2011-09-20 16:16 ` Steve Dickson
@ 2011-09-20 16:19 ` Christoph Hellwig
2011-09-20 16:33 ` Chuck Lever
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2011-09-20 16:19 UTC (permalink / raw)
To: Steve Dickson; +Cc: Christoph Hellwig, Linux NFS Mailing list
On Tue, Sep 20, 2011 at 12:16:50PM -0400, Steve Dickson wrote:
> > What is thje rationale for this? And who would want to move it at
> > compile time, not at run time?
> It all has to do with statd not running as root...
>
> During the rpm installation, a state directory is created and
> the uid/gid are set to rpcuser id. When statd fires up, those
> uid/gids are obtained and used to set the process's uid/gid so
> the daemon does not run as root..
>
> The default NFS state directory is /var/lib/nfs. Since other
> processes, like mountd and exportfs, read and write to that,
> we don't want to muck around with its ownership. So a statd
> directory is created and the ownership of that directory
> is mucked with.
Shouldn't the configure option then be about running statd non-root
and all things required for it? E.g. do the set*uid, creating the
new dir with the right owner and permissions, and using it?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] statd: Decouple statd's state directory from the NFS state directory
2011-09-20 16:19 ` Christoph Hellwig
@ 2011-09-20 16:33 ` Chuck Lever
0 siblings, 0 replies; 6+ messages in thread
From: Chuck Lever @ 2011-09-20 16:33 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Steve Dickson, Linux NFS Mailing list
On Sep 20, 2011, at 12:19 PM, Christoph Hellwig wrote:
> On Tue, Sep 20, 2011 at 12:16:50PM -0400, Steve Dickson wrote:
>>> What is thje rationale for this? And who would want to move it at
>>> compile time, not at run time?
>> It all has to do with statd not running as root...
>>
>> During the rpm installation, a state directory is created and
>> the uid/gid are set to rpcuser id. When statd fires up, those
>> uid/gids are obtained and used to set the process's uid/gid so
>> the daemon does not run as root..
>>
>> The default NFS state directory is /var/lib/nfs. Since other
>> processes, like mountd and exportfs, read and write to that,
>> we don't want to muck around with its ownership. So a statd
>> directory is created and the ownership of that directory
>> is mucked with.
>
> Shouldn't the configure option then be about running statd non-root
> and all things required for it? E.g. do the set*uid, creating the
> new dir with the right owner and permissions, and using it?
I agree that these days we would choose command line options or build-time settings instead of reading the ownership of our state directory, but it's an ancient formal administrative interface.
We're not creating anything new here, just making it easier for distros to customize.
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-09-20 16:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-20 15:03 [PATCH 1/1] statd: Decouple statd's state directory from the NFS state directory Steve Dickson
2011-09-20 15:40 ` Christoph Hellwig
2011-09-20 16:16 ` Steve Dickson
2011-09-20 16:19 ` Christoph Hellwig
2011-09-20 16:33 ` Chuck Lever
2011-09-20 15:43 ` Chuck Lever
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).