Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Benjamin Coddington <bcodding@redhat.com>
CC:     Neil Brown <neilb@suse.de>, Steve Dickson <SteveD@RedHat.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client
 uniquifiers
Thread-Topic: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client
 uniquifiers
Thread-Index: AQHYHqhMlT21uFtPcU6l7DrJSxeLtayNFxwAgAFGrYCAACKIAIAALKMAgAAKL4CAAAdIAIAACC0AgAAEqYCAAAnVAIAABAgAgANWcoCAALuNgIAASZiAgANdMwCAAAl0AA==
Date:   Wed, 16 Feb 2022 19:35:22 +0000
Message-ID: <9FC005FB-370E-4AFA-AD80-8599CBFCC1E0@oracle.com>
References: <cover.1644515977.git.bcodding@redhat.com>
 <9c046648bfd9c8260ec7bd37e0a93f7821e0842f.1644515977.git.bcodding@redhat.com>
 <7642FA55-F3F2-4813-86E2-1B65185E6B36@oracle.com>
 <3d2992df-7ef7-50ba-4f11-f4de588620d2@redhat.com>
 <DDB59BD9-8C29-45C3-ABAF-B25EDDB63E09@oracle.com>
 <D0908E76-C163-4DBF-A93C-665492EB9DB2@redhat.com>
 <E2C56D5B-AC77-48D1-9AF6-268406648657@oracle.com>
 <4657F9AE-3B9E-4992-9334-3FF1CF18EF31@redhat.com>
 <C7533D80-25B3-4722-94A9-0440C48B8574@oracle.com>
 <945849B4-BE30-434C-88E9-8E901AAFA638@redhat.com>
 <06B01290-E375-455E-A6D7-419CA653A0D1@oracle.com>
 <948D8123-E310-4A35-BF04-C030F20EA83C@redhat.com>
 <164479707170.27779.15384523062754338136@noble.neil.brown.name>
 <863AB69A-D5D6-4F22-950C-E5F468CD4552@redhat.com>
 <42AAFEDD-F4EE-4A91-BD23-E08B1149EF1C@oracle.com>
 <3AF29DC6-2EEB-4C3E-BD6C-BE31910921AE@redhat.com>
