diff mbox

[rpcbind] Move default state-dir to /run/rpcbind

Message ID 877f8almcf.fsf@notabene.neil.brown.name
State New
Headers show

Commit Message

NeilBrown Nov. 11, 2016, 3:36 a.m. UTC
rpcbind can save state in a file to allow restart without forgetting
about running services.

The default location is currently "/tmp" which is an over-used
directory that isn't really suitable for system files.
The modern preferences would be a subdirectory of "/run", which can
be selected with a ./configure option.  That subdirectory would still need
to be created by something.

It is trivial for rpcbind to create the directory itself, and harmless
to try if it already exists, so:
- add a "mkdir" when saving state data
- change the default to /run/rpcbind (currently used by Debian)
- remove the default settign in the code, just use the one
  in configure.ac

Signed-off-by: NeilBrown <neilb@suse.com>
---
 configure.ac    | 4 ++--
 src/warmstart.c | 5 +----
 2 files changed, 3 insertions(+), 6 deletions(-)

Comments

Mike Frysinger Nov. 11, 2016, 9 p.m. UTC | #1
On 11 Nov 2016 14:36, NeilBrown wrote:
> rpcbind can save state in a file to allow restart without forgetting
> about running services.
> 
> The default location is currently "/tmp" which is an over-used
> directory that isn't really suitable for system files.
> The modern preferences would be a subdirectory of "/run", which can
> be selected with a ./configure option.  That subdirectory would still need
> to be created by something.

the portable path is /var/cache instead of /run.  i don't think libtirpc
should be configuring itself to assume Linux by default.
-mike
NeilBrown Nov. 13, 2016, 11:09 p.m. UTC | #2
On Sat, Nov 12 2016, Mike Frysinger wrote:

> [ Unknown signature status ]
> On 11 Nov 2016 14:36, NeilBrown wrote:
>> rpcbind can save state in a file to allow restart without forgetting
>> about running services.
>> 
>> The default location is currently "/tmp" which is an over-used
>> directory that isn't really suitable for system files.
>> The modern preferences would be a subdirectory of "/run", which can
>> be selected with a ./configure option.  That subdirectory would still need
>> to be created by something.
>
> the portable path is /var/cache instead of /run.  i don't think libtirpc
> should be configuring itself to assume Linux by default.

In principle I agree.  But is /var/cache really a good choice?
We don't want the state files to persist over a reboot, and I strongly
suspect that /var/cache is designed to do exactly that.

Are there agree standards that are broader than Linux that we can look
to?
FHS defines /var/run (or even /run) but I suspect it is linux-only.

https://en.wikipedia.org/wiki/Unix_filesystem#Conventional_directory_layout

only suggests /tmp as something that won't survive reboot.
Maybe we should continue to use the /tmp filesystem, but move to a
subdirectory:
 /tmp/rpcbind
??

Thanks,
NeilBrown
Mike Frysinger Nov. 14, 2016, 7:12 p.m. UTC | #3
On 14 Nov 2016 10:09, NeilBrown wrote:
> On Sat, Nov 12 2016, Mike Frysinger wrote:
> > On 11 Nov 2016 14:36, NeilBrown wrote:
> >> rpcbind can save state in a file to allow restart without forgetting
> >> about running services.
> >> 
> >> The default location is currently "/tmp" which is an over-used
> >> directory that isn't really suitable for system files.
> >> The modern preferences would be a subdirectory of "/run", which can
> >> be selected with a ./configure option.  That subdirectory would still need
> >> to be created by something.
> >
> > the portable path is /var/cache instead of /run.  i don't think libtirpc
> > should be configuring itself to assume Linux by default.
> 
> In principle I agree.  But is /var/cache really a good choice?
> We don't want the state files to persist over a reboot, and I strongly
> suspect that /var/cache is designed to do exactly that.
> 
> Are there agree standards that are broader than Linux that we can look
> to?
> FHS defines /var/run (or even /run) but I suspect it is linux-only.

