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? Ben