From: Dave Chinner <david@fromorbit.com>
To: fdmanana@kernel.org
Cc: fstests@vger.kernel.org, linux-btrfs@vger.kernel.org,
Filipe Manana <fdmanana@suse.com>
Subject: Re: [PATCH 1/2] fstests: fix btrfs test failures after commit 27d077ec0bda
Date: Thu, 24 Dec 2015 10:49:45 +1100 [thread overview]
Message-ID: <20151223234945.GI26718@dastard> (raw)
In-Reply-To: <1450750960-5172-1-git-send-email-fdmanana@kernel.org>
On Tue, Dec 22, 2015 at 02:22:40AM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> Commit 27d077ec0bda (common: use mount/umount helpers everywhere) made
> a few btrfs test fail for 2 different reasons:
>
> 1) Some tests (btrfs/029 and btrfs/031) use $SCRATCH_MNT as a mount
> point for some subvolume created in $TEST_DEV, therefore calling
> _scratch_unmount does not work as it passes $SCRATCH_DEV as the
> argument to the umount program. This is intentional to test reflinks
> accross different mountpoints of the same filesystem but for different
> subvolumes;
The correct way to fix this is to stop abusing $SCRATCH_MNT and
to instead use a local mount point on the test device....
> 2) For multiple devices filesystems (btrfs/003 and btrfs/011) that test
> the device replace feature, we need to unmount using the mount path
> ($SCRATCH_MNT) because unmounting using one of the devices as an
> argument ($SCRATCH_DEV) does not always work - after replace operations
> we get in /proc/mounts a device other than $SCRATCH_DEV associated
> with the mount point $SCRATCH_MNT (this is mentioned in a comment at
> btrfs/011 for example), so we need to pass that other device to the
> umount program or pass it the mount point.
Which says to that _scratch_unmount should be using $SCRATCH_MNT
rather than $SCRATCH_DEV. That would fix the problem without needing
to modify any of the tests, right?
> Using $SCRATCH_MNT as a mountpoint for a device other than $SCRATCH_DEV is
> misleading, but that's a different problem that existed long before and
> this change attempts only to fix the regression from 27d077ec0bda.
It may be misleading, but that's the fundamental problem that needs
fixing. As always, we should be trying to fix the root cause of the
problem, not working around them...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
next prev parent reply other threads:[~2015-12-23 23:49 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-22 2:22 [PATCH 1/2] fstests: fix btrfs test failures after commit 27d077ec0bda fdmanana
2015-12-22 12:40 ` Eryu Guan
2015-12-23 23:49 ` Dave Chinner [this message]
2015-12-24 12:13 ` Filipe Manana
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=20151223234945.GI26718@dastard \
--to=david@fromorbit.com \
--cc=fdmanana@kernel.org \
--cc=fdmanana@suse.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