All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Three little nfs-utils fixes.
@ 2018-12-03  0:58 NeilBrown
  2018-12-03  0:58 ` [PATCH 1/3] nfs.conf: allow empty assignments NeilBrown
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: NeilBrown @ 2018-12-03  0:58 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Justin Mitchell, linux-nfs

First fixes a regression I found when importing the latest upstream at
SUSE.

Others are related to ensuring systemd starts thing properly in cases
that hadn't been tested before.

Thanks,
NeilBrown


---

NeilBrown (3):
      nfs.conf: allow empty assignments.
      Let systemd know when rpc.statd is needed.
      systemd: run statd-notify even when nfs-client isn't enabled.


 support/nfs/conffile.c      |    5 -----
 systemd/rpc-statd.service   |    1 +
 tests/nfsconf/01-errors.exp |    1 -
 utils/statd/start-statd     |    7 ++++++-
 4 files changed, 7 insertions(+), 7 deletions(-)

--
Signature


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] nfs.conf: allow empty assignments.
  2018-12-03  0:58 [PATCH 0/3] Three little nfs-utils fixes NeilBrown
@ 2018-12-03  0:58 ` NeilBrown
  2018-12-03  0:58 ` [PATCH 3/3] systemd: run statd-notify even when nfs-client isn't enabled NeilBrown
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: NeilBrown @ 2018-12-03  0:58 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Justin Mitchell, linux-nfs

A recent commit caused an error message (but didn't actually
trigger an error) for a config file line like:

  foo =

There is no good reason to treat this as an error, and we (SUSE) have
established practice of expecting these to be accepted.
Specifically "/etc/nfs.conf" includes "/etc/sysconfig/nfs" which
contains lots of empty definitions.

So remove the error message.

Fixes: 1c2c18806800 ("nfs.conf: Removed buffer overruns")
Signed-off-by: NeilBrown <neilb@suse.com>
---
 support/nfs/conffile.c      |    5 -----
 tests/nfsconf/01-errors.exp |    1 -
 2 files changed, 6 deletions(-)

diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index 0e39aca6b468..77c5790c893c 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -405,11 +405,6 @@ conf_parse_line(int trans, char *line, const char *filename, int lineno, char **
 			  "missing tag in assignment", filename, lineno);
 		return;
 	}
-	if (*val == '\0') {
-		xlog_warn("config error at %s:%d: "
-			  "missing value in assignment", filename, lineno);
-		return;
-	}
 
 	if (strcasecmp(line, "include")==0) {
 		/* load and parse subordinate config files */
diff --git a/tests/nfsconf/01-errors.exp b/tests/nfsconf/01-errors.exp
index 2bf1b8c7f65b..0b985b46267e 100644
--- a/tests/nfsconf/01-errors.exp
+++ b/tests/nfsconf/01-errors.exp
@@ -4,7 +4,6 @@ nfsconf: config error at 01-errors.conf:10: non-matched ']', ignoring until next
 nfsconf: config error at 01-errors.conf:11: ignoring line not in a section
 nfsconf: config error at 01-errors.conf:14: line not empty and not an assignment
 nfsconf: config error at 01-errors.conf:15: missing tag in assignment
-nfsconf: config error at 01-errors.conf:16: missing value in assignment
 nfsconf: config error at 01-errors.conf:18: unmatched quotes
 [four]
  four = foo = bar



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] Let systemd know when rpc.statd is needed.
  2018-12-03  0:58 [PATCH 0/3] Three little nfs-utils fixes NeilBrown
  2018-12-03  0:58 ` [PATCH 1/3] nfs.conf: allow empty assignments NeilBrown
  2018-12-03  0:58 ` [PATCH 3/3] systemd: run statd-notify even when nfs-client isn't enabled NeilBrown
@ 2018-12-03  0:58 ` NeilBrown
  2018-12-10 21:26 ` [PATCH 0/3] Three little nfs-utils fixes Steve Dickson
  3 siblings, 0 replies; 5+ messages in thread
From: NeilBrown @ 2018-12-03  0:58 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Justin Mitchell, linux-nfs

A recent change to set IgnoreOnIsolate for rpc-statd
isn't quite sufficient (though it doesn't hurt).
While rpc-statd does remain when
  systemctl isolate multi-user
is run, its dependencies don't remain, so rpcbind might
get killed, which makes rpc.statd rather useless.

The reason this is all an issue is that systemd doesn't know that
rpc-statd is needed - mount.nfs explicitly starts it rather than
having a dependency start it.
This can be rectified by explicitly telling systemd about the
dependency using "systemctl add-wants".  This can be done in the
start-statd script, at the same time that rpc-statd is started.

As --runtime dependency is used so that it doesn't persist across
reboots.  A new dependency will be created on next boot if an NFSv3
filesystem is mounted.

With this in place, both rpc.statd and rpcbind remain.
Actually, rpcbind.service is stopped, but rpcbind.socket remains,
and when anything tries to contact rpcbind, rpcbind.service
is automatically started and it re-reads its saved state.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 utils/statd/start-statd |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/utils/statd/start-statd b/utils/statd/start-statd
index 82715b40c1af..54ced822016a 100755
--- a/utils/statd/start-statd
+++ b/utils/statd/start-statd
@@ -20,7 +20,12 @@ fi
 # First try systemd if it's installed.
 if [ -d /run/systemd/system ]; then
     # Quit only if the call worked.
-    systemctl start rpc-statd.service && exit
+    if systemctl start rpc-statd.service; then
+        # Ensure systemd knows not to stop rpc.statd or its dependencies
+        # on 'systemctl isolate ..'
+        systemctl add-wants --runtime remote-fs.target rpc-statd.service
+        exit 0
+    fi
 fi
 
 cd /



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] systemd: run statd-notify even when nfs-client isn't enabled.
  2018-12-03  0:58 [PATCH 0/3] Three little nfs-utils fixes NeilBrown
  2018-12-03  0:58 ` [PATCH 1/3] nfs.conf: allow empty assignments NeilBrown
