From: Eryu Guan <eguan@redhat.com>
To: Anand Jain <anand.jain@oracle.com>
Cc: fstests@vger.kernel.org, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v2] fstests: btrfs: fix 006 adds _runnt_btrfs_util_prog()
Date: Thu, 23 Jun 2016 19:18:43 +0800 [thread overview]
Message-ID: <20160623111843.GL3226@eguan.usersys.redhat.com> (raw)
In-Reply-To: <24a2491d-91d7-0e45-5008-e784b7a699fd@oracle.com>
On Thu, Jun 23, 2016 at 07:03:59PM +0800, Anand Jain wrote:
>
>
> On 06/23/2016 06:53 PM, Eryu Guan wrote:
[snip]
> > > diff --git a/common/rc b/common/rc
> > > index a44fb8750220..2a10fbb2d341 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -3114,6 +3114,17 @@ _min_dio_alignment()
> > > fi
> > > }
> > >
> > > +run_check_dontfail()
> > > +{
> > > + echo "# $@" >> $seqres.full 2>&1
> > > + "$@" >> $seqres.full 2>&1 || echo "failed: '$@'"
> > > +}
> > > +
> > > +_runnt_btrfs_util_prog()
> > > +{
> > > + run_check_dontfail $BTRFS_UTIL_PROG $*
> > > +}
> > > +
> > > run_check()
> > > {
> > > echo "# $@" >> $seqres.full 2>&1
> > > diff --git a/tests/btrfs/006 b/tests/btrfs/006
> > > index 715fd80fb6fc..9d1fe09e07de 100755
> > > --- a/tests/btrfs/006
> > > +++ b/tests/btrfs/006
> > > @@ -79,7 +79,7 @@ echo "== Show filesystem by UUID"
> > > $BTRFS_UTIL_PROG filesystem show $UUID | _filter_btrfs_filesystem_show $TOTAL_DEVS $UUID
> > >
> > > echo "== Sync filesystem"
> > > -$BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT | _filter_scratch
> > > +_runnt_btrfs_util_prog filesystem sync $SCRATCH_MNT
> >
> > Still, I don't think this helper is necessary.
> >
> > $BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT >/dev/null
> >
> > doesn't _fail on failure, output error messages breaks golden image, and
> > is much simpler. Do I miss anything?
>
> runnt_btrfs_util_prog() checks the return status of the command,
> if failed (!0) it will echo so to break the golden image.
So it fails silently now? (return non-zero value and print no error
message) That seems a btrfs-progs bug to me.. It should print error
messages to stderr on failure, so we don't have to check the return
value explicitly.
Thanks,
Eryu
next prev parent reply other threads:[~2016-06-23 11:18 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-23 10:37 [PATCH v2] fstests: btrfs: fix 006 adds _runnt_btrfs_util_prog() Anand Jain
2016-06-23 10:53 ` Eryu Guan
2016-06-23 11:03 ` Anand Jain
2016-06-23 11:18 ` Eryu Guan [this message]
2016-06-23 11:34 ` Anand Jain
2016-06-23 11:54 ` Filipe Manana
2016-06-23 11:59 ` Anand Jain
2016-06-23 12:11 ` Eryu Guan
2016-06-23 12:24 ` Anand Jain
2016-06-23 13:17 ` [PATCH v3] fstests: btrfs: fix 006 Anand Jain
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160623111843.GL3226@eguan.usersys.redhat.com \
--to=eguan@redhat.com \
--cc=anand.jain@oracle.com \
--cc=fstests@vger.kernel.org \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).