Hi-
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?
--
Chuck Lever
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....
--b.
On Mon, Sep 5, 2022 at 12:33 PM Chuck Lever III <[email protected]> wrote:
>
> Hi-
>
> 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?
>
Is there any more info about this (like exported file system or
something or a network trace)? I typically hit this issue when I get a
client and it doesn't have nfs4_getfacl command installed.
>
> --
> Chuck Lever
>
>
>
> On Sep 6, 2022, at 11:30 AM, Olga Kornievskaia <[email protected]> wrote:
>
> On Mon, Sep 5, 2022 at 12:33 PM Chuck Lever III <[email protected]> wrote:
>>
>> Hi-
>>
>> 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?
>>
>
> Is there any more info about this (like exported file system or
> something or a network trace)?
I see this failure on all filesystem types I test.
> I typically hit this issue when I get a
> client and it doesn't have nfs4_getfacl command installed.
I just checked, my client has nfs4_getfacl installed.
--
Chuck Lever
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);
+ }
out_unlock:
if (attr->na_seclabel && attr->na_seclabel->len)
Does that seem reasonable?
Thanks,
NeilBrown
> On Sep 6, 2022, at 7:11 PM, NeilBrown <[email protected]> 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
On Wed, 07 Sep 2022, Chuck Lever III wrote:
>
> > On Sep 6, 2022, at 7:11 PM, NeilBrown <[email protected]> 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