[PATCH] xfs: change return value check to golden image check

[PATCH] xfs: change return value check to golden image check

[Zorro Lang]

xfs/133 and xfs/138 use too much code to do "return value" check,
it's not necessary. For the code can be more readable and clear,
I change "return value" check to golden image check.

Signed-off-by: Zorro Lang <zlang@redhat.com>
---
 tests/xfs/133     | 20 +++++++-------------
 tests/xfs/133.out |  7 +++++++
 tests/xfs/138     | 26 ++++++++++++--------------
 tests/xfs/138.out | 12 ++++++++++++
 4 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/tests/xfs/133 b/tests/xfs/133
index 330221f..82c38b1 100755
--- a/tests/xfs/133
+++ b/tests/xfs/133
@@ -46,8 +46,6 @@ _cleanup()
 rm -f $seqres.full
 
 # real QA test starts here
-
-# Modify as appropriate.
 _supported_fs xfs
 _supported_os Linux
 _require_scratch
@@ -72,22 +70,18 @@ $qa_project:10
 EOF
 
 	$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \
-		-c "project -s $qa_project" $SCRATCH_MNT >>$seqres.full 2>&1
-	[ $? -ne 0 ] && _fail "Initalize project=$qa_project Failed"
+		-c "project -s $qa_project" $SCRATCH_MNT > /dev/null
 
 	$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \
-		-c "limit -p bsoft=100m bhard=200m $qa_project" $SCRATCH_MNT >>$seqres.full 2>&1
-	[ $? -ne 0 ] && _fail "Create limit for project=$qa_project Failed"
+		-c "limit -p bsoft=100m bhard=200m $qa_project" $SCRATCH_MNT
 
-	$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \
-		-c "quota -p -v $qa_project" $SCRATCH_MNT 2>>$seqres.full | grep -qw $qa_project
-	[ $? -ne 0 ] && _fail "Query project=$qa_project Failed"
+	echo "=== quota command output ==="
+	$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid \
+		-c "quota -p -v -b $qa_project" $SCRATCH_MNT | _filter_quota
 
+	echo "=== report command output ==="
 	$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \
-		-c "report -p" $SCRATCH_MNT 2>>$seqres.full | grep -qw $qa_project
-	[ $? -ne 0 ] && _fail "Report project=$qa_project Failed"
-
-	return 0
+		-c "report -p -N -b" $SCRATCH_MNT | _filter_quota
 }
 
 # Test project
diff --git a/tests/xfs/133.out b/tests/xfs/133.out
index 189cb69..21cfd0d 100644
--- a/tests/xfs/133.out
+++ b/tests/xfs/133.out
@@ -1 +1,8 @@
 QA output created by 133
+=== quota command output ===
+Disk quotas for Project 123456-project (10)
+Filesystem Blocks Quota Limit Warn/Time Mounted on
+SCRATCH_DEV 0 102400 204800 00 [--------] SCRATCH_MNT
+=== report command output ===
+123456-project 0 102400 204800 00 [--------]
+
diff --git a/tests/xfs/138 b/tests/xfs/138
index 6b17608..c6a6f50 100755
--- a/tests/xfs/138
+++ b/tests/xfs/138
@@ -47,8 +47,6 @@ _cleanup()
 rm -f $seqres.full
 
 # real QA test starts here
-
-# Modify as appropriate.
 _supported_fs xfs
 _supported_os Linux
 _require_scratch
@@ -63,20 +61,20 @@ _qmount_option "uquota,gquota"
 _qmount
 
 # user test
-xfs_quota -x -c "limit -u bsoft=100m bhard=200m 123456-fsgqa" $SCRATCH_MNT >>$seqres.full 2>&1
-[ $? -ne 0 ] && _fail "Create limit for user 123456-fsgqa Failed"
-xfs_quota -x -c "quota -u -v 123456-fsgqa" $SCRATCH_MNT 2>>$seqres.full | grep -qw 123456-fsgqa
-[ $? -ne 0 ] && _fail "Query user 123456-fsgqa Failed"
-xfs_quota -x -c "report -u" $SCRATCH_MNT 2>>$seqres.full | grep -qw 123456-fsgqa
-[ $? -ne 0 ] && _fail "Report user 123456-fsgqa Failed"
+echo "== user test =="
+$XFS_QUOTA_PROG -x -c "limit -u bsoft=100m bhard=200m 123456-fsgqa" $SCRATCH_MNT
+echo "=== quota command output ==="
+$XFS_QUOTA_PROG -c "quota -u -b -N -v 123456-fsgqa" $SCRATCH_MNT | _filter_quota
+echo "=== report command output ==="
+$XFS_QUOTA_PROG -x -c "report -u -b -N" $SCRATCH_MNT | grep -v "^root " | _filter_quota
 
 # group test
