MIME-Version: 1.0
References: <20221109133306.167037-1-dwysocha@redhat.com> <CC98B38C-2E79-40CE-BD9B-6E336BBD6552@oracle.com>
 <CALF+zO=u2p3BuH1nGt7DMekbRiJZGUuZZRF-D7pGULw5VoMo_A@mail.gmail.com> <2817011C-299D-46B4-A8E2-D5EDA9FE1D08@oracle.com>
In-Reply-To: <2817011C-299D-46B4-A8E2-D5EDA9FE1D08@oracle.com>
From:   David Wysochanski <dwysocha@redhat.com>
Date:   Mon, 23 Jan 2023 11:07:51 -0500
Message-ID: <CALF+zOmKL8mDX8HDdVEwijbdHDmeXyKwCC2H2wtE=WrjOYJe3w@mail.gmail.com>
Subject: Re: [PATCH] Documentation: Fix sysfs path for the NFSv4 client identifier
To:     Chuck Lever III <chuck.lever@oracle.com>
Cc:     Anna Schumaker <anna.schumaker@netapp.com>,
        Benjamin Coddington <bcodding@redhat.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Jan 23, 2023 at 9:48 AM Chuck Lever III <chuck.lever@oracle.com> wrote:
>
> Hi DW -
>
> > On Jan 23, 2023, at 8:52 AM, David Wysochanski <dwysocha@redhat.com> wrote:
> >
> > On Wed, Nov 9, 2022 at 8:58 AM Chuck Lever III <chuck.lever@oracle.com> wrote:
> >>
> >>
> >>
> >>> On Nov 9, 2022, at 8:33 AM, Dave Wysochanski <dwysocha@redhat.com> wrote:
> >>>
> >>> The sysfs path for the NFS4 client identfier should start with
> >>> the path component of 'nfs' for the kset, and then the 'net'
> >>> path component for the netns object, followed by the
> >>> 'nfs_client' path component for the NFS client kobject,
> >>> and ending with 'identifier' for the netns_client_id
> >>> kobj_attribute.
> >>
> >> Seems like the redundancy has some purpose and is baked-in to
> >> the path. I'd rather leave the sleeping dog as it is, enjoying
> >> the morning sun.
> >>
> >> Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
> >>
> > Hey Chuck,
> >
> > I just realized the patch is still outstanding.  Now when I re-read
> > your comment, I'm not sure what it means.  Your comment sounds
> > like you feel the patch may not be necessary, but then you have
> > a "Reviewed-by".
>
> I was responding to Ben's suggestion elsewhere that the pathname
> had redundant components and should be simplified. After reviewing
> the code (and your patch) it appears that all the components are
> necessary to have, so the document change you proposed is
> appropriate.
>
Ah ok yes I remember now, thanks for explaining.

>
> > I am not sure if you mean this should be applied or not.
>
> Reviewed-by: means "OK to apply". The documentation is incorrect.
>
Ok.

>
> > To be clear, the existing sysfs path does not exist, and we got a
> > complaint about this documentation inaccuracy, hence my patch.
>
> Can you dig up the complaint bug and suggest a Link: tag to add?
>
It was such a small issue I was not planning to create a separate
bug, but I can do so easily if you would like that.

The comment was a private comment that came later on in this
bug, which you commented on a while back when there was discussion
on how to generate a unique id.
https://bugzilla.redhat.com/show_bug.cgi?id=1801326#c45
The bug was repurposed to documentation only.


>
> >>> Fixes: a28faaddb2be ("Documentation: Add an explanation of NFSv4 client identifiers")
> >>> Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
> >>> ---
> >>> Documentation/filesystems/nfs/client-identifier.rst | 4 ++--
> >>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/filesystems/nfs/client-identifier.rst b/Documentation/filesystems/nfs/client-identifier.rst
> >>> index 5147e15815a1..a94c7a9748d7 100644
> >>> --- a/Documentation/filesystems/nfs/client-identifier.rst
> >>> +++ b/Documentation/filesystems/nfs/client-identifier.rst
> >>> @@ -152,7 +152,7 @@ string:
> >>>      via the kernel command line, or when the "nfs" module is
> >>>      loaded.
> >>>
> >>> -    /sys/fs/nfs/client/net/identifier
> >>> +    /sys/fs/nfs/net/nfs_client/identifier
> >>>      This virtual file, available since Linux 5.3, is local to the
> >>>      network namespace in which it is accessed and so can provide
> >>>      distinction between network namespaces (containers) when the
> >>> @@ -164,7 +164,7 @@ then that uniquifier can be used. For example, a uniquifier might
> >>> be formed at boot using the container's internal identifier:
> >>>
> >>>    sha256sum /etc/machine-id | awk '{print $1}' \\
> >>> -        > /sys/fs/nfs/client/net/identifier
> >>> +        > /sys/fs/nfs/net/nfs_client/identifier
> >>>
> >>> Security considerations
> >>> -----------------------
> >>> --
> >>> 2.31.1
> >>>
> >>
> >> --
> >> Chuck Lever
>
> --
> Chuck Lever
>
>
>