Re: [PATCH] generic/135: fix standard error output from _scratch_mkfs

Re: [PATCH] generic/135: fix standard error output from _scratch_mkfs

[Zirong Lang]

2016年2月22日 14:12，"Xiao Yang" <yangx.jy@cn.fujitsu.com>写道：
>
>
>> 2016年2月22日 11:41，"Xiao Yang" <yangx.jy@cn.fujitsu.com> 写道：
>> >
>> > Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
>> > ---
>> >  tests/generic/135 |    2 +-
>> >  1 files changed, 1 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/tests/generic/135 b/tests/generic/135
>> > index 52418f6..5aa880d 100755
>> > --- a/tests/generic/135
>> > +++ b/tests/generic/135
>> > @@ -41,7 +41,7 @@ _supported_os Linux IRIX
>> >
>> >  _require_odirect
>> >  _require_scratch
>> > -_scratch_mkfs >/dev/null
>> > +_scratch_mkfs &>/dev/null
>>
>> Hi,
>>
>> Although I'm not the offical reviewer, but I think that stderr output can help us to find mkfs failure from golden image diff. So keep the stderr output maybe better.
>>
>> If you think your patch is necessary, you'd better give a appropriate reason to explain why we need to ignore that stderr output?
>>
>> Thanks,
>> Zorro
>
> mkfs succees with unexpected message when running generic/135, i.e.
> [root@localhost ~]# mkfs.ext4 /dev/sdb1 >/dev/null
> mke2fs 1.42.9 (28-Dec-2013)

Hi xiao,

Now I understand what you mean, and I can reproduce that problem. This
is a regression bug which bring in by commit:

4a5cda8 xfstests: do not unmount tmpfs during remount

That patch remove "2>&1". I think that is a miss, so you can simply bring
that back. After you explain the reason in your commit message clearly, I
think maintainer will glad to check it:)

Thanks,
Zorro

>
> thanks,
> xiao yang
>
>> >  _scratch_mount
>> >
>> >  cd $SCRATCH_MNT
>> > --
>> > 1.7.1
>> >
>> >
>> >
>> > --
>> > 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] generic/135: fix standard error output from _scratch_mkfs

[Xiao Yang]

于 2016/02/22 17:40, Zirong Lang 写道:
> 2016年2月22日 14:12，"Xiao Yang"<yangx.jy@cn.fujitsu.com>写道：
>>
>>> 2016年2月22日 11:41，"Xiao Yang"<yangx.jy@cn.fujitsu.com>  写道：
>>>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>>>> ---
>>>>   tests/generic/135 |    2 +-
>>>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/tests/generic/135 b/tests/generic/135
>>>> index 52418f6..5aa880d 100755
>>>> --- a/tests/generic/135
>>>> +++ b/tests/generic/135
>>>> @@ -41,7 +41,7 @@ _supported_os Linux IRIX
>>>>
>>>>   _require_odirect
>>>>   _require_scratch
>>>> -_scratch_mkfs>/dev/null
>>>> +_scratch_mkfs&>/dev/null
>>> Hi,
>>>
>>> Although I'm not the offical reviewer, but I think that stderr output can help us to find mkfs failure from golden image diff. So keep the stderr output maybe better.
>>>
>>> If you think your patch is necessary, you'd better give a appropriate reason to explain why we need to ignore that stderr output?
>>>
>>> Thanks,
>>> Zorro
>> mkfs succees with unexpected message when running generic/135, i.e.
>> [root@localhost ~]# mkfs.ext4 /dev/sdb1>/dev/null
>> mke2fs 1.42.9 (28-Dec-2013)
> Hi xiao,
>
> Now I understand what you mean, and I can reproduce that problem. This
> is a regression bug which bring in by commit:
>
> 4a5cda8 xfstests: do not unmount tmpfs during remount
>
> That patch remove "2>&1". I think that is a miss, so you can simply bring
> that back. After you explain the reason in your commit message clearly, I
> think maintainer will glad to check it:)
>
> Thanks,
> Zorro
>
thanks for your suggestion, I will expalin the reason in my commit 
message clearly.
thanks,
xiao yang
>> thanks,
>> xiao yang
>>
>>>>   _scratch_mount
>>>>
>>>>   cd $SCRATCH_MNT
>>>> --
>>>> 1.7.1
>>>>
>>>>
>>>>
>>>> --
>>>> 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
>>
>
> .
>

[PATCH] generic/135: fix standard error output from _scratch_mkfs

[Xiao Yang]

'mke2fs 1.42.9 (28-Dec-2013)' produces as standard error when
we run generic/135 in ext4 file system. This is a regression
bug which bring in by commit:
4a5cda8 xfstests: do not unmount tmpfs during remount
That patch remove '2>&1', so i bring that back.

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 tests/generic/135 |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tests/generic/135 b/tests/generic/135
index 52418f6..5aa880d 100755
--- a/tests/generic/135
+++ b/tests/generic/135
@@ -41,7 +41,7 @@ _supported_os Linux IRIX
 
 _require_odirect
 _require_scratch
