[LTP] [PATCH 1/1] io_destroy01: TCONF when unsupported

[LTP] [PATCH 1/1] io_destroy01: TCONF when unsupported

[Petr Vorel]

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 = {
-- 
2.28.0

[LTP] [PATCH 1/1] io_destroy01: TCONF when unsupported

[Yang Xu]

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 = {

[LTP] [PATCH 1/1] io_destroy01: TCONF when unsupported

[Cyril Hrubis]

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.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/1] io_destroy01: TCONF when unsupported

[Petr Vorel]

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

[LTP] [PATCH 1/1] io_destroy01: TCONF when unsupported

[Petr Vorel]

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

[LTP] [PATCH 1/1] io_destroy01: TCONF when unsupported

[Cyril Hrubis]

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.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/1] io_destroy01: TCONF when unsupported

[Petr Vorel]

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

[LTP] [PATCH 1/1] io_destroy01: TCONF when unsupported

[Yang Xu]

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
>
>
> .
>

[LTP] [PATCH 1/1] io_destroy01: TCONF when unsupported

[Richard Palethorpe]

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.

-- 
Thank you,
Richard.

[LTP] [PATCH 1/1] io_destroy01: TCONF when unsupported

[Cyril Hrubis]

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.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/1] io_destroy01: TCONF when unsupported

[Petr Vorel]

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