-xfs_quota -x -c "limit -g bsoft=100m bhard=200m 123456-fsgqa" $SCRATCH_MNT >>$seqres.full 2>&1
-[ $? -ne 0 ] && _fail "Create limit for group 123456-fsgqa Failed"
-xfs_quota -x -c "quota -g -v 123456-fsgqa" $SCRATCH_MNT 2>>$seqres.full | grep -qw 123456-fsgqa
-[ $? -ne 0 ] && _fail "Query group 123456-fsgqa Failed"
-xfs_quota -x -c "report -u" $SCRATCH_MNT 2>>$seqres.full | grep -qw 123456-fsgqa
-[ $? -ne 0 ] && _fail "Report group 123456-fsgqa Failed"
+echo "== group test =="
+$XFS_QUOTA_PROG -x -c "limit -g bsoft=100m bhard=200m 123456-fsgqa" $SCRATCH_MNT
+echo "=== quota command output ==="
+$XFS_QUOTA_PROG -c "quota -g -b -N -v 123456-fsgqa" $SCRATCH_MNT | _filter_quota
+echo "=== report command output ==="
+$XFS_QUOTA_PROG -x -c "report -u -b -N" $SCRATCH_MNT | grep -v "^root " | _filter_quota
 
 # success, all done
 status=0
diff --git a/tests/xfs/138.out b/tests/xfs/138.out
index f3eb411..71929c4 100644
--- a/tests/xfs/138.out
+++ b/tests/xfs/138.out
@@ -1 +1,13 @@
 QA output created by 138
+== user test ==
+=== quota command output ===
+SCRATCH_DEV 0 102400 204800 00 [--------] SCRATCH_MNT
+=== report command output ===
+123456-fsgqa 0 102400 204800 00 [--------]
+
+== group test ==
+=== quota command output ===
+SCRATCH_DEV 0 102400 204800 00 [--------] SCRATCH_MNT
+=== report command output ===
+123456-fsgqa 0 102400 204800 00 [--------]
+
-- 
1.9.3

Re: [PATCH] xfs: change return value check to golden image check

[Dave Chinner]

On Fri, Feb 12, 2016 at 12:37:36AM +0800, Zorro Lang wrote:
> xfs/133 and xfs/138 use too much code to do "return value" check,
> it's not necessary. For the code can be more readable and clear,
> I change "return value" check to golden image check.
> 
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
>  tests/xfs/133     | 20 +++++++-------------
>  tests/xfs/133.out |  7 +++++++
>  tests/xfs/138     | 26 ++++++++++++--------------
>  tests/xfs/138.out | 12 ++++++++++++
>  4 files changed, 38 insertions(+), 27 deletions(-)

This cause a xfs/133 failure like this on my systems:

--- tests/xfs/133.out   2016-02-19 10:40:57.043131919 +1100
+++ /home/dave/src/xfstests-dev/results//xfs/xfs/133.out.bad    2016-02-19 12:24:53.173589432 +1100
@@ -4,5 +4,6 @@
 Filesystem Blocks Quota Limit Warn/Time Mounted on
 SCRATCH_DEV 0 102400 204800 00 [--------] SCRATCH_MNT
 === report command output ===
+(null) 0 0 0 00 [--------]
 123456-project 0 102400 204800 00 [--------]
 

Can you see if you can reproduce it?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] xfs: change return value check to golden image check

[Zirong Lang]

----- 原始邮件 -----
> 发件人: "Dave Chinner" <david@fromorbit.com>
> 收件人: "Zorro Lang" <zlang@redhat.com>
> 抄送: fstests@vger.kernel.org, eguan@redhat.com
> 发送时间: 星期五, 2016年 2 月 19日 上午 9:33:16
> 主题: Re: [PATCH] xfs: change return value check to golden image check
> 
> On Fri, Feb 12, 2016 at 12:37:36AM +0800, Zorro Lang wrote:
> > xfs/133 and xfs/138 use too much code to do "return value" check,
> > it's not necessary. For the code can be more readable and clear,
> > I change "return value" check to golden image check.
> > 
> > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > ---
> >  tests/xfs/133     | 20 +++++++-------------
> >  tests/xfs/133.out |  7 +++++++
> >  tests/xfs/138     | 26 ++++++++++++--------------
> >  tests/xfs/138.out | 12 ++++++++++++
> >  4 files changed, 38 insertions(+), 27 deletions(-)
> 
> This cause a xfs/133 failure like this on my systems:
> 
> --- tests/xfs/133.out   2016-02-19 10:40:57.043131919 +1100
> +++ /home/dave/src/xfstests-dev/results//xfs/xfs/133.out.bad    2016-02-19
> 12:24:53.173589432 +1100
> @@ -4,5 +4,6 @@
>  Filesystem Blocks Quota Limit Warn/Time Mounted on
>  SCRATCH_DEV 0 102400 204800 00 [--------] SCRATCH_MNT
>  === report command output ===
> +(null) 0 0 0 00 [--------]
>  123456-project 0 102400 204800 00 [--------]
>  
> 
> Can you see if you can reproduce it?

Hi Dave,

Thanks for review this patch:)

I can't reproduce that problem. It shows xfs_quota report one more strange
line, I can't sure what cause it by read xfs/133.

