| Message ID | CANYNYEEy2vf2rxLFeQ0hkstPrvF=eeA-joc0imGZt96Q+_r44w@mail.gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Prefer generator to static systemd units | expand |
Hi Steve, Neil, On Fri, Jul 28, 2023 at 01:06:49PM -0300, Andreas Hasenack wrote: > Hi, > > in Debian and Ubuntu, the configuration file /etc/nfs.conf is only > placed on disk in the postinst script[1]. In this scenario it's possible > to have the nfs-common generators run before /etc/nfs.conf exists[2], > via another package's postinst calling systemctl daemon-reload. Since > there is no /etc/nfs.conf yet, defaults are assumed and the generators > exit silently, and the corresponding static units are used. > > But in Debian/Ubuntu, the rpc_pipefs directory is /run/rpc_pipefs, and > not the one specified in the static units, and thus we get it mounted in > the wrong directory. > > It seems best to always rely on the generators, as they will always be > able to produce the correct target and mount units. > > For reference, this was first brought up in this thread[3]. > > Producing an upstream set of patches was a bit confusing, since these > systemd units are highly distro dependent. They are not even installed > via `make install` because of this, so I have more confidence in the > first patch of the series. > > I produced a Debian package with these two patches applied on top of > Debian's 2.6.3[6], and ran the DEP8 tests of nfs-utils[4] and autofs[5], > which exercise some simple v3 and v4 mounts, with and without kerberos. > These tests passed[7][8] (ephemeral links, will be gone once the PPA is > destroyed). > > 1. https://git.launchpad.net/ubuntu/+source/nfs-utils/tree/debian/nfs-common.postinst?h=applied/ubuntu/devel#n6 > 2. https://bugs.launchpad.net/ubuntu/+source/nfs-utils/+bug/1971935/comments/22 > 3. https://marc.info/?l=linux-nfs&m=165729895515639&w=4 > 4. https://git.launchpad.net/ubuntu/+source/nfs-utils/tree/debian/tests?h=applied/ubuntu/lunar-devel > 5. https://git.launchpad.net/ubuntu/+source/autofs/tree/debian/tests?h=applied/ubuntu/lunar-devel > 6. https://code.launchpad.net/~ahasenack/ubuntu/+source/nfs-utils/+git/nfs-utils/+ref/upstream-nfs-utils-test > 7. https://autopkgtest.ubuntu.com/results/autopkgtest-mantic-ahasenack-nfs-upstream-test/mantic/amd64/a/autofs/20230728_135149_0895b@/log.gz > 8. https://autopkgtest.ubuntu.com/results/autopkgtest-mantic-ahasenack-nfs-upstream-test/mantic/amd64/n/nfs-utils/20230728_150122_3ef18@/log.gz > > Andreas Hasenack (2): > Always run the rpc_pipefs generator > Use the generated units instead of static ones > > configure.ac | 8 +------- > systemd/Makefile.am | 5 ----- > systemd/rpc-pipefs-generator.c | 3 --- > systemd/rpc_pipefs.target | 3 --- > systemd/rpc_pipefs.target.in | 3 --- > systemd/var-lib-nfs-rpc_pipefs.mount | 10 ---------- > systemd/var-lib-nfs-rpc_pipefs.mount.in | 10 ---------- > 7 files changed, 1 insertion(+), 41 deletions(-) > delete mode 100644 systemd/rpc_pipefs.target > delete mode 100644 systemd/rpc_pipefs.target.in > delete mode 100644 systemd/var-lib-nfs-rpc_pipefs.mount > delete mode 100644 systemd/var-lib-nfs-rpc_pipefs.mount.in Is this patch series as prposed by Andreas acceptable upstream? We have this change in Debian since the 1:2.6.3-1 upload, https://tracker.debian.org/news/1442835/accepted-nfs-utils-1263-1-source-into-unstable/, with no regression reported TTBOMK. For reference, the patch series is here in the linux-nfs archives (referencing it here explicitly as b4 mbox seems not to get all the 3 mails when requesting the cover letter): https://lore.kernel.org/linux-nfs/CANYNYEEy2vf2rxLFeQ0hkstPrvF=eeA-joc0imGZt96Q+_r44w@mail.gmail.com/ https://lore.kernel.org/linux-nfs/CANYNYEFKtw+_Y-NrOoQt9G9eund2C0=XMrXBj8mt1L=ebrSkLQ@mail.gmail.com/ https://lore.kernel.org/linux-nfs/CANYNYEHETbcqmEhE7BB57bCH03J-XT986Bb+DucdpbV8KHeZug@mail.gmail.com/ Regards, Salvatore
Hi Steve, Neil, On Tue, Sep 05, 2023 at 05:09:33PM +0200, Salvatore Bonaccorso wrote: > Hi Steve, Neil, > > On Fri, Jul 28, 2023 at 01:06:49PM -0300, Andreas Hasenack wrote: > > Hi, > > > > in Debian and Ubuntu, the configuration file /etc/nfs.conf is only > > placed on disk in the postinst script[1]. In this scenario it's possible > > to have the nfs-common generators run before /etc/nfs.conf exists[2], > > via another package's postinst calling systemctl daemon-reload. Since > > there is no /etc/nfs.conf yet, defaults are assumed and the generators > > exit silently, and the corresponding static units are used. > > > > But in Debian/Ubuntu, the rpc_pipefs directory is /run/rpc_pipefs, and > > not the one specified in the static units, and thus we get it mounted in > > the wrong directory. > > > > It seems best to always rely on the generators, as they will always be > > able to produce the correct target and mount units. > > > > For reference, this was first brought up in this thread[3]. > > > > Producing an upstream set of patches was a bit confusing, since these > > systemd units are highly distro dependent. They are not even installed > > via `make install` because of this, so I have more confidence in the > > first patch of the series. > > > > I produced a Debian package with these two patches applied on top of > > Debian's 2.6.3[6], and ran the DEP8 tests of nfs-utils[4] and autofs[5], > > which exercise some simple v3 and v4 mounts, with and without kerberos. > > These tests passed[7][8] (ephemeral links, will be gone once the PPA is > > destroyed). > > > > 1. https://git.launchpad.net/ubuntu/+source/nfs-utils/tree/debian/nfs-common.postinst?h=applied/ubuntu/devel#n6 > > 2. https://bugs.launchpad.net/ubuntu/+source/nfs-utils/+bug/1971935/comments/22 > > 3. https://marc.info/?l=linux-nfs&m=165729895515639&w=4 > > 4. https://git.launchpad.net/ubuntu/+source/nfs-utils/tree/debian/tests?h=applied/ubuntu/lunar-devel > > 5. https://git.launchpad.net/ubuntu/+source/autofs/tree/debian/tests?h=applied/ubuntu/lunar-devel > > 6. https://code.launchpad.net/~ahasenack/ubuntu/+source/nfs-utils/+git/nfs-utils/+ref/upstream-nfs-utils-test > > 7. https://autopkgtest.ubuntu.com/results/autopkgtest-mantic-ahasenack-nfs-upstream-test/mantic/amd64/a/autofs/20230728_135149_0895b@/log.gz > > 8. https://autopkgtest.ubuntu.com/results/autopkgtest-mantic-ahasenack-nfs-upstream-test/mantic/amd64/n/nfs-utils/20230728_150122_3ef18@/log.gz > > > > Andreas Hasenack (2): > > Always run the rpc_pipefs generator > > Use the generated units instead of static ones > > > > configure.ac | 8 +------- > > systemd/Makefile.am | 5 ----- > > systemd/rpc-pipefs-generator.c | 3 --- > > systemd/rpc_pipefs.target | 3 --- > > systemd/rpc_pipefs.target.in | 3 --- > > systemd/var-lib-nfs-rpc_pipefs.mount | 10 ---------- > > systemd/var-lib-nfs-rpc_pipefs.mount.in | 10 ---------- > > 7 files changed, 1 insertion(+), 41 deletions(-) > > delete mode 100644 systemd/rpc_pipefs.target > > delete mode 100644 systemd/rpc_pipefs.target.in > > delete mode 100644 systemd/var-lib-nfs-rpc_pipefs.mount > > delete mode 100644 systemd/var-lib-nfs-rpc_pipefs.mount.in > > Is this patch series as prposed by Andreas acceptable upstream? > > We have this change in Debian since the 1:2.6.3-1 upload, > https://tracker.debian.org/news/1442835/accepted-nfs-utils-1263-1-source-into-unstable/, > with no regression reported TTBOMK. > > For reference, the patch series is here in the linux-nfs archives > (referencing it here explicitly as b4 mbox seems not to get all the 3 > mails when requesting the cover letter): > https://lore.kernel.org/linux-nfs/CANYNYEEy2vf2rxLFeQ0hkstPrvF=eeA-joc0imGZt96Q+_r44w@mail.gmail.com/ > https://lore.kernel.org/linux-nfs/CANYNYEFKtw+_Y-NrOoQt9G9eund2C0=XMrXBj8mt1L=ebrSkLQ@mail.gmail.com/ > https://lore.kernel.org/linux-nfs/CANYNYEHETbcqmEhE7BB57bCH03J-XT986Bb+DucdpbV8KHeZug@mail.gmail.com/ Anything we can do here, to have this upstreamed? Or is there something missing to make it possible? Regards, Salvatore
On Wed, 22 Nov 2023, Salvatore Bonaccorso wrote: > Hi Steve, Neil, > > On Tue, Sep 05, 2023 at 05:09:33PM +0200, Salvatore Bonaccorso wrote: > > Hi Steve, Neil, > > > > On Fri, Jul 28, 2023 at 01:06:49PM -0300, Andreas Hasenack wrote: > > > Hi, > > > > > > in Debian and Ubuntu, the configuration file /etc/nfs.conf is only > > > placed on disk in the postinst script[1]. In this scenario it's possible > > > to have the nfs-common generators run before /etc/nfs.conf exists[2], > > > via another package's postinst calling systemctl daemon-reload. Since > > > there is no /etc/nfs.conf yet, defaults are assumed and the generators > > > exit silently, and the corresponding static units are used. > > > > > > But in Debian/Ubuntu, the rpc_pipefs directory is /run/rpc_pipefs, and > > > not the one specified in the static units, and thus we get it mounted in > > > the wrong directory. > > > > > > It seems best to always rely on the generators, as they will always be > > > able to produce the correct target and mount units. > > > > > > For reference, this was first brought up in this thread[3]. > > > > > > Producing an upstream set of patches was a bit confusing, since these > > > systemd units are highly distro dependent. They are not even installed > > > via `make install` because of this, so I have more confidence in the > > > first patch of the series. > > > > > > I produced a Debian package with these two patches applied on top of > > > Debian's 2.6.3[6], and ran the DEP8 tests of nfs-utils[4] and autofs[5], > > > which exercise some simple v3 and v4 mounts, with and without kerberos. > > > These tests passed[7][8] (ephemeral links, will be gone once the PPA is > > > destroyed). > > > > > > 1. https://git.launchpad.net/ubuntu/+source/nfs-utils/tree/debian/nfs-common.postinst?h=applied/ubuntu/devel#n6 > > > 2. https://bugs.launchpad.net/ubuntu/+source/nfs-utils/+bug/1971935/comments/22 > > > 3. https://marc.info/?l=linux-nfs&m=165729895515639&w=4 > > > 4. https://git.launchpad.net/ubuntu/+source/nfs-utils/tree/debian/tests?h=applied/ubuntu/lunar-devel > > > 5. https://git.launchpad.net/ubuntu/+source/autofs/tree/debian/tests?h=applied/ubuntu/lunar-devel > > > 6. https://code.launchpad.net/~ahasenack/ubuntu/+source/nfs-utils/+git/nfs-utils/+ref/upstream-nfs-utils-test > > > 7. https://autopkgtest.ubuntu.com/results/autopkgtest-mantic-ahasenack-nfs-upstream-test/mantic/amd64/a/autofs/20230728_135149_0895b@/log.gz > > > 8. https://autopkgtest.ubuntu.com/results/autopkgtest-mantic-ahasenack-nfs-upstream-test/mantic/amd64/n/nfs-utils/20230728_150122_3ef18@/log.gz > > > > > > Andreas Hasenack (2): > > > Always run the rpc_pipefs generator > > > Use the generated units instead of static ones > > > > > > configure.ac | 8 +------- > > > systemd/Makefile.am | 5 ----- > > > systemd/rpc-pipefs-generator.c | 3 --- > > > systemd/rpc_pipefs.target | 3 --- > > > systemd/rpc_pipefs.target.in | 3 --- > > > systemd/var-lib-nfs-rpc_pipefs.mount | 10 ---------- > > > systemd/var-lib-nfs-rpc_pipefs.mount.in | 10 ---------- > > > 7 files changed, 1 insertion(+), 41 deletions(-) > > > delete mode 100644 systemd/rpc_pipefs.target > > > delete mode 100644 systemd/rpc_pipefs.target.in > > > delete mode 100644 systemd/var-lib-nfs-rpc_pipefs.mount > > > delete mode 100644 systemd/var-lib-nfs-rpc_pipefs.mount.in > > > > Is this patch series as prposed by Andreas acceptable upstream? > > > > We have this change in Debian since the 1:2.6.3-1 upload, > > https://tracker.debian.org/news/1442835/accepted-nfs-utils-1263-1-source-into-unstable/, > > with no regression reported TTBOMK. > > > > For reference, the patch series is here in the linux-nfs archives > > (referencing it here explicitly as b4 mbox seems not to get all the 3 > > mails when requesting the cover letter): > > https://lore.kernel.org/linux-nfs/CANYNYEEy2vf2rxLFeQ0hkstPrvF=eeA-joc0imGZt96Q+_r44w@mail.gmail.com/ > > https://lore.kernel.org/linux-nfs/CANYNYEFKtw+_Y-NrOoQt9G9eund2C0=XMrXBj8mt1L=ebrSkLQ@mail.gmail.com/ > > https://lore.kernel.org/linux-nfs/CANYNYEHETbcqmEhE7BB57bCH03J-XT986Bb+DucdpbV8KHeZug@mail.gmail.com/ > > Anything we can do here, to have this upstreamed? > > Or is there something missing to make it possible? > Hi, thanks for being persistent.... Allowing generators to run before /etc/nfs.conf exists seems like a bug... Maybe that situation could be improved a bit by having the package install some config directly into e.g. /etc/nfs.conf.d/rpc_pipe.conf Also I cannot see how we can "always rely on the generators" if they might be run before /etc/nfs.conf is created. However I agree that the current idiosyncratic behaviour, where sometimes the generator is and sometimes it isn't, is not good and bound to cause confusion. So I like the patches and think they should be applied. I would probably combine them both into one as they are working in the same direction and having one without the other seems even less consistent. Reviewed-by: NeilBrown <neilb@suse.de> Thanks, NeilBrown