-_scratch_mkfs >/dev/null
+_scratch_mkfs &>/dev/null
 _scratch_mount
 
 cd $SCRATCH_MNT
-- 
1.7.1

Re: [PATCH] generic/135: fix standard error output from _scratch_mkfs

[Eryu Guan]

On Tue, Feb 23, 2016 at 09:35:25AM +0800, Xiao Yang wrote:
> 'mke2fs 1.42.9 (28-Dec-2013)' produces as standard error when
> we run generic/135 in ext4 file system. This is a regression
> bug which bring in by commit:
> 4a5cda8 xfstests: do not unmount tmpfs during remount
> That patch remove '2>&1', so i bring that back.

Thanks for the revision! But I noticed that Ted's patch fixed it too,

"common: filter warning messages from mkfs.ext4"

which filters the warnings in _scratch_mkfs_ext4, so that other mkfs
errors can be noticed in the test. So I think it's a better fix :)

Thanks,
Eryu

> 
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
>  tests/generic/135 |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/tests/generic/135 b/tests/generic/135
> index 52418f6..5aa880d 100755
> --- a/tests/generic/135
> +++ b/tests/generic/135
> @@ -41,7 +41,7 @@ _supported_os Linux IRIX
>  
>  _require_odirect
>  _require_scratch
> -_scratch_mkfs >/dev/null
> +_scratch_mkfs &>/dev/null
>  _scratch_mount
>  
>  cd $SCRATCH_MNT
> -- 
> 1.7.1
> 
> 
> 
> --
> 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] generic/135: fix standard error output from _scratch_mkfs

[Theodore Ts'o]

On Tue, Feb 23, 2016 at 11:26:52AM +0800, Eryu Guan wrote:
> On Tue, Feb 23, 2016 at 09:35:25AM +0800, Xiao Yang wrote:
> > 'mke2fs 1.42.9 (28-Dec-2013)' produces as standard error when
> > we run generic/135 in ext4 file system. This is a regression
> > bug which bring in by commit:
> > 4a5cda8 xfstests: do not unmount tmpfs during remount
> > That patch remove '2>&1', so i bring that back.
> 
> Thanks for the revision! But I noticed that Ted's patch fixed it too,
> 
> "common: filter warning messages from mkfs.ext4"
> 
> which filters the warnings in _scratch_mkfs_ext4, so that other mkfs
> errors can be noticed in the test. So I think it's a better fix :)

Yes, I had revised patch that added the 2>&1 but that wasn't the one
that ended up xfstests repository.  I started with a patch to add it
back that looked much like yours (except I used '2>&1' instead of '>&'),
but I started thinking about the fact that this would cause errors to
be masked which is not cool.

I believe Dave or Cristoph has said that they don't like redirecting
stderr to /dev/null for _scratch_mount, and it seems to me that
assuming my change makes it in, it would probably be good to get rid
of the other stderr redirections to /dev/null.

					- Ted

Re: [PATCH] generic/135: fix standard error output from _scratch_mkfs

[Xiao Yang]

于 2016/02/23 11:26, Eryu Guan 写道:
> On Tue, Feb 23, 2016 at 09:35:25AM +0800, Xiao Yang wrote:
>> 'mke2fs 1.42.9 (28-Dec-2013)' produces as standard error when
>> we run generic/135 in ext4 file system. This is a regression
>> bug which bring in by commit:
>> 4a5cda8 xfstests: do not unmount tmpfs during remount
>> That patch remove '2>&1', so i bring that back.
> Thanks for the revision! But I noticed that Ted's patch fixed it too,
>
> "common: filter warning messages from mkfs.ext4"
>
> which filters the warnings in _scratch_mkfs_ext4, so that other mkfs
> errors can be noticed in the test. So I think it's a better fix :)
>
> Thanks,
> Eryu
>
Hi
After applying Ted's pacth, 'mke2fs 1.42.9 (28-Dec-2013)' still produces 
when we run
generic/135 in ext4 file system. I think that this is standard error 
instead of warning .
>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>> ---
>>   tests/generic/135 |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/tests/generic/135 b/tests/generic/135
>> index 52418f6..5aa880d 100755
>> --- a/tests/generic/135
>> +++ b/tests/generic/135
>> @@ -41,7 +41,7 @@ _supported_os Linux IRIX
>>
>>   _require_odirect
>>   _require_scratch
>> -_scratch_mkfs>/dev/null
>> +_scratch_mkfs&>/dev/null
>>   _scratch_mount
>>
>>   cd $SCRATCH_MNT
>> -- 
>> 1.7.1
>>
>>
>>
>> --
>> 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
>
>

[PATCH] generic/135: fix standard error output from _scratch_mkfs

[Xiao Yang]

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 tests/generic/135 |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tests/generic/135 b/tests/generic/135
index 52418f6..5aa880d 100755
--- a/tests/generic/135
+++ b/tests/generic/135
@@ -41,7 +41,7 @@ _supported_os Linux IRIX
 
 _require_odirect
 _require_scratch
-_scratch_mkfs >/dev/null
+_scratch_mkfs &>/dev/null
 _scratch_mount
 
 cd $SCRATCH_MNT
-- 
1.7.1