diff mbox series

[COMMITTED,1/1] nfs05_make_tree: Restore 5 min timeout

Message ID 20220616162339.19322-1-pvorel@suse.cz
State Accepted
Headers show
Series [COMMITTED,1/1] nfs05_make_tree: Restore 5 min timeout | expand

Commit Message

Petr Vorel June 16, 2022, 4:23 p.m. UTC
nfs05_make_tree is a long running test. The previous default timeout 5
min was enough after reducing runtime in 05be61cf6.

But the new default 30 sec introduced in the concept of max runtime is
definitely not enough. Restore the previous timeout 5 min.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/network/nfs/nfs_stress/nfs05_make_tree.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Cyril Hrubis June 17, 2022, 9:13 a.m. UTC | #1
Hi!
> nfs05_make_tree is a long running test. The previous default timeout 5
> min was enough after reducing runtime in 05be61cf6.
> 
> But the new default 30 sec introduced in the concept of max runtime is
> definitely not enough. Restore the previous timeout 5 min.

Looking at the code this is actually a helper binary rather than a test
itself. I think that the problem here is that we mix the shell test
library and C library and the test is a mess of both with unexpected
consequencies.

I guess that it would probably implement main() in the nfs05_make_tree.c
instead as we do for the various tools in testcases/lib/*.c. That would
at least make sure that we will not break anything with changes intended
for tests and not for helper binaries as this one.

> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>  testcases/network/nfs/nfs_stress/nfs05_make_tree.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/testcases/network/nfs/nfs_stress/nfs05_make_tree.c b/testcases/network/nfs/nfs_stress/nfs05_make_tree.c
> index fdc13bd5d..5456c1bf0 100644
> --- a/testcases/network/nfs/nfs_stress/nfs05_make_tree.c
> +++ b/testcases/network/nfs/nfs_stress/nfs05_make_tree.c
> @@ -215,4 +215,5 @@ static struct tst_test test = {
>  	.options = opts,
>  	.test_all = do_test,
>  	.setup = setup,
> +	.max_runtime = 300,
>  };
> -- 
> 2.36.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp
Petr Vorel June 17, 2022, 1:37 p.m. UTC | #2
> Hi!
> > nfs05_make_tree is a long running test. The previous default timeout 5
> > min was enough after reducing runtime in 05be61cf6.

> > But the new default 30 sec introduced in the concept of max runtime is
> > definitely not enough. Restore the previous timeout 5 min.

> Looking at the code this is actually a helper binary rather than a test
> itself. I think that the problem here is that we mix the shell test
> library and C library and the test is a mess of both with unexpected
> consequencies.
Yes.

> I guess that it would probably implement main() in the nfs05_make_tree.c
> instead as we do for the various tools in testcases/lib/*.c. That would
> at least make sure that we will not break anything with changes intended
> for tests and not for helper binaries as this one.

Helper binaries does not use LTP C API. I'd like to keep it (to have SAFE_*()),
I guess you're not against. Therefore just using TST_NO_DEFAULT_MAIN
and adding main(), right?

Kind regards,
Petr

> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> >  testcases/network/nfs/nfs_stress/nfs05_make_tree.c | 1 +
> >  1 file changed, 1 insertion(+)

> > diff --git a/testcases/network/nfs/nfs_stress/nfs05_make_tree.c b/testcases/network/nfs/nfs_stress/nfs05_make_tree.c
> > index fdc13bd5d..5456c1bf0 100644
> > --- a/testcases/network/nfs/nfs_stress/nfs05_make_tree.c
> > +++ b/testcases/network/nfs/nfs_stress/nfs05_make_tree.c
> > @@ -215,4 +215,5 @@ static struct tst_test test = {
> >  	.options = opts,
> >  	.test_all = do_test,
> >  	.setup = setup,
> > +	.max_runtime = 300,
> >  };
> > -- 
> > 2.36.1


> > -- 
> > Mailing list info: https://lists.linux.it/listinfo/ltp
Cyril Hrubis June 17, 2022, 3:09 p.m. UTC | #3
Hi!
> > I guess that it would probably implement main() in the nfs05_make_tree.c
> > instead as we do for the various tools in testcases/lib/*.c. That would
> > at least make sure that we will not break anything with changes intended
> > for tests and not for helper binaries as this one.
> 
> Helper binaries does not use LTP C API. I'd like to keep it (to have SAFE_*()),
> I guess you're not against. Therefore just using TST_NO_DEFAULT_MAIN
> and adding main(), right?

I guess that would work, but I will have to double check the tst_test.c
library.

Also I'm not sure that this would be the best solution, mixing C and
shell code was always akward and maybe we can figure out how to make it
better.
diff mbox series

Patch

diff --git a/testcases/network/nfs/nfs_stress/nfs05_make_tree.c b/testcases/network/nfs/nfs_stress/nfs05_make_tree.c
index fdc13bd5d..5456c1bf0 100644
--- a/testcases/network/nfs/nfs_stress/nfs05_make_tree.c
+++ b/testcases/network/nfs/nfs_stress/nfs05_make_tree.c
@@ -215,4 +215,5 @@  static struct tst_test test = {
 	.options = opts,
 	.test_all = do_test,
 	.setup = setup,
+	.max_runtime = 300,
 };