tests: add btrfs mount tests
PATCH 1/1 tests: add btrfs mount tests
btrfs needs a special support in mount. Add a testcase for btrfs specific problems.
Coverage:352740e8: bind mounts pointing to btrfs subvolume2cd28fc8: mounting default subvolumed2f82678: use of "auto"618a8814: use of "subvolid"
--- 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
tests/expected/mount/fstab-btrfs new file +1 -0
@@ -0,0 +1 @@ +Success
tests/ts/mount/fstab-btrfs new file +99 -0
99 lines not on this page · raw message
Signature · 13 lines
2.7.0 -- Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: [email protected] 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
Re: [PATCH] tests: add btrfs mount tests (fails!)
Quoted 1 line · Stanislav Brabec
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?
Signature · 10 lines
Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: [email protected] 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
Re: [PATCH] tests: add btrfs mount tests (fails!)
Quoted 1 line · Stanislav Brabec
(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.)
Signature · 10 lines
Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: [email protected] 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
Re: [PATCH] tests: add btrfs mount tests (fails!)
Quoted 2 lines · Stanislav Brabec
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".
Signature · 10 lines
Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: [email protected] 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
another mount -a problem, not related to btrfs
In reply to Stanislav BrabecDne 3.2.2016 v 18:00 Stanislav Brabec napsal(a):
Quoted 7 lines
Quoted 2 lines · Stanislav Brabec
(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.
Signature · 10 lines
Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: [email protected] 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
Karel Zak
maintainerRe: [PATCH] tests: add btrfs mount tests (fails!)
In reply to Stanislav BrabecQuoted 8 lines · Stanislav Brabec
Quoted 1 line · Stanislav Brabec
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
Signature · 2 lines
Karel Zak <[email protected]> http://karelzak.blogspot.com
Re: [PATCH] tests: add btrfs mount tests (fails!)
Quoted 8 lines · Karel Zak
Quoted 2 lines · Stanislav Brabec
Stanislav Brabec wrote:
+mkdir -p s3/bind-mntSo, 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.
Signature · 10 lines
Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: [email protected] 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
Re: [PATCH] tests: add btrfs mount tests (fails!)
Quoted 5 lines · Stanislav Brabec
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.
Signature · 10 lines
Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: [email protected] 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
PATCH 1/1 tests: add btrfs mount tests
btrfs needs a special support in mount. Add a testcase for btrfs specific problems.
Coverage:352740e8: bind mounts pointing to btrfs2cd28fc8: mounting default subvolumed2f82678: use of "auto"618a8814: use of "subvolid"
--- 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
tests/expected/mount/fstab-btrfs new file +1 -0
@@ -0,0 +1 @@ +Success
tests/expected/mount/fstab-btrfs-auto new file +1 -0
@@ -0,0 +1 @@ +Success
tests/expected/mount/fstab-btrfs-btrfs new file +1 -0
@@ -0,0 +1 @@ +Success
tests/ts/mount/fstab-btrfs new file +128 -0
128 lines not on this page · raw message
Signature · 13 lines
2.7.0 -- Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: [email protected] 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
Quoted 9 lines · Stanislav Brabec
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
Signature · 2 lines
Karel Zak <[email protected]> http://karelzak.blogspot.com
Karel Zak
maintainerRe: [PATCH] tests: add btrfs mount tests (fails!)
In reply to Stanislav BrabecQuoted 18 lines · Stanislav Brabec
Quoted 5 lines · Stanislav Brabec
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
Quoted 5 lines
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
Signature · 2 lines
Karel Zak <[email protected]> http://karelzak.blogspot.com
Re: [PATCH] tests: add btrfs mount tests (fails!)
In reply to Stanislav BrabecQuoted 13 lines · 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.ext4source.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
Quoted 6 lines
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.
Quoted 5 lines
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.
Signature · 10 lines
Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: [email protected] 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
Re: [PATCH] tests: add btrfs mount tests (fails!)
Quoted 4 lines · On Feb 19, 2016 at 16:02 Stanislav Brabec wrote:
Quoted 3 lines · 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:
Quoted 8 lines
Quoted 5 lines
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(+)
libmount/src/tab.c +54 -0
@@ -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, charchar *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, charchar *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, charDBG (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)
Signature · 13 lines
2.7.2 -- Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: [email protected] 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
Quoted 2 lines · Stanislav Brabec
btrfs needs a special support in mount. Add a testcase for btrfs specific problems.
IMO this test needs some more error handling, see below.
Quoted 70 lines
Coverage:
352740e8: bind mounts pointing to btrfs subvolume2cd28fc8: mounting default subvolumed2f82678: use of "auto"618a8814: use of "subvolid"Signed-off-by Stanislav Brabec <[email protected]>--- 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(quoted diff) +0
-0b/tests/expected/mount/fstab-btrfs new file mode 100644 index 0000000..3582111
tests/expected/mount/fstab-btrfs new file +1
-0@@ -0,0 +1 @@ +Successtests/ts/mount/fstab-btrfs new file +9
-0@@ -0,0 +1,99 @@ +#!/bin/bash + +# +# Copyright (C) 2016 Stanislav Brabec <[email protected]> +# +# 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 publishedby +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version.
tests/ts/mount/fstab-btrfs new file +26
-0+# +# 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"tests/ts/mount/fstab-btrfs new file +1
-0+pushd . >/dev/null
tests/ts/mount/fstab-btrfs line 52
Quoted 10 lines
tests/ts/mount/fstab-btrfs new file +10
-0+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.
Quoted 50 lines
tests/ts/mount/fstab-btrfs new file +3
-0+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 ../../../..
tests/ts/mount/fstab-btrfs new file +9
-0+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 entryin fstab +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT-default" "btrfs" ""
tests/ts/mount/fstab-btrfs new file +2
-0+# 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" +
tests/ts/mount/fstab-btrfs new file +2
-0+# 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_OUTPUTtests/ts/mount/fstab-btrfs new file +5
-0+ +ts_fstab_clean + +# Tests with fs == auto +# mounting default subvolume, deep in the structure, without entryin fstab +ts_fstab_add "$DEVICE" "$TS_MOUNTPOINT-default" "auto" ""
tests/ts/mount/fstab-btrfs new file +2
-0+# 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" +
tests/ts/mount/fstab-btrfs new file +3
-0+# 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_OUTPUTtests/ts/mount/fstab-btrfs new file +6
-1+ +ts_fstab_clean + +ts_log "Success" +ts_finalize + --2.7.0
Quoted 7 lines · 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.Quoted 1 line
+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.
Quoted 1 line
Quoted 1 line
+NON_DEFAULT_SUBVOLID=$(btrfs subvol list "$TS_MOUNTPOINT-create" |
NON_DEFAULT_SUBVOLID will be empty.
Quoted 1 line
Quoted 1 line
"subvolid=$NON_DEFAULT_SUBVOLID" +
I am not sure how kernel will interpret subvolid=.
Signature · 10 lines
Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: [email protected] 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
Quoted 12 lines · Stanislav Brabec
Quoted 7 lines · 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.Quoted 1 line
+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.
Quoted 6 lines
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.
Quoted 8 lines
Quoted 2 lines
Quoted 2 lines
+NON_DEFAULT_SUBVOLID=$(btrfs subvol list "$TS_MOUNTPOINT-create" |
NON_DEFAULT_SUBVOLIDwill be empty.Quoted 1 line
Quoted 1 line
"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