On Wed, 07 Sep 2022, Chuck Lever III wrote:
>
> > On Sep 6, 2022, at 7:11 PM, NeilBrown <neilb@xxxxxxx> wrote:
> >
> > On Tue, 06 Sep 2022, J. Bruce Fields wrote:
> >> On Mon, Sep 05, 2022 at 04:29:16PM +0000, Chuck Lever III wrote:
> >>> Bruce reminded me I'm not the only one seeing this failure
> >>> these days:
> >>>
> >>>> nfs/001 4s ... - output mismatch (see /root/xfstests-dev/results//nfs/001.out.bad)
> >>>> --- tests/nfs/001.out 2019-12-20 17:34:10.569343364 -0500
> >>>> +++ /root/xfstests-dev/results//nfs/001.out.bad 2022-09-04 20:01:35.502462323 -0400
> >>>> @@ -1,2 +1,2 @@
> >>>> QA output created by 001
> >>>> -203
> >>>> +3
> >>>> ...
> >>>
> >>> I'm looking at about 5 other priority bugs at the moment. Can
> >>> someone else do a little triage?
> >>
> >> For what it's worth, a bisect lands on
> >> c0cbe70742f4a70893cd6e5f6b10b6e89b6db95b "NFSD: add posix ACLs to struct
> >> nfsd_attrs".
> >>
> >> Haven't really looked at nfs/001 except to note it does have something
> >> to do with ACLs, so that checks out....
> >
> > I see the problem.
> > acl setting was moved to nfsd_setattr().
> > But nfsd_setattr() contains the lines
> >
> > if (!iap->ia_valid)
> > return 0;
> >
> > In the setacl case, ia_valid is 0.
> >
> > Maybe something like:
> >
> > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > index 528afc3be7af..0582a5b16237 100644
> > --- a/fs/nfsd/vfs.c
> > +++ b/fs/nfsd/vfs.c
> > @@ -395,9 +395,6 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
> > if (S_ISLNK(inode->i_mode))
> > iap->ia_valid &= ~ATTR_MODE;
> >
> > - if (!iap->ia_valid)
> > - return 0;
> > -
> > nfsd_sanitize_attrs(inode, iap);
> >
> > if (check_guard && guardtime != inode->i_ctime.tv_sec)
> > @@ -448,8 +445,10 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
> > goto out_unlock;
> > }
> >
> > - iap->ia_valid |= ATTR_CTIME;
> > - host_err = notify_change(&init_user_ns, dentry, iap, NULL);
> > + if (iap->ia_valid) {
> > + iap->ia_valid |= ATTR_CTIME;
> > + host_err = notify_change(&init_user_ns, dentry, iap, NULL);
> > + }
>
> Unless I'm missing something, host_err might not be initialized
> in the !iap->ia_valid case.
Yes, of course. Thanks.
I'll post a proper patch which I have now tested.
Thanks,
NeilBrown
