Received-SPF: softfail (google.com: domain of transitioning linux-nfs-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <778c3e11-62e1-00b1-0cbc-514abe1d1eac@redhat.com>
Date:   Tue, 8 Feb 2022 17:30:17 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [nfs-utils PATCH] nfs4id: a tool to create and persist nfs4
 client uniquifiers
Content-Language: en-US
To:     Benjamin Coddington <bcodding@redhat.com>
Cc:     linux-nfs@vger.kernel.org
References: <c2e8b7c06352d3cad3454de096024fff80e638af.1643979161.git.bcodding@redhat.com>
 <6f01c382-8da5-5673-30db-0c0099d820b5@redhat.com>
 <0AB20C82-6200-46E0-A76C-62345DAF8A3A@redhat.com>
 <6cfb516d-0747-a749-b310-1368a2186307@redhat.com>
 <E013276C-7605-4E2B-A650-C61C6FC5BADF@redhat.com>
From:   Steve Dickson <steved@redhat.com>
In-Reply-To: <E013276C-7605-4E2B-A650-C61C6FC5BADF@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hey!

On 2/8/22 3:00 PM, Benjamin Coddington wrote:
> On 8 Feb 2022, at 14:52, Steve Dickson wrote:
> 
>> On 2/8/22 11:22 AM, Benjamin Coddington wrote:
>>> On 8 Feb 2022, at 11:04, Steve Dickson wrote:
>>>
>>>> Hello,
>>>>
>>>> On 2/4/22 7:56 AM, Benjamin Coddington wrote:
>>>>> The nfs4id program will either create a new UUID from a random 
>>>>> source or
>>>>> derive it from /etc/machine-id, else it returns a UUID that has 
>>>>> already
>>>>> been written to /etc/nfs4-id.  This small, lightweight tool is 
>>>>> suitable for
>>>>> execution by systemd-udev in rules to populate the nfs4 client 
>>>>> uniquifier.
>>>>>
>>>>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
>>>>> ---
>>>>>   .gitignore               |   1 +
>>>>>   configure.ac             |   4 +
>>>>>   tools/Makefile.am        |   1 +
>>>>>   tools/nfs4id/Makefile.am |   8 ++
>>>>>   tools/nfs4id/nfs4id.c    | 184 
>>>>> +++++++++++++++++++++++++++++++++++++++
>>>>>   tools/nfs4id/nfs4id.man  |  29 ++++++
>>>>>   6 files changed, 227 insertions(+)
>>>>>   create mode 100644 tools/nfs4id/Makefile.am
>>>>>   create mode 100644 tools/nfs4id/nfs4id.c
>>>>>   create mode 100644 tools/nfs4id/nfs4id.man
>>>> Just a nit... naming convention... In the past
>>>> we never put the protocol version in the name.
>>>> Do a ls tools and utils directory and you
>>>> see what I mean....
>>>>
>>>> Would it be a problem to change the name from
>>>> nfs4id to nfsid?
>>>
>>> Not at all..
>> Good...
> 
> I didn't orginally do that because I thought it might be confusing for some
> folks who want `nfsid` to display their kerberos identity.  There's a BZ
> open for that!
> 
> Do you think that's a problem?  I feel like it's a problem.
> 
>>> and I think there's a lot of room for naming discussions about
>>> the file to store the id too:
>>>
>>> Trond sent /etc/nfs4_uuid
>>> Neil suggests /etc/netns/NAME/nfs.conf.d/identity.conf
>>> Ben sent /etc/nfs4-id (to match /etc/machine-id)
>> Question... is it kosher to be writing /etc which is
>> generally on the root filesystem?
> 
> Sure, why not?
In general, writes to /etc are only happen when packages
are installed and removed... any real time writes go
to /var or /run (which is not persistent).

> 
>> By far Neil suggestion is the most intriguing... but
>> on the containers I'm looking at there no /etc/netns
>> directory.
> 
> Not yet -- you can create it.
"you" meaning who? the nfs-utils install or network
namespace env? Or is it, when /etc/netns exists
there is a network namespace and we should use
that dir?

> 
>> I had to install the iproute package to do the
>> "ip netns identify" which returns NULL...
>> also adds yet another dependency on nfs-utils.
> 
> We don't need the dependency..this little binary is just a helper for a 
> udev
> rule.  Trond already wrote his version of this.  :)  This one's just trying
> to be a little lighter (whoa, we don't need all of bash!).
Fair enough.

> 
>> So if "ip netns identify" does return NULL what directory
>> path should be used?
> 
> I think thats out of scope..  if udevd is running in a container, and has a
> rule that uses this tool, then the container either is likely to have
> already customized its /etc.  Or perhaps we can make the udev rule 
> namespace
> aware (I need to look into what's available to udev's rule environment when
> it runs in a namespace).
With all do respect... that is a lot of hand waving :-)
I'll wait to see what comes you come up with.

> 
>> I'm all for making things container friendly but I'm
>> also a fan of keeping things simple... So
>> how about /etc/nfs.conf.d/identity.conf or
>> /etc/nfs.conf.d/nfsid.conf?
> 
> Really, because its not a configuration.  Its value persistence, and we 
> /really/
> don't want people configuring it.
Good point... and I got it... I was just trying to used
the parsing code we already have in place.

> 
>>>
>>> Maybe the tool wants an option to specify the file?
>> This is probably not a bad idea... It is a common practice
> 
> OK, I'll do that.
Thanks!

steved.

> 
> Ben
>