PR#1: Drop %post chown rpcuser /var/lib/nfs/statd - rpms/nfs-utils

rpms / nfs-utils

#1 Drop %post chown rpcuser /var/lib/nfs/statd

Opened 6 years ago by walters. Modified 6 years ago

rpms/ walters/nfs-utils drop-post-chown into rawhide

Drop %post chown rpcuser /var/lib/nfs/statd

Colin Walters • 6 years ago

b8d1dfb

nfs-utils.spec

file modified

-3

		`@@ -220,9 +220,6 @@`
		`%systemd_post nfs-config`
		`%systemd_post nfs-server`

		`- # Make sure statd used the correct uid/gid.`
		`- chown -R rpcuser:rpcuser /var/lib/nfs/statd`
		`-`
		`%preun`
		`if [ $1 -eq 0 ]; then`
		`%systemd_preun nfs-client.target`

walters commented 6 years ago

Digging through the git history quickly, this seems to be a workaround
for an ancient upgrade issue. We really shouldn't be
doing this on new installs, and really not on on every upgrade either.

The particular reason I'm making this change is that rpm-ostree
explicitly errors out when scripts fail (because we can), and this
will fail in an rpm-ostree environment as the real /var is not accessible
to scripts.

More information:

walters commented 6 years ago

Can you review this patch, please?

steved commented 6 years ago

I'm a bit concerned about removing that chown since it
ensures NFS v3 locking will work (aka rpc.statd will have
access to that directory).

How testing the the directory exists before doing the chown
something like:
[ -d /var/lib/nfs/statd ] && chown -R rpcuser:rpcuser /var/lib/nfs/statd

walters commented 6 years ago

But yum/librpm (and rpm-ostree) already do the chown due to:
%dir %attr(700,rpcuser,rpcuser) %{_sharedstatedir}/nfs/statd

So I understand being conservative and I'm willing to make the change you suggested...but the only scenario I can think of where this would help is upgrading a truly ancient system (where NFS isn't working anyways because the perms are wrong?).

walters commented 6 years ago

FWIW I tested this with a local build, then installing it in a F27 container via yum, and verified:

ls -ald /var/lib/nfs/statd/
drwx------. 4 rpcuser rpcuser 30 Nov  8 14:50 /var/lib/nfs/statd/

steved commented 6 years ago

But yum/librpm (and rpm-ostree) already do the chown due to:
%dir %attr(700,rpcuser,rpcuser) %{_sharedstatedir}/nfs/statd
I see your point now... I'll make the change on the next build..