All of lore.kernel.org
 help / color / mirror / Atom feed
From: "NeilBrown" <neilb@suse.de>
To: "Benjamin Coddington" <bcodding@redhat.com>
Cc: "Trond Myklebust" <trondmy@hammerspace.com>,
	steved@redhat.com, linux-nfs@vger.kernel.org
Subject: Re: [nfs-utils PATCH] nfs4id: a tool to create and persist nfs4 client uniquifiers
Date: Mon, 14 Feb 2022 10:24:18 +1100	[thread overview]
Message-ID: <164479465866.27779.3680126986096452561@noble.neil.brown.name> (raw)
In-Reply-To: <299337F3-E83F-49EC-BB24-C9B859C9FB6D@redhat.com>

On Sat, 12 Feb 2022, Benjamin Coddington wrote:
> On 10 Feb 2022, at 17:54, NeilBrown wrote:
> 
> > On Thu, 10 Feb 2022, Benjamin Coddington wrote:
> >>
> >> Yes, but even better than having the tool do the writing is to have 
> >> udev do
> >> it, because udev makes the problem of when and who will execute this 
> >> tool go
> >> away, and the entire process is configurable for anyone that needs to 
> >> change
> >> any part of it or use their own methods of generating/storing ids.
> >
> > I really don't understand the focus on udev.
> >
> > Something, somewhere, deliberately creates the new network namespace.
> > It then deliberately configures that namespace - creating a virtual
> > device maybe, adding an IP address, setting a default route or 
> > whatever.
> > None of that is done by udev rules (is it)?
> > Setting the NFS identity is just another part of configuring the new
> > network namespace.
> >
> > udev is great when we don't know exactly when an event will happen, 
> > but
> > we want to respond when it does.
> > That doesn't match the case of creating a new network namespace.  Some
> > code deliberately creates it and is perfectly positioned to then
> > configure it.
> 
> I think there's so many ways to create a new network namespace that we 
> can't
> reasonably be expected to try to insert out problem into all of them.

I 100% agree.  Similarly there are lots of init systems and we don't try
to provide configuration for each one to ensure - e.g.  - that sm-notify
is run at the correct time.
But we *do* provide configuration for one - systemd.  This is partly
because it is widely used, but largely because the distro that I
personally help maintain uses it.  So I added those systemd/* files.
(and then others helped improve them).

> Handling the event from the kernel allows us to make a best-effort 
> default attempt.

That "best" effort is not actually very good.

Might I suggest that we take a similar approach to the systemd config.
You choose whatever container system is important to you are the moment,
determine how best to integrate the required support with that system,
make sure the tool works well for that case, provide any config files
that might be generally useful for anyone using that container system,
and in the documentation for the tool explain generally when it must run
and why, and give an example for the system that you know.

If someone else uses a different container system, they are free to
create a solution based on your example, and welcome to submit patches -
either to nfs-utils or to that container system.

Obviously you make the tool reasonably general without over-engineering,
but don't try to solve problems that you don't even know really exist.
I would suggest the tool be passed one of:
 - a unique string 
 - a file containing a unique string
 - a file in which a randomly generated unique string may be stored if
   it doesn't already contain one
 - (maybe) nothing, in which case a new random identity is generate each
   time.  This could be useful for older kernels.

> 
> > udev is async.  How certain can we be that the udev event will be 
> > fully
> > handled before the first mount attempt?
> 
> Good point.  We can't at all be certain.
> 
> We can start over completely from here..
> 
> We can have mount.nfs /also/ try to configure the id.. this is more 
> robust.

If mount.nfs is going to do it, then the "also" is pointless.  There
would be no point in any other code doing it.  However I'm no longer as
keen on mount.nfs as I was.

> 
> We can have mount.nfs do a round of udev settle..

My experience with "udev settle" with md does not make me want to depend
on it.  It has it's place and does useful things, but not always quite
what I want.

> 
> Are there other options?

Document the requirement and make it "someone elses problem".
Your kernel patch to provide a random default mean a lack of
configuration will only hurt the container which lacks it - which nicely
localises the problem.

Thanks,
NeilBrown

  reply	other threads:[~2022-02-13 23:24 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-04 12:56 [nfs-utils PATCH] nfs4id: a tool to create and persist nfs4 client uniquifiers Benjamin Coddington
2022-02-04 15:17 ` Chuck Lever III
2022-02-04 15:49   ` Benjamin Coddington
2022-02-04 18:45     ` Chuck Lever III
2022-02-04 19:44       ` Benjamin Coddington
2022-02-05 17:35         ` Chuck Lever III
2022-02-08  3:14       ` NeilBrown
2022-02-08 11:43         ` Benjamin Coddington
2022-02-08 16:04 ` Steve Dickson
2022-02-08 16:21   ` Chuck Lever III
2022-02-08 19:29     ` Steve Dickson
2022-02-08 21:18       ` Chuck Lever III
2022-02-08 22:39         ` Steve Dickson
2022-02-10 13:28           ` Benjamin Coddington
2022-02-10 15:21             ` Chuck Lever III
2022-02-10 15:47               ` Benjamin Coddington
2022-02-10 16:25                 ` Chuck Lever III
2022-02-10 16:41                   ` Benjamin Coddington
2022-02-10 17:11             ` Steve Dickson
2022-02-08 16:22   ` Benjamin Coddington
2022-02-08 19:52     ` Steve Dickson
2022-02-08 20:00       ` Benjamin Coddington
2022-02-08 22:30         ` Steve Dickson
2022-02-09 13:55           ` Benjamin Coddington
2022-02-09 15:23             ` Steve Dickson
2022-02-09 21:21       ` NeilBrown
2022-02-09 21:45         ` Trond Myklebust
2022-02-09 23:58           ` NeilBrown
2022-02-10 12:25             ` Benjamin Coddington
2022-02-10 22:54               ` NeilBrown
2022-02-11 13:35                 ` Benjamin Coddington
2022-02-13 23:24                   ` NeilBrown [this message]
2022-02-14 11:34                     ` Benjamin Coddington

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=164479465866.27779.3680126986096452561@noble.neil.brown.name \
    --to=neilb@suse.de \
    --cc=bcodding@redhat.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=steved@redhat.com \
    --cc=trondmy@hammerspace.com \
    /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.