Re: [nfs-utils PATCH v2 4/4] systemd: add a generator for the rpc_pipefs mountpoint

[NeilBrown <neilb@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 3424 bytes --]

On Thu, Apr 06 2017, Scott Mayhew wrote:

> On Thu, 06 Apr 2017, NeilBrown wrote:
>
>> On Wed, Apr 05 2017, Scott Mayhew wrote:
>> 
>> > The nfs.conf has config options for the rpc_pipefs mountpoint.
>> > Currently, changing these from the default also requires manually
>> > overriding the systemd unit files that are hard-coded to mount the
>> > filesystem on /var/lib/nfs/rpc_pipefs.
>> >
>> > This patch adds a generator that creates a mount unit file for the
>> > rpc_pipefs when a non-default value is specified in /etc/nfs.conf, as
>> > well as a target unit file to override the dependencies for the systemd
>> > units using the rpc_pipefs.  The blkmapd, idmapd, and gssd service unit
>> > files have been modified to define their dependencies on the rpc_pipefs
>> > mountpoint indirectly via the rpc_pipefs target unit file.
>> >
>> > This patch also removes the dependency on the rpc_pipefs from the
>> > rpc-svcgssd.service unit file.  rpc.svcgssd uses the sunrpc cache
>> > mechanism to exchange data with the kernel, not the rpc_pipefs.
>> >
>> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
>> 
>> I was reading through this, which I really thought would be a good patch
>> that I would give an Ack to, and looking at some of the detail involved,
>> I started to wonder if we were really doing the right thing here.
>> You go to some trouble to make the name of the .mount unit file match
>> the name of the location of the mountpoint.  Is that really necessary?
>> 
>> Unit files created by systemd-fstab-generator do follow that pattern,
>> but they don't have to.
>
>> 
>> Maybe we should just:
>> 
>> 1/ rename var-lib-nfs-rpc_pipefs.mount to rpc_pipefs.mount,
>>    and modify the dependencies accordingly.
>> 
> The mount doesn't work when I change the name.
>
> [root@fedora25 ~]# systemctl status rpc_pipefs.mount
> ● rpc_pipefs.mount - RPC Pipe File System
>    Loaded: error (Reason: Invalid argument)
>    Active: inactive (dead)
>     Where: /var/lib/nfs/rpc_pipefs
>      What: sunrpc
>
> Apr 06 08:17:59 localhost.localdomain systemd[1]: rpc_pipefs.mount: Where= setting doesn't match unit name. Refusing.
>
>
> Looking back at systemd.mount(5), it looks like they do have to follow
> that pattern:
>
>        Mount units must be named after the mount point directories they control.
>        Example: the mount point /home/lennart must be configured in a unit file
>        home-lennart.mount. For details about the escaping logic used to convert a
>        file system path to a unit name, see systemd.unit(5). Note that mount units
>        cannot be templated, nor is possible to add multiple names to a mount unit
>        by creating additional symlinks to it.
>

My turn not to read to the end of the man page I see...

That's a strange restriction that seem inconsistent with everything else
systemd does.  Maybe it it helpful for implementing "RequiresMountsFor"
but I cannot think of any other justification.
Anyway, I guess we have to live with it.

I bothered me that you needed to include this:
>> > +	fprintf(f, "DefaultDependencies=no\n");
>> > +	fprintf(f, "After=systemd-tmpfiles-setup.service\n");
>> > +	fprintf(f, "Conflicts=umount.target\n");

in the code, but I can't see anything we can do about that.

So:
 Reviewed-by: NeilBrown <neilb@suse.com>

for this patch.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]