@ 2018-12-03  0:58 ` NeilBrown
  2018-12-03  0:58 ` [PATCH 2/3] Let systemd know when rpc.statd is needed NeilBrown
  2018-12-10 21:26 ` [PATCH 0/3] Three little nfs-utils fixes Steve Dickson
  3 siblings, 0 replies; 5+ messages in thread
From: NeilBrown @ 2018-12-03  0:58 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Justin Mitchell, linux-nfs

When NFS filesytems are mounted, nfs-client.target really should
be enabled.  However it is possible to mount NFS filesystems
without this (providing gss isn't used) and it mostly works.

One aspect that doesn't work is that sm-notify isn't run, so the server
isn't told to drop any locks from the previous client instance.
This can result in confusing failures: if a client crashes while
holding a lock, it won't be able to get the same lock after a reboot.

While this isn't a complete solution (nfs-client really should be
enabled), adding a dependency from rpc-statd to rpc-statd-notify is
easy, has no down sides, and could help avoid confusion.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 systemd/rpc-statd.service |    1 +
 1 file changed, 1 insertion(+)

diff --git a/systemd/rpc-statd.service b/systemd/rpc-statd.service
index 1f4e6a8b92ab..3e92cf71add0 100644
--- a/systemd/rpc-statd.service
+++ b/systemd/rpc-statd.service
@@ -4,6 +4,7 @@ DefaultDependencies=no
 Conflicts=umount.target
 Requires=nss-lookup.target rpcbind.socket
 Wants=network-online.target
+Wants=rpc-statd-notify.service
 After=network-online.target nss-lookup.target rpcbind.socket
 
 PartOf=nfs-utils.service



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3] Three little nfs-utils fixes.
  2018-12-03  0:58 [PATCH 0/3] Three little nfs-utils fixes NeilBrown
                   ` (2 preceding siblings ...)
  2018-12-03  0:58 ` [PATCH 2/3] Let systemd know when rpc.statd is needed NeilBrown
@ 2018-12-10 21:26 ` Steve Dickson
  3 siblings, 0 replies; 5+ messages in thread
From: Steve Dickson @ 2018-12-10 21:26 UTC (permalink / raw)
  To: NeilBrown; +Cc: linux-nfs



On 12/2/18 7:58 PM, NeilBrown wrote:
> First fixes a regression I found when importing the latest upstream at
> SUSE.
> 
> Others are related to ensuring systemd starts thing properly in cases
> that hadn't been tested before.
> 
> Thanks,
> NeilBrown
Committed... 

steved.

> 
> 
> ---
> 
> NeilBrown (3):
>       nfs.conf: allow empty assignments.
>       Let systemd know when rpc.statd is needed.
>       systemd: run statd-notify even when nfs-client isn't enabled.
> 
> 
>  support/nfs/conffile.c      |    5 -----
>  systemd/rpc-statd.service   |    1 +
>  tests/nfsconf/01-errors.exp |    1 -
>  utils/statd/start-statd     |    7 ++++++-
>  4 files changed, 7 insertions(+), 7 deletions(-)
> 
> --
> Signature
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-12-10 21:26 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-03  0:58 [PATCH 0/3] Three little nfs-utils fixes NeilBrown
2018-12-03  0:58 ` [PATCH 1/3] nfs.conf: allow empty assignments NeilBrown
2018-12-03  0:58 ` [PATCH 3/3] systemd: run statd-notify even when nfs-client isn't enabled NeilBrown
2018-12-03  0:58 ` [PATCH 2/3] Let systemd know when rpc.statd is needed NeilBrown
2018-12-10 21:26 ` [PATCH 0/3] Three little nfs-utils fixes Steve Dickson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.