| Message ID | 2a649d6a51055ac36352a73add403037acc537ef.1381493768.git.sbehrens@giantdisaster.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Oct 11, 2013 at 02:18:23PM +0200, Stefan Behrens wrote: > Currently the attempt to reset the device statistics with > 'btrfs device stat -z ...' on read-only filesystems does not return an > error. The statistics that are hold in main memory are reset but the > statistics that are stored in the filesystem are not reset. > > Fix it by returning EROFS in this case. > > This is the reproducer: > > #!/bin/sh > TEST_DEV1=/dev/sdzzzzz1 > TEST_DEV2=/dev/sdzzzzz2 > TEST_MNT=/mnt > echo 0 25165824 linear $TEST_DEV1 0 | dmsetup create foom > echo 0 25165824 linear $TEST_DEV2 0 | dmsetup create foon > mkfs.btrfs -f -d raid1 -m raid1 /dev/mapper/foom /dev/mapper/foon > mount /dev/mapper/foom $TEST_MNT > # switch to I/O error mode: > echo 0 25165824 error | dmsetup reload foon > dmsetup resume foon > # cause I/O errors: > touch ${TEST_MNT}/foo > sync > # switch dm back to I/O good mode: > echo 0 25165824 linear $TEST_DEV1 0 | dmsetup reload foon > dmsetup resume foon > umount ${TEST_MNT} > btrfs dev scan > mount /dev/mapper/foom ${TEST_MNT} -o ro > # will report counters != 0, should fail with EROFS > btrfs device stat -z ${TEST_MNT} > # will report counters == 0: > btrfs device stat ${TEST_MNT} > umount ${TEST_MNT} > mount /dev/mapper/foom ${TEST_MNT} -o ro > # will report counters != 0, i.e., the 'btrfs device stat -z' failed to > # clear the counters on disk, only the counters in main memory had been > # cleared: > btrfs device stat ${TEST_MNT} > umount ${TEST_MNT} > dmsetup remove foom; dmsetup remove foon > Hey look something else that should go into xfstests, Josef -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 11 Oct 2013 09:47:33 -0400, Josef Bacik wrote:
> Hey look something else that should go into xfstests,
I don't think so. It's a bug that is there from the very beginning, not
a regression. We can't catch all possible errors (non-regressions) with
xfstests. We would spend all time for writing xfstests, there wouldn't
be any time left to actually add bugs.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 11, 2013 at 04:07:37PM +0200, Stefan Behrens wrote: > On Fri, 11 Oct 2013 09:47:33 -0400, Josef Bacik wrote: > > Hey look something else that should go into xfstests, > > I don't think so. It's a bug that is there from the very beginning, not > a regression. We can't catch all possible errors (non-regressions) with > xfstests. We would spend all time for writing xfstests, there wouldn't > be any time left to actually add bugs. > Sure it's been there forever, but now that we've fixed it I'd like to make sure we don't ever have to fix it again. Josef -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index fe0f2ef..646d10d 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -6250,6 +6250,8 @@ int btrfs_get_dev_stats(struct btrfs_root *root, "btrfs: get dev_stats failed, not yet valid\n"); return -ENODEV; } else if (stats->flags & BTRFS_DEV_STATS_RESET) { + if (root->fs_info->sb->s_flags & MS_RDONLY) + return -EROFS; for (i = 0; i < BTRFS_DEV_STAT_VALUES_MAX; i++) { if (stats->nr_items > i) stats->values[i] =
Currently the attempt to reset the device statistics with 'btrfs device stat -z ...' on read-only filesystems does not return an error. The statistics that are hold in main memory are reset but the statistics that are stored in the filesystem are not reset. Fix it by returning EROFS in this case. This is the reproducer: #!/bin/sh TEST_DEV1=/dev/sdzzzzz1 TEST_DEV2=/dev/sdzzzzz2 TEST_MNT=/mnt echo 0 25165824 linear $TEST_DEV1 0 | dmsetup create foom echo 0 25165824 linear $TEST_DEV2 0 | dmsetup create foon mkfs.btrfs -f -d raid1 -m raid1 /dev/mapper/foom /dev/mapper/foon mount /dev/mapper/foom $TEST_MNT # switch to I/O error mode: echo 0 25165824 error | dmsetup reload foon dmsetup resume foon # cause I/O errors: touch ${TEST_MNT}/foo sync # switch dm back to I/O good mode: echo 0 25165824 linear $TEST_DEV1 0 | dmsetup reload foon dmsetup resume foon umount ${TEST_MNT} btrfs dev scan mount /dev/mapper/foom ${TEST_MNT} -o ro # will report counters != 0, should fail with EROFS btrfs device stat -z ${TEST_MNT} # will report counters == 0: btrfs device stat ${TEST_MNT} umount ${TEST_MNT} mount /dev/mapper/foom ${TEST_MNT} -o ro # will report counters != 0, i.e., the 'btrfs device stat -z' failed to # clear the counters on disk, only the counters in main memory had been # cleared: btrfs device stat ${TEST_MNT} umount ${TEST_MNT} dmsetup remove foom; dmsetup remove foon Signed-off-by: Stefan Behrens <sbehrens@giantdisaster.de> --- fs/btrfs/volumes.c | 2 ++ 1 file changed, 2 insertions(+)