libltp: add support for .needs_rofs for EROFS check
v2 cover libltp: add support for .needs_rofs for EROFS check
Some tests go through losetup, create, format and mount filesystems only to run tests for 'EROFS' return value from system calls. The tests end up being flaky depending on the tools available on the platform. e.g. mkfs.<filesystem> tool is required for mounting a device with filesystem.
If the test is only to check for EROFS, this can be achieved by simply doing a 'tmpfs' read-only mount in $tmpdir. So, this series adds a ".needs_rofs" flag to 'struct tst_test' that can be used to indicate that the test only needs a read-only filesystem mount ot test for 'EROFS' return value. If the flag is set, the library will first attempt to mount 'tmpfs' filesystem at tst_test->mntpoint and fallback to the original create, format and mount a loopback device if that fails.
Example of tests that can benefit from this are:
access04, mkdirat02, mknodat02, acct01, fchown04, mknod07 etc.
This also allows for these tests to successfully run on Android.
Sandeep Patil (2):
libltp: add support to mount tmpfs for EROFS testing syscalls/access04: use .needs_rofs flag for EROFS testing
include/tst_test.h | 1 + lib/tst_test.c | 36 ++++++++++++++++++++++++----- testcases/kernel/syscalls/access/access04.c | 3 +-- 3 files changed, 32 insertions(+), 8 deletions(-)
Signature · 1 line
2.14.1.581.gf28d330327-goog
PATCH v2 1/2 libltp: add support to mount tmpfs for EROFS testing
Some tests go through losetup, create, format and mount filesystems only to test for 'EROFS' return value from system calls. The tests end up being flaky depending on the tools available on the platform. e.g. mkfs.<filesystem> tool is required for mounting a device with filesystem.
If the test is only to check for EROFS, this can be achieved by simply by the read-only mounting 'tmpfs' at 'mntpoint' and fallback to the original way if that fails.
Example of tests that can benefit from this are:
access04, mkdirat02, mknodat02, acct01, fchown04, mknod07 etc.
This also allows for these tests to successfully run on Android.
---
v1->v2
------
- Changed the interface to be at a higher level, i.e. instead of making tests specify that they need a read-only tmpfs mount, we do that automatically in the library as a preference and fallback to the old way if that doesn't succeed.
- The read-only bind mount of "/" option was tried and it worked just fine with an Android system, but broke my ubuntu instance badly (I lost root). I could never find out why, since hte patch worked fine on Android and it correctly did the bind mount. However, I figured if there is any chance of possible breakage, we should simply avoid that path for now.
include/tst_test.h | 1 + lib/tst_test.c | 36 ++++++++++++++++++++++++++++++------ 2 files changed, 31 insertions(+), 6 deletions(-)
include/tst_test.h +1 -0
@@ -124,6 +124,7 @@ struct tst_test { int needs_checkpoints:1; int format_device:1; int mount_device:1; + int needs_rofs:1;/* Minimal device size in megabytes */ unsigned int dev_min_size;
lib/tst_test.c +30 -6
@@ -21,6 +21,7 @@#include <string.h> #include <stdlib.h> #include <errno.h> +#include <sys/mount.h> #include <sys/types.h> #include <sys/wait.h> #include <sys/time.h>
@@ -41,7 +42,7 @@ struct tst_test *tst_test;static int iterations =1 ;static float duration =-1 ;static pid_t main_pid, lib_pid; -static int device_mounted; +static int mntpoint_active;struct results {int passed;
@@ -701,7 +702,32 @@ static void do_setup(int argc, char *argv[])if (needs_tmpdir () && !tst_tmpdir_created ())tst_tmpdir (); -if (tst_test->needs_device) { +if (tst_test->mntpoint) +SAFE_MKDIR (tst_test->mntpoint,0777 ); + +if (tst_test->needs_rofs) { +if (!tst_test->mntpoint) { +tst_brk (TBROK , +"tst_test->mntpoint must be set!" ); + } + +/* If we failed to do read-only bind mount for '/'. Fallback to +* using a device with empty read-only filesystem. +*/ +if (mount (NULL , tst_test->mntpoint,"tmpfs" ,MS_RDONLY ,NULL )) { +tst_res (TINFO ,"Cannot mount tmpfs read-only at %s, " +"setting up a device instead\n" , + tst_test->mntpoint); + tst_test->mount_device =1 ; + tst_test->needs_device =1 ; + tst_test->format_device =1 ; + tst_test->mnt_flags =MS_RDONLY ; + }else { + mntpoint_active =1 ; + } + } + +if (tst_test->needs_device && !mntpoint_active) { tdev.dev =tst_acquire_device_ (NULL , tst_test->dev_min_size);if (!tdev.dev)
@@ -721,16 +747,14 @@ static void do_setup(int argc, char *argv[]) }if (tst_test->mount_device) { -if (!tst_test->mntpoint) {tst_brk (TBROK ,"tst_test->mntpoint must be set!" ); } -SAFE_MKDIR (tst_test->mntpoint,0777 );SAFE_MOUNT (tdev.dev, tst_test->mntpoint, tdev.fs_type, tst_test->mnt_flags, tst_test->mnt_data); - device_mounted =1 ; + mntpoint_active =1 ; } }
@@ -751,7 +775,7 @@ static void do_test_setup(void)static void do_cleanup (void ) { -if (device_mounted) +if (mntpoint_active)tst_umount (tst_test->mntpoint);if (tst_test->needs_device && tdev.dev)
Signature · 1 line
2.14.1.581.gf28d330327-goog
PATCH v2 1/2 libltp: add support to mount tmpfs for EROFS testing
----- Original Message -----
Quoted 24 lines
Some tests go through losetup, create, format and mount filesystems only to test for 'EROFS' return value from system calls. The tests end up being flaky depending on the tools available on the platform. e.g. mkfs.<filesystem> tool is required for mounting a device with filesystem.
If the test is only to check for EROFS, this can be achieved by simply by the read-only mounting 'tmpfs' at 'mntpoint' and fallback to the original way if that fails.
Example of tests that can benefit from this are:
access04, mkdirat02, mknodat02, acct01, fchown04, mknod07 etc.This also allows for these tests to successfully run on Android.
Signed-off-by Sandeep Patil <[email protected]>---
v1->v2
------
- Changed the interface to be at a higher level, i.e. instead of making tests specify that they need a read-only tmpfs mount, we do that automatically in the library as a preference and fallback to the old way if that doesn't succeed.
Quoted 6 lines
- The read-only bind mount of "/" option was tried and it worked just fine with an Android system, but broke my ubuntu instance badly (I lost root).
I could never find out why, since hte patch worked fine on Android and it correctly did the bind mount. However, I figured if there is any chance of possible breakage, we should simply avoid that path for now.
I tried this too, with bind mounting only one directory inside TMPDIR:
SAFE_MOUNT(source, target, NULL, MS_BIND, NULL); SAFE_MOUNT(NULL, target, NULL, MS_REMOUNT | MS_RDONLY | MS_BIND, NULL);
and it worked fine, but it turns out it's supported since 2.6.26.
This patch (which uses tmpfs for EROFS only) looks OK to me, and it allows us to add RO bind mount later as fallback if there's need for it (e.g. tmpfs not available on some distros).
I tested it as far back as RHEL5.6 (2.6.18 kernel).
Regards,
Jan
PATCH v2 1/2 libltp: add support to mount tmpfs for EROFS testing
Quoted 35 lines · Jan Stancek
----- Original Message -----
Quoted 24 lines
Some tests go through losetup, create, format and mount filesystems only to test for 'EROFS' return value from system calls. The tests end up being flaky depending on the tools available on the platform. e.g. mkfs.<filesystem> tool is required for mounting a device with filesystem.
If the test is only to check for EROFS, this can be achieved by simply by the read-only mounting 'tmpfs' at 'mntpoint' and fallback to the original way if that fails.
Example of tests that can benefit from this are:
access04, mkdirat02, mknodat02, acct01, fchown04, mknod07 etc.This also allows for these tests to successfully run on Android.
Signed-off-by Sandeep Patil <[email protected]>---
v1->v2
------
- Changed the interface to be at a higher level, i.e. instead of making tests specify that they need a read-only tmpfs mount, we do that automatically in the library as a preference and fallback to the old way if that doesn't succeed.Quoted 6 lines
- The read-only bind mount of "/" option was tried and it worked just fine with an Android system, but broke my ubuntu instance badly (I lost root).
I could never find out why, since hte patch worked fine on Android and it correctly did the bind mount. However, I figured if there is any chance of possible breakage, we should simply avoid that path for now.
Quoted 4 lines
I tried this too, with bind mounting only one directory inside TMPDIR:
SAFE_MOUNT(source, target, NULL, MS_BIND, NULL);SAFE_MOUNT(NULL, target, NULL, MS_REMOUNT | MS_RDONLY | MS_BIND, NULL);
and it worked fine, but it turns out it's supported since 2.6.26.
Yes, it worked find for me too. The only place I can imagine something went wrong was probably some mistake in the code to check for appropriate return to make sure '/' is indeed bind mounted as read-only. If it is not then there is a very real chance of the test tear down deleting the bind-mounted '/' recursively as it clears the $tmpdir (where we bind mounted '/')
I did not use SAFE_MOUNT (I couldn't) because we want to fall back to the original way of doing things if bind-mount doesn't work. So, I think the sequence of event as it happened on the broken ubuntu instance in my case may have been -
1. bind mount '/' to /tmp/foo - success
2. remount /tmp/foo as read-only - fail
3. retry umount(/tmp/foo) 10 times - fail
4. rm -rf /tmp/foo
So, at the end I found things deleted from / :( and wasn't able to diagnose what went wrong. This was clearly a problem since somehow the test code at the time must have failed to detect #2 somehow...
.. Anyway, we can do best-effort to do tmpfs, bind mount, create-format-mount device.
Quoted 6 lines
This patch (which uses tmpfs for EROFS only) looks OK to me, and it allows us to add RO bind mount later as fallback if there's need for it (e.g. tmpfs not available on some distros).
I tested it as far back as RHEL5.6 (2.6.18 kernel).
Thanks for the test Jan. I will resend with your tested-by.
Quoted 3 lines
Regards,
Jan
PATCH v2 2/2 syscalls/access04: use .needs_rofs flag for EROFS testing
In reply to Sandeep PatilPrefer to use the .needs_rofs flag instead of requiring format and mount a filesystem image for testing 'EROFS' return value from access()
--- testcases/kernel/syscalls/access/access04.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
testcases/kernel/syscalls/access/access04.c +1 -2
@@ -125,9 +125,8 @@ static struct tst_test test = { .needs_tmpdir = 1, .needs_root = 1, .forks_child = 1, - .mount_device = 1, + .needs_rofs = 1, .mntpoint = MNT_POINT, - .mnt_flags = MS_RDONLY, .setup = setup, .test = verify_access, };
Signature · 1 line
2.14.1.581.gf28d330327-goog