On 16 Feb 2022, at 14:35, Chuck Lever III wrote:
On Feb 16, 2022, at 2:01 PM, Benjamin Coddington <bcodding@xxxxxxxxxx>
wrote:
Coming back to this.. so it seems at least part of our disagreement
about
the name had to do with a misunderstanding of what the tool did.
I understand what your implementation does. I don't
understand why it works that way.
The disagreement is that I feel like you're trying to
abstract the operation of this little tool in ways that
no-one asked for. It's purpose is very narrow and
completely related to the NFSv4 client ID.
Just to finally clear the air about it: the tool does not write to
sysfs, it
doesn't try to modify the client in any way. I'm going to leave it
up to
the distros.
Seems to me that the distros care only about where the
persistence file is located. I can't see a problem with
Neil's suggestion that the tool also write into sysfs.
The location of the sysfs API is invariant; there should
be no need to configure it or expose it.
Can you give a reason why the tool should /not/ write
into sysfs?
So that there is a division of work. Systemd-udevd handles the event
and
sets the attribute, not the tool. Systemd-udevd is already set up to
have
the appropriate selinux contexts and rights to make these changes.
Systemd-udevd's logging can clearly show the action and result instead
of it being hidden away inside this tool. Systemd-udevd doesn't expect
programs to make the changes, its designed around the idea that programs
provide information and it makes the changes.
You can see how many issues we got into by imagining what would happen
if
users ran this tool. If the tool doesn't modify the client, that's one
less
worry we have upstream, its the distro's problem.
If the tool writes to sysfs, then someone executing it manually can
change
the client's id with a live mount. That's bad. That's less likely to
happen if its all tucked away in a udev rule. I know the
counter-arguement
"you can write to sysfs yourself", but I still think its better this
way.
There's increased flexibility and utility - if an expert sysadmin wants
to
distinguish groups of clients, they can customize their udev rule to add
custom strings to the uniquifier using the many additional points of
data
available in udev rules.
Considering this, I think your only remaining objection to "nfsuuid"
is that it
might return data other than a uuid if someone points it at a file
that
contains data other than a uuid.
I can fix that. And its probably not a bad idea either. The nfsuuid
tool
can ensure that the persisted data is a uuid.
Why is that necessary? I agree that any random string will
do, as long as it is the same after client reboots and is
globally unique. It can be a BLAKE2 hash or anything else.
Is there a hard requirement that the uniquifier is in the
form of an RFC 4122 UUID? (if there is, the requirement
should be explained in the man page).
I have no problem with using a UUID. I just don't believe
there's a hard requirement that it /must/ be a UUID.
There's no hard requirement, as you know. Its just an extremely useful
datatype for this purpose. I'm sure we could make a `nfsblake2`, but I
can't imagine why.
Maybe I also need to change the man page or description of the patch
to be
clearer about what the tool does. Any suggestions there?
I've made some in previous responses. The rules about
reboot persistence and global uniqueness are paramount.
Ok, I can re-read the threads and find them and include them.
So I initially agreed with Trond's statement that the
client ID uniquifier is not specific to a particular
mount request, so doesn't belong in mount.nfs.
All still true. But...
If mount.nfs handles it instead, you don't need a
separate tool (naming problem solved), there's no lag
between when the uniquifier is set and the first
SETCLIENTID (race condition solved), no udev rule is
needed (no additional implementation complexity), and
no man page and no new module parameters (no
additional administrative configuration complexity).
Yep. The race is a problem.
What's not to like?
Not much. How can we figure out what we want to do as a community
before
doing it all the ways?
Ben