xfs/133 only set one project quota for 123456-project, I don't think it will
report one more project quota message.

I can add 'grep 123456-project' to make sure it will only report 123456-project.
But I can't do that before I sure that problem you met is not a xfsprogs bug.

So would you please give me more information about your test environment, especially
about xfs project quota?

I will do more exception test, try to reproduce it.

Thanks,
Zorro Lang



> 
> Cheers,
> 
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
> 

Re: [PATCH] xfs: change return value check to golden image check

[Zirong Lang]

----- 原始邮件 -----
> 发件人: "Dave Chinner" <david@fromorbit.com>
> 收件人: "Zorro Lang" <zlang@redhat.com>
> 抄送: fstests@vger.kernel.org, eguan@redhat.com
> 发送时间: 星期五, 2016年 2 月 19日 上午 9:33:16
> 主题: Re: [PATCH] xfs: change return value check to golden image check
> 
> On Fri, Feb 12, 2016 at 12:37:36AM +0800, Zorro Lang wrote:
> > xfs/133 and xfs/138 use too much code to do "return value" check,
> > it's not necessary. For the code can be more readable and clear,
> > I change "return value" check to golden image check.
> > 
> > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > ---
> >  tests/xfs/133     | 20 +++++++-------------
> >  tests/xfs/133.out |  7 +++++++
> >  tests/xfs/138     | 26 ++++++++++++--------------
> >  tests/xfs/138.out | 12 ++++++++++++
> >  4 files changed, 38 insertions(+), 27 deletions(-)
> 
> This cause a xfs/133 failure like this on my systems:
> 
> --- tests/xfs/133.out   2016-02-19 10:40:57.043131919 +1100
> +++ /home/dave/src/xfstests-dev/results//xfs/xfs/133.out.bad    2016-02-19
> 12:24:53.173589432 +1100
> @@ -4,5 +4,6 @@
>  Filesystem Blocks Quota Limit Warn/Time Mounted on
>  SCRATCH_DEV 0 102400 204800 00 [--------] SCRATCH_MNT
>  === report command output ===
> +(null) 0 0 0 00 [--------]
>  123456-project 0 102400 204800 00 [--------]

Wow, I reproduce this problem after I updated my xfsprogs-dev from:

12dd365 xfs_quota: modify commands which can't handle multiple types

to:

1abecda xfsprogs: Release v4.5.0-rc1

That's your newest Release recently. I need more check for this new failure.

Thanks,
Zorro

>  
> 
> Can you see if you can reproduce it?
> 
> Cheers,
> 
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
> 

Re: [PATCH] xfs: change return value check to golden image check

[Zirong Lang]

----- 原始邮件 -----
> 发件人: "Zirong Lang" <zlang@redhat.com>
> 收件人: "Dave Chinner" <david@fromorbit.com>
> 抄送: fstests@vger.kernel.org, eguan@redhat.com
> 发送时间: 星期五, 2016年 2 月 19日 下午 11:35:23
> 主题: Re: [PATCH] xfs: change return value check to golden image check
> 
> 
> 
> ----- 原始邮件 -----
> > 发件人: "Dave Chinner" <david@fromorbit.com>
> > 收件人: "Zorro Lang" <zlang@redhat.com>
> > 抄送: fstests@vger.kernel.org, eguan@redhat.com
> > 发送时间: 星期五, 2016年 2 月 19日 上午 9:33:16
> > 主题: Re: [PATCH] xfs: change return value check to golden image check
> > 
> > On Fri, Feb 12, 2016 at 12:37:36AM +0800, Zorro Lang wrote:
> > > xfs/133 and xfs/138 use too much code to do "return value" check,
> > > it's not necessary. For the code can be more readable and clear,
> > > I change "return value" check to golden image check.
> > > 
> > > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > > ---
> > >  tests/xfs/133     | 20 +++++++-------------
> > >  tests/xfs/133.out |  7 +++++++
> > >  tests/xfs/138     | 26 ++++++++++++--------------
> > >  tests/xfs/138.out | 12 ++++++++++++
> > >  4 files changed, 38 insertions(+), 27 deletions(-)
> > 
> > This cause a xfs/133 failure like this on my systems:
> > 
> > --- tests/xfs/133.out   2016-02-19 10:40:57.043131919 +1100
> > +++ /home/dave/src/xfstests-dev/results//xfs/xfs/133.out.bad    2016-02-19
> > 12:24:53.173589432 +1100
> > @@ -4,5 +4,6 @@
> >  Filesystem Blocks Quota Limit Warn/Time Mounted on
> >  SCRATCH_DEV 0 102400 204800 00 [--------] SCRATCH_MNT
> >  === report command output ===
> > +(null) 0 0 0 00 [--------]
> >  123456-project 0 102400 204800 00 [--------]
> 
> Wow, I reproduce this problem after I updated my xfsprogs-dev from:
> 
> 12dd365 xfs_quota: modify commands which can't handle multiple types

Sorry, 12dd365 is my private commit, which haven't been merged. It should from

1b9fecf libxfs: reset dirty buffer priority on lookup

