> On Apr 15, 2021, at 11:33 AM, Steve Dickson <SteveD@xxxxxxxxxx> 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@xxxxxxxxxx> 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