Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
From:   "NeilBrown" <neilb@suse.de>
To:     "Christoph Hellwig" <hch@lst.de>
Cc:     "J.  Bruce Fields" <bfields@fieldses.org>,
        "Chuck Lever" <chuck.lever@oracle.com>,
        "Christoph Hellwig" <hch@lst.de>, linux-nfs@vger.kernel.org
Subject: Re: [PATCH] NFSD: drop support for ancient file-handles
In-reply-to: <20210827151505.GA19199@lst.de>
References: <162995209561.7591.4202079352301963089@noble.neil.brown.name>,
 <20210827151505.GA19199@lst.de>
Date:   Sat, 28 Aug 2021 09:24:12 +1000
Message-id: <163010665269.7591.11571286577204380127@noble.neil.brown.name>
Precedence: bulk

On Sat, 28 Aug 2021, Christoph Hellwig wrote:
> On Thu, Aug 26, 2021 at 02:28:15PM +1000, NeilBrown wrote:
> > This patch also moves the nfsfh.h from the include/uapi directory into
> > fs/nfsd.  I can find no evidence of it being used anywhere outside the
> > kernel.  Certainly nfs-utils and wireshark do not use it.
> 
> That sounds fine, but I'd split this into a separate patch.

Thanks for your review.  Your suggestions seem appropriate.
I'll send out revised patches in a couple of days.

NeilBrown


> 
> > fh_base and fh_pad are occasionally used to refer to the whole
> > filehandle.  These are replaced with "fh_raw" which is hopefully more
> > meaningful.
> 
> I think that kind of cleanup should also be a separate patch.  That
> being said as far as I can tell fh_raw is only ever used in context
> where we can just pass a void pointer.  So just giving the struct
> for the "new" file handle after fh_size a name and passing that
> would be much cleaner than a union with a char array.
> 
> 
> > I found
> >  https://www.spinics.net/lists/linux-nfs/msg43280.html
> >  "Re: [PATCH] nfsd: clean up fh_auth usage"
> > from 2014 where moving nfsfh.h out of uapi was considered but not
> > actioned. Christoph said he would "do some research if the
> > uapi <linux/nfsd/*.h> headers are used anywhere at all".  I can find no
> > report on the result of that research.  My own research turned up
> > nothing.
> 
> I can't remember doing much of research, and certainly not of finding
> anything.
> 
> > -	memcpy((char*)&fh.fh_handle.fh_base, f->data, f->size);
> > +	memcpy((char*)&fh.fh_handle.fh_raw, f->data, f->size);
> 
> Indedpendnt on what we're going to pass here, I don't think we
> should need cast like this one (there are a few more).
> 
>