> 
> to:
> 
> 1abecda xfsprogs: Release v4.5.0-rc1
> 
> That's your newest Release recently. I need more check for this new failure.
> 
> Thanks,
> Zorro
> 
> >  
> > 
> > Can you see if you can reproduce it?
> > 
> > Cheers,
> > 
> > Dave.
> > --
> > Dave Chinner
> > david@fromorbit.com
> > 
> 

Re: [PATCH] xfs: change return value check to golden image check

[Eric Sandeen]

On 2/19/16 9:35 AM, Zirong Lang wrote:
> 
> 
> ----- 原始邮件 -----
>> 发件人: "Dave Chinner" <david@fromorbit.com>
>> 收件人: "Zorro Lang" <zlang@redhat.com>
>> 抄送: fstests@vger.kernel.org, eguan@redhat.com
>> 发送时间: 星期五, 2016年 2 月 19日 上午 9:33:16
>> 主题: Re: [PATCH] xfs: change return value check to golden image check
>>
>> On Fri, Feb 12, 2016 at 12:37:36AM +0800, Zorro Lang wrote:
>>> xfs/133 and xfs/138 use too much code to do "return value" check,
>>> it's not necessary. For the code can be more readable and clear,
>>> I change "return value" check to golden image check.
>>>
>>> Signed-off-by: Zorro Lang <zlang@redhat.com>
>>> ---
>>>  tests/xfs/133     | 20 +++++++-------------
>>>  tests/xfs/133.out |  7 +++++++
>>>  tests/xfs/138     | 26 ++++++++++++--------------
>>>  tests/xfs/138.out | 12 ++++++++++++
>>>  4 files changed, 38 insertions(+), 27 deletions(-)
>>
>> This cause a xfs/133 failure like this on my systems:
>>
>> --- tests/xfs/133.out   2016-02-19 10:40:57.043131919 +1100
>> +++ /home/dave/src/xfstests-dev/results//xfs/xfs/133.out.bad    2016-02-19
>> 12:24:53.173589432 +1100
>> @@ -4,5 +4,6 @@
>>  Filesystem Blocks Quota Limit Warn/Time Mounted on
>>  SCRATCH_DEV 0 102400 204800 00 [--------] SCRATCH_MNT
>>  === report command output ===
>> +(null) 0 0 0 00 [--------]

I need to dig, but this may be a result of GETNEXTQUOTA additions to
xfs_quota.

We can now find IDs on disk that don't exist in the user database, and
we would not have reported them before.

Perhaps change the test to report ids not names, to debug it and see
which one it is finding?

I'm guessing it's ID 0, but I have to think about whether that's correct
to show or not...

Thanks,
-Eric

>>  123456-project 0 102400 204800 00 [--------]
> 
> Wow, I reproduce this problem after I updated my xfsprogs-dev from:
> 
> 12dd365 xfs_quota: modify commands which can't handle multiple types
> 
> to:
> 
> 1abecda xfsprogs: Release v4.5.0-rc1
> 
> That's your newest Release recently. I need more check for this new failure.
> 
> Thanks,
> Zorro
> 
>>  
>>
>> Can you see if you can reproduce it?
>>
>> Cheers,
>>
>> Dave.
>> --
>> Dave Chinner
>> david@fromorbit.com
>>
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] xfs: change return value check to golden image check

[Eric Sandeen]

On 2/19/16 9:58 AM, Eric Sandeen wrote:
> 
> 
> On 2/19/16 9:35 AM, Zirong Lang wrote:
>>
>>
>> ----- 原始邮件 -----
>>> 发件人: "Dave Chinner" <david@fromorbit.com>
>>> 收件人: "Zorro Lang" <zlang@redhat.com>
>>> 抄送: fstests@vger.kernel.org, eguan@redhat.com
>>> 发送时间: 星期五, 2016年 2 月 19日 上午 9:33:16
>>> 主题: Re: [PATCH] xfs: change return value check to golden image check
>>>
>>> On Fri, Feb 12, 2016 at 12:37:36AM +0800, Zorro Lang wrote:
>>>> xfs/133 and xfs/138 use too much code to do "return value" check,
>>>> it's not necessary. For the code can be more readable and clear,
>>>> I change "return value" check to golden image check.
>>>>
>>>> Signed-off-by: Zorro Lang <zlang@redhat.com>
>>>> ---
>>>>  tests/xfs/133     | 20 +++++++-------------
>>>>  tests/xfs/133.out |  7 +++++++
>>>>  tests/xfs/138     | 26 ++++++++++++--------------
>>>>  tests/xfs/138.out | 12 ++++++++++++
>>>>  4 files changed, 38 insertions(+), 27 deletions(-)
>>>
>>> This cause a xfs/133 failure like this on my systems:
>>>
>>> --- tests/xfs/133.out   2016-02-19 10:40:57.043131919 +1100
>>> +++ /home/dave/src/xfstests-dev/results//xfs/xfs/133.out.bad    2016-02-19
>>> 12:24:53.173589432 +1100
>>> @@ -4,5 +4,6 @@
>>>  Filesystem Blocks Quota Limit Warn/Time Mounted on
>>>  SCRATCH_DEV 0 102400 204800 00 [--------] SCRATCH_MNT
>>>  === report command output ===
>>> +(null) 0 0 0 00 [--------]
> 
> I need to dig, but this may be a result of GETNEXTQUOTA additions to
> xfs_quota.
> 
> We can now find IDs on disk that don't exist in the user database, and
> we would not have reported them before.
> 
> Perhaps change the test to report ids not names, to debug it and see
> which one it is finding?
> 
> I'm guessing it's ID 0, but I have to think about whether that's correct
> to show or not...