In-Reply-To: <3AF29DC6-2EEB-4C3E-BD6C-BE31910921AE@redhat.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?824vpKQJBOAJu1Va/rV6I6vX1R8PVaD882K1vJusY1Miou8pBst4IPz/7afA?=
 =?us-ascii?Q?NTw3Mfn4UawhG1vPrc3KTPSyVo0LyG7i6YvGx0PoxlsewEJK8GYusL5kRp73?=
 =?us-ascii?Q?XzSyBFxivZ/mkFPziZtt97XYC48x3toMD+fG8iwKhwaMm59qX3StTKDZL8RL?=
 =?us-ascii?Q?RquyieOnqz/gkdnF6c8M8t8hWnbsUZQ3twhOWRGoUht9eQaOa5Gk0FHv+S0A?=
 =?us-ascii?Q?dXpxcdbfWnatj/+CB8Plp8nhd5rCRJnQKLgaPHnpS3XXQwImhdqFgb6E7CrA?=
 =?us-ascii?Q?RPM0764mAKJsvBqLuzCZ79SWxSBmq1yy4AMchXWFYh14ngtrFdtgdCcFMRZB?=
 =?us-ascii?Q?3WIR8nhnjxYvxQQTcl3CMsKhYuBQOHvmKRkgobYBbThQsQkjIcI5u8H5A2vS?=
 =?us-ascii?Q?PqAlw6D1NX8o8PzwqHqGzw8qcWOUYuzBn0+Ukn/S84T9wrlcfI3NnJBmXZXm?=
 =?us-ascii?Q?LPtq/r3FeSQeLuF39nZVdz+cKCfPm+U61Q0CJXmXldZfNQGrwdf7RT0/c2vJ?=
 =?us-ascii?Q?LgzyVvUtjjjmrYYp7CW968CRUHtKfD/UPiwmJnD9dFCpRZvwpTI0+LZL25eE?=
 =?us-ascii?Q?E84mRyEVQFTqR4Xa4UVBQ8l0IUxyYilu/uIz5nrqYo1VvnN1nj8JlF/GJTCQ?=
 =?us-ascii?Q?9JoqwmqqQOYfmG8nTxF7IAGBgIddDd1Q/Nrv4p20/Bc3WBfv+AN1Ks8tCBPy?=
 =?us-ascii?Q?+QuaHTF7lNYyS8tQsVGchm6waJeZuwKfKMYlnTqI37WWi4yudMUj1qa9rYvi?=
 =?us-ascii?Q?MdCQ6y2eWCtWlQyaFGmcUvMW6yAPBpmaZGBx9hQJqvRYDd8hSCvW+4rof36b?=
 =?us-ascii?Q?PzYVWd+8AMFilmbN+AO+8OFRbp5bWg3QI8uqsj2bDJmoXkYO7+57RmJ6dxnO?=
 =?us-ascii?Q?m4z94VZOI/ZwkfoJ0KTO/2x4XS4Xpx0mdfMxNWCEai2T2oOVqR6cp/V1g+Ve?=
 =?us-ascii?Q?oDySMS278aCfgWYuyMMuqflYU5qxSrC4kVZiafg3VdoRfo7IbQrcuSrmHN5F?=
 =?us-ascii?Q?cZ403IMS+sKOvO7/If0iwVE2txkXPHfiRQMNkqPyW42MSnI3gnJmRzsV92gb?=
 =?us-ascii?Q?Iy/thmmxejzDH/yL1FxOC9noNPktMprVr97LhAlW4HWGdnRmXGZwXMZdLpHb?=
 =?us-ascii?Q?Uq9f9fezMyMOyGHk6x0YysmXeGtcCavJrVbEFRR29RVtXEQh2rRTLK8VRMqm?=
 =?us-ascii?Q?N6/s6DZXoGrc51m0ccUfjuXwCJF9nYQM6P3DVTU2JYX5VtmdUy23aPC4g2MU?=
 =?us-ascii?Q?j9GXQ54Y6FrOL0n5LopjcVqoARTHBlUTVhd9FIaJQqh9Bx8GiCT+P4Bd6BWi?=
 =?us-ascii?Q?WUWkV3wnESCaI0c35MgPT7FHAAMPUyuOtCK+cChcAaJgR0CKWR3SyWeVcqsK?=
 =?us-ascii?Q?tsRLlyqdfm3P7+vhz2wBwZ0OPuVrewvpXB4cqr93/V4uqQq/5qywh3OhWYAb?=
 =?us-ascii?Q?J5sZe0u/46w2xlg8sWovUZNA8JpCrXRSmLHBSbeMFla/KDfmeJ2geY24jECQ?=
 =?us-ascii?Q?WZjUyyzs/6iv7eGgw2RnEY70Ei22JuOxx0xBUBYO+ZkE7UjMSnNFjW0SM++u?=
 =?us-ascii?Q?KZC9PIa40EkzDIz/9LxTheaYzo735LNDnLFPcaShoOPxOeBHNfckmmKGBVu+?=
 =?us-ascii?Q?0W8rRVOi8jfHmBbK7i3fjtA=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <D22506E481517642988C6630D2469DE4@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1b6387e7-f872-42a6-5863-08d9f1837958
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Feb 2022 19:35:22.7385
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Y/FJJ0Tv3L48Y32pYdVg+B4Z+vZV+gVZN04Pl4v0XU2sRoKvmYqTnDF+KakdVKFpIkV5KgEHjWtOxrR8ZDe3RA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR10MB3614
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10260 signatures=675971
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 spamscore=0
 phishscore=0 bulkscore=0 suspectscore=0 malwarescore=0 mlxscore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2201110000 definitions=main-2202160109