/var/run should work across systems i believe.  at least BSD's support it.
-mike
Steve Dickson Nov. 14, 2016, 7:26 p.m. UTC | #4
On 11/14/2016 02:12 PM, Mike Frysinger wrote:
> On 14 Nov 2016 10:09, NeilBrown wrote:
>> On Sat, Nov 12 2016, Mike Frysinger wrote:
>>> On 11 Nov 2016 14:36, NeilBrown wrote:
>>>> rpcbind can save state in a file to allow restart without forgetting
>>>> about running services.
>>>>
>>>> The default location is currently "/tmp" which is an over-used
>>>> directory that isn't really suitable for system files.
>>>> The modern preferences would be a subdirectory of "/run", which can
>>>> be selected with a ./configure option.  That subdirectory would still need
>>>> to be created by something.
>>> the portable path is /var/cache instead of /run.  i don't think libtirpc
>>> should be configuring itself to assume Linux by default.
>> In principle I agree.  But is /var/cache really a good choice?
>> We don't want the state files to persist over a reboot, and I strongly
>> suspect that /var/cache is designed to do exactly that.
>>
>> Are there agree standards that are broader than Linux that we can look
>> to?
>> FHS defines /var/run (or even /run) but I suspect it is linux-only.
> /var/run should work across systems i believe.  at least BSD's support it.
In the Red Hat distos /var/run is a symbolic link to /run and the systemd
folks have asked us to use /run instead of /var/run

steved.

--
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
Mike Frysinger Nov. 14, 2016, 8:12 p.m. UTC | #5
On 14 Nov 2016 14:26, Steve Dickson wrote:
> On 11/14/2016 02:12 PM, Mike Frysinger wrote:
> > On 14 Nov 2016 10:09, NeilBrown wrote:
> >> On Sat, Nov 12 2016, Mike Frysinger wrote:
> >>> On 11 Nov 2016 14:36, NeilBrown wrote:
> >>>> rpcbind can save state in a file to allow restart without forgetting
> >>>> about running services.
> >>>>
> >>>> The default location is currently "/tmp" which is an over-used
> >>>> directory that isn't really suitable for system files.
> >>>> The modern preferences would be a subdirectory of "/run", which can
> >>>> be selected with a ./configure option.  That subdirectory would still need
> >>>> to be created by something.
> >>> the portable path is /var/cache instead of /run.  i don't think libtirpc
> >>> should be configuring itself to assume Linux by default.
> >> In principle I agree.  But is /var/cache really a good choice?
> >> We don't want the state files to persist over a reboot, and I strongly
> >> suspect that /var/cache is designed to do exactly that.
> >>
> >> Are there agree standards that are broader than Linux that we can look
> >> to?
> >> FHS defines /var/run (or even /run) but I suspect it is linux-only.
> > /var/run should work across systems i believe.  at least BSD's support it.
>
> In the Red Hat distos /var/run is a symbolic link to /run and the systemd
> folks have asked us to use /run instead of /var/run

yes, but we already know that's not an acceptable default -- /run today
is Linux specific.  the question i was responding to here is if there's
a portable location that is better than /tmp.

Linux distros already know that for many packages they need to pass
flags to get /run behavior.  rpcbind is no different.
-mike
NeilBrown Nov. 15, 2016, 6:36 a.m. UTC | #6
On Tue, Nov 15 2016, Steve Dickson wrote:

