2018-12-03 00:58:46

by NeilBrown

[permalink] [raw]
Subject: [PATCH 0/3] Three little nfs-utils fixes.

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



2018-12-03 00:58:50

by NeilBrown

[permalink] [raw]
Subject: [PATCH 1/3] nfs.conf: allow empty assignments.

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



2018-12-03 00:58:55

by NeilBrown

[permalink] [raw]
Subject: [PATCH 2/3] Let systemd know when rpc.statd is needed.

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 <[email protected]>
---
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 /



2018-12-03 00:59:00

by NeilBrown

[permalink] [raw]
Subject: [PATCH 3/3] systemd: run statd-notify even when nfs-client isn't enabled.

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



2018-12-10 21:26:56

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 0/3] Three little nfs-utils fixes.



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
>