From: Petr Vorel <pvorel@suse.cz>
To: Cyril Hrubis <chrubis@suse.cz>
Cc: Richard Palethorpe <rpalethorpe@suse.com>, ltp@lists.linux.it
Subject: Re: [LTP] [PATCH v6 5/8] tst_test.sh: Add $TST_ALL_FILESYSTEMS
Date: Fri, 16 Sep 2022 22:27:34 +0200 [thread overview]
Message-ID: <YyTcNlcwetdsXQgA@pevik> (raw)
In-Reply-To: <YyR3apctP+uns07p@yuki>
> Hi!
> > > Whatever the API ends up I guess that we should check the support and
> > > mkfs here as well in order to TCONF early before we create a device and
> > > attempt to format/mount it.
> > I was worried to add mkfs.foo dependency unless there is a requirement for it
> > (i.e. TST_NEEDS_DEVICE=1). Do you really want to add this dependency for *all*
> > tests? (I'd prefer no, i.e. to keep here check only for test not being skipped,
> > because check for mkfs.foo is in tst_mkfs which does the job, thus not only we'd
> > add an extra unneeded dependency but also duplicity in the check).
> Isn't this mostly theoretical problem? I mean how common it would be to
> have test that tests different filesystem but does not need
> corresponding mkfs?
Have you noticed !=, i.e.:
if [ "$TST_ALL_FILESYSTEMS" != 1 ]; then
The check here is for single filesystem testing, just with disabled on some
filesystems (i.e. skip on tmpfs). Equivalent of fcntl33.c (skips on tmpfs,
ramfs - BTW not in fs_type_whitelist, thus not being checked, nfs; but not using
.all_filesystems).
That's why I'm not going to add check for kernel support and mkfs.foo (hope I
was correct you overlooked !=, or did I miss something?)
For testing on all filesystems (TST_ALL_FILESYSTEMS=1) there is obviously check
for kernel and mkfs support in testcases/lib/tst_supported_fs.c.
> > But if you want it, i.e. if we require mkfs.$TST_FS_TYPE (unless tmpfs of
> > course, which is handled properly) here, then we don't need to separate checks
> > for mkfs.foo and skip list in testcases/lib/tst_supported_fs.c.
> I would go that way for now.
I dare to merge this approach:
* all filesystems
tst_supported_fs [-s skip_list]
print the list of supported filesystems
if fs_type is supported and not in skip_list (optional),
print list of supported filesystems and return 0
if fs_type isn't supported or in skip_list, return 1
* single filesystem
tst_supported_fs fs_type
if fs_type is supported, return 0 otherwise return 1
tst_supported_fs -s skip_list fs_type
if fs_type is in skip_list, return 1 otherwise return 0
> I guess that the cleanest solution would be keeping the funcionality
> really orthogonal, i.e. separating the kernel and mkfs checks so that we
> would have -m to enable mkfs check -k to enable kernel check and -s to
> enable skiplist. But that is something that can and should be done after
> the relase.
If really needed, sure.
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
next prev parent reply other threads:[~2022-09-16 20:27 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-15 9:36 [LTP] [PATCH v6 0/8] shell: df01.sh: $TST_ALL_FILESYSTEMS Petr Vorel
2022-09-15 9:36 ` [LTP] [PATCH v6 1/8] tst_supported_fs: Implement skip list Petr Vorel
2022-09-15 9:36 ` [LTP] [PATCH v6 2/8] zram01.sh: Use tst_supported_fs -s tmpfs Petr Vorel
2022-09-16 11:33 ` Cyril Hrubis
2022-09-15 9:36 ` [LTP] [PATCH v6 3/8] tst_supported_fs: Support skip list when query single fs Petr Vorel
2022-09-16 11:45 ` Cyril Hrubis
2022-09-16 12:09 ` Petr Vorel
2022-09-16 12:28 ` Cyril Hrubis
2022-09-16 12:39 ` Petr Vorel
2022-09-16 12:10 ` Petr Vorel
2022-09-15 9:36 ` [LTP] [PATCH v6 4/8] shell: Add $TST_SKIP_FILESYSTEMS + tests Petr Vorel
2022-09-16 11:50 ` Cyril Hrubis
2022-09-15 9:36 ` [LTP] [PATCH v6 5/8] tst_test.sh: Add $TST_ALL_FILESYSTEMS Petr Vorel
2022-09-16 12:45 ` Cyril Hrubis
2022-09-16 13:09 ` Petr Vorel
2022-09-16 13:17 ` Cyril Hrubis
2022-09-16 20:27 ` Petr Vorel [this message]
2022-09-15 9:36 ` [LTP] [PATCH v6 6/8] tst_test.sh: Allow | after whitelisted variable Petr Vorel
2022-09-16 13:02 ` Cyril Hrubis
2022-09-15 9:36 ` [LTP] [PATCH v6 7/8] shell: Add tests for TST_ALL_FILESYSTEMS=1 Petr Vorel
2022-09-15 9:36 ` [LTP] [PATCH v6 8/8] df01.sh: Convert to TST_ALL_FILESYSTEMS=1 Petr Vorel
2022-09-16 13:10 ` Cyril Hrubis
2022-09-16 13:11 ` Cyril Hrubis
2022-09-16 20:39 ` Petr Vorel
2022-09-16 13:12 ` Petr Vorel
2022-09-16 21:22 ` [LTP] [PATCH v6 0/8] shell: df01.sh: $TST_ALL_FILESYSTEMS Petr Vorel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YyTcNlcwetdsXQgA@pevik \
--to=pvorel@suse.cz \
--cc=chrubis@suse.cz \
--cc=ltp@lists.linux.it \
--cc=rpalethorpe@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.