[PATCH 0/2] Prefer generator to static systemd units

[PATCH 0/2] Prefer generator to static systemd units

[Andreas Hasenack]

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

-- 
2.39.2

Re: [PATCH 0/2] Prefer generator to static systemd units

[Salvatore Bonaccorso]

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

Re: [PATCH 0/2] Prefer generator to static systemd units

[Salvatore Bonaccorso]

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

Re: [PATCH 0/2] Prefer generator to static systemd units

[NeilBrown]

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