X-Proofpoint-GUID: SkZQGL4PCLI8rp7qt7Z6gxm7KNfUgvY6
X-Proofpoint-ORIG-GUID: SkZQGL4PCLI8rp7qt7Z6gxm7KNfUgvY6
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Feb 16, 2022, at 2:01 PM, Benjamin Coddington <bcodding@redhat.com> wr=
ote:
>=20
> On 14 Feb 2022, at 10:39, Chuck Lever III wrote:
>=20
>>> On Feb 14, 2022, at 6:15 AM, Benjamin Coddington <bcodding@redhat.com> =
wrote:
>>>=20
>>> On 13 Feb 2022, at 19:04, NeilBrown wrote:
>>>=20
>>>> On Sat, 12 Feb 2022, Benjamin Coddington wrote:
>>>>> On 11 Feb 2022, at 15:51, Chuck Lever III wrote:
>>>>>=20
>>>>>>> On Feb 11, 2022, at 3:16 PM, Benjamin Coddington
>>>>>>> <bcodding@redhat.com> wrote:
>>>>>>>=20
>>>>>>> On 11 Feb 2022, at 15:00, Chuck Lever III wrote:
>>>>>>>=20
>>>>>>>>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington
>>>>>>>>> <bcodding@redhat.com> wrote:
>>>>>>>>>=20
>>>>>>>>> All the arguments for exacting tolerances on how it should be nam=
ed
>>>>>>>>> apply
>>>>>>>>> equally well to anything that implies its output will be used for
>>>>>>>>> nfs client
>>>>>>>>> ids, or host ids.
>>>>>>>>=20
>>>>>>>> I completely disagree with this assessment.
>>>>>>>=20
>>>>>>> But how, and in what way?  The tool just generates uuids, and spits
>>>>>>> them
>>>>>>> out, or it spits out whatever's in the file you specify, up to 64
>>>>>>> chars.  If
>>>>>>> we can't have uuid in the name, how can we have NFS or machine-id o=
r
>>>>>>> host-id?
>>>>>>=20
>>>>>> We don't have a tool called "string" to get and set the DNS name of
>>>>>> the local host. It's called "hostname".
>>>>>>=20
>>>>>> The purpose of the proposed tool is to persist a unique string to be
>>>>>> used as part of an NFS client ID. I would like to name the tool base=
d
>>>>>> on that purpose, not based on the way the content of the persistent
>>>>>> file happens to be arranged some of the time.
>>>>>>=20
>>>>>> When the tool generates the string, it just happens to be a UUID. It
>>>>>> doesn't have to be. The tool could generate a digest of the boot tim=
e
>>>>>> or the current time. In fact, one of those is usually part of certai=
n
>>>>>> types of a UUID anyway. The fact that it is a UUID is totally not
>>>>>> relevant. We happen to use a UUID because it has certain global
>>>>>> uniqueness properties. (By the way, perhaps the man page could menti=
on
>>>>>> that global uniqueness is important for this identifier. Anything wi=
th
>>>>>> similar guaranteed global uniqueness could be used).
>>>>>>=20
>>>>>> You keep admitting that the tool can output something that isn't a
>>>>>> UUID. Doesn't that make my argument for me: that the tool doesn't
>>>>>> generate a UUID, it manages a persistent host identifier. Just like
>>>>>> "hostname." Therefore "nfshostid". I really don't see how nfshostid
>>>>>> is just as miserable as nfsuuid -- hence, I completely disagree
>>>>>> that "all arguments ... apply equally well".
>>>>>=20
>>>>> Yes - your arguement is a good one.   I wasn't clear enough admitting
>>>>> you
>>>>> were right two emails ago, sorry about that.
>>>>>=20
>>>>> However, I still feel the same argument applied to "nfshostid"
>>>>> disqualifies
>>>>> it as well.  It doesn't output the nfshostid.  That, if it even conta=
ins
>>>>> the
>>>>> part outputted, is more than what's written out.
>>>>>=20
>>>>> In my experience with linux tools, nfshostid sounds like something I =
can
>>>>> use
>>>>> to set or retrieve the identifier for an NFS host, and this little to=
ol
>>>>> does
>>>>> not do that.
>>>>>=20
>>>>=20
>>>> I agree.  This tool primarily does 1 thing - it sets a string which wi=
ll
>>>> be the uniquifier using the the client_owner4.  So I think the word
>>>> "set" should appear in the name.  I also think the name should start "=
nfs".
>>>> I don't much care whether it is
>>>> nfssetid
>>>> nfs-set-uuid
>>>> nfssetowner
>>>> nfssetuniquifier
>>>> nfssetidentity
>>>> nfsidset
>>>> though perhaps I'd prefer
>>>> nfs=3Dset=3Did
>>>>=20
>>>> If not given any args, it should probably print a usage message rather
>>>> than perform a default action, to reduce the number of holes in feet.
>>>>=20
>>>> .... Naming  - THE hard problem of computer engineering ....
>>>=20
>>> No, it does NOT set the uniquifier string.  It returns it on stdout.  I=
f
>>> you run it without args it just prints the string.  Its completely harm=
less.
>>=20
>> OK, my understanding was that if you run it as root, and the
>> string isn't already set, it does indeed set a value in the
>> persistence file.
>>=20
>> That should be harmless, though. Once it is set, it is always
>> the same afterwards, and that's fine. Someone who just types
>> in the command to see what it does can't damage their
>> metatarsals; the worst that happens is the persistence file
>> is never used again.
>>=20
>> I agree that's not very "set"-like.
>>=20
>> nfsgetuniquifier
>> nfsguestuniquifier
>> nfsnsuniquifier
>> ns-uniquifier
>>=20
>> Use with copious amounts of tab completion.
>=20
> 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?


> Considering this, I think your only remaining objection to "nfsuuid" is t=
hat it
> might return data other than a uuid if someone points it at a file that
> contains data other than a uuid.
>=20
> I can fix that.  And its probably not a bad idea either.  The nfsuuid too=
l
> 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.


> Maybe I also need to change the man page or description of the patch to b=
e
> 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.

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

What's not to like?


--
Chuck Lever