[LTP] [PATCH 1/1] tst_test: Fix tests using device without .filesystems

[LTP] [PATCH 1/1] tst_test: Fix tests using device without .filesystems

[Petr Vorel]

cce618891 introduced a regression for tests which use the test loop
device (e.g. .mount_device = 1 or .format_device = 1) without
.filesystems array (execveat03, ioctl04, inotify0[378], fanotify10,
mount02, umount0[1-3], umount2_02).

Call in that case prepare_device() with the default filesystem type.

Fixes: cce618891 ("lib: tst_test: Add per filesystem mkfs and mount opts")
Reported-by: Petr Cervinka <pcervinka@suse.com>
Suggested-by: Andrea Cervesato <andrea.cervesato@suse.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Hi,

is there a more elegant way to fix this?

Kind regards,
Petr

 lib/tst_test.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/tst_test.c b/lib/tst_test.c
index b49c248ae..6d258cd28 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1355,8 +1355,11 @@ static void do_setup(int argc, char *argv[])
 
 		tdev.fs_type = default_fs_type();
 
-		if (!tst_test->all_filesystems && count_fs_descs() == 1)
-			prepare_device(&tst_test->filesystems[0]);
+		if (!tst_test->all_filesystems && count_fs_descs() <= 1) {
+			prepare_device(tst_test->filesystems ?
+				       &tst_test->filesystems[0] :
+				       &(struct tst_fs){.type = tdev.fs_type});
+		}
 	}
 
 	if (tst_test->needs_overlay && !tst_test->mount_device)
-- 
2.45.2


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] tst_test: Fix tests using device without .filesystems

[Cyril Hrubis]

Hi!
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index b49c248ae..6d258cd28 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -1355,8 +1355,11 @@ static void do_setup(int argc, char *argv[])
>  
>  		tdev.fs_type = default_fs_type();
>  
> -		if (!tst_test->all_filesystems && count_fs_descs() == 1)
> -			prepare_device(&tst_test->filesystems[0]);
> +		if (!tst_test->all_filesystems && count_fs_descs() <= 1) {
> +			prepare_device(tst_test->filesystems ?
> +				       &tst_test->filesystems[0] :
> +				       &(struct tst_fs){.type = tdev.fs_type});

Another option would be to change prepare_device() so that it works fine
with NULL tst_fs pointer, then we could simply do:

	prepare_device(tst_test->filesystems);


And for that we would have to do something like this:

diff --git a/lib/tst_test.c b/lib/tst_test.c
index b49c248ae..ec6deea9b 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1101,6 +1101,9 @@ static void prepare_device(struct tst_fs *fs)
 {
        const char *mnt_data;
        char buf[1024];
+       struct tst_fs dummy = {};
+
+       fs = fs ? &dummy : fs;
 
        const char *const extra[] = {fs->mkfs_size_opt, NULL};


> +		}
>  	}
>  
>  	if (tst_test->needs_overlay && !tst_test->mount_device)
> -- 
> 2.45.2
> 

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] tst_test: Fix tests using device without .filesystems

[Petr Vorel]

Hi Cyril,

> Hi!
> > diff --git a/lib/tst_test.c b/lib/tst_test.c
> > index b49c248ae..6d258cd28 100644
> > --- a/lib/tst_test.c
> > +++ b/lib/tst_test.c
> > @@ -1355,8 +1355,11 @@ static void do_setup(int argc, char *argv[])

> >  		tdev.fs_type = default_fs_type();

