> 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.
> out_unlock:
> if (attr->na_seclabel && attr->na_seclabel->len)
>
>
> Does that seem reasonable?
>
> Thanks,
> NeilBrown
--
Chuck Lever
