All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: Eryu Guan <guan@eryu.me>
Cc: Josef Bacik <josef@toxicpanda.com>,
	fstests@vger.kernel.org, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] fstests: generic/204: fail if the mkfs fails
Date: Sun, 1 Aug 2021 11:57:44 -0400	[thread overview]
Message-ID: <YQbEeAYqXAAMGn7G@mit.edu> (raw)
In-Reply-To: <YQaZUK880CRVf6Sn@desktop>

On Sun, Aug 01, 2021 at 08:53:36PM +0800, Eryu Guan wrote:
> > So the underlying disk is 1TB in size, and we ended up using this 1T
> > filesystem when _scratch_mkfs_sized failed?
> > 
> > But we have done _try_wipe_scratch_devs before each test to make sure we
> > don't use previous scratch dev accidently (just like this case), and the
> > subsesquent _scratch_mount will fail and fail the whole test. So it's
> > not clear to me what caused the failure you hit.

The call to _try_wipe_scratch_devs was added in 2019.  My commit to
add:

	|| _notrun "mkfs.${FSTYP} failed"

dates from 2017.  So the reason I was seeing the problem was because
it was before we started running wipefs between tests.

That being said, I've checked a recent test run, and the _notrun
hasn't triggered recently.  Looking at the git history, it looks like
a large number of tests had their arguments to _scratch_mkfs_sized
adjusted upwards to avoid failures when running with 64k block sizes
on powerpc.

Going back to generic/204, I see why Josef ran into issues, however.
even though we are running wipefs before each test.  In the case of
generic/204, it runs _scratch_mkfs to determine the blocksize, and
then it runs _scratch_mkfs_sized --- and if it fails, the file system
is left at the full size of the scratch file system, and then
generic/204 takes a vey long time.

So even if we can rely on wipefs causing the tests to fail, maybe we
should just add a check for mkfs failure to _scratch_mkfs_sized?  I
think that's a better fix than Josef's proposed patch to generic/204.
One benefit of adding the check to _scratch_mkfs_sized is we can
supply a clearer explanation of the failure since the failure would be
"mkfs failed" as opposed to "mount: /vdc: wrong fs type, bad option,
bad superblock on /dev/vdc, missing codepage or helper program, or
other error."

It might also make sense to adjust the size passed to
_scratch_mkfs_sized in generic/204 to be a something slightly larger,
since otherwise it's pretty much guaranteed that generic/204 will
start failing on PowerPC when testing with a 64k block size.

Cheers,

						- Ted

      reply	other threads:[~2021-08-01 16:00 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-29 20:35 [PATCH] fstests: generic/204: fail if the mkfs fails Josef Bacik
2021-07-30  3:52 ` Theodore Ts'o
2021-08-01 12:42 ` Eryu Guan
2021-08-01 12:53   ` Eryu Guan
2021-08-01 15:57     ` Theodore Ts'o [this message]

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=YQbEeAYqXAAMGn7G@mit.edu \
    --to=tytso@mit.edu \
    --cc=fstests@vger.kernel.org \
    --cc=guan@eryu.me \
    --cc=josef@toxicpanda.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.