| Message ID | 20200929073501.4598-1-pvorel@suse.cz |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/1] io_destroy01: TCONF when unsupported | expand |
Hi Petr Why not using kconfig in tst_test struct. Best Regards Yang Xu > to fix test on kernel built without CONFIG_AIO=y. > > Cleanup: use return instead of else, join string. > > Signed-off-by: Petr Vorel<pvorel@suse.cz> > --- > Hi, > > simple patch, but can wait after release. > I can move cleanup into separate commit. > > Kind regards, > Petr > > testcases/kernel/syscalls/io_destroy/io_destroy01.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/testcases/kernel/syscalls/io_destroy/io_destroy01.c b/testcases/kernel/syscalls/io_destroy/io_destroy01.c > index 560d7b3fb..5c5c59335 100644 > --- a/testcases/kernel/syscalls/io_destroy/io_destroy01.c > +++ b/testcases/kernel/syscalls/io_destroy/io_destroy01.c > @@ -32,14 +32,19 @@ static void verify_io_destroy(void) > return; > } > > + if (TST_RET == -ENOSYS) { > + tst_res(TCONF, "io_destroy() not supported"); > + return; > + } > + > if (TST_RET == -EINVAL) { > tst_res(TPASS, > "io_destroy() failed as expected, returned -EINVAL"); > - } else { > - tst_res(TFAIL, "io_destroy() failed unexpectedly, " > - "returned -%s expected -EINVAL", > - tst_strerrno(-TST_RET)); > + return; > } > + > + tst_res(TFAIL, "io_destroy() failed unexpectedly, returned -%s expected -EINVAL", > + tst_strerrno(-TST_RET)); > } > > static struct tst_test test = {
Hi! > to fix test on kernel built without CONFIG_AIO=y. > > Cleanup: use return instead of else, join string. This is both obviously ok, but the cleanup should be in a separate patch. You can add my reviewed-by if you split the patch into two. Also is this the only io_* test that fails? Looking at the testcases we have the io_setup01 that looks like it will fail with TBROK if io_setup() returns -ENOSYS.
Hi Yang, > Hi Petr > Why not using kconfig in tst_test struct. We should probably use tst_syscall(). But using kconfig is not 1) needed (tst_syscall() most reliable way) 2) kconfig requires having kernel config available, which is not on some platforms. (I plan to introduce TWARN_IF_NO_CONFIG to print only warning for platforms which don't have kconfig available, see https://github.com/linux-test-project/ltp/issues/700). Kind regards, Petr
Hi Cyril, > Hi! > > to fix test on kernel built without CONFIG_AIO=y. > > Cleanup: use return instead of else, join string. > This is both obviously ok, but the cleanup should be in a separate > patch. +1, I'll do. I suggested to use tst_syscall(), but we probably want here to use libc wrapper I guess, thus I'll merge this (split into 2 commits). > You can add my reviewed-by if you split the patch into two. > Also is this the only io_* test that fails? > Looking at the testcases we have the io_setup01 that looks like it will > fail with TBROK if io_setup() returns -ENOSYS. Thanks, I'll send a separate patch for this. Kind regards, Petr
Hi! > > > to fix test on kernel built without CONFIG_AIO=y. > > > > Cleanup: use return instead of else, join string. > > > This is both obviously ok, but the cleanup should be in a separate > > patch. > +1, I'll do. > I suggested to use tst_syscall(), but we probably want here to use libc wrapper These are from libaio library, which is why they return negative errors instead of setting errno.
Hi, > > > > to fix test on kernel built without CONFIG_AIO=y. > > > > Cleanup: use return instead of else, join string. > > > This is both obviously ok, but the cleanup should be in a separate > > > patch. > > +1, I'll do. > > I suggested to use tst_syscall(), but we probably want here to use libc wrapper > These are from libaio library, which is why they return negative errors > instead of setting errno. Right, thanks for info. Merged fixed version (2 commits). Kind regards, Petr
Hi Petr > Hi Yang, > >> Hi Petr >> Why not using kconfig in tst_test struct. > > We should probably use tst_syscall(). > But using kconfig is not 1) needed (tst_syscall() most reliable way) 2) kconfig > requires having kernel config available, which is not on some platforms. I see. At the first, I prefer to like to use kconfig because it only tests io_destory one time on kernel without CONFIG_AIO when we use -i parameters, but using libaio wrapper is stable. So it is ok. > > (I plan to introduce TWARN_IF_NO_CONFIG to print only warning for platforms > which don't have kconfig available, see > https://github.com/linux-test-project/ltp/issues/700). I have seen this ticket, if I understand it corretly, if platform has kernel config without needed kconfig, test cases still will report TCONF if platform doesn't have kernel config, test cases only reports warning(system doesn't have kernel config file) and continue to run test and it maybe lead to a unexpected stat/failure. And we don't add extra check in tests case such as ENOSYS/EINVAL check because it is redundant against kconfig check. Best Regards Yang Xu > > Kind regards, > Petr > > > . >
Hello, Yang Xu <xuyang2018.jy@cn.fujitsu.com> writes: > Hi Petr > >> Hi Yang, >> >>> Hi Petr >>> Why not using kconfig in tst_test struct. >> >> We should probably use tst_syscall(). >> But using kconfig is not 1) needed (tst_syscall() most reliable way) 2) kconfig >> requires having kernel config available, which is not on some platforms. > I see. At the first, I prefer to like to use kconfig because it only > tests io_destory one time on kernel without CONFIG_AIO when we use -i > parameters, but using libaio wrapper is stable. So it is ok. Kconfig can even be wrong (most likely with out-of-tree patches) or we might look at the wrong option as things are moved around sometimes. This is another instance of us trying to guess if something can be done without trying it. I guess there are some things where it is not as simple as trying a system call, but in cases where ENOSYS will be returned there is no need to look at the config except maybe for diagnostic purposes. If you are concerned about '-i' then you can even just make in invalid syscall in setup to see what error it returns.
Hi!
> Why not using kconfig in tst_test struct.
Well because this is much simpler and does the same job.
There is a different case for kconfig, i.e. some people would like to be
able to syntetize kconfig for testing by extracting the data from tests,
but that is completely different problem and would need a different
solution.
Hi, > Yang Xu <xuyang2018.jy@cn.fujitsu.com> writes: > > Hi Petr > >> Hi Yang, > >>> Hi Petr > >>> Why not using kconfig in tst_test struct. > >> We should probably use tst_syscall(). > >> But using kconfig is not 1) needed (tst_syscall() most reliable way) 2) kconfig > >> requires having kernel config available, which is not on some platforms. > > I see. At the first, I prefer to like to use kconfig because it only > > tests io_destory one time on kernel without CONFIG_AIO when we use -i > > parameters, but using libaio wrapper is stable. So it is ok. > Kconfig can even be wrong (most likely with out-of-tree patches) or we > might look at the wrong option as things are moved around > sometimes. This is another instance of us trying to guess if something > can be done without trying it. > I guess there are some things where it is not as simple as trying a > system call, but in cases where ENOSYS will be returned there is no need > to look at the config except maybe for diagnostic purposes. If you are > concerned about '-i' then you can even just make in invalid syscall in > setup to see what error it returns. @Yang: Yes, have a look at various foo_supported_by_kernel() helper functions in include/lapi/ (added by Viresh Kumar). Kind regards, Petr
diff --git a/testcases/kernel/syscalls/io_destroy/io_destroy01.c b/testcases/kernel/syscalls/io_destroy/io_destroy01.c index 560d7b3fb..5c5c59335 100644 --- a/testcases/kernel/syscalls/io_destroy/io_destroy01.c +++ b/testcases/kernel/syscalls/io_destroy/io_destroy01.c @@ -32,14 +32,19 @@ static void verify_io_destroy(void) return; } + if (TST_RET == -ENOSYS) { + tst_res(TCONF, "io_destroy() not supported"); + return; + } + if (TST_RET == -EINVAL) { tst_res(TPASS, "io_destroy() failed as expected, returned -EINVAL"); - } else { - tst_res(TFAIL, "io_destroy() failed unexpectedly, " - "returned -%s expected -EINVAL", - tst_strerrno(-TST_RET)); + return; } + + tst_res(TFAIL, "io_destroy() failed unexpectedly, returned -%s expected -EINVAL", + tst_strerrno(-TST_RET)); } static struct tst_test test = {
to fix test on kernel built without CONFIG_AIO=y. Cleanup: use return instead of else, join string. Signed-off-by: Petr Vorel <pvorel@suse.cz> --- Hi, simple patch, but can wait after release. I can move cleanup into separate commit. Kind regards, Petr testcases/kernel/syscalls/io_destroy/io_destroy01.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)