> > -		if (!tst_test->all_filesystems && count_fs_descs() == 1)
> > -			prepare_device(&tst_test->filesystems[0]);
> > +		if (!tst_test->all_filesystems && count_fs_descs() <= 1) {
> > +			prepare_device(tst_test->filesystems ?
> > +				       &tst_test->filesystems[0] :
> > +				       &(struct tst_fs){.type = tdev.fs_type});

> Another option would be to change prepare_device() so that it works fine
> with NULL tst_fs pointer, then we could simply do:

> 	prepare_device(tst_test->filesystems);


> And for that we would have to do something like this:

> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index b49c248ae..ec6deea9b 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -1101,6 +1101,9 @@ static void prepare_device(struct tst_fs *fs)
>  {
>         const char *mnt_data;
>         char buf[1024];
> +       struct tst_fs dummy = {};
> +
> +       fs = fs ? &dummy : fs;

This looks more elegant, but I get coredump.
I also think you need to have count_fs_descs() <= 1 and maybe define fs_type
(not sure about it).

Feel free to post v2.

Kind regards,
Petr

>         const char *const extra[] = {fs->mkfs_size_opt, NULL};


> > +		}
> >  	}

> >  	if (tst_test->needs_overlay && !tst_test->mount_device)
> > -- 
> > 2.45.2

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] tst_test: Fix tests using device without .filesystems

[Cyril Hrubis]

Hi!
> > +       fs = fs ? &dummy : fs;

This is a stupid typo, it should be fs = fs ? fs : &dummy instead.

> This looks more elegant, but I get coredump.
> I also think you need to have count_fs_descs() <= 1 and maybe define fs_type
> (not sure about it).

You need count_fs_descs() <= 1 but the fs type is used from the
tst_device structure instead. So the whole patch should be:

diff --git a/lib/tst_test.c b/lib/tst_test.c
index b49c248ae..c6ed5d562 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1101,6 +1101,9 @@ static void prepare_device(struct tst_fs *fs)
 {
        const char *mnt_data;
        char buf[1024];
+       struct tst_fs dummy = {};
+
+       fs = fs ? fs: &dummy;

        const char *const extra[] = {fs->mkfs_size_opt, NULL};

@@ -1355,8 +1358,8 @@ static void do_setup(int argc, char *argv[])

                tdev.fs_type = default_fs_type();

-               if (!tst_test->all_filesystems && count_fs_descs() == 1)
-                       prepare_device(&tst_test->filesystems[0]);
+               if (!tst_test->all_filesystems && count_fs_descs() <= 1)
+                       prepare_device(tst_test->filesystems);
        }

        if (tst_test->needs_overlay && !tst_test->mount_device)



-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] tst_test: Fix tests using device without .filesystems

[Petr Vorel]

> Hi!
> > > +       fs = fs ? &dummy : fs;

> This is a stupid typo, it should be fs = fs ? fs : &dummy instead.

Ah, obviously. I was blind not notice this.

> > This looks more elegant, but I get coredump.
> > I also think you need to have count_fs_descs() <= 1 and maybe define fs_type
> > (not sure about it).

> You need count_fs_descs() <= 1 but the fs type is used from the
> tst_device structure instead. So the whole patch should be:

> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index b49c248ae..c6ed5d562 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -1101,6 +1101,9 @@ static void prepare_device(struct tst_fs *fs)
>  {
>         const char *mnt_data;
>         char buf[1024];
> +       struct tst_fs dummy = {};
> +
> +       fs = fs ? fs: &dummy;

nit: FYI shorten syntax works even on 4.8.5:
	fs = fs ?: &dummy;

>         const char *const extra[] = {fs->mkfs_size_opt, NULL};

> @@ -1355,8 +1358,8 @@ static void do_setup(int argc, char *argv[])

>                 tdev.fs_type = default_fs_type();

> -               if (!tst_test->all_filesystems && count_fs_descs() == 1)
> -                       prepare_device(&tst_test->filesystems[0]);
> +               if (!tst_test->all_filesystems && count_fs_descs() <= 1)
> +                       prepare_device(tst_test->filesystems);
>         }

>         if (tst_test->needs_overlay && !tst_test->mount_device)

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Tested-by: Petr Vorel <pvorel@suse.cz>

Feel free to merge it.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] tst_test: Fix tests using device without .filesystems

[Cyril Hrubis]

Hi!
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Tested-by: Petr Vorel <pvorel@suse.cz>
> 
> Feel free to merge it.

Pushed, thanks for the fix.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp