Merge ~ahasenack/ubuntu/+source/nfs-utils:kinetic-nfs-utils-generator into ubuntu/+source/nfs-utils:ubuntu/devel

[Andreas Hasenack]

This fixes LP: #1971935

PPA: https://launchpad.net/~ahasenack/+archive/ubuntu/nfs-utils-generator

Test case at the end.

The diff is very small, don't be scared by the wall of text below.

A longer explanation about what is happening can be seen in https://bugs.launchpad.net/ubuntu/+source/nfs-utils/+bug/1971935/comments/22

quick tl;dr about systemd generators: they are small binary files in /lib/systemd/system-generators/ that get run very early in the boot, or whenever systemctl daemon-reload is issued, and their intent is to create systemd unit files. For example, the fstab generator produces individual mount units for the lines in /etc/fstab, and so on.

nfs-common ships a var-lib-nfs-rpc_pipefs.mount unit, which mounts rpc_pipefs in /var/lib/nfs/rpc_pipefs. The tricky thing with systemd units is that the name of the unit needs to correspond to the mount point. Keep that in mind.

The rpc_pipefs mount point is a configurable option in /etc/nfs.conf. The compile-time default is /var/lib/nfs/rpc_pipefs, but debian/ubuntu change that to /run/rpc_pipefs in the shipped configuration file. And users could decide to change it again to something else. In that case, the var-lib-nfs-rpc_pipefs.mount unit would be mounting it in the wrong place, somewhere nobody else is looking for it.

Therefore upstream introduced the rpc-pipefs-generator generator. When it runs, it checks the /etc/nfs.conf file to see if the rpc_pipefs mount point was changed from the default value. If it's set at default, it exits silently, and eventually var-lib-nfs-rpc_pipefs.mount will mount it in the expected place. But if the mount point was chenged in /etc/nfs.conf, then the generator will produce a new mount unit, called run-rpc_pipefs.mount (remember the name has to match the mount point), and a corrresponding new rpc_pipefs.target target. Since these units will be in /run/systemd/generator, they take precedence over the ones shipped in /lib/systemd/system/, and the rpc_pipefs mount point will be correctly done in /run/rpc_pipefs, matching the configuration in /etc/nfs.conf.

