On Thu, Jul 13, 2023 at 5:16 PM Benjamin Coddington <bcodding@xxxxxxxxxx> wrote:
>
> On 13 Jul 2023, at 17:08, Benjamin Coddington wrote:
>
> > On 13 Jul 2023, at 15:54, Olga Kornievskaia wrote:
> >
> >> From: Olga Kornievskaia <kolga@xxxxxxxxxx>
> >>
> >> Currently, if the OPEN compound experiencing an error and needs to
> >> get the file attributes separately, it will send a stand alone
> >> GETATTR but it would use the filehandle from the results of
> >> the OPEN compound. In case of the CLAIM_FH OPEN, nfs_openres's fh
> >> is zero value. That generate a GETATTR that's sent with a zero
> >> value filehandle, and results in the server returning an error.
> >>
> >> Instead, for the CLAIM_FH OPEN, take the filehandle that was used
> >> in the PUTFH of the OPEN compound.
> >>
> >> Signed-off-by: Olga Kornievskaia <kolga@xxxxxxxxxx>
> >> ---
> >> fs/nfs/nfs4proc.c | 6 +++++-
> >> 1 file changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> >> index 8edc610dc1d3..0b1b49f01c5b 100644
> >> --- a/fs/nfs/nfs4proc.c
> >> +++ b/fs/nfs/nfs4proc.c
> >> @@ -2703,8 +2703,12 @@ static int _nfs4_proc_open(struct nfs4_opendata *data,
> >> return status;
> >> }
> >> if (!(o_res->f_attr->valid & NFS_ATTR_FATTR)) {
> >> + struct nfs_fh *fh = &o_res->fh;
> >> +
> >> nfs4_sequence_free_slot(&o_res->seq_res);
> >> - nfs4_proc_getattr(server, &o_res->fh, o_res->f_attr, NULL);
> >> + if (o_arg->claim == NFS4_OPEN_CLAIM_FH)
> >> + fh = NFS_FH(d_inode(data->dentry));
> >> + nfs4_proc_getattr(server, fh, o_res->f_attr, NULL);
> >> }
> >> return 0;
> >> }
> >
> > Looks good, but why not just use o_arg->fh? Maybe also an opportunity
> > to fix the whitespace damage a few lines before this hunk too.
> >
> > Reviewed-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
>
> .. and should we handle the other OPEN_CLAIM cases that use the filehandle
> as well?
I was pondering about that but I think the other OPEN CLAIM that use
the FH directly are recovery opens. I'm not sure if they need this.
>
> Ben
>
