Re: [PATCH 0/3] Enable the setting of a kernel module parameter from nfs.conf

Chuck Lever III
2021-04-15T16:37:18Z

> On Apr 15, 2021, at 11:33 AM, Steve Dickson <SteveD@RedHat.com> wrote:
>
> Hey Chuck!
>
> On 4/14/21 7:26 PM, Chuck Lever III wrote:
>> Hi Steve-
>>
>>> On Apr 14, 2021, at 2:10 PM, Steve Dickson <SteveD@redhat.com> wrote:
>>>
>>> ﻿This is a tweak of the patch set Alice Mitchell posted last July [1].
>>
>> That approach was dropped last July because it is not container-aware.
>> It should be simple for someone to write a udev script that uses the
>> existing sysfs API that can update nfs4_client_id in a namespace. I
>> would prefer the sysfs/udev approach for setting nfs4_client_id,
>> since it is container-aware and makes this setting completely
>> automatic (zero touch).
> As I said in in my cover letter, I see this more as introduction of
> a mechanism more than a way to set the unique id.

Yep, I got that.

I'm not addressing the question of whether adding a
mechanism to set a module parameter in nfs.conf is good
or not. I'm saying nfs4_client_id is not an appropriate
parameter to add to nfs.conf. Can you pick another
module parameter as an example for your mechanism?

> The mechanism being
> a way to set kernel module params from nfs.conf. The setting of
> the id is just a side effect...
>
> Why spread out the NFS configuration? Why not
> just keep it in one place... aka nfs.conf?

We need to understand whether a module parameter is not
going to work in nfs.conf because that setting needs to
be namespace-aware. In this case, this setting does indeed
need to be namespace-aware. nfs.conf is not aware of
network namespaces.

> Plus we could document all the kernel params in nfs.conf
> and the nfs.conf man page. The only documentation I know
> of is in the kernel tree.

OK, but that's not relevant to whether nfs.conf is the
right place to set nfs4_client_id.

> As far as not being container-aware... that might true
> but it does not mean its not useful to set the id from
> nfs.conf...

Yes, it does mean that in that case. It's completely
broken to use the same nfs4_client_id in every network
namespace.

> Actual I have customers asking for this type
> of functionality

Ask yourself why they might want it. It's probably because
we don't set it correctly currently. If we have a way to
automatically get it right every time, there's really no
need for this setting to be exposed.

I do agree that it's long past time we should be setting
nfs4_client_id properly. I would rather see a udev script
developed (you, me, or Alice could do it in an afternoon)
first. If that doesn't meet the actual customer need, then
we can revisit.

> steved.
>>
>>
>>> It enables the setting of the nfs4_unique_id kernel module
>>> parameter from /etc/nfs.conf.
>>
>>> Things I tweaked:
>>>
>>> * Introduce a new [kernel] section in nfs.conf which only
>>> contains the nfs4_unique_id setting... For now...
>>>
>>> * nfs4_unique_id can be set to two different values
>>>
>>> - nfs4_unique_id = ${machine-id} will use /etc/machine-id
>>> as the unique id.
>>> - nfs4_unique_id = ${hostname} will use the system's hostname
>>> as the unique id.
>>>
>>> * The new nfs-config systemd service need to be enabled for the
>>> /etc/modprobe.d/nfs.conf file to be created with
>>> the "options nfs nfs4_unique_id=" set.
>>>
>>> I see this patch set is not a way to set the nfs4_unique_id
>>> module parameter... I see it as a beginning of a way to set
>>> all module parameters from /etc/nfs.conf, which I think
>>> is a good thing...
>>>
>>> [1] https://www.spinics.net/lists/linux-nfs/msg78658.html
>>>
>>> Alice Mitchell (3):
>>> nfs-utils: Enable the retrieval of raw config settings without
>>> expansion
>>> nfs-utils: Add support for further ${variable} expansions in nfs.conf
>>> nfs-utils: Update nfs4_unique_id module parameter from the nfs.conf
>>> value
>>>
>>> configure.ac | 1 +
>>> nfs.conf | 4 +-
>>> support/include/conffile.h | 1 +
>>> support/nfs/conffile.c | 283 ++++++++++++++++++++++++++++++++--
>>> systemd/Makefile.am | 3 +
>>> systemd/nfs-client.target | 3 +
>>> systemd/nfs-conf-export.sh | 28 ++++
>>> systemd/nfs-config.service.in | 18 +++
>>> systemd/nfs.conf.man | 19 ++-
>>> tools/nfsconf/nfsconf.man | 10 +-
>>> tools/nfsconf/nfsconfcli.c | 22 ++-
>>> 11 files changed, 372 insertions(+), 20 deletions(-)
>>> create mode 100755 systemd/nfs-conf-export.sh
>>> create mode 100644 systemd/nfs-config.service.in
>>>
>>> --
>>> 2.30.2
>>>
>

--
Chuck Lever