Due to this bug here, however, that's not always happening, and the resoning is described in https://bugs.launchpad.net/ubuntu/+source/nfs-utils/+bug/1971935/comments/22. In debian it's not as bad, because of the two mount points, one is correct (lucky perhaps? Don't know why debian got 2 mount points). But in Ubuntu it's bad because we only get one mount point, and in the wrong place, there the other nfs daemons won't look for it, and fail.

The fix I'm applying in this branch is to not ship the default mount and target units for rpc_pipefs, and just always rely on the ones produced by the generator. It's not yet a fool proof system, because the user could still change the mountpoint in /etc/nfs.conf, but the generator will only update things on a reboot, or if systemctl daemon-reload is called. But hopefully someone who knows what rpc-pipefs is used for, and changes it, will know the actual mount also has to be redone.

I sent this upstream to the mailing list[1], got a tentative response. My debian salsa PR[2] has no comments yet.

A person affected by this bug commented[3] that the change fixed the problem for them.

I figured let's land this in kinetic and see if there is any ill effect, while we wait for upstream or debian to comment. At some point, though, we will have to decide to SRU this or not.

# Quick test case

On a kinetic *VM*:
sudo apt udpate && sudo apt install autofs
mount -t rpc_pipefs

If you get /var/lib/nfs/rpc_pipefs in the mount output, instead of /run/rpc_pipefs, that's the bug.

1. https://marc.info/?l=linux-nfs&m=165754514206399&w=4
2. https://salsa.debian.org/kernel-team/nfs-utils/-/merge_requests/18
3. https://bugs.launchpad.net/ubuntu/+source/nfs-utils/+bug/1971935/comments/24

[Bryce Harrington (bryce)]

I've read through all the discussions on bugs and mailing lists. I don't quite understand the "Debian gets two mounts; Ubuntu gets one" behavior/situation, but with that posited the rest of the analysis does seem to follow.

stirling: ~$ multipass launch daily:kinetic
Launched: meteoric-mudfish
stirling: ~$ multipass exec meteoric-mudfish -- /bin/bash
To run a command as administrator (user "root"), use "sudo <command>".
See "man sudo_root" for details.
ubuntu@meteoric-mudfish:~$ sudo -s

## Vanilla VM:
root@meteoric-mudfish:/home/ubuntu# mount -t rpc_pipefs
sunrpc on /var/lib/nfs/rpc_pipefs type rpc_pipefs (rw,relatime)

## With PPA installed:
root@meteoric-mudfish:/home/ubuntu# sudo add-apt-repository ppa:ahasenack/nfs-utils-generator
...
root@meteoric-mudfish:/home/ubuntu# apt-get update; apt-get full-upgrade
...
root@meteoric-mudfish:/home/ubuntu# umount /var/lib/nfs/rpc_pipefs
root@meteoric-mudfish:/home/ubuntu# mount -t rpc_pipefs
root@meteoric-mudfish:/home/ubuntu# mount | grep pipefs
root@meteoric-mudfish:/home/ubuntu# ls /run
NetworkManager dmeventd-client needrestart snapd.socket
agetty.reload dmeventd-server netns sshd
autofs-running fsck network sshd.pid
autofs.pid initctl rpcbind sudo
blkid initramfs rpcbind.lock systemd
cloud-init lock rpcbind.sock tmpfiles.d
console-setup log screen udev
credentials lvm sendsigs.omit.d udisks2
crond.pid machine-id shm user
crond.reboot motd.dynamic sm-notify.pid utmp
cryptsetup mount snapd uuidd
dbus multipathd.pid snapd-snap.socket
root@meteoric-mudfish:/home/ubuntu# reboot
...

Broadcast message from root@meteoric-mudfish on pts/1 (Fri 2022-07-22 16:14:14 PDT):

The system is going down for reboot NOW!

root@meteoric-mudfish:/home/ubuntu# stirling: ~$ multipass exec meteoric-mudfish -- /bin/bash
                                                                                                                                                                                                                     ubuntu@meteoric-mudfish:~$ mount -t rpc_pipefs
sunrpc on /run/rpc_pipefs type rpc_pipefs (rw,relatime)
ubuntu@meteoric-mudfish:~$

ubuntu@meteoric-mudfish:~$ ls /run
NetworkManager dmeventd-client network sshd
agetty.reload dmeventd-server rpc_pipefs sshd.pid
autofs-running fsck rpcbind sudo
autofs.pid initctl rpcbind.lock systemd
blkid initramfs rpcbind.sock tmpfiles.d
cloud-init lock screen udev
console-setup log sendsigs.omit.d udisks2
credentials lvm shm user
crond.pid motd.dynamic sm-notify.pid utmp
crond.reboot mount snapd uuidd
cryptsetup multipathd.pid snapd-snap.socket
dbus netns snapd.socket

I agree with your assessment to just get it into kinetic for testing for now. Ideally, would be good...

review: Approve

[git-ubuntu bot (git-ubuntu-bot)]

Approvers: ahasenack, bryce
Uploaders: ahasenack, bryce
MP auto-approved

review: Approve

[Andreas Hasenack (ahasenack)]

I was waiting a bit more on upstream, and debian, to comment, but that didn't happen, and the longer we wait, the longer this remains unused by the community at large, so I uploaded it now to kinetic:

Uploading nfs-utils_2.6.1-2ubuntu2.dsc
Uploading nfs-utils_2.6.1-2ubuntu2.debian.tar.xz
Uploading nfs-utils_2.6.1-2ubuntu2_source.buildinfo
Uploading nfs-utils_2.6.1-2ubuntu2_source.changes

diff --git a/debian/changelog b/debian/changelog
index 9abfe3e..d008716 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,13 @@
+nfs-utils (1:2.6.1-2ubuntu2) kinetic; urgency=medium
+
+  * Rely on the generator units for the rpc_pipefs mount
+    (LP: #1971935):
+    - d/p/always-run-generator.patch: run the generator even if the
+      config differs from the built-in default
+    - d/rules: exclude the units we will let the generator produce
+
+ -- Andreas Hasenack <[e-mail]>  Thu, 07 Jul 2022 21:08:11 +0000
+
 nfs-utils (1:2.6.1-2ubuntu1) kinetic; urgency=medium
 
   * Merge with Debian unstable (LP: #1974233). Remaining changes:
diff --git a/debian/patches/always-run-generator.patch b/debian/patches/always-run-generator.patch
new file mode 100644
index 0000000..ba420c5
--- /dev/null
+++ b/debian/patches/always-run-generator.patch
@@ -0,0 +1,23 @@
+Description: Always run the generator
+ Run the generator even if the pipefs-directory setting is the default one.
+Author: Andreas Hasenack <[e-mail]>
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1971935
+Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1014429
+Forwarded: https://marc.info/?l=linux-nfs&m=165754514206399&w=4
+Last-Updated: 2022-07-12
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+diff --git a/systemd/rpc-pipefs-generator.c b/systemd/rpc-pipefs-generator.c
+index c24db567..7c42431f 100644
+--- a/systemd/rpc-pipefs-generator.c
++++ b/systemd/rpc-pipefs-generator.c
+@@ -139,9 +139,6 @@ int main(int argc, char *argv[])
+ 	s = conf_get_str("general", "pipefs-directory");
+ 	if (!s)
+ 		exit(0);
+-	if (strlen(s) == strlen(RPC_PIPEFS_DEFAULT) &&
+-			strcmp(s, RPC_PIPEFS_DEFAULT) == 0)
+-		exit(0);
+ 
+ 	if (is_non_pipefs_mountpoint(s))
+ 		exit(1);
diff --git a/debian/patches/series b/debian/patches/series
index 155ed0e..e3c5a5e 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -9,3 +9,4 @@ svcgssd-fix-use-after-free.patch
 svcgssd-display-principal-if-set.patch
 svcgssd-document-missing-options.patch
 nfs-conf-manpage-missing-svcgssd-options.patch
+always-run-generator.patch
diff --git a/debian/rules b/debian/rules
index 50c34f2..32e4ca2 100755
--- a/debian/rules
+++ b/debian/rules
@@ -28,6 +28,10 @@ override_dh_fixperms:
 	dh_fixperms
 	chmod u+s debian/nfs-common/sbin/mount.nfs
 
+override_dh_install:
+	# we will let the generator produce these units, see LP: #1971935
+	dh_install -Xvar-lib-nfs-rpc_pipefs.mount -Xrpc_pipefs.target
+
 override_dh_installinit:
 	dh_installinit -pnfs-common -R
 	install -m 0755 debian/nfs-kernel-server.init debian/nfs-kernel-server/etc/init.d/nfs-kernel-server