| Message ID | 20240709103324.183993-1-pvorel@suse.cz |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/1] tst_test: Fix tests using device without .filesystems | expand |
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 >
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
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)
> 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
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.
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)
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(-)