Ok, with Zorro's help, we see that this is a result of GETNEXTQUOTA.

With that in place, "report" shows all active quotas, skipping only
if XFS_IS_DQUOT_UNINITIALIZED().  But project ID 0 has 4 inodes
accounted for:

# xfs_db -c "dquot -p 0" -c print /dev/...
...
diskdq.bcount = 0
diskdq.icount = 4
diskdq.itimer = 0
diskdq.btimer = 0
...

We never reported ID 0 before, because it was not in the projects file.
But it looks active, so GETNEXTQUOTA finds and returns it now.

I'm not actually sure what the best way is to fix this; I was even on
the fence about using GETNEXTQUOTA for project quotas at all, because
we always have a local file of projects to iterate anyway.

We could explicitly look up id 0 and not show it if it's not in the
projects file.

We could not use GETNEXTQUOTA in the kernel for project quotas.

We could skip printing id 0 altogether in xfs_quota

We could filter it out in the test ...

-Eric

Re: [PATCH] xfs: change return value check to golden image check

[Zirong Lang]

----- 原始邮件 -----
> 发件人: "Eric Sandeen" <sandeen@sandeen.net>
> 收件人: "Zirong Lang" <zlang@redhat.com>, "Dave Chinner" <david@fromorbit.com>
> 抄送: fstests@vger.kernel.org, eguan@redhat.com
> 发送时间: 星期六, 2016年 2 月 20日 上午 12:35:08
> 主题: Re: [PATCH] xfs: change return value check to golden image check
> 
> 
> 
> On 2/19/16 9:58 AM, Eric Sandeen wrote:
> > 
> > 
> > On 2/19/16 9:35 AM, Zirong Lang wrote:
> >>
> >>
> >> ----- 原始邮件 -----
> >>> 发件人: "Dave Chinner" <david@fromorbit.com>
> >>> 收件人: "Zorro Lang" <zlang@redhat.com>
> >>> 抄送: fstests@vger.kernel.org, eguan@redhat.com
> >>> 发送时间: 星期五, 2016年 2 月 19日 上午 9:33:16
> >>> 主题: Re: [PATCH] xfs: change return value check to golden image check
> >>>
> >>> On Fri, Feb 12, 2016 at 12:37:36AM +0800, Zorro Lang wrote:
> >>>> xfs/133 and xfs/138 use too much code to do "return value" check,
> >>>> it's not necessary. For the code can be more readable and clear,
> >>>> I change "return value" check to golden image check.
> >>>>
> >>>> Signed-off-by: Zorro Lang <zlang@redhat.com>
> >>>> ---
> >>>>  tests/xfs/133     | 20 +++++++-------------
> >>>>  tests/xfs/133.out |  7 +++++++
> >>>>  tests/xfs/138     | 26 ++++++++++++--------------
> >>>>  tests/xfs/138.out | 12 ++++++++++++
> >>>>  4 files changed, 38 insertions(+), 27 deletions(-)
> >>>
> >>> This cause a xfs/133 failure like this on my systems:
> >>>
> >>> --- tests/xfs/133.out   2016-02-19 10:40:57.043131919 +1100
> >>> +++ /home/dave/src/xfstests-dev/results//xfs/xfs/133.out.bad
> >>> 2016-02-19
> >>> 12:24:53.173589432 +1100
> >>> @@ -4,5 +4,6 @@
> >>>  Filesystem Blocks Quota Limit Warn/Time Mounted on
> >>>  SCRATCH_DEV 0 102400 204800 00 [--------] SCRATCH_MNT
> >>>  === report command output ===
> >>> +(null) 0 0 0 00 [--------]
> > 
> > I need to dig, but this may be a result of GETNEXTQUOTA additions to
> > xfs_quota.
> > 
> > We can now find IDs on disk that don't exist in the user database, and
> > we would not have reported them before.
> > 
> > Perhaps change the test to report ids not names, to debug it and see
> > which one it is finding?
> > 
> > I'm guessing it's ID 0, but I have to think about whether that's correct
> > to show or not...
> 
> Ok, with Zorro's help, we see that this is a result of GETNEXTQUOTA.
> 
> With that in place, "report" shows all active quotas, skipping only
> if XFS_IS_DQUOT_UNINITIALIZED().  But project ID 0 has 4 inodes
> accounted for:
> 
> # xfs_db -c "dquot -p 0" -c print /dev/...
> ...
> diskdq.bcount = 0
> diskdq.icount = 4
> diskdq.itimer = 0
> diskdq.btimer = 0
> ...
> 
> We never reported ID 0 before, because it was not in the projects file.
> But it looks active, so GETNEXTQUOTA finds and returns it now.
> 
> I'm not actually sure what the best way is to fix this; I was even on
> the fence about using GETNEXTQUOTA for project quotas at all, because
> we always have a local file of projects to iterate anyway.
> 
> We could explicitly look up id 0 and not show it if it's not in the
> projects file.
> 
> We could not use GETNEXTQUOTA in the kernel for project quotas.
> 
> We could skip printing id 0 altogether in xfs_quota
> 
> We could filter it out in the test ...