> On 11/10/2016 10:36 PM, NeilBrown wrote:
>> rpcbind can save state in a file to allow restart without forgetting
>> about running services.
>>
>> The default location is currently "/tmp" which is an over-used
>> directory that isn't really suitable for system files.
>> The modern preferences would be a subdirectory of "/run", which can
>> be selected with a ./configure option.  That subdirectory would still need
>> to be created by something.
>>
>> It is trivial for rpcbind to create the directory itself, and harmless
>> to try if it already exists, so:
>> - add a "mkdir" when saving state data
>> - change the default to /run/rpcbind (currently used by Debian)
>> - remove the default settign in the code, just use the one
>>   in configure.ac
> I'm all for move the warmstart directory to /run but why don't we have systemd
> create the directory via a  tmpfiles.d( config file... some like
>
> #Type Path      Mode UID GID Age Argument
> D  /run/rpcbind 0700 rpc rpc - -
>
> The only thing I'm not sure about is how it would get installed...
> I guess it would some type of Makefile.ac entry??

Because not everyone uses systemd?

Someone at SUSE recently moved the state directory to /run/rpcbind and
used tmpfiles.d exactly as you describe to create /run/rpcbind.  Then
found they needed to do something extra and different for dracut.  I
haven't looked into why, but it is presumably because while dracut does
use systemd, it doesn't use it quite the same way that normal system boot
uses it.
I've found this with configuring mdadm too.  I got the udev and systemd
configuration just right, and then found that while dracut uses udev and
systemd, I still need to add extra stuff to dracut.  And then there was
another boot environment which used udev but not systemd, so it needed
different tweaking.

So I think we are less likely to run into strange problems if we just
get rpcbind to create its own directory.  Certainly rely on systemd to
do the things that systemd does best (like start the service), but don't
rely on it to do things we can easily do ourselves.

Thanks,
NeilBrown
Steve Dickson Nov. 15, 2016, 4:02 p.m. UTC | #7
On 11/15/2016 01:36 AM, NeilBrown wrote:
> On Tue, Nov 15 2016, Steve Dickson wrote:
>
>> On 11/10/2016 10:36 PM, NeilBrown wrote:
>>> rpcbind can save state in a file to allow restart without forgetting
>>> about running services.
>>>
>>> The default location is currently "/tmp" which is an over-used
>>> directory that isn't really suitable for system files.
>>> The modern preferences would be a subdirectory of "/run", which can
>>> be selected with a ./configure option.  That subdirectory would still need
>>> to be created by something.
>>>
>>> It is trivial for rpcbind to create the directory itself, and harmless
>>> to try if it already exists, so:
>>> - add a "mkdir" when saving state data
>>> - change the default to /run/rpcbind (currently used by Debian)
>>> - remove the default settign in the code, just use the one
>>>   in configure.ac
>> I'm all for move the warmstart directory to /run but why don't we have systemd
>> create the directory via a  tmpfiles.d( config file... some like
>>
>> #Type Path      Mode UID GID Age Argument
>> D  /run/rpcbind 0700 rpc rpc - -
>>
>> The only thing I'm not sure about is how it would get installed...
>> I guess it would some type of Makefile.ac entry??
> Because not everyone uses systemd?
Yeah... I figured as much.
> Someone at SUSE recently moved the state directory to /run/rpcbind and
> used tmpfiles.d exactly as you describe to create /run/rpcbind.  Then
> found they needed to do something extra and different for dracut.  I
> haven't looked into why, but it is presumably because while dracut does
> use systemd, it doesn't use it quite the same way that normal system boot
> uses it.
I didn't have this problem when I move rpcbind in RHEL7 to use /run/rpcbind via tmpfiles.d
The biggest problem I had was migrating the warmstart files to the new place.
Having /run/rpcbind exist after boot made that much easier. 
> I've found this with configuring mdadm too.  I got the udev and systemd
> configuration just right, and then found that while dracut uses udev and
> systemd, I still need to add extra stuff to dracut.  And then there was
> another boot environment which used udev but not systemd, so it needed
> different tweaking.
>
> So I think we are less likely to run into strange problems if we just
> get rpcbind to create its own directory.  Certainly rely on systemd to
> do the things that systemd does best (like start the service), but don't
> rely on it to do things we can easily do ourselves.
Another potential problem is SElinux... Its never a fan of daemon creating things
on the fly... but I guess that is my problem 8-)

steved.
 
>
> Thanks,
> NeilBrown
>

--
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
NeilBrown Nov. 15, 2016, 8:28 p.m. UTC | #8
On Wed, Nov 16 2016, Steve Dickson wrote:

> On 11/15/2016 01:36 AM, NeilBrown wrote:
>> On Tue, Nov 15 2016, Steve Dickson wrote:
>>
>>> On 11/10/2016 10:36 PM, NeilBrown wrote:
>>>> rpcbind can save state in a file to allow restart without forgetting
>>>> about running services.
>>>>
>>>> The default location is currently "/tmp" which is an over-used
>>>> directory that isn't really suitable for system files.
>>>> The modern preferences would be a subdirectory of "/run", which can
>>>> be selected with a ./configure option.  That subdirectory would still need
>>>> to be created by something.
>>>>
>>>> It is trivial for rpcbind to create the directory itself, and harmless
>>>> to try if it already exists, so:
>>>> - add a "mkdir" when saving state data
>>>> - change the default to /run/rpcbind (currently used by Debian)
>>>> - remove the default settign in the code, just use the one
>>>>   in configure.ac
>>> I'm all for move the warmstart directory to /run but why don't we have systemd
>>> create the directory via a  tmpfiles.d( config file... some like
>>>
>>> #Type Path      Mode UID GID Age Argument
>>> D  /run/rpcbind 0700 rpc rpc - -
>>>
>>> The only thing I'm not sure about is how it would get installed...
>>> I guess it would some type of Makefile.ac entry??
>> Because not everyone uses systemd?
> Yeah... I figured as much.
>> Someone at SUSE recently moved the state directory to /run/rpcbind and
>> used tmpfiles.d exactly as you describe to create /run/rpcbind.  Then
>> found they needed to do something extra and different for dracut.  I
>> haven't looked into why, but it is presumably because while dracut does
>> use systemd, it doesn't use it quite the same way that normal system boot
>> uses it.
> I didn't have this problem when I move rpcbind in RHEL7 to use /run/rpcbind via tmpfiles.d
> The biggest problem I had was migrating the warmstart files to the new place.
> Having /run/rpcbind exist after boot made that much easier. 

That would be a bit fiddlely.
Might be easiest to create a symlink /run/rpcind -> old-location when
the package is installed, then let the proper directory be created on
boot.
Only then you cannot clean up the old directory at install time.

>> I've found this with configuring mdadm too.  I got the udev and systemd
>> configuration just right, and then found that while dracut uses udev and
>> systemd, I still need to add extra stuff to dracut.  And then there was
>> another boot environment which used udev but not systemd, so it needed
>> different tweaking.
>>
>> So I think we are less likely to run into strange problems if we just
>> get rpcbind to create its own directory.  Certainly rely on systemd to
>> do the things that systemd does best (like start the service), but don't
>> rely on it to do things we can easily do ourselves.
> Another potential problem is SElinux... Its never a fan of daemon creating things
> on the fly... but I guess that is my problem 8-)

rpcbind would only create the directory if it doesn't exist.  If you
need to create it with special SElinux things, there is no harm in doing
that first.

Thanks,
NeilBrown
diff mbox

Patch

diff --git a/configure.ac b/configure.ac
index f84921eb27fb..fe7d0b068439 100644
--- a/configure.ac
+++ b/configure.ac
@@ -22,8 +22,8 @@  AC_ARG_ENABLE([warmstarts],
 AM_CONDITIONAL(WARMSTART, test x$enable_warmstarts = xyes)
 
 AC_ARG_WITH([statedir],
-  AS_HELP_STRING([--with-statedir=ARG], [use ARG as state dir @<:@default=/tmp@:>@])
-  ,, [with_statedir=/tmp])
+  AS_HELP_STRING([--with-statedir=ARG], [use ARG as state dir @<:@default=/run/rpcbind@:>@])
+  ,, [with_statedir=/run/rpcbind])
 AC_SUBST([statedir], [$with_statedir])
 
 AC_ARG_WITH([rpcuser],
diff --git a/src/warmstart.c b/src/warmstart.c
index 122a058b7954..3896027e62ba 100644
--- a/src/warmstart.c
+++ b/src/warmstart.c
@@ -48,10 +48,6 @@ 
 
 #include "rpcbind.h"
 
-#ifndef RPCBIND_STATEDIR
-#define RPCBIND_STATEDIR "/tmp"
-#endif
-
 /* These files keep the pmap_list and rpcb_list in XDR format */
 #define	RPCBFILE	RPCBIND_STATEDIR "/rpcbind.xdr"
 #ifdef PORTMAP
@@ -141,6 +137,7 @@  error:
 void
 write_warmstart()
 {
+	(void) mkdir(RPCBIND_STATEDIR, 0770);
 	(void) write_struct(RPCBFILE, (xdrproc_t)xdr_rpcblist_ptr, &list_rbl);
 #ifdef PORTMAP
 	(void) write_struct(PMAPFILE, (xdrproc_t)xdr_pmaplist_ptr, &list_pml);