All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Trond Myklebust <trondmy@hammerspace.com>
To: "ajmitchell@redhat.com" <ajmitchell@redhat.com>,
	"SteveD@RedHat.com" <SteveD@RedHat.com>,
	"chuck.lever@oracle.com" <chuck.lever@oracle.com>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 0/3] Enable the setting of a kernel module parameter from nfs.conf
Date: Thu, 15 Apr 2021 23:30:15 +0000	[thread overview]
Message-ID: <bf7a7563989477a30490e3982665f90bdcfa1016.camel@hammerspace.com> (raw)
In-Reply-To: <366FA143-AB3E-4320-8329-7EA247ADB22B@oracle.com>

On Thu, 2021-04-15 at 16:37 +0000, Chuck Lever III wrote:
> 
> 
> > 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.
> 

Right. The only sensible solution in a containerised world is a udev
script that sets /sys/fs/nfs/net/nfs_client/identifier when triggered.

Note that we really want something that generates a random uuid, and
then persists it so that it can be retrieved on reboot or restart of
the container. Something similar to systemd-machine-id-setup, but that
can be called from udev.

> 
> > 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
> 
> 
> 

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



  reply	other threads:[~2021-04-15 23:30 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-14 18:10 [PATCH 0/3] Enable the setting of a kernel module parameter from nfs.conf Steve Dickson
2021-04-14 18:10 ` [PATCH 1/3] nfs-utils: Enable the retrieval of raw config settings without expansion Steve Dickson
2021-05-06 17:29   ` Steve Dickson
2021-04-14 18:10 ` [PATCH 2/3] nfs-utils: Add support for further ${variable} expansions in nfs.conf Steve Dickson
2021-04-14 18:10 ` [PATCH 3/3] nfs-utils: Update nfs4_unique_id module parameter from the nfs.conf value Steve Dickson
2021-04-14 23:26 ` [PATCH 0/3] Enable the setting of a kernel module parameter from nfs.conf Chuck Lever III
2021-04-15 15:33   ` Steve Dickson
2021-04-15 16:37     ` Chuck Lever III
2021-04-15 23:30       ` Trond Myklebust [this message]
2021-04-16  0:40         ` Trond Myklebust
2021-04-17 16:33           ` Steve Dickson
2021-04-17 18:09             ` Trond Myklebust
2021-04-17 16:18       ` Steve Dickson
2021-04-17 16:36         ` Chuck Lever III
2021-04-17 17:50           ` Steve Dickson
2021-04-18 16:51             ` Chuck Lever III
2021-04-20 13:11               ` Steve Dickson
2021-04-20 14:09                 ` Chuck Lever III
2021-04-20 14:31                   ` Trond Myklebust
2021-04-20 17:18                     ` J. Bruce Fields
2021-04-20 17:28                       ` Trond Myklebust
2021-04-20 17:40                         ` bfields
2021-04-20 17:53                           ` Trond Myklebust
2021-04-20 18:16                             ` bfields
2021-04-20 19:30                               ` Steve Dickson
2021-04-20 18:47                     ` Steve Dickson
2021-04-20 18:26                   ` Steve Dickson
2021-05-13  0:29     ` NeilBrown
2021-05-18 12:38       ` Steve Dickson
2021-05-21  2:39         ` NeilBrown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bf7a7563989477a30490e3982665f90bdcfa1016.camel@hammerspace.com \
    --to=trondmy@hammerspace.com \
    --cc=SteveD@RedHat.com \
    --cc=ajmitchell@redhat.com \
    --cc=chuck.lever@oracle.com \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.