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

[Zirong Lang <zlang@redhat.com>]

----- 原始邮件 -----
> 发件人: "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
> > 
>