Maybe the pquota 0 problem will effect other cases except xfs/133 (maybe not,
I haven't tested that). So if we think it's a case problem, we need to check
all cases which report/query xfs project quota.

So I should wait for the decision about how to deal with GETNEXTQUOTA on project quota.

Thanks,
Zorro

> 
> -Eric
> 

Re: [PATCH] xfs: change return value check to golden image check

[Zirong Lang]

----- 原始邮件 -----
> 发件人: "Zirong Lang" <zlang@redhat.com>
> 收件人: "Eric Sandeen" <sandeen@sandeen.net>
> 抄送: "Dave Chinner" <david@fromorbit.com>, fstests@vger.kernel.org, eguan@redhat.com
> 发送时间: 星期六, 2016年 2 月 20日 上午 12:52:07
> 主题: Re: [PATCH] xfs: change return value check to golden image check
> 
> 
> 
> ----- 原始邮件 -----
> > 发件人: "Eric Sandeen" <sandeen@sandeen.net>
> > 收件人: "Zirong Lang" <zlang@redhat.com>, "Dave Chinner" <david@fromorbit.com>
> > 抄送: fstests@vger.kernel.org, eguan@redhat.com
> > 发送时间: 星期六, 2016年 2 月 20日 上午 12:35:08
> > 主题: Re: [PATCH] xfs: change return value check to golden image check
> > 
> > 
> > 
> > On 2/19/16 9:58 AM, Eric Sandeen wrote:
> > > 
> > > 
> > > On 2/19/16 9:35 AM, Zirong Lang wrote:
> > >>
> > >>
> > >> ----- 原始邮件 -----
> > >>> 发件人: "Dave Chinner" <david@fromorbit.com>
> > >>> 收件人: "Zorro Lang" <zlang@redhat.com>
> > >>> 抄送: fstests@vger.kernel.org, eguan@redhat.com
> > >>> 发送时间: 星期五, 2016年 2 月 19日 上午 9:33:16
> > >>> 主题: Re: [PATCH] xfs: change return value check to golden image check
> > >>>
> > >>> On Fri, Feb 12, 2016 at 12:37:36AM +0800, Zorro Lang wrote:
> > >>>> xfs/133 and xfs/138 use too much code to do "return value" check,
> > >>>> it's not necessary. For the code can be more readable and clear,
> > >>>> I change "return value" check to golden image check.
> > >>>>
> > >>>> Signed-off-by: Zorro Lang <zlang@redhat.com>
> > >>>> ---
> > >>>>  tests/xfs/133     | 20 +++++++-------------
> > >>>>  tests/xfs/133.out |  7 +++++++
> > >>>>  tests/xfs/138     | 26 ++++++++++++--------------
> > >>>>  tests/xfs/138.out | 12 ++++++++++++
> > >>>>  4 files changed, 38 insertions(+), 27 deletions(-)
> > >>>
> > >>> This cause a xfs/133 failure like this on my systems:
> > >>>
> > >>> --- tests/xfs/133.out   2016-02-19 10:40:57.043131919 +1100
> > >>> +++ /home/dave/src/xfstests-dev/results//xfs/xfs/133.out.bad
> > >>> 2016-02-19
> > >>> 12:24:53.173589432 +1100
> > >>> @@ -4,5 +4,6 @@
> > >>>  Filesystem Blocks Quota Limit Warn/Time Mounted on
> > >>>  SCRATCH_DEV 0 102400 204800 00 [--------] SCRATCH_MNT
> > >>>  === report command output ===
> > >>> +(null) 0 0 0 00 [--------]
> > > 
> > > I need to dig, but this may be a result of GETNEXTQUOTA additions to
> > > xfs_quota.
> > > 
> > > We can now find IDs on disk that don't exist in the user database, and
> > > we would not have reported them before.
> > > 
> > > Perhaps change the test to report ids not names, to debug it and see
> > > which one it is finding?
> > > 
> > > I'm guessing it's ID 0, but I have to think about whether that's correct
> > > to show or not...
> > 
> > Ok, with Zorro's help, we see that this is a result of GETNEXTQUOTA.
> > 
> > With that in place, "report" shows all active quotas, skipping only
> > if XFS_IS_DQUOT_UNINITIALIZED().  But project ID 0 has 4 inodes
> > accounted for:
> > 
> > # xfs_db -c "dquot -p 0" -c print /dev/...
> > ...
> > diskdq.bcount = 0
> > diskdq.icount = 4
> > diskdq.itimer = 0
> > diskdq.btimer = 0
> > ...
> > 
> > We never reported ID 0 before, because it was not in the projects file.
> > But it looks active, so GETNEXTQUOTA finds and returns it now.
> > 
> > I'm not actually sure what the best way is to fix this; I was even on
> > the fence about using GETNEXTQUOTA for project quotas at all, because
> > we always have a local file of projects to iterate anyway.
> > 
> > We could explicitly look up id 0 and not show it if it's not in the
> > projects file.
> > 
> > We could not use GETNEXTQUOTA in the kernel for project quotas.
> > 
> > We could skip printing id 0 altogether in xfs_quota
> > 
> > We could filter it out in the test ...
> 
> Maybe the pquota 0 problem will effect other cases except xfs/133 (maybe not,
> I haven't tested that). So if we think it's a case problem, we need to check
> all cases which report/query xfs project quota.
> 
> So I should wait for the decision about how to deal with GETNEXTQUOTA on
> project quota.

Hi Dave, Eric

So What do should I do, for you can merge this patch?

Send V2, add 'grep $qa_project'?

Send V2, add project quota 0 output into 133.out ?

Send another patch, add a common filter to filter project quota 0, and
try to find and modify all related cases?

Thanks,
Zorro


> 
> Thanks,
> Zorro
> 
> > 
> > -Eric
> > 
> 

Re: [PATCH] xfs: change return value check to golden image check

[Eric Sandeen]

On 2/19/16 11:07 AM, Zirong Lang wrote:
> 
> 
> ----- 原始邮件 -----
>> 发件人: "Zirong Lang" <zlang@redhat.com>
>> 收件人: "Eric Sandeen" <sandeen@sandeen.net>
>> 抄送: "Dave Chinner" <david@fromorbit.com>, fstests@vger.kernel.org, eguan@redhat.com
>> 发送时间: 星期六, 2016年 2 月 20日 上午 12:52:07
>> 主题: Re: [PATCH] xfs: change return value check to golden image check
>>
>>
>>
>> ----- 原始邮件 -----
>>> 发件人: "Eric Sandeen" <sandeen@sandeen.net>
>>> 收件人: "Zirong Lang" <zlang@redhat.com>, "Dave Chinner" <david@fromorbit.com>
>>> 抄送: fstests@vger.kernel.org, eguan@redhat.com
>>> 发送时间: 星期六, 2016年 2 月 20日 上午 12:35:08
>>> 主题: Re: [PATCH] xfs: change return value check to golden image check
>>>
>>>
>>>
>>> On 2/19/16 9:58 AM, Eric Sandeen wrote:
>>>>
>>>>
>>>> On 2/19/16 9:35 AM, Zirong Lang wrote:
>>>>>
>>>>>
>>>>> ----- 原始邮件 -----
>>>>>> 发件人: "Dave Chinner" <david@fromorbit.com>
>>>>>> 收件人: "Zorro Lang" <zlang@redhat.com>
>>>>>> 抄送: fstests@vger.kernel.org, eguan@redhat.com
>>>>>> 发送时间: 星期五, 2016年 2 月 19日 上午 9:33:16
>>>>>> 主题: Re: [PATCH] xfs: change return value check to golden image check
>>>>>>
>>>>>> On Fri, Feb 12, 2016 at 12:37:36AM +0800, Zorro Lang wrote:
>>>>>>> xfs/133 and xfs/138 use too much code to do "return value" check,
>>>>>>> it's not necessary. For the code can be more readable and clear,
>>>>>>> I change "return value" check to golden image check.
>>>>>>>
>>>>>>> Signed-off-by: Zorro Lang <zlang@redhat.com>
>>>>>>> ---
>>>>>>>  tests/xfs/133     | 20 +++++++-------------
>>>>>>>  tests/xfs/133.out |  7 +++++++
>>>>>>>  tests/xfs/138     | 26 ++++++++++++--------------
>>>>>>>  tests/xfs/138.out | 12 ++++++++++++
>>>>>>>  4 files changed, 38 insertions(+), 27 deletions(-)
>>>>>>
>>>>>> This cause a xfs/133 failure like this on my systems:
>>>>>>
>>>>>> --- tests/xfs/133.out   2016-02-19 10:40:57.043131919 +1100
>>>>>> +++ /home/dave/src/xfstests-dev/results//xfs/xfs/133.out.bad
>>>>>> 2016-02-19
>>>>>> 12:24:53.173589432 +1100
>>>>>> @@ -4,5 +4,6 @@
>>>>>>  Filesystem Blocks Quota Limit Warn/Time Mounted on
>>>>>>  SCRATCH_DEV 0 102400 204800 00 [--------] SCRATCH_MNT
>>>>>>  === report command output ===
>>>>>> +(null) 0 0 0 00 [--------]
>>>>
>>>> I need to dig, but this may be a result of GETNEXTQUOTA additions to
>>>> xfs_quota.
>>>>
>>>> We can now find IDs on disk that don't exist in the user database, and
>>>> we would not have reported them before.
>>>>
>>>> Perhaps change the test to report ids not names, to debug it and see
>>>> which one it is finding?
>>>>
>>>> I'm guessing it's ID 0, but I have to think about whether that's correct
>>>> to show or not...
>>>
>>> Ok, with Zorro's help, we see that this is a result of GETNEXTQUOTA.
>>>
>>> With that in place, "report" shows all active quotas, skipping only
>>> if XFS_IS_DQUOT_UNINITIALIZED().  But project ID 0 has 4 inodes
>>> accounted for:
>>>
>>> # xfs_db -c "dquot -p 0" -c print /dev/...
>>> ...
>>> diskdq.bcount = 0
>>> diskdq.icount = 4
>>> diskdq.itimer = 0
>>> diskdq.btimer = 0
>>> ...
>>>
>>> We never reported ID 0 before, because it was not in the projects file.
>>> But it looks active, so GETNEXTQUOTA finds and returns it now.
>>>
>>> I'm not actually sure what the best way is to fix this; I was even on
>>> the fence about using GETNEXTQUOTA for project quotas at all, because
>>> we always have a local file of projects to iterate anyway.
>>>
>>> We could explicitly look up id 0 and not show it if it's not in the
>>> projects file.
>>>
>>> We could not use GETNEXTQUOTA in the kernel for project quotas.
>>>
>>> We could skip printing id 0 altogether in xfs_quota
>>>
>>> We could filter it out in the test ...
>>
>> Maybe the pquota 0 problem will effect other cases except xfs/133 (maybe not,
>> I haven't tested that). So if we think it's a case problem, we need to check
>> all cases which report/query xfs project quota.
>>
>> So I should wait for the decision about how to deal with GETNEXTQUOTA on
>> project quota.
> 
> Hi Dave, Eric
> 
> So What do should I do, for you can merge this patch?
> 
> Send V2, add 'grep $qa_project'?
> 
> Send V2, add project quota 0 output into 133.out ?
> 
> Send another patch, add a common filter to filter project quota 0, and
> try to find and modify all related cases?

Let's decide what the right thing to do in kernelspace/userspace is first,
then we can adjust tests if needed.

-Eric

Re: [PATCH] xfs: change return value check to golden image check

[Dave Chinner]

On Fri, Feb 19, 2016 at 10:06:32AM -0500, Zirong Lang wrote:
> 
> 
> ----- 原始邮件 -----
> > 发件人: "Dave Chinner" <david@fromorbit.com>
> > 收件人: "Zorro Lang" <zlang@redhat.com>
> > 抄送: fstests@vger.kernel.org, eguan@redhat.com
> > 发送时间: 星期五, 2016年 2 月 19日 上午 9:33:16
> > 主题: Re: [PATCH] xfs: change return value check to golden image check
> > 
> > On Fri, Feb 12, 2016 at 12:37:36AM +0800, Zorro Lang wrote:
> > > xfs/133 and xfs/138 use too much code to do "return value" check,
> > > it's not necessary. For the code can be more readable and clear,
> > > I change "return value" check to golden image check.
> > > 
> > > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > > ---
> > >  tests/xfs/133     | 20 +++++++-------------
> > >  tests/xfs/133.out |  7 +++++++
> > >  tests/xfs/138     | 26 ++++++++++++--------------
> > >  tests/xfs/138.out | 12 ++++++++++++
> > >  4 files changed, 38 insertions(+), 27 deletions(-)
> > 
> > This cause a xfs/133 failure like this on my systems:
> > 
> > --- tests/xfs/133.out   2016-02-19 10:40:57.043131919 +1100
> > +++ /home/dave/src/xfstests-dev/results//xfs/xfs/133.out.bad    2016-02-19
> > 12:24:53.173589432 +1100
> > @@ -4,5 +4,6 @@
> >  Filesystem Blocks Quota Limit Warn/Time Mounted on
> >  SCRATCH_DEV 0 102400 204800 00 [--------] SCRATCH_MNT
> >  === report command output ===
> > +(null) 0 0 0 00 [--------]
> >  123456-project 0 102400 204800 00 [--------]
> >  
> > 
> > Can you see if you can reproduce it?
> 
> Hi Dave,
> 
> Thanks for review this patch:)
> 
> I can't reproduce that problem. It shows xfs_quota report one more strange
> line, I can't sure what cause it by read xfs/133.
> 
> xfs/133 only set one project quota for 123456-project, I don't think it will
> report one more project quota message.

I suspect it is the default quota limit changes that are causing
this, causing a dquot for project ID 0 to be created.

> I can add 'grep 123456-project' to make sure it will only report 123456-project.
> But I can't do that before I sure that problem you met is not a xfsprogs bug.
> 
> So would you please give me more information about your test environment, especially
> about xfs project quota?

Everything is at current top of tree.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com