Re: [PATCH v4 2/2] xfs/006: new case to test xfs fail_at_unmount error handling

[Zirong Lang <zlang@redhat.com>]

----- 原始邮件 -----
> 发件人: "Eric Sandeen" <sandeen@sandeen.net>
> 收件人: "Zirong Lang" <zlang@redhat.com>, "Dave Chinner" <david@fromorbit.com>
> 抄送: "Eryu Guan" <eguan@redhat.com>, fstests@vger.kernel.org, sandeen@redhat.com, cem@redhat.com
> 发送时间: 星期三, 2016年 6 月 22日 上午 11:04:55
> 主题: Re: [PATCH v4 2/2] xfs/006: new case to test xfs fail_at_unmount error handling
> 
> On 6/21/16 8:42 PM, Zirong Lang wrote:
> > Hi Dave
> > 
> > ----- 原始邮件 -----
> >> 发件人: "Dave Chinner" <david@fromorbit.com>
> >> 收件人: "Eryu Guan" <eguan@redhat.com>
> >> 抄送: "Zorro Lang" <zlang@redhat.com>, fstests@vger.kernel.org,
> >> sandeen@redhat.com, cem@redhat.com
> >> 发送时间: 星期三, 2016年 6 月 22日 上午 8:00:40
> >> 主题: Re: [PATCH v4 2/2] xfs/006: new case to test xfs fail_at_unmount error
> >> handling
> >>
> >> On Tue, Jun 21, 2016 at 03:08:18PM +0800, Eryu Guan wrote:
> >>> On Mon, Jun 20, 2016 at 09:24:33PM +0800, Zorro Lang wrote:
> >>>> +# real QA test starts here
> >>>> +_supported_fs xfs
> >>>> +_supported_os Linux
> >>>> +_require_dm_target error
> >>>> +_require_scratch
> >>>> +
> >>>> +_scratch_mkfs > $seqres.full 2>&1
> >>>> +_require_fs_sysfs $SCRATCH_DEV error/fail_at_unmount
> >>>
> >>> Usually we call _require_xxx before mkfs and do the real test, a comment
> >>> to explain why we need to mkfs first would be good.
> >>
> >> Ok, so why do we need to test the scratch device for this
> >> sysfs file check? We've already got the test device mounted, and
> >> filesystems tend to present identical sysfs control files for all
> >> mounted filesystems.
> >>
> >> i.e. this _require_fs_sysfs() function could just drop the device
> >> and check the test device for whether the sysfs entry exists. If it
> >> doesn't, then the scratch device isn't going to have it, either.
> > 
> > Hmm... at first I thought about if I should use TEST_DEV to do
> > _require_fs_sysfs
> > checking. But I'm not sure if different devices maybe bring different sysfs
> > attributes in, if someone make a special device in one case? So I give one
> > more
> > argument about device name.
> 
> Sorry, I was kind of thinking this as well on my first review, but I let it
> pass.
> 
> I would say that for now, let's just use TEST_DEV.  There is no reason to
> code
> around "what-if" scenarios.  If a filesystem ends up needing a special mkfs
> or mount option to expose a sysfs tunable in the future, we can add a device
> name
> at that point. Until then, I don't think there is any reason, and the "mkfs
> first,
> then require" is definitely a little bit out of the ordinary.

Sorry, I should follow your first review suggestion. I will change to use TEST_DEV:)

Thanks,
 -Zorro

> 
> -Eric
>