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

[Chuck Lever III <chuck.lever@oracle.com>]

> 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