From: NeilBrown <neilb@suse.com>
To: Scott Mayhew <smayhew@redhat.com>,
        Steve Dickson <SteveD@RedHat.com>
Date: Thu, 30 Mar 2017 17:35:48 +1100
Cc: linux-nfs@vger.kernel.org
Subject: Re: [nfs-utils PATCH] systemd: add generators for the rpc_pipefs mountpoint
In-Reply-To: <20170329180207.xkcxepra442j6vv6@tonberry.usersys.redhat.com>
References: <20170328133703.19325-1-smayhew@redhat.com> <2aace10d-1b9b-b8e8-4f85-caf31f5f51bf@RedHat.com> <20170329180207.xkcxepra442j6vv6@tonberry.usersys.redhat.com>
Message-ID: <87a8835l17.fsf@notabene.neil.brown.name>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
        micalg=pgp-sha256; protocol="application/pgp-signature"
Sender: linux-nfs-owner@vger.kernel.org

--=-=-=
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable

On Wed, Mar 29 2017, Scott Mayhew wrote:

> On Wed, 29 Mar 2017, Steve Dickson wrote:
>
>> Hello,
>>=20
>> On 03/28/2017 09:37 AM, Scott Mayhew wrote:
>> > The nfs.conf and idmapd.conf files both have config options for the
>> > pipefs mountpoint.  Currently, changing these from the defaults also
>> > requires manually overriding the systemd unit files that are hard-coded
>> > to mount the filesystem on /var/lib/nfs/rpc_pipefs.
>> The Pipefs-Directory config variable is not documented in either
>> idmapd.conf(5) or rpc.idmapd(8) or nfsidmap(5) so the only way
>> to know about it as to read the code. I would not call that
>> a supported interface and can easily be removed. IMHO.
>> The same thing goes for the Cache-Expiration variable.=20
>
> So you're saying to document the Pipefs-Directory and Cache-Expiration
> variables, not remove them... right?  I think they should be documented
> in idmapd.conf(5) since the other pages both refer to idmapd.conf(5).
>>=20
>> >=20
>> > This patch adds two generators to create drop-in config files to
>> > override the dependencies for the systemd units that require the pipef=
s.
>> > There are two because rpc.idmapd uses a separate configuration file.  =
If
>> > idmapd's configurations are ever folded into the nfs.conf, then the
>> > idmapd-rpc-pipefs-generator.c can be deleted and generate=3D1 can be
>> > specified for idmapd in rpc-pipefs-generator.c.
>> So I'm thinking we just read Pipefs-Directory out of=20
>> one spot that would be /etc/nfs.conf.
>
> I agree but then rpc.idmapd and nfsidmap should be modified to read
> /etc/nfs.conf instead of /etc/idmapd.conf by default, but that could be
> confusing unless some of the section names and/or variable names from
> /etc/idmapd.conf were changed up a bit.  That's quite a bit more than
> I'm trying to accomplish here.
>
>> That way we would only=20
>> have to support one of these generators.
>
> If your issue is that there are two generators then I can fold them into
> single one... then if the idmapd.conf ever get folded into nfs.conf it's
> simply a matter of deleting a few lines of code.  The only reason I made
> two generators is becuase it made more sense to me that way.  I'm fine
> either way.
>
>>=20
>> Also please document the Pipefs-Directory variable in both
>> the nfs.conf man page and in the example nfs.conf file
>> in the git tree.=20
>
> It's actually already documented in both, under the gssd section.
>
>>=20
>> >=20
>> > This patch also adds a unit file to mount the pipefs on /run/rpc_pipef=
s,
>> > since that is the most logical alternate location for the pipefs
>> > filesystem.  Users wanting to use some other location besides
>> > /var/lib/nfs/rpc_pipefs or /run/rpc_pipefs would have to create their
>> > own systemd mount unit file, but the generators would take care of the
>> > rest.
>> I'm not sure I understand why this new unit file, run-rpc_pipefs.mount,
>> is needed, especially since it is not being install (aka no updates
>> to the Makefile.am files).
>
> I forgot to update the Makefile.am by mistake.  I definitely want the
> new unit file.  The idea is to give users a choice between
> /var/lib/nfs/rpc_pipes and /run/rpc_pipefs without requiring them to go
> manually messing around with systemd configs.

Could the new unit file be generated by the generator???
i.e. the generator creates the foo.mount based on the nfs.conf file
contents, as well as create the dependencies between services and this
file.

??

NeilBrown

--=-=-=
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEG8Yp69OQ2HB7X0l6Oeye3VZigbkFAljcp0QACgkQOeye3VZi
gbnTkBAArNhbLsLq5qCoeEVbn8aBEWkXdi4CihNena64x0BOiXOv5R0iAS3wKhah
CfQvQsuj9lLTVTQ84YfKeQg3r/mQbK3PJqWzevHKr+EfnTiB+fa0w0ltvR2BESTr
/g7ARPBmWBqJCIZXNbpIaIqIx/4jHZ0/e5rXJLSTIv6fgZaSZxVAJ2FoQGCr+eNW
sRP9+eTm0MCsKrt3YdZrxlReD/59yZEQfs6a23vu7gSJgL3Sf6vYSH46WgfMYqFc
gbfRuIuyt/mgNffSRQH3SHV9Q6Dh5grfr7cSvAKgmmhBk8ZT+PAY4XVs2gAHxflk
g4KUM2GN0wRDDv3z4b7zZmFKLm7Sk5sdXPj5vdSHl3BfddEDwB5hicMo7B390R++
yko/zUoXQRjBZ9YwHptRr9X328eu63Yk5Yk8cOZGLOTISe81PbALA1F/xMhUm+OH
wDyB9SSrkcah8q3HIM4o0nRV2+oYMp0ifqIZfgOBVAjiHhYCOoTRNNq/R3+9czit
DRyImOmehR94yTqJ3N5+M5UkbkbGrmo9KmFUvXT1JN17h1nzdUpCxgxyDoN0mShI
rnJDhgJ06fF17yEzR4DYQ8KKtcRL6EuVJaxe3zOZi+eF+g5pcBXGqqT+wOZOxjD0
52il/XvPfqBCz6K0NXp/Lx+RMDO0abqAHE6IsTidyc0N18iEiog=
=3lxa
-----END PGP SIGNATURE-----
--=-=-=--