* [PATCH] tests: add btrfs mount tests @ 2016-02-02 18:00 Stanislav Brabec 2016-02-02 20:14 ` [PATCH] tests: add btrfs mount tests (fails!) Stanislav Brabec 2016-03-14 1:20 ` [PATCH] tests: add btrfs mount tests Ruediger Meier 0 siblings, 2 replies; 16+ messages in thread From: Stanislav Brabec @ 2016-02-02 18:00 UTC (permalink / raw) To: util-linux btrfs needs a special support in mount. Add a testcase for btrfs specific problems. Coverage: 352740e8: bind mounts pointing to btrfs subvolume 2cd28fc8: mounting default subvolume d2f82678: use of "auto" 618a8814: use of "subvolid" Signed-off-by: Stanislav Brabec <sbrabec@suse.cz> --- tests/expected/mount/fstab-btrfs | 1 + tests/ts/mount/fstab-btrfs | 99 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 tests/expected/mount/fstab-btrfs create mode 100755 tests/ts/mount/fstab-btrfs diff --git a/tests/expected/mount/fstab-btrfs b/tests/expected/mount/fstab-btrfs new file mode 100644 index 0000000..3582111 --- /dev/null +++ b/tests/expected/mount/fstab-btrfs @@ -0,0 +1 @@ +Success diff --git a/tests/ts/mount/fstab-btrfs b/tests/ts/mount/fstab-btrfs new file mode 100755 index 0000000..1c1d0fd --- /dev/null +++ b/tests/ts/mount/fstab-btrfs @@ -0,0 +1,99 @@ +#!/bin/bash + +# +# Copyright (C) 2016 Stanislav Brabec <sbrabec@suse.cz> +# +# This file is part of util-linux. +# +# This file is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This file is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +TS_TOPDIR="${0%/*}/../.." +TS_DESC="btrfs (fstab)" + +. $TS_TOPDIR/functions.sh +ts_init "$*" + +ts_check_test_command "$TS_CMD_MOUNT" +ts_check_test_command "$TS_CMD_UMOUNT" + +ts_skip_nonroot +ts_check_losetup +ts_check_prog "mkfs.btrfs" +ts_check_prog "btrfs" + +ts_device_init 42 +DEVICE=$TS_LODEV +[ -d "$TS_MOUNTPOINT-create" ] || mkdir -p "$TS_MOUNTPOINT-create" +[ -d "$TS_MOUNTPOINT-default" ] || mkdir -p "$TS_MOUNTPOINT-default" +[ -d "$TS_MOUNTPOINT-subvol" ] || mkdir -p "$TS_MOUNTPOINT-subvol" +[ -d "$TS_MOUNTPOINT-subvolid" ] || mkdir -p "$TS_MOUNTPOINT-subvolid" +[ -d "$TS_MOUNTPOINT-bind" ] || mkdir -p "$TS_MOUNTPOINT-bind" +mkfs.btrfs -f -d single -m single $DEVICE &> /dev/null || ts_die "Cannot make btrfs on $DEVICE" + +$TS_CMD_MOUNT -o loop "$DEVICE" "$TS_MOUNTPOINT-create" +pushd . >/dev/null +cd "$TS_MOUNTPOINT-create" +mkdir -p d0/dd0/ddd0 +cd ./d0/dd0/ddd0 +touch file{1..5} +btrfs subvol create s1 >/dev/null +cd ./s1 +touch file{1..5} +mkdir -p d1/dd1/ddd1 +cd ./d1/dd1/ddd1 +btrfs subvol create s2 >/dev/null +DEFAULT_SUBVOLID=$(btrfs inspect rootid s2) +btrfs subvol set-default $DEFAULT_SUBVOLID . >/dev/null +NON_DEFAULT_SUBVOLID=$(btrfs subvol list "$TS_MOUNTPOINT-create" | while read dummy id rest ; do if test $id = $DEFAULT_SUBVOLID ; then continue ; fi ; echo $id ; done) +cd ../../../.. +mkdir -p d2/dd2/ddd2 +cd ./d2/dd2/ddd2 +btrfs subvol create s3 >/dev/null +popd >/dev/null +NON_DEFAULT_SUBVOL=d0/dd0/ddd0/d2/dd2/ddd2/s3 +$TS_CMD_UMOUNT "$TS_MOUNTPOINT-create" + +# Tests with fs == btrfs +# mounting default subvolume, deep in the structure, without entry in fstab +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT-default" "btrfs" "" +# mounting default subvolume, deep in the structure +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT-subvol" "btrfs" "subvol=$NON_DEFAULT_SUBVOL" +# mounting non-default subvolume +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT-subvolid" "btrfs" "subvolid=$NON_DEFAULT_SUBVOLID" + +# test bind mount pointing to subvolume root +ts_fstab_add "$TS_MOUNTPOINT-subvol" "$TS_MOUNTPOINT-bind" "auto" "bind" + +$TS_CMD_MOUNT -a 2>&1 >> $TS_OUTPUT +$TS_CMD_MOUNT -a 2>&1 >> $TS_OUTPUT + +ts_fstab_clean + +# Tests with fs == auto +# mounting default subvolume, deep in the structure, without entry in fstab +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT-default" "auto" "" +# mounting default subvolume, deep in the structure +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT-subvol" "auto" "subvol=$NON_DEFAULT_SUBVOL" +# mounting non-default subvolume +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT-subvolid" "auto" "subvolid=$NON_DEFAULT_SUBVOLID" + +# test bind mount pointing to subvolume sub-directory +mkdir -p "$TS_MOUNTPOINT-subvol/bind-mnt" +ts_fstab_add "$TS_MOUNTPOINT-subvol/bind-mnt" "$TS_MOUNTPOINT-bind" "auto" "bind" + +$TS_CMD_MOUNT -a 2>&1 >> $TS_OUTPUT +$TS_CMD_MOUNT -a 2>&1 >> $TS_OUTPUT + +ts_fstab_clean + +ts_log "Success" +ts_finalize + -- 2.7.0 -- Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: sbrabec@suse.com Lihovarská 1060/12 tel: +49 911 7405384547 190 00 Praha 9 fax: +420 284 084 001 Czech Republic http://www.suse.cz/ PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] tests: add btrfs mount tests (fails!) 2016-02-02 18:00 [PATCH] tests: add btrfs mount tests Stanislav Brabec @ 2016-02-02 20:14 ` Stanislav Brabec 2016-02-03 17:00 ` Stanislav Brabec 2016-02-11 9:43 ` [PATCH] tests: add btrfs mount tests (fails!) Karel Zak 2016-03-14 1:20 ` [PATCH] tests: add btrfs mount tests Ruediger Meier 1 sibling, 2 replies; 16+ messages in thread From: Stanislav Brabec @ 2016-02-02 20:14 UTC (permalink / raw) To: util-linux, David Sterba Stanislav Brabec wrote: > Coverage: I just found a bug in my testcase: To prevent race bind-mnt has to be created in time of fs creation. btrfs subvol create s3 >/dev/null +mkdir -p s3/bind-mnt popd >/dev/null But, what is worse, I found a problem in previous patches. Things are even more complicated than I thought, and patches still not cover all cases. Up to now, I have been thinking that device + subvolid is unique identifier in the mountinfo. But it is not true, at least not when bind mounts take its role. Using bind mounts that point to a sub-directory of a subvolume of btrfs results in two lines in mountinfo that have the same subvolid but different subvol. Second "mount -a" then fails badly. Not only that it mounts the bind mount second time, it also mistakenly mounts another volume over a previously mounted volume. dev/loop0 on /home/sbrabec/VCS/util-linux/tests/output/mount/fstab-btrfs-mnt-subvol type btrfs (rw,relatime,space_cache,subvolid=258,subvol=/d0/dd0/ddd0/d2/dd2/ddd2/s3/bind-mnt) => totally wrong We are back on the beginning. I have to re-evaluate all patches I sent up to now. TODO: Check whether it is possible to directly mount sub-directory of sub-volume that is not a subvolume per self. If yes: - We will need to introduce full path evaluation of btrfs sub-volume, and always compare it with mountinfo. If not: - We will have to search the whole mountinfo for a particular subvolid and and find a shortest subvol string. David, could you bring some light into it? -- Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: sbrabec@suse.com Lihovarská 1060/12 tel: +49 911 7405384547 190 00 Praha 9 fax: +420 284 084 001 Czech Republic http://www.suse.cz/ PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] tests: add btrfs mount tests (fails!) 2016-02-02 20:14 ` [PATCH] tests: add btrfs mount tests (fails!) Stanislav Brabec @ 2016-02-03 17:00 ` Stanislav Brabec 2016-02-03 18:39 ` Stanislav Brabec 2016-02-10 16:03 ` another mount -a problem, not related to btrfs Stanislav Brabec 2016-02-11 9:43 ` [PATCH] tests: add btrfs mount tests (fails!) Karel Zak 1 sibling, 2 replies; 16+ messages in thread From: Stanislav Brabec @ 2016-02-03 17:00 UTC (permalink / raw) To: util-linux, David Sterba Stanislav Brabec wrote: > (rw,relatime,space_cache,subvolid=258,subvol=/d0/dd0/ddd0/d2/dd2/ddd2/s3/bind-mnt) After a discussion with David, we came to the conclusion that the problem is caused by bug in the mountinfo provided by kernel: It reports invalid subvol=/d0/dd0/ddd0/d2/dd2/ddd2/s3/bind-mnt, which breaks both the logic inside new patches, and the convention that it is possible to create a mount command from the options reported by mountinfo. After a further research, we found, that kernel is currently unfixable in a correct way without refactoring of btrfs or changes in vfs. It implies that fstab support for btrfs is not fully fixable. But it is possible to write a heuristic, that can work in most cases, and in other cases it will behave at least reasonably (i. e. not mount again, not mount incorrect directories etc.) Here is the case that CANNOT be fixed correctly: Suppose you have following fstab: /dev/sda1 / btrfs subvol=.snapshots/1/snapshot 0 0 /dev/sda1 /.snapshots btrfs subvol=.snapshots 0 0 Now suppose additional line: /var /mnt/var none bind 0 0 And following line: .snapshots/1/snapshot/var /mnt/var none bind 0 0 However both lines mount the same part of btrfs hierarchy, files inside will have different inode numbers for each. mountinfo provided by the current btrfs is not sufficient to discriminate between them, and what is even worse, in second case it reports incorrect subvolid. As we cannot expect correct fix for that in next say few years, util-linux has to live with lies, incorrect subvol, unmountable subvolid. Conclusion: Don't suppose unique relation between subvol and subvolid. Always go through the whole mountinfo, and find a shortest string corresponding to the particular subvolid. It is the correct one. In case of bind mounts, don't trust subvolid and subvol at all. The only real information is the source. btrfs guesses the rest, often correctly, sometimes incorrectly. subvol is just a copy of this path (possibly unusable in real mount command), subvolid is the id of the nearest parent node, not the real subvolid of the volume the bind mount points to. If device and source matches, the best guess heuristic says, that the bind mount is probably already mounted. (It could be false, but we cannot detect it.) -- Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: sbrabec@suse.com Lihovarská 1060/12 tel: +49 911 7405384547 190 00 Praha 9 fax: +420 284 084 001 Czech Republic http://www.suse.cz/ PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] tests: add btrfs mount tests (fails!) 2016-02-03 17:00 ` Stanislav Brabec @ 2016-02-03 18:39 ` Stanislav Brabec 2016-02-10 16:03 ` another mount -a problem, not related to btrfs Stanislav Brabec 1 sibling, 0 replies; 16+ messages in thread From: Stanislav Brabec @ 2016-02-03 18:39 UTC (permalink / raw) To: util-linux, David Sterba Stanislav Brabec wrote: > However both lines mount the same part of btrfs hierarchy, files inside > will have different inode numbers for each. Fix: inode number will be the same, independently on subvolume actually used, and the reported subvolid. But the mount reference count differs depending on the bind mount source. In the first case, I can run umount /.snapshots in the second, I will get EBUSY. => The "mount -a" problem can be reasonably fixed, but depending on the algorithm, we could encounter unfixable problem during "umount -a". -- Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: sbrabec@suse.com Lihovarská 1060/12 tel: +49 911 7405384547 190 00 Praha 9 fax: +420 284 084 001 Czech Republic http://www.suse.cz/ PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76 ^ permalink raw reply [flat|nested] 16+ messages in thread
* another mount -a problem, not related to btrfs 2016-02-03 17:00 ` Stanislav Brabec 2016-02-03 18:39 ` Stanislav Brabec @ 2016-02-10 16:03 ` Stanislav Brabec 1 sibling, 0 replies; 16+ messages in thread From: Stanislav Brabec @ 2016-02-10 16:03 UTC (permalink / raw) To: util-linux, David Sterba Dne 3.2.2016 v 18:00 Stanislav Brabec napsal(a): > Stanislav Brabec wrote: >> (rw,relatime,space_cache,subvolid=258,subvol=/d0/dd0/ddd0/d2/dd2/ddd2/s3/bind-mnt) >> > > After a discussion with David, we came to the conclusion that the > problem is caused by bug in the mountinfo provided by kernel: > While working on a fix, I found a new bug, not related to btrfs: truncate -s10M source.img mkdir -p /source/subdir mkdir -p /dest /sbin/mkfs.ext4 source.img losetup /dev/loop0 $PWD/source.img echo "/dev/loop0 /source ext4 defaults 0 0" >>/etc/fstab mount -a mkdir -p /source/subdir mount -o bind /source /dest echo "/source/subdir /dest auto bind 0 0" >>/etc/fstab mount -a Got: /dev/loop0 on /source type ext4 (rw,relatime,data=ordered) /dev/loop0 on /dest type ext4 (rw,relatime,data=ordered) /dev/loop0 on /dest type ext4 (rw,relatime,data=ordered) /dev/loop0 on /source type ext4 (rw,relatime,data=ordered) and empty /source It apparently should not happen. -- Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: sbrabec@suse.com Lihovarská 1060/12 tel: +49 911 7405384547 190 00 Praha 9 fax: +420 284 084 001 Czech Republic http://www.suse.cz/ PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] tests: add btrfs mount tests (fails!) 2016-02-02 20:14 ` [PATCH] tests: add btrfs mount tests (fails!) Stanislav Brabec 2016-02-03 17:00 ` Stanislav Brabec @ 2016-02-11 9:43 ` Karel Zak 2016-02-11 13:47 ` Stanislav Brabec 1 sibling, 1 reply; 16+ messages in thread From: Karel Zak @ 2016-02-11 9:43 UTC (permalink / raw) To: Stanislav Brabec; +Cc: util-linux, David Sterba On Tue, Feb 02, 2016 at 09:14:48PM +0100, Stanislav Brabec wrote: > Stanislav Brabec wrote: > >Coverage: > > I just found a bug in my testcase: > To prevent race bind-mnt has to be created in time of fs creation. > > btrfs subvol create s3 >/dev/null > +mkdir -p s3/bind-mnt So, this is enough to fix the testcase, right? I'd like to merge it. Karel -- Karel Zak <kzak@redhat.com> http://karelzak.blogspot.com ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] tests: add btrfs mount tests (fails!) 2016-02-11 9:43 ` [PATCH] tests: add btrfs mount tests (fails!) Karel Zak @ 2016-02-11 13:47 ` Stanislav Brabec 2016-02-11 16:34 ` Stanislav Brabec 2016-02-19 15:02 ` Stanislav Brabec 0 siblings, 2 replies; 16+ messages in thread From: Stanislav Brabec @ 2016-02-11 13:47 UTC (permalink / raw) To: Karel Zak; +Cc: util-linux, David Sterba Karel Zak wrote: > On Tue, Feb 02, 2016 at 09:14:48PM +0100, Stanislav Brabec wrote: >> Stanislav Brabec wrote: >> +mkdir -p s3/bind-mnt > > So, this is enough to fix the testcase, right? I'd like to merge it. > > Karel > Well, not. There are missing umount commands in the middle of the test, which trigger following problem: truncate -s10M source.img mkdir -p /source/subdir mkdir -p /dest /sbin/mkfs.ext4 source.img losetup /dev/loop0 $PWD/source.img mount -text4 /dev/loop0 /source mkdir -p /source/subdir mount -o bind /source /dest mount -o bind /source/subdir /dest Mounting a bind mount over a root of existing bind mount causes mounting over the bind mount source. I expected mounting over the target directory I specified. I am not sure, whether it is correct behavior, but for sure it is surprising. Additionally, I am working on a reproducer of an invalid "mount -a" behavior triggered by kernel bug reporting false subvol. Now I have only a reproducer outside testsuite: #/btrfs_test.img /btrfs_mnt_root auto loop,subvol=/ 0 0 #/btrfs_test.img /btrfs_mnt auto loop,subvolid=257 0 0 #/btrfs_mnt_root/d0/dd0/ddd0/s1/bind-mnt /btrfs_bind auto bind 0 0 Round 1: Uncomment 1st and 3rd line and call mount -a. Round 2: Uncomment 2nd line and call mount -a. Result: Only two mounts exist. I am working on a subtest that will trigger it inside the test suite. I already have a possible fix for it, but I want to confirm that it is correct before sending it upstream. -- Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: sbrabec@suse.com Lihovarská 1060/12 tel: +49 911 7405384547 190 00 Praha 9 fax: +420 284 084 001 Czech Republic http://www.suse.cz/ PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] tests: add btrfs mount tests (fails!) 2016-02-11 13:47 ` Stanislav Brabec @ 2016-02-11 16:34 ` Stanislav Brabec 2016-02-11 18:07 ` [PATCH] tests: add btrfs mount tests Stanislav Brabec 2016-02-12 10:07 ` [PATCH] tests: add btrfs mount tests (fails!) Karel Zak 2016-02-19 15:02 ` Stanislav Brabec 1 sibling, 2 replies; 16+ messages in thread From: Stanislav Brabec @ 2016-02-11 16:34 UTC (permalink / raw) To: Karel Zak; +Cc: util-linux, David Sterba Stanislav Brabec wrote: > I am working on a subtest that will trigger it inside the test suite. > > I already have a possible fix for it, but I want to confirm that it is > correct before sending it upstream. > I just have a reproducer of the new problem. The newly found problem is not btrfs speficic: echo "/ext4.img /ext4_mnt1 auto loop 0 0" >>/etc/fstab mount -a echo "/ext4.img /ext4_mnt2 auto loop 0 0" >>/etc/fstab mount -a mount umount /ext4_mnt1 /ext4_mnt2 sed -i /ext4.img/d /etc/fstab Result: Nothing was mounted into /ext4_mnt2 (This is the problem with loop files in fstab, which I mentioned in my past mails, but was not able to reproduce.) It also means, that I still don't have a reproducer that will demonstrate mount confused by an incorrect subvol provided by kernel. -- Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: sbrabec@suse.com Lihovarská 1060/12 tel: +49 911 7405384547 190 00 Praha 9 fax: +420 284 084 001 Czech Republic http://www.suse.cz/ PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] tests: add btrfs mount tests 2016-02-11 16:34 ` Stanislav Brabec @ 2016-02-11 18:07 ` Stanislav Brabec 2016-02-12 9:38 ` Karel Zak 2016-02-12 10:07 ` [PATCH] tests: add btrfs mount tests (fails!) Karel Zak 1 sibling, 1 reply; 16+ messages in thread From: Stanislav Brabec @ 2016-02-11 18:07 UTC (permalink / raw) To: Karel Zak; +Cc: util-linux, David Sterba btrfs needs a special support in mount. Add a testcase for btrfs specific problems. Coverage: 352740e8: bind mounts pointing to btrfs 2cd28fc8: mounting default subvolume d2f82678: use of "auto" 618a8814: use of "subvolid" Signed-off-by: Stanislav Brabec <sbrabec@suse.cz> --- tests/expected/mount/fstab-btrfs | 1 + tests/expected/mount/fstab-btrfs-auto | 1 + tests/expected/mount/fstab-btrfs-btrfs | 1 + tests/ts/mount/fstab-btrfs | 128 +++++++++++++++++++++++++++++++++ 4 files changed, 131 insertions(+) create mode 100644 tests/expected/mount/fstab-btrfs create mode 100644 tests/expected/mount/fstab-btrfs-auto create mode 100644 tests/expected/mount/fstab-btrfs-btrfs create mode 100755 tests/ts/mount/fstab-btrfs diff --git a/tests/expected/mount/fstab-btrfs b/tests/expected/mount/fstab-btrfs new file mode 100644 index 0000000..3582111 --- /dev/null +++ b/tests/expected/mount/fstab-btrfs @@ -0,0 +1 @@ +Success diff --git a/tests/expected/mount/fstab-btrfs-auto b/tests/expected/mount/fstab-btrfs-auto new file mode 100644 index 0000000..3582111 --- /dev/null +++ b/tests/expected/mount/fstab-btrfs-auto @@ -0,0 +1 @@ +Success diff --git a/tests/expected/mount/fstab-btrfs-btrfs b/tests/expected/mount/fstab-btrfs-btrfs new file mode 100644 index 0000000..3582111 --- /dev/null +++ b/tests/expected/mount/fstab-btrfs-btrfs @@ -0,0 +1 @@ +Success diff --git a/tests/ts/mount/fstab-btrfs b/tests/ts/mount/fstab-btrfs new file mode 100755 index 0000000..0b11fd6 --- /dev/null +++ b/tests/ts/mount/fstab-btrfs @@ -0,0 +1,128 @@ +#!/bin/bash + +# +# Copyright (C) 2016 Stanislav Brabec <sbrabec@suse.cz> +# +# This file is part of util-linux. +# +# This file is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This file is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +TS_TOPDIR="${0%/*}/../.." +TS_DESC="btrfs (fstab)" + +. $TS_TOPDIR/functions.sh +ts_init "$*" + +ts_check_test_command "$TS_CMD_MOUNT" +ts_check_test_command "$TS_CMD_UMOUNT" + +ts_skip_nonroot +ts_check_losetup +ts_check_prog "mkfs.btrfs" +ts_check_prog "btrfs" + +TS_MOUNTPOINT_ANY="$TS_MOUNTPOINT" +TS_MOUNTPOINT_CREATE="$TS_MOUNTPOINT-create" +TS_MOUNTPOINT_DEFAULT="$TS_MOUNTPOINT-default" +TS_MOUNTPOINT_SUBVOL="$TS_MOUNTPOINT-subvol" +TS_MOUNTPOINT_SUBVOLID="$TS_MOUNTPOINT-subvolid" +TS_MOUNTPOINT_BIND="$TS_MOUNTPOINT-bind" + +ts_device_init 42 +DEVICE=$TS_LODEV +[ -d "$TS_MOUNTPOINT_CREATE" ] || mkdir -p "$TS_MOUNTPOINT_CREATE" +[ -d "$TS_MOUNTPOINT_DEFAULT" ] || mkdir -p "$TS_MOUNTPOINT_DEFAULT" +[ -d "$TS_MOUNTPOINT_SUBVOL" ] || mkdir -p "$TS_MOUNTPOINT_SUBVOL" +[ -d "$TS_MOUNTPOINT_SUBVOLID" ] || mkdir -p "$TS_MOUNTPOINT_SUBVOLID" +[ -d "$TS_MOUNTPOINT_BIND" ] || mkdir -p "$TS_MOUNTPOINT_BIND" +mkfs.btrfs -f -d single -m single $DEVICE &> /dev/null || ts_die "Cannot make btrfs on $DEVICE" + +$TS_CMD_MOUNT -o loop "$DEVICE" "$TS_MOUNTPOINT_CREATE" +pushd . >/dev/null +cd "$TS_MOUNTPOINT_CREATE" +mkdir -p d0/dd0/ddd0 +cd ./d0/dd0/ddd0 +touch file{1..5} +btrfs subvol create s1 >/dev/null +cd ./s1 +touch file{1..5} +mkdir bind-point +mkdir -p d1/dd1/ddd1 +cd ./d1/dd1/ddd1 +btrfs subvol create s2 >/dev/null +DEFAULT_SUBVOLID=$(btrfs inspect rootid s2) +btrfs subvol set-default $DEFAULT_SUBVOLID . >/dev/null +NON_DEFAULT_SUBVOLID=$(btrfs subvol list "$TS_MOUNTPOINT_CREATE" | while read dummy id rest ; do if test $id = $DEFAULT_SUBVOLID ; then continue ; fi ; echo $id ; done) +cd ../../../.. +mkdir -p d2/dd2/ddd2 +cd ./d2/dd2/ddd2 +btrfs subvol create s3 >/dev/null +mkdir -p s3/bind-mnt +popd >/dev/null +NON_DEFAULT_SUBVOL=d0/dd0/ddd0/d2/dd2/ddd2/s3 +$TS_CMD_UMOUNT "$TS_MOUNTPOINT_CREATE" + + +ts_init_subtest "btrfs" +# Tests with fs == btrfs +# mounting default subvolume, deep in the structure, without entry in fstab +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT_DEFAULT" "btrfs" "" +# mounting default subvolume, deep in the structure +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT_SUBVOL" "btrfs" "subvol=$NON_DEFAULT_SUBVOL" +# mounting non-default subvolume +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT_SUBVOLID" "btrfs" "subvolid=$NON_DEFAULT_SUBVOLID" +# test bind mount pointing to subvolume root +ts_fstab_add "$TS_MOUNTPOINT_SUBVOLID" "$TS_MOUNTPOINT_BIND" "auto" "bind" + +$TS_CMD_MOUNT -a 2>&1 >> $TS_OUTPUT +$TS_CMD_MOUNT -a 2>&1 >> $TS_OUTPUT + +$TS_CMD_UMOUNT "$TS_MOUNTPOINT_BIND" 2>&1 >> $TS_OUTPUT +$TS_CMD_UMOUNT "$TS_MOUNTPOINT_DEFAULT" 2>&1 >> $TS_OUTPUT +$TS_CMD_UMOUNT "$TS_MOUNTPOINT_SUBVOL" 2>&1 >> $TS_OUTPUT +$TS_CMD_UMOUNT "$TS_MOUNTPOINT_SUBVOLID" 2>&1 >> $TS_OUTPUT + +# check that everything was unmounted +$TS_CMD_MOUNT | grep "$TS_MOUNTPOINT_ANY" 2>&1 >> $TS_OUTPUT + +ts_fstab_clean +ts_log "Success" +ts_finalize_subtest + +ts_init_subtest "auto" +# Tests with fs == auto +# mounting default subvolume, deep in the structure, without entry in fstab +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT_DEFAULT" "auto" "" +# mounting default subvolume, deep in the structure +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT_SUBVOL" "auto" "subvol=$NON_DEFAULT_SUBVOL" +# mounting non-default subvolume +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT_SUBVOLID" "auto" "subvolid=$NON_DEFAULT_SUBVOLID" +# test bind mount pointing to subvolume sub-directory +ts_fstab_add "$TS_MOUNTPOINT_SUBVOL/bind-mnt" "$TS_MOUNTPOINT_BIND" "auto" "bind" + +$TS_CMD_MOUNT -a 2>&1 >> $TS_OUTPUT +$TS_CMD_MOUNT -a 2>&1 >> $TS_OUTPUT + +$TS_CMD_UMOUNT "$TS_MOUNTPOINT_BIND" 2>&1 >> $TS_OUTPUT +$TS_CMD_UMOUNT "$TS_MOUNTPOINT_DEFAULT" 2>&1 >> $TS_OUTPUT +$TS_CMD_UMOUNT "$TS_MOUNTPOINT_SUBVOL" 2>&1 >> $TS_OUTPUT +$TS_CMD_UMOUNT "$TS_MOUNTPOINT_SUBVOLID" 2>&1 >> $TS_OUTPUT + +# check that everything was unmounted +$TS_CMD_MOUNT | grep "$TS_MOUNTPOINT_ANY" 2>&1 >> $TS_OUTPUT + +ts_fstab_clean +ts_log "Success" +ts_finalize_subtest + + +ts_finalize + -- 2.7.0 -- Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: sbrabec@suse.com Lihovarská 1060/12 tel: +49 911 7405384547 190 00 Praha 9 fax: +420 284 084 001 Czech Republic http://www.suse.cz/ PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] tests: add btrfs mount tests 2016-02-11 18:07 ` [PATCH] tests: add btrfs mount tests Stanislav Brabec @ 2016-02-12 9:38 ` Karel Zak 0 siblings, 0 replies; 16+ messages in thread From: Karel Zak @ 2016-02-12 9:38 UTC (permalink / raw) To: Stanislav Brabec; +Cc: util-linux, David Sterba On Thu, Feb 11, 2016 at 07:07:22PM +0100, Stanislav Brabec wrote: > tests/expected/mount/fstab-btrfs | 1 + > tests/expected/mount/fstab-btrfs-auto | 1 + > tests/expected/mount/fstab-btrfs-btrfs | 1 + > tests/ts/mount/fstab-btrfs | 128 +++++++++++++++++++++++++++++++++ > 4 files changed, 131 insertions(+) > create mode 100644 tests/expected/mount/fstab-btrfs > create mode 100644 tests/expected/mount/fstab-btrfs-auto > create mode 100644 tests/expected/mount/fstab-btrfs-btrfs > create mode 100755 tests/ts/mount/fstab-btrfs Applied, thanks! (Seems you're familiar with our test suite now, let's hope we will see more tests from you;-) Karel -- Karel Zak <kzak@redhat.com> http://karelzak.blogspot.com ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] tests: add btrfs mount tests (fails!) 2016-02-11 16:34 ` Stanislav Brabec 2016-02-11 18:07 ` [PATCH] tests: add btrfs mount tests Stanislav Brabec @ 2016-02-12 10:07 ` Karel Zak 1 sibling, 0 replies; 16+ messages in thread From: Karel Zak @ 2016-02-12 10:07 UTC (permalink / raw) To: Stanislav Brabec; +Cc: util-linux, David Sterba On Thu, Feb 11, 2016 at 05:34:27PM +0100, Stanislav Brabec wrote: > Stanislav Brabec wrote: > >I am working on a subtest that will trigger it inside the test suite. > > > >I already have a possible fix for it, but I want to confirm that it is > >correct before sending it upstream. > > > I just have a reproducer of the new problem. > > The newly found problem is not btrfs speficic: > > echo "/ext4.img /ext4_mnt1 auto loop 0 0" >>/etc/fstab > mount -a > echo "/ext4.img /ext4_mnt2 auto loop 0 0" >>/etc/fstab > mount -a > mount > umount /ext4_mnt1 /ext4_mnt2 > sed -i /ext4.img/d /etc/fstab > > Result: > Nothing was mounted into /ext4_mnt2 > > (This is the problem with loop files in fstab, which I mentioned in my past > mails, but was not able to reproduce.) Fixed. There was "break" rather than "continue" in the mnt_table_is_fs_mounted(). Karel -- Karel Zak <kzak@redhat.com> http://karelzak.blogspot.com ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] tests: add btrfs mount tests (fails!) 2016-02-11 13:47 ` Stanislav Brabec 2016-02-11 16:34 ` Stanislav Brabec @ 2016-02-19 15:02 ` Stanislav Brabec 2016-03-09 19:13 ` Stanislav Brabec 1 sibling, 1 reply; 16+ messages in thread From: Stanislav Brabec @ 2016-02-19 15:02 UTC (permalink / raw) To: Karel Zak; +Cc: util-linux, David Sterba On Feb 11 2016 at 14:47 Stanislav Brabec wrote: > There are missing umount commands in the middle of the test, which > trigger following problem: > > truncate -s10M source.img > mkdir -p /source/subdir > mkdir -p /dest > /sbin/mkfs.ext4 source.img > losetup /dev/loop0 $PWD/source.img > mount -text4 /dev/loop0 /source > mkdir -p /source/subdir > mount -o bind /source /dest > mount -o bind /source/subdir /dest > > Mounting a bind mount over a root of existing bind mount causes mounting > over the bind mount source. I expected mounting over the target > directory I specified. > > I am not sure, whether it is correct behavior, but for sure it is > surprising. We discussed it with David and considered that it is an unexpected but correct behavior, that is a consequence of systemd introducing new default behavior for mounts. All bind mounts are shared by default, which causes propagation of a mount inside a target (including the target root) into a source. > Additionally, I am working on a reproducer of an invalid "mount -a" > behavior triggered by kernel bug reporting false subvol. > I already have a possible fix for it, but I want to confirm that it is > correct before sending it upstream. I finally found a reproducer. It is a negative reproducer, i. e. fstab that should cause error, but it does not. I am still not sure, whether positive reproducer exists. I will send a fix and testcase in a separate thread. After that, all discovered btrfs mount issues will be finally fixed. There is another known problem with non intuitive output of lsblk with btrfs, but it is above the scope of this thread. -- Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: sbrabec@suse.com Lihovarská 1060/12 tel: +49 911 7405384547 190 00 Praha 9 fax: +420 284 084 001 Czech Republic http://www.suse.cz/ PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] tests: add btrfs mount tests (fails!) 2016-02-19 15:02 ` Stanislav Brabec @ 2016-03-09 19:13 ` Stanislav Brabec 0 siblings, 0 replies; 16+ messages in thread From: Stanislav Brabec @ 2016-03-09 19:13 UTC (permalink / raw) To: Karel Zak; +Cc: util-linux, David Sterba On Feb 19, 2016 at 16:02 Stanislav Brabec wrote: > On Feb 11 2016 at 14:47 Stanislav Brabec wrote: >> There are missing umount commands in the middle of the test, which >> trigger following problem: >> >> Additionally, I am working on a reproducer of an invalid "mount -a" >> behavior triggered by kernel bug reporting false subvol. > >> I already have a possible fix for it, but I want to confirm that it is >> correct before sending it upstream. > > I finally found a reproducer. > Well, now I have a real reproducer that is not triggered by another bug. My findings: 1) The way to reproduce is extremely obscure, because not only subvolid has to match, but also target. 2) There is no way to fix it in an easy way. I suggested a new algorithm searching for shortest subvol, but it would be vulnerable to bad evaluation as well. So I decided to keep the code as it is. Way to reproduce: To confuse the algorithm, there has to be a mounted something else into the target than fstab specifies. Step 1: /dev/loop0 /btrfs_mnt_root btrfs subvol=/ 0 0 /btrfs_mnt_root/d0/dd0/ddd0/s1/d1/dd1/ddd1/s2/bind-mnt /btrfs_mnt_bind btrfs bind,private 0 0 It will create following mountinfo: 299 72 0:83 / /btrfs_mnt_root rw,relatime shared:211 - btrfs /dev/loop0 rw,space_cache,subvolid=5,subvol=/ 312 72 0:83 /d0/dd0/ddd0/s1/d1/dd1/ddd1/s2/bind-mnt /btrfs_mnt_bind rw,relatime shared:211 - btrfs /dev/loop0 rw,space_cache,subvolid=258,subvol=/d0/dd0/ddd0/s1/d1/dd1/ddd1/s2/bind-mnt Note that subvolid 258 does not correspond to / nor to d0/dd0/ddd0/s1/d1/dd1/ddd1/s2/bind-mnt, but it corresponds to d0/dd0/ddd0/s1/d1/dd1/ddd1/s2. Now remove both lines from fstab and add: /dev/loop0 /btrfs_mnt_bind btrfs defaults 0 0 Default subvolume is d0/dd0/ddd0/s1/d1/dd1/ddd1/s2, but the algorithm (both the existing and the previously suggested one) falsely assumes that it is /d0/dd0/ddd0/s1/d1/dd1/ddd1/s2/bind-mnt, because both subvolid and target match. In theory, the new fstab should cause mounting the default subvolume over the existing one, but nothing happens. Summary: - The full fix would mean evaluation of the btrfs subvolume tree structure (several ioctl() with possible need of disk lookups) just to find, whether it is already mounted or no. - The reproducer is really obscure, and nothing from a real life cannot trigger it. - The full fix will be vulnerable to another unfixable problems (e. g. when somebody changes the default subvolume path while the device is mounted). => I propose to keep this part of code with a known bug. Here is the the previously proposed (obsolete) fix: --- libmount/src/tab.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/libmount/src/tab.c b/libmount/src/tab.c index 102ed25..556d835 100644 --- a/libmount/src/tab.c +++ b/libmount/src/tab.c @@ -1224,6 +1224,18 @@ static char *remove_mountpoint_from_path(const char *path, const char *mnt) } #ifdef HAVE_BTRFS_SUPPORT +/* All currently existing kernels reports incorrect values for bind + * mounts. Instead of subvol/subvolid of a subvolume used for creating of bind + * mount, it reports subvolid corresponding to nearest upper subvol node and + * subvol pointing to the current dir relative to btrfs root. + * + * The current kernel btrfs implementation has not access to proper VFS + * structures, so it is just guessing. Fixing this will need refactoring of + * kernel btrfs subvol implementation. It is not expected to happen anytime + * soon. Set this define to suppose that kernels are buggy, and don't blindly + * trust to the reported mountinfo values. */ +#define KERNEL_BTRFS_REPORTS_FALSE_SUBVOL 1 + static int get_btrfs_fs_root(struct libmnt_table *tb, struct libmnt_fs *fs, char **root) { char *vol = NULL, *p; @@ -1236,6 +1248,12 @@ static int get_btrfs_fs_root(struct libmnt_table *tb, struct libmnt_fs *fs, char char *target; struct libmnt_fs *f; char subvolidstr[sizeof(stringify_value(UINT64_MAX))]; +#ifdef KERNEL_BTRFS_REPORTS_FALSE_SUBVOL + struct libmnt_fs *t; + struct libmnt_iter itr; + char *optval = NULL; + size_t optvalsz = 0, valsz, bestvalsz = PATH_MAX; +#endif DBG(BTRFS, ul_debug(" found subvolid=%s, checking", vol)); @@ -1248,9 +1266,24 @@ static int get_btrfs_fs_root(struct libmnt_table *tb, struct libmnt_fs *fs, char goto err; DBG(BTRFS, ul_debug(" tring target=%s subvolid=%s", target, subvolidstr)); +#ifndef KERNEL_BTRFS_REPORTS_FALSE_SUBVOL f = mnt_table_find_target_with_option(tb, target, "subvolid", subvolidstr, MNT_ITER_BACKWARD); +#else + f = NULL; + valsz = strlen(subvolidstr); + mnt_reset_iter(&itr, MNT_ITER_BACKWARD); + while (mnt_table_next_fs(tb, &itr, &t) == 0) { + if (mnt_fs_streq_target(t, target) + && mnt_fs_get_option(t, "subvolid", &optval, &optvalsz) == 0 + && optvalsz == valsz + && strncmp(optval, subvolidstr, optvalsz) == 0 + && optvalsz < bestvalsz) + f = t; + } +#endif + if (!tb->cache) free(target); if (!f) @@ -1271,6 +1304,12 @@ static int get_btrfs_fs_root(struct libmnt_table *tb, struct libmnt_fs *fs, char char *target; struct libmnt_fs *f; char default_id_str[sizeof(stringify_value(UINT64_MAX))]; +#ifdef KERNEL_BTRFS_REPORTS_FALSE_SUBVOL + struct libmnt_fs *t; + struct libmnt_iter itr; + char *optval = NULL; + size_t optvalsz = 0, valsz, bestvalsz = PATH_MAX; +#endif DBG(BTRFS, ul_debug(" subvolid/subvol not found, checking default")); @@ -1295,9 +1334,24 @@ static int get_btrfs_fs_root(struct libmnt_table *tb, struct libmnt_fs *fs, char DBG(BTRFS, ul_debug(" tring target=%s default subvolid=%s", target, default_id_str)); +#ifndef KERNEL_BTRFS_REPORTS_FALSE_SUBVOL f = mnt_table_find_target_with_option(tb, target, "subvolid", default_id_str, MNT_ITER_BACKWARD); +#else + f = NULL; + valsz = strlen(default_id_str); + mnt_reset_iter(&itr, MNT_ITER_BACKWARD); + while (mnt_table_next_fs(tb, &itr, &t) == 0) { + if (mnt_fs_streq_target(t, target) + && mnt_fs_get_option(t, "subvolid", &optval, &optvalsz) == 0 + && optvalsz == valsz + && strncmp(optval, default_id_str, optvalsz) == 0 + && optvalsz < bestvalsz) + f = t; + } +#endif + if (!tb->cache) free(target); if (!f) -- 2.7.2 -- Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: sbrabec@suse.com Lihovarská 1060/12 tel: +49 911 7405384547 190 00 Praha 9 fax: +420 284 084 001 Czech Republic http://www.suse.cz/ PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] tests: add btrfs mount tests 2016-02-02 18:00 [PATCH] tests: add btrfs mount tests Stanislav Brabec 2016-02-02 20:14 ` [PATCH] tests: add btrfs mount tests (fails!) Stanislav Brabec @ 2016-03-14 1:20 ` Ruediger Meier 2016-03-14 14:19 ` Stanislav Brabec 1 sibling, 1 reply; 16+ messages in thread From: Ruediger Meier @ 2016-03-14 1:20 UTC (permalink / raw) To: Stanislav Brabec; +Cc: util-linux On Tuesday 02 February 2016, Stanislav Brabec wrote: > btrfs needs a special support in mount. Add a testcase for btrfs > specific problems. IMO this test needs some more error handling, see below. > Coverage: > 352740e8: bind mounts pointing to btrfs subvolume > 2cd28fc8: mounting default subvolume > d2f82678: use of "auto" > 618a8814: use of "subvolid" > > Signed-off-by: Stanislav Brabec <sbrabec@suse.cz> > --- > tests/expected/mount/fstab-btrfs | 1 + > tests/ts/mount/fstab-btrfs | 99 > ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 100 > insertions(+) > create mode 100644 tests/expected/mount/fstab-btrfs > create mode 100755 tests/ts/mount/fstab-btrfs > > diff --git a/tests/expected/mount/fstab-btrfs > b/tests/expected/mount/fstab-btrfs new file mode 100644 > index 0000000..3582111 > --- /dev/null > +++ b/tests/expected/mount/fstab-btrfs > @@ -0,0 +1 @@ > +Success > diff --git a/tests/ts/mount/fstab-btrfs b/tests/ts/mount/fstab-btrfs > new file mode 100755 > index 0000000..1c1d0fd > --- /dev/null > +++ b/tests/ts/mount/fstab-btrfs > @@ -0,0 +1,99 @@ > +#!/bin/bash > + > +# > +# Copyright (C) 2016 Stanislav Brabec <sbrabec@suse.cz> > +# > +# This file is part of util-linux. > +# > +# This file is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published > by +# the Free Software Foundation; either version 2 of the License, > or +# (at your option) any later version. > +# > +# This file is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +TS_TOPDIR="${0%/*}/../.." > +TS_DESC="btrfs (fstab)" > + > +. $TS_TOPDIR/functions.sh > +ts_init "$*" > + > +ts_check_test_command "$TS_CMD_MOUNT" > +ts_check_test_command "$TS_CMD_UMOUNT" > + > +ts_skip_nonroot > +ts_check_losetup > +ts_check_prog "mkfs.btrfs" > +ts_check_prog "btrfs" > + > +ts_device_init 42 > +DEVICE=$TS_LODEV > +[ -d "$TS_MOUNTPOINT-create" ] || mkdir -p "$TS_MOUNTPOINT-create" > +[ -d "$TS_MOUNTPOINT-default" ] || mkdir -p "$TS_MOUNTPOINT-default" > +[ -d "$TS_MOUNTPOINT-subvol" ] || mkdir -p "$TS_MOUNTPOINT-subvol" > +[ -d "$TS_MOUNTPOINT-subvolid" ] || mkdir -p > "$TS_MOUNTPOINT-subvolid" +[ -d "$TS_MOUNTPOINT-bind" ] || mkdir -p > "$TS_MOUNTPOINT-bind" +mkfs.btrfs -f -d single -m single $DEVICE &> > /dev/null || ts_die "Cannot make btrfs on $DEVICE" + > +$TS_CMD_MOUNT -o loop "$DEVICE" "$TS_MOUNTPOINT-create" > +pushd . >/dev/null > +cd "$TS_MOUNTPOINT-create" > +mkdir -p d0/dd0/ddd0 > +cd ./d0/dd0/ddd0 > +touch file{1..5} > +btrfs subvol create s1 >/dev/null > +cd ./s1 > +touch file{1..5} > +mkdir -p d1/dd1/ddd1 > +cd ./d1/dd1/ddd1 > +btrfs subvol create s2 >/dev/null For example if you comment out above line (or if it would fail) then all subtests below still succeed. This can't be right. > +DEFAULT_SUBVOLID=$(btrfs inspect rootid s2) > +btrfs subvol set-default $DEFAULT_SUBVOLID . >/dev/null > +NON_DEFAULT_SUBVOLID=$(btrfs subvol list "$TS_MOUNTPOINT-create" | > while read dummy id rest ; do if test $id = $DEFAULT_SUBVOLID ; then > continue ; fi ; echo $id ; done) +cd ../../../.. > +mkdir -p d2/dd2/ddd2 > +cd ./d2/dd2/ddd2 > +btrfs subvol create s3 >/dev/null > +popd >/dev/null > +NON_DEFAULT_SUBVOL=d0/dd0/ddd0/d2/dd2/ddd2/s3 > +$TS_CMD_UMOUNT "$TS_MOUNTPOINT-create" > + > +# Tests with fs == btrfs > +# mounting default subvolume, deep in the structure, without entry > in fstab +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT-default" "btrfs" "" > +# mounting default subvolume, deep in the structure > +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT-subvol" "btrfs" > "subvol=$NON_DEFAULT_SUBVOL" +# mounting non-default subvolume > +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT-subvolid" "btrfs" > "subvolid=$NON_DEFAULT_SUBVOLID" + > +# test bind mount pointing to subvolume root > +ts_fstab_add "$TS_MOUNTPOINT-subvol" "$TS_MOUNTPOINT-bind" "auto" > "bind" + > +$TS_CMD_MOUNT -a 2>&1 >> $TS_OUTPUT > +$TS_CMD_MOUNT -a 2>&1 >> $TS_OUTPUT > + > +ts_fstab_clean > + > +# Tests with fs == auto > +# mounting default subvolume, deep in the structure, without entry > in fstab +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT-default" "auto" "" > +# mounting default subvolume, deep in the structure > +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT-subvol" "auto" > "subvol=$NON_DEFAULT_SUBVOL" +# mounting non-default subvolume > +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT-subvolid" "auto" > "subvolid=$NON_DEFAULT_SUBVOLID" + > +# test bind mount pointing to subvolume sub-directory > +mkdir -p "$TS_MOUNTPOINT-subvol/bind-mnt" > +ts_fstab_add "$TS_MOUNTPOINT-subvol/bind-mnt" "$TS_MOUNTPOINT-bind" > "auto" "bind" + > +$TS_CMD_MOUNT -a 2>&1 >> $TS_OUTPUT > +$TS_CMD_MOUNT -a 2>&1 >> $TS_OUTPUT > + > +ts_fstab_clean > + > +ts_log "Success" > +ts_finalize > + > -- > 2.7.0 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] tests: add btrfs mount tests 2016-03-14 1:20 ` [PATCH] tests: add btrfs mount tests Ruediger Meier @ 2016-03-14 14:19 ` Stanislav Brabec 2016-03-14 23:27 ` Ruediger Meier 0 siblings, 1 reply; 16+ messages in thread From: Stanislav Brabec @ 2016-03-14 14:19 UTC (permalink / raw) To: Ruediger Meier; +Cc: util-linux On Mar 14, 2016 at 02:20 Ruediger Meier wrote: > On Tuesday 02 February 2016, Stanislav Brabec wrote: > IMO this test needs some more error handling, see below. > >> +btrfs subvol create s2 >/dev/null > > For example if you comment out above line (or if it would fail) then > all subtests below still succeed. This can't be right. Well, it is a test suite for mount, not btrfs. I did not add any error checks in the code that creates the testing image. Grepping through the whole test suite, I see that more than a half of all tests do not check for mkfs failure. Here we just have a multi-line mkfs. It is possible to add a check for every command there, and fail the test if the command fails or returns something unexpected. >> +NON_DEFAULT_SUBVOLID=$(btrfs subvol list "$TS_MOUNTPOINT-create" | NON_DEFAULT_SUBVOLID will be empty. >> "subvolid=$NON_DEFAULT_SUBVOLID" + I am not sure how kernel will interpret subvolid=. -- Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: sbrabec@suse.com Lihovarská 1060/12 tel: +49 911 7405384547 190 00 Praha 9 fax: +420 284 084 001 Czech Republic http://www.suse.cz/ PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] tests: add btrfs mount tests 2016-03-14 14:19 ` Stanislav Brabec @ 2016-03-14 23:27 ` Ruediger Meier 0 siblings, 0 replies; 16+ messages in thread From: Ruediger Meier @ 2016-03-14 23:27 UTC (permalink / raw) To: Stanislav Brabec; +Cc: util-linux On Monday 14 March 2016, Stanislav Brabec wrote: > On Mar 14, 2016 at 02:20 Ruediger Meier wrote: > > On Tuesday 02 February 2016, Stanislav Brabec wrote: > > IMO this test needs some more error handling, see below. > > > >> +btrfs subvol create s2 >/dev/null > > > > For example if you comment out above line (or if it would fail) > > then all subtests below still succeed. This can't be right. > > Well, it is a test suite for mount, not btrfs. I did not add any > error checks in the code that creates the testing image. > Grepping through the whole test suite, I see that more than a half of > all tests do not check for mkfs failure. Here we just have a > multi-line mkfs. > > It is possible to add a check for every command there, and fail the > test if the command fails or returns something unexpected. You are right. I've added already some lines to skip outdated btrfs-tools. Nethertheless both subtests succeed even when they operate on an empty device without any filesystem or subvolume. Ideally a test should be skipped if the system is broken. But if we don't have "skip rules" implemented yet then it should better fail than succeed. I know that we have a lot of such issues in our test suite. Just commented this one because it was recently added and I've noticed it on existing systems. > >> +NON_DEFAULT_SUBVOLID=$(btrfs subvol list "$TS_MOUNTPOINT-create" > >> | > > NON_DEFAULT_SUBVOLID will be empty. > > >> "subvolid=$NON_DEFAULT_SUBVOLID" + > > I am not sure how kernel will interpret subvolid=. Maybe just log the btrfs return codes to TS_OUTPUT. If we see failure in real life then we know that something has to be fixed, either the skip rules or the code to create subvols. cu, Rudi ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2016-03-14 23:27 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-02 18:00 [PATCH] tests: add btrfs mount tests Stanislav Brabec 2016-02-02 20:14 ` [PATCH] tests: add btrfs mount tests (fails!) Stanislav Brabec 2016-02-03 17:00 ` Stanislav Brabec 2016-02-03 18:39 ` Stanislav Brabec 2016-02-10 16:03 ` another mount -a problem, not related to btrfs Stanislav Brabec 2016-02-11 9:43 ` [PATCH] tests: add btrfs mount tests (fails!) Karel Zak 2016-02-11 13:47 ` Stanislav Brabec 2016-02-11 16:34 ` Stanislav Brabec 2016-02-11 18:07 ` [PATCH] tests: add btrfs mount tests Stanislav Brabec 2016-02-12 9:38 ` Karel Zak 2016-02-12 10:07 ` [PATCH] tests: add btrfs mount tests (fails!) Karel Zak 2016-02-19 15:02 ` Stanislav Brabec 2016-03-09 19:13 ` Stanislav Brabec 2016-03-14 1:20 ` [PATCH] tests: add btrfs mount tests Ruediger Meier 2016-03-14 14:19 ` Stanislav Brabec 2016-03-14 23:27 ` Ruediger Meier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox