[LTP] Test library API changes

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
I'm back after my vacation and I had a some time to experiment with the
LTP test API. My intention is to make the changes in the next
development cycle. I.e. we will concentrate on the release in upcoming
weeks and once that is taken care of I would like to discuss these
changes.

I've pushed one cleanup that removes tst_res() and tst_brk() from the
codebase today since there was only one test using this interface and
that was a typo anyway.

Two patches that actually implement sketch of the new API are in forked
LTP repository at:

https://github.com/metan-ucw/ltp

The interesting files are:

https://github.com/metan-ucw/ltp/blob/master/include/tst_test.h
and
https://github.com/metan-ucw/ltp/blob/master/lib/tst_test.c
and one testcase that has been converted to the new library:
https://github.com/metan-ucw/ltp/blob/master/testcases/kernel/syscalls/preadv/preadv01.c

The general intention is to move as much code to the library as possible
in order to avoid common mistakes and also to let the programmer
concentrate on the test itself rather than spending time figuring out
the test library API.

One of the important changes is that the library functionality is
exported as bitflags in the test structure rather than function calls,
this makes the library much easier to use since this eliminates all
problems with ordering of the resource inits/cleanups.

Another big change is that the process no longer runs the test function
itself but rather than that export pointer to the function that runs the
test. This would allow us to finally fix problems with test timeouts
etc. since we can easily fork in the test library before we start
execution the test code and watch for segfaults/timeouts etc.

The code I've wrote just a proof of concept and by no means complete.
It's intended to start a conversation (I can send it as a patch here as
well if desired, but I do not think that it makes sense for so early
prototype).

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: ltp@lists.linux.it
> Sent: Tuesday, 5 January, 2016 12:11:36 PM
> Subject: [LTP] Test library API changes
> 
> Hi!
> I'm back after my vacation and I had a some time to experiment with the
> LTP test API. My intention is to make the changes in the next
> development cycle. I.e. we will concentrate on the release in upcoming
> weeks and once that is taken care of I would like to discuss these
> changes.
> 
> I've pushed one cleanup that removes tst_res() and tst_brk() from the
> codebase today since there was only one test using this interface and
> that was a typo anyway.
> 
> Two patches that actually implement sketch of the new API are in forked
> LTP repository at:
> 
> https://github.com/metan-ucw/ltp
> 
> The interesting files are:
> 
> https://github.com/metan-ucw/ltp/blob/master/include/tst_test.h
> and
> https://github.com/metan-ucw/ltp/blob/master/lib/tst_test.c
> and one testcase that has been converted to the new library:
> https://github.com/metan-ucw/ltp/blob/master/testcases/kernel/syscalls/preadv/preadv01.c
> 
> The general intention is to move as much code to the library as possible
> in order to avoid common mistakes and also to let the programmer
> concentrate on the test itself rather than spending time figuring out
> the test library API.
> 
> One of the important changes is that the library functionality is
> exported as bitflags in the test structure rather than function calls,
> this makes the library much easier to use since this eliminates all
> problems with ordering of the resource inits/cleanups.
> 
> Another big change is that the process no longer runs the test function
> itself but rather than that export pointer to the function that runs the
> test. This would allow us to finally fix problems with test timeouts
> etc. since we can easily fork in the test library before we start
> execution the test code and watch for segfaults/timeouts etc.
> 
> The code I've wrote just a proof of concept and by no means complete.
> It's intended to start a conversation (I can send it as a patch here as
> well if desired, but I do not think that it makes sense for so early
> prototype).

Hi,

1) https://github.com/metan-ucw/ltp/blob/master/include/tst_test.h#L65
I did like there is option for test_all(). One concern I had was
that forcing everybody to use test(int) was too restrictive.
For example, you want to pass more parameters than just testcase #.
Also not all tests follow "static struct tcase {" idiom, with single
test function that is fed different arguments. I think test_all()
gives us more flexibility. I was also thinking if library should
care about "tcnt".

2) https://github.com/metan-ucw/ltp/blob/master/lib/tst_test.c#L218-L219
This also looks more strict requirement than what we had until now.
It requires to count exactly how many PASS/FAIL there are.
Is extra PASS or SKIP message a reason to fail the test with brk?

3) cleanup and children
I know, it's a proof of concept :-).

4) After first reading, I'm not sure I have clear picture of changes in API.
From what I gathered these are main changes:
  test.h -> tst_test.h
  tst_resm -> tst_res
  tst_brkm -> tst_brk
  safe_macros.h -> tst_safe_macros.h (now included via tst_test.h)

Did I miss anything?

Regards,
Jan

> 
> --
> Cyril Hrubis
> chrubis@suse.cz
> 
> --
> Mailing list info: http://lists.linux.it/listinfo/ltp
> 

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
> 1) https://github.com/metan-ucw/ltp/blob/master/include/tst_test.h#L65
> I did like there is option for test_all(). One concern I had was
> that forcing everybody to use test(int) was too restrictive.
> For example, you want to pass more parameters than just testcase #.
> Also not all tests follow "static struct tcase {" idiom, with single
> test function that is fed different arguments. I think test_all()
> gives us more flexibility. I was also thinking if library should
> care about "tcnt".

My primary concern for adding the test_all() was a bit different. There
are testcases where tests depends on each other and must be executed in
exact order. So instead of adding overly complicated way how to express
the dependencies I've added a possibility to run all tests in one
function. But yes the concern that enforcing too strict format may
complicate the conversion of the testcases was there as well.

> 2) https://github.com/metan-ucw/ltp/blob/master/lib/tst_test.c#L218-L219
> This also looks more strict requirement than what we had until now.
> It requires to count exactly how many PASS/FAIL there are.
> Is extra PASS or SKIP message a reason to fail the test with brk?

That depends. If we want to produce more structured/detailed logs than
PASS/FAIL/SKIP for one test binary we should be more strict about this.
Since at the moment single skipped testcase marks the whole test as
skipped. I would like to get to the state where the test result rather
says: "9 passed 1 skipped".

> 3) cleanup and children
> I know, it's a proof of concept :-).

I wasn't looking into thread safety either. So far it's just a sketch of
API + very minimal implementation.

> 4) After first reading, I'm not sure I have clear picture of changes in API.
> From what I gathered these are main changes:
>   test.h -> tst_test.h
>   tst_resm -> tst_res
>   tst_brkm -> tst_brk
>   safe_macros.h -> tst_safe_macros.h (now included via tst_test.h)

I've tried to do the changes in a way that old and new library can
coexist for some time, since I guess that the conversion would take
quite a lot of time. That is the main reason for the tst_resm -> tst_res
change. The safe_macros header was split into two, the new one does not
take the cleanup parameter and calls the same functions (as the old one)
but with with NULL passed a the callback. What wasn't done yet is
rerouting tst_resm()/tst_brkm() to the new library in case that new
library is used so that rest of the code in lib/ can be called from the
new library.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
Since we successfuly released LTP I've restored the effort to create
better test library, updated changes are at:

https://github.com/metan-ucw/ltp

There are couple of smaller changes:

* The minimal kernel version is now stored in tst_test structure
  (which avoids calling tst_kvercmp() in setup())

* The tst_resm() and tst_brkm() are now rerouted to new library
  in case that test was started using the new library and in case
  that cleanup is NULL for tst_brkm().

  Now the SAFE_MACROS() and rest of the library calls works from newlib
  tests as well. Though most of the library calls would end up with
  wrappers without the cleanup callback just like I did for the
  SAFE_MACROS() allready.

* Unless TST_NO_DEFAULT_MAIN is defined the tst_test.h adds default main
  that picks up struct tst_test named test and run the tests.

* tst_syscall() has been added since the ltp_syscall() quietly adds
  cleanup parameter to tst_brkm()


There are two more tests converted:

https://github.com/metan-ucw/ltp/blob/master/testcases/kernel/syscalls/add_key/add_key01.c
https://github.com/metan-ucw/ltp/blob/master/testcases/kernel/syscalls/add_key/add_key02.c

And preadv test is updated:

https://github.com/metan-ucw/ltp/blob/master/testcases/kernel/syscalls/preadv/preadv01.c


My current plan is to try to convert a few more testcases and edhance
the new lib API on the go, then work more on the library internals that
are pretty minimalistic at the moment.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
New vesion at:

https://github.com/metan-ucw/ltp

Some fixes:

* Fixed a few SAFE_MACROS() (removed forgotten callback parameter)

* Fixed tst_lib.c to compile (defined TST_NO_DEFAULT_MAIN)

* TERRNO and TTERRNO works now


And new functionality:

I've changed SAFE_CLOSE() to set the fd paramter to -1 on sucessful
exit, which makes it easier to use together with the pattern common in
cleanup() which does fd > 0 && close(fd). How do you like this idea?


And also another change I spend some time thinking about is that tcnt is
now divided into two parts. The tcnt strictly defines number of tests
done by the test() function and the newly introduced acnt (assertion
count) defines number of PASS/FAIL reported by the function when it's
called. This makes it easier to work with following pattern:


	fd = create_fd()
	if (fd < 0)
		tst_brk(TBROK, ...);

	if (bar)
		tst_res(TPASS, ...);
	else
		tst_res(TFAIL, ...);

	if (bar)
		tst_res(TPASS, ...);
	else
		tst_res(TFAIL, ...);


Now if we define acnt = 2 in the test structure and tcnt = 2 the library
would expect that there are two tests done by the function and that it
could be called twice with i = 0 and i = 1. Does this sound reasonable
as well?

There is also new converted test creat01:

https://github.com/metan-ucw/ltp/blob/master/testcases/kernel/syscalls/creat/creat01.c

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
This time I've added functionality to handle child processes:

https://github.com/metan-ucw/ltp/blob/master/include/tst_test.h#L48

This function forks a child, runs the function and propagates the
results to the parent process via pipe.

The function is implemented at:

https://github.com/metan-ucw/ltp/blob/master/lib/tst_test.c#L129

And used in newly converted test at:

https://github.com/metan-ucw/ltp/blob/master/testcases/kernel/syscalls/creat/creat04.c

Whith this the core test library API is more or less sketched.

PS: I would appreciate any comments.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
> This time I've added functionality to handle child processes:
> 
> https://github.com/metan-ucw/ltp/blob/master/include/tst_test.h#L48
> 
> This function forks a child, runs the function and propagates the
> results to the parent process via pipe.
> 
> The function is implemented at:
> 
> https://github.com/metan-ucw/ltp/blob/master/lib/tst_test.c#L129
> 
> And used in newly converted test at:
> 
> https://github.com/metan-ucw/ltp/blob/master/testcases/kernel/syscalls/creat/creat04.c
> 
> Whith this the core test library API is more or less sketched.

And I was writing faster than thinking. We will need another one where
starting and reaping child is split between two functions. I will work
on that tomorrow.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
> And I was writing faster than thinking. We will need another one where
> starting and reaping child is split between two functions. I will work
> on that tomorrow.

And I've split the function into two now:

https://github.com/metan-ucw/ltp/blob/master/include/tst_test.h#L50

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Tuesday, 9 February, 2016 6:46:18 PM
> Subject: Re: [LTP] Test library API changes
> 
> Hi!
> > And I was writing faster than thinking. We will need another one where
> > starting and reaping child is split between two functions. I will work
> > on that tomorrow.
> 
> And I've split the function into two now:
> 
> https://github.com/metan-ucw/ltp/blob/master/include/tst_test.h#L50

I had a look and have some comments/questions?

1. Why is tcnt not ARRAY_SIZE(tcases)?
   https://github.com/metan-ucw/ltp/blob/master/testcases/kernel/syscalls/creat/creat04.c#L83

static struct tst_test test = {
	.tid = "creat04",
	.tcnt = 2,

2. It would be nice if we could pass any parameters to child.
Atm. child func accepts only int and only way to share something else
appears to be passing it via global variables, which may become
confusing for bigger tests.

3. Sum based on tcnt still worries me because number of assertions per
test may vary:
   if (sum != tcnt * acnt * iterations)
	tst_brk(TBROK, "Number passes/fails/skipps does not match number of tests");

What if we verified instead, that after every testcase, at least one of
pass/fail/skip increased?

4. suggestion: improved safeguard for cleanup
Record pid of main process at start and call cleanup only if current
pid is recorded pid. This doesn't make it depend on using specific
API call to run child process.

Regards,
Jan

> 
> --
> Cyril Hrubis
> chrubis@suse.cz
> 

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
> I had a look and have some comments/questions?
> 
> 1. Why is tcnt not ARRAY_SIZE(tcases)?
>    https://github.com/metan-ucw/ltp/blob/master/testcases/kernel/syscalls/creat/creat04.c#L83

My mistake. Will fix that.

> static struct tst_test test = {
> 	.tid = "creat04",
> 	.tcnt = 2,
> 
> 2. It would be nice if we could pass any parameters to child.
> Atm. child func accepts only int and only way to share something else
> appears to be passing it via global variables, which may become
> confusing for bigger tests.

I was thinking about passing a pointer just as pthread_create() does as
well. Let's go this way instead.

> 3. Sum based on tcnt still worries me because number of assertions per
> test may vary:
>    if (sum != tcnt * acnt * iterations)
> 	tst_brk(TBROK, "Number passes/fails/skipps does not match number of tests");
> 
> What if we verified instead, that after every testcase, at least one of
> pass/fail/skip increased?

That is a good idea. I will rework it this way.

> 4. suggestion: improved safeguard for cleanup
> Record pid of main process at start and call cleanup only if current
> pid is recorded pid. This doesn't make it depend on using specific
> API call to run child process.

There is another problem that has to be solved if we want to go this
way. We have to pass the results from the child to the parent and since
it's more complicated than single PASS/FAIL/SKIP/BROK now, we have to
pass it somehow. So we would have to create a communication channel
anyway. Which means either opening one by default before the test starts
or doing that in tst_fork() wrapper.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
> 4. suggestion: improved safeguard for cleanup
> Record pid of main process at start and call cleanup only if current
> pid is recorded pid. This doesn't make it depend on using specific
> API call to run child process.

You are right, we should avoid calling cleanup() and internal do_exit()
in case that process was forked by plain tst_fork(), which is reasonable
action if process needs child and do not need to use tst_resm()
reporting inside. I would say that not doing exit(0) from the child is a
mistake in this case, but we should certainly at least issue an error
message. And also we must not call the callback if such child called
tst_brk(TBROK,..) (which is desirable so that we can use SAFE_MACROS()
in such cases).

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
I've redone the child handling and now it does:

* If test sets forks_child flag in test structure pipe is opened at the
  start of the test which is used to propagate test results from a child
  to parent

* Child processes are created via SAFE_FORK(), which flushes stdout,
  just in case there is something there, checks for for fork() failure
  and clears the result structure.

* After each test run (call to test() function), wait() is called until
  it returns ECHILD.

  If child was succesfully waited(), pipe is examined and if it contains
  test results, these are added to parent results.

  We look into the pipe after each sucessful wait() in order not to fill
  up the pipe capacity with many children.

* If child calls tst_brk(), exit() with non-zero value is called which is
  handled in parent wait() and the parent exits with tst_brk() as well.

  Techincaly any child that does not exit with 0 cause main test process
  to report TCONF/TBROK.

  At the moment the tst_brk() semantics is to exit the whole test since
  something unexpected happened. Which seems to be right course of
  action since failing SAFE_MACRO() in child should really cause main
  test process to exit.

  So if tst_brk() stays as it is we would need to add another call that
  can exit the child (would do the same action as returning from the
  test() function, i.e. write results and do exit(0)).

The code is at the same place at:

https://github.com/metan-ucw/ltp

Few test/example programs:

https://github.com/metan-ucw/ltp/blob/master/lib/newlib_tests/test05.c
https://github.com/metan-ucw/ltp/blob/master/lib/newlib_tests/test06.c
https://github.com/metan-ucw/ltp/blob/master/lib/newlib_tests/test07.c

There are probably stil a few rough edges: children forked from
children, waiting for rest of the children after one of them called
tst_brk(), etc. But the basic functionality seems to work fine.

As usuall comments are welcome.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Thursday, 11 February, 2016 5:03:13 PM
> Subject: Re: [LTP] Test library API changes
> 
> Hi!
> I've redone the child handling and now it does:
> 
> * If test sets forks_child flag in test structure pipe is opened at the
>   start of the test which is used to propagate test results from a child
>   to parent

Hi,

I was thinking about mapping a piece of shared memory for "struct results",
then we wouldn't have to worry about how/when child terminates and how
big buffer for resutls we need.

And if it was always there, it is one less parameter (forks_child) user
needs to provide.

> 
> * Child processes are created via SAFE_FORK(), which flushes stdout,
>   just in case there is something there, checks for for fork() failure
>   and clears the result structure.
> 
> * After each test run (call to test() function), wait() is called until
>   it returns ECHILD.
> 
>   If child was succesfully waited(), pipe is examined and if it contains
>   test results, these are added to parent results.
> 
>   We look into the pipe after each sucessful wait() in order not to fill
>   up the pipe capacity with many children.
> 
> * If child calls tst_brk(), exit() with non-zero value is called which is
>   handled in parent wait() and the parent exits with tst_brk() as well.
> 
>   Techincaly any child that does not exit with 0 cause main test process
>   to report TCONF/TBROK.
> 
>   At the moment the tst_brk() semantics is to exit the whole test since
>   something unexpected happened. Which seems to be right course of
>   action since failing SAFE_MACRO() in child should really cause main
>   test process to exit.

We have tests that crash/kill child on purpose. As I recall some examples
are mprotect and oom tests.

These tests would need to wait for child themselves, since non-zero exit
code is expected and we don't want reap_children() to see them and stop
the test.

> 
>   So if tst_brk() stays as it is we would need to add another call that
>   can exit the child (would do the same action as returning from the
>   test() function, i.e. write results and do exit(0)).

Other notes:

write_result calls "exit(0);", and run_tests calls exit as well after
it calls write_result.

If we stay with pipe and write_result, can we use atexit() to call it?
That would allow child to exit anywhere with "exit()" instead of making
sure it returns all the way to run_tests function.

Regards,
Jan

> 
> The code is at the same place at:
> 
> https://github.com/metan-ucw/ltp
> 
> Few test/example programs:
> 
> https://github.com/metan-ucw/ltp/blob/master/lib/newlib_tests/test05.c
> https://github.com/metan-ucw/ltp/blob/master/lib/newlib_tests/test06.c
> https://github.com/metan-ucw/ltp/blob/master/lib/newlib_tests/test07.c
> 
> There are probably stil a few rough edges: children forked from
> children, waiting for rest of the children after one of them called
> tst_brk(), etc. But the basic functionality seems to work fine.
> 
> As usuall comments are welcome.
> 
> --
> Cyril Hrubis
> chrubis@suse.cz
> 

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
> I was thinking about mapping a piece of shared memory for "struct results",
> then we wouldn't have to worry about how/when child terminates and how
> big buffer for resutls we need.

I did so as well, but then you need a temporary directory and backing
file and futexes/atomic operations to coordinate access to it. But we
would need the mutexes/atomic operations for threads as well...

> And if it was always there, it is one less parameter (forks_child) user
> needs to provide.

I've added it since it will serve another purpose as well. If we want to
do tst_sig() automatically before we start testcases we need to know if
process is expected to fork() children.

> > * Child processes are created via SAFE_FORK(), which flushes stdout,
> >   just in case there is something there, checks for for fork() failure
> >   and clears the result structure.
> > 
> > * After each test run (call to test() function), wait() is called until
> >   it returns ECHILD.
> > 
> >   If child was succesfully waited(), pipe is examined and if it contains
> >   test results, these are added to parent results.
> > 
> >   We look into the pipe after each sucessful wait() in order not to fill
> >   up the pipe capacity with many children.
> > 
> > * If child calls tst_brk(), exit() with non-zero value is called which is
> >   handled in parent wait() and the parent exits with tst_brk() as well.
> > 
> >   Techincaly any child that does not exit with 0 cause main test process
> >   to report TCONF/TBROK.
> > 
> >   At the moment the tst_brk() semantics is to exit the whole test since
> >   something unexpected happened. Which seems to be right course of
> >   action since failing SAFE_MACRO() in child should really cause main
> >   test process to exit.
> 
> We have tests that crash/kill child on purpose. As I recall some examples
> are mprotect and oom tests.
>
> These tests would need to wait for child themselves, since non-zero exit
> code is expected and we don't want reap_children() to see them and stop
> the test.

This is the reason the library does not store children pids or counts
forked children. All it cares about are child processes that outlived
the main test process. If the main test process waits() it's children
and the children does crash/exit() they never end up in the library code
so the child would not write the result and the parent will not reap it
since it was waited() allready. It will leak the pipe file descriptor in
the child in this case, but that will be closed at exit() by system.

Or did I miss something?

> >   So if tst_brk() stays as it is we would need to add another call that
> >   can exit the child (would do the same action as returning from the
> >   test() function, i.e. write results and do exit(0)).
> 
> Other notes:
> 
> write_result calls "exit(0);", and run_tests calls exit as well after
> it calls write_result.

Right my mistake.

> If we stay with pipe and write_result, can we use atexit() to call it?
> That would allow child to exit anywhere with "exit()" instead of making
> sure it returns all the way to run_tests function.

That would break the case when the test process does wait() for it's
own children since we will end up writing results anyway.

Maybe the page of shared memory is right way to propagate the results
after all. I will think about it.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
The up to date code is at the same place as usual.
(https://github.com/metan-ucw/ltp)

Apart from added support of acquiring a device, checkpoints and
resource files the IPC was rewritten to shared memory.

Now the library, before the test is started, mmaps a page backed by a
file on /dev/shm/. The first few bytes are used for shared struct
results, the rest is used by checkpoints if checkpoints are needed. The
struct result values are incremented by gcc atomic operations (which is
thread safe as well), the open question is if we need a fallback to
inline assembler if these are not supported. Then there is a special
function to open checkpoint shm file from a process started by a exec (a
few of the testcases does so) since it must open the shm rather than a
file in a temporary directory.

There are still some rough edges and the code should be reviewed, but at
least 90% of the functionality is there and working.

As usuall comments are welcome :)

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Tuesday, 16 February, 2016 10:19:58 PM
> Subject: Re: [LTP] Test library API changes
> 
> Hi!
> The up to date code is at the same place as usual.
> (https://github.com/metan-ucw/ltp)
> 
> Apart from added support of acquiring a device, checkpoints and
> resource files the IPC was rewritten to shared memory.
> 
> Now the library, before the test is started, mmaps a page backed by a
> file on /dev/shm/. The first few bytes are used for shared struct
> results, the rest is used by checkpoints if checkpoints are needed. The
> struct result values are incremented by gcc atomic operations (which is
> thread safe as well), the open question is if we need a fallback to
> inline assembler if these are not supported. Then there is a special
> function to open checkpoint shm file from a process started by a exec (a
> few of the testcases does so) since it must open the shm rather than a
> file in a temporary directory.
> 
> There are still some rough edges and the code should be reviewed, but at
> least 90% of the functionality is there and working.
> 
> As usuall comments are welcome :)

Hi,

My first impression of this version is that it looks simpler,
I find it easier to follow, but that could also be explained
by fact that I'm reading it for 3rd time :-).

Here are some comments/questions/ideas:

1. acnt appears to be unused now

2. Can we keep ltp_syscall() and call correct brk func with some magic?

3. new/old files names do not follow obvious pattern
For example: tst_dev.h vs tst_device.h
I'm thinking if we can give all old same kind of prefix, like tst1_*
to distinguish them from new api files.

4. ipc_fd = open(name, O_CREAT | O_EXCL | O_RDWR, 0600);
Should we allow other users to read/write too? For example,
what if I change euid and then fork? Or do we expect to always
fork and then change euid?

5. setup_ipc ideas/comments (some of these are connected)

5a) Why not always setup results and checkpoints?
    If we always map file, it doesn't cost us anything
    to initialize checkpoints. needs_checkpoints could be
    removed.
    Also processes spawned via exec could report results
    easily to main process if we always initialized both.

5b) split setup_ipc into setup_ipc and open_ipc,
    That should eliminate some code from tst_checkpoint_open()

5c) What if we stored ipc path to env variable?

setup_ipc
  generates tmp name based on test name: ltp_ipc_path
  for convenience will initialize also envp array:
    ltp_only_ipc_env[] = { "LTP_IPC_PATH="$ltp_ipc_path, NULL }
  creates ipc file

open_ipc(ipc_path)
  if results and checkpoints already initialized
    return
  if ipc_path == NULL
    ipc_path = getenv("LTP_IPC_PATH")
  if ipc_path == NULL
    brk()
  open ipc file and initialize both results and checkpoints

TST_CHECKPOINT_INIT calls open_ipc(NULL)
  process spawned via exec could call either of them,
  effect would be the same

5d) Always unlink ipc file in setup if we have /proc
    Main pid keeps file open and unlinks it.
    ltp_ipc_path would be set to "/proc/$main_pid/fd/$fd"

Regards,
Jan

> 
> --
> Cyril Hrubis
> chrubis@suse.cz
> 

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
> My first impression of this version is that it looks simpler,
> I find it easier to follow, but that could also be explained
> by fact that I'm reading it for 3rd time :-).

Well the SHM code is a bit easier than the pipes but not that much
easier, so I would say it's a combination of both.

> Here are some comments/questions/ideas:
>
> 1. acnt appears to be unused now

I haven't removed it yet... I'm still thinking how we can generate
things like TAP output that expects us to print exact number of tests on
the first line followed by exact number of result lines and make as
little rules for result reporting as possible.

Maybe we can just say that testcases that define test_all do one just
one test and produce one result per such test and do more detailed
reporting only for testcases that define tcnt.

Or we can turn on the detailed reporting only if acnt has been set.

> 2. Can we keep ltp_syscall() and call correct brk func with some magic?

Well we can split the header as we did with the rest of them, do you
think that it's worth of it?

> 3. new/old files names do not follow obvious pattern
> For example: tst_dev.h vs tst_device.h
> I'm thinking if we can give all old same kind of prefix, like tst1_*
> to distinguish them from new api files.

I would go for tst_old_ prefix in this case.

We can easily change all but safe_macros.h that is explicitly included
in ~200 testcases.

> 4. ipc_fd = open(name, O_CREAT | O_EXCL | O_RDWR, 0600);
> Should we allow other users to read/write too? For example,
> what if I change euid and then fork? Or do we expect to always
> fork and then change euid?

We have four tests that uses checkpoint from child processes, none of
them does change euid, so I've just put 0600 there. We can always change
that if needed. Or do you think that we should just enable it now since
it does not matter much?

> 5. setup_ipc ideas/comments (some of these are connected)
> 
> 5a) Why not always setup results and checkpoints?
>     If we always map file, it doesn't cost us anything
>     to initialize checkpoints. needs_checkpoints could be
>     removed.

That depends. If we want to be able to change the library internals in
the future we have to force the test to explicitly list what it's using,
otherwise we would have to anotate all tests that use checkpoints in the
futere in case that they wouldn't be initialized as a byproduct of
result propagation.

In short if we decide that shared memory is what we will use from now on
till the end of the world we can remove it. :)

>     Also processes spawned via exec could report results
>     easily to main process if we always initialized both.

Well we can but I see much value in this since execed() test processes
are quite rare (~10) and all of them seems to report PASS/FAIL using the
return value.

> 5b) split setup_ipc into setup_ipc and open_ipc,
>     That should eliminate some code from tst_checkpoint_open()

Sure, there are still some leftovers since I've commited first working
version late in the evening :).

> 5c) What if we stored ipc path to env variable?
> 
> setup_ipc
>   generates tmp name based on test name: ltp_ipc_path
>   for convenience will initialize also envp array:
>     ltp_only_ipc_env[] = { "LTP_IPC_PATH="$ltp_ipc_path, NULL }
>   creates ipc file

Hmm, that way the test would have to explicitly pass it to the execve().

I would rather make it reasonably unique but decideable without
explicitly passing variables around.

The current approach would not work when the the process is execed by an
child of the test process, which is not ideal either.

> open_ipc(ipc_path)
>   if results and checkpoints already initialized
>     return
>   if ipc_path == NULL
>     ipc_path = getenv("LTP_IPC_PATH")
>   if ipc_path == NULL
>     brk()
>   open ipc file and initialize both results and checkpoints
> 
> TST_CHECKPOINT_INIT calls open_ipc(NULL)
>   process spawned via exec could call either of them,
>   effect would be the same
>
> 5d) Always unlink ipc file in setup if we have /proc
>     Main pid keeps file open and unlinks it.
>     ltp_ipc_path would be set to "/proc/$main_pid/fd/$fd"

This is a good idea, that way the memory would be freed even when the
main test process fails to do cleanup.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Wednesday, 17 February, 2016 4:54:06 PM
> Subject: Re: [LTP] Test library API changes
> 
> Hi!
> > My first impression of this version is that it looks simpler,
> > I find it easier to follow, but that could also be explained
> > by fact that I'm reading it for 3rd time :-).
> 
> Well the SHM code is a bit easier than the pipes but not that much
> easier, so I would say it's a combination of both.
> 
> > Here are some comments/questions/ideas:
> >
> > 1. acnt appears to be unused now
> 
> I haven't removed it yet... I'm still thinking how we can generate
> things like TAP output that expects us to print exact number of tests on
> the first line followed by exact number of result lines and make as
> little rules for result reporting as possible.
> 
> Maybe we can just say that testcases that define test_all do one just
> one test and produce one result per such test and do more detailed
> reporting only for testcases that define tcnt.
> 
> Or we can turn on the detailed reporting only if acnt has been set.
> 
> > 2. Can we keep ltp_syscall() and call correct brk func with some magic?
> 
> Well we can split the header as we did with the rest of them, do you
> think that it's worth of it?

I was thinking some ifdef magic. It has same signature in both
versions of API, so adding new function with different name,
that does pretty much the same seems like unnecessary complication.

> 
> > 3. new/old files names do not follow obvious pattern
> > For example: tst_dev.h vs tst_device.h
> > I'm thinking if we can give all old same kind of prefix, like tst1_*
> > to distinguish them from new api files.
> 
> I would go for tst_old_ prefix in this case.
> 
> We can easily change all but safe_macros.h that is explicitly included
> in ~200 testcases.
> 
> > 4. ipc_fd = open(name, O_CREAT | O_EXCL | O_RDWR, 0600);
> > Should we allow other users to read/write too? For example,
> > what if I change euid and then fork? Or do we expect to always
> > fork and then change euid?
> 
> We have four tests that uses checkpoint from child processes, none of
> them does change euid, so I've just put 0600 there. We can always change
> that if needed. Or do you think that we should just enable it now since
> it does not matter much?

I thought it's more. I'm OK we keep it 0600 and see if/when we hit a problem.

> 
> > 5. setup_ipc ideas/comments (some of these are connected)
> > 
> > 5a) Why not always setup results and checkpoints?
> >     If we always map file, it doesn't cost us anything
> >     to initialize checkpoints. needs_checkpoints could be
> >     removed.
> 
> That depends. If we want to be able to change the library internals in
> the future we have to force the test to explicitly list what it's using,
> otherwise we would have to anotate all tests that use checkpoints in the
> futere in case that they wouldn't be initialized as a byproduct of
> result propagation.

OK, that's a good point.

> 
> In short if we decide that shared memory is what we will use from now on
> till the end of the world we can remove it. :)
> 
> >     Also processes spawned via exec could report results
> >     easily to main process if we always initialized both.
> 
> Well we can but I see much value in this since execed() test processes
> are quite rare (~10) and all of them seems to report PASS/FAIL using the
> return value.
> 
> > 5b) split setup_ipc into setup_ipc and open_ipc,
> >     That should eliminate some code from tst_checkpoint_open()
> 
> Sure, there are still some leftovers since I've commited first working
> version late in the evening :).
> 
> > 5c) What if we stored ipc path to env variable?
> > 
> > setup_ipc
> >   generates tmp name based on test name: ltp_ipc_path
> >   for convenience will initialize also envp array:
> >     ltp_only_ipc_env[] = { "LTP_IPC_PATH="$ltp_ipc_path, NULL }
> >   creates ipc file
> 
> Hmm, that way the test would have to explicitly pass it to the execve().

True, but it would be rare, as you said it's for ~10 testcases.

> 
> I would rather make it reasonably unique but decideable without
> explicitly passing variables around.

Should we consider multiple instances running at a time? I do
recall that tools/pounder21 allows running things in parallel.
(Not sure if anyone runs more instances of same test though)

Regards,
Jan

> 
> The current approach would not work when the the process is execed by an
> child of the test process, which is not ideal either.
> 
> > open_ipc(ipc_path)
> >   if results and checkpoints already initialized
> >     return
> >   if ipc_path == NULL
> >     ipc_path = getenv("LTP_IPC_PATH")
> >   if ipc_path == NULL
> >     brk()
> >   open ipc file and initialize both results and checkpoints
> > 
> > TST_CHECKPOINT_INIT calls open_ipc(NULL)
> >   process spawned via exec could call either of them,
> >   effect would be the same
> >
> > 5d) Always unlink ipc file in setup if we have /proc
> >     Main pid keeps file open and unlinks it.
> >     ltp_ipc_path would be set to "/proc/$main_pid/fd/$fd"
> 
> This is a good idea, that way the memory would be freed even when the
> main test process fails to do cleanup.
> 
> --
> Cyril Hrubis
> chrubis@suse.cz
> 

[LTP] Test library API changes

[Alexey Kodanev]

Hi,
On 02/17/2016 06:54 PM, Cyril Hrubis wrote:
> Hi!
>> Here are some comments/questions/ideas:
>>
>> 1. acnt appears to be unused now
> I haven't removed it yet... I'm still thinking how we can generate
> things like TAP output that expects us to print exact number of tests on
> the first line followed by exact number of result lines and make as
> little rules for result reporting as possible.
>
> Maybe we can just say that testcases that define test_all do one just
> one test and produce one result per such test and do more detailed
> reporting only for testcases that define tcnt.
>
> Or we can turn on the detailed reporting only if acnt has been set.

Not sure, but if I read TAP spec correctly, it doesn't strictly define
that it should beon the first line, rather appear only once at the start
or at the end (if the number of test points is not known).

So if tcnt for the test plan, what is 'acnt' for? ... I didn't find the
analogy in the TAP spec.

Looking at the 'struct tst_test',

* the names like 'tcnt' and 'acnt' seem not clear. May be we should name 
'tcnt' as
test_planor test_count?

* also, why bitfields in the struct tst_test have different number of 
storage bits?

   int needs_tmpdir:1;
   int needs_root:2;
   int forks_child:3;
   int needs_device:4;
   int needs_checkpoints:5;

if their value is just '0' or '1', ':1' - should be instead.

Best regards,
Alexey

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
> > I haven't removed it yet... I'm still thinking how we can generate
> > things like TAP output that expects us to print exact number of tests on
> > the first line followed by exact number of result lines and make as
> > little rules for result reporting as possible.
> >
> > Maybe we can just say that testcases that define test_all do one just
> > one test and produce one result per such test and do more detailed
> > reporting only for testcases that define tcnt.
> >
> > Or we can turn on the detailed reporting only if acnt has been set.
> 
> Not sure, but if I read TAP spec correctly, it doesn't strictly define
> that it should beon the first line, rather appear only once at the start
> or at the end (if the number of test points is not known).

I missed this part. I guess that we can always generate the number of
tests after the test was run and even when the number of tests is needed
in the result log beforehand we can just write the whole log once the
test finishes.

Explicit number of tests defined beforhand has only one advantage. We
would be able to detect a cases where the test has exited cleanly before
the test has really finished.

> So if tcnt for the test plan, what is 'acnt' for? ... I didn't find the
> analogy in the TAP spec.

Technically now the tcnt defines number of tests implemented by the
test() function. The acnt was meant as number of assertions but is
unused at the moment.

I.e. if test() function prints two PASS/FAIL statements during a call
acnt would have been set to 2 and the total number of PASS/FAIL messages
would be tcnt * acnt. But Jan thinks that this would be unnecessarily
strict requirement.

> Looking at the 'struct tst_test',
> 
> * the names like 'tcnt' and 'acnt' seem not clear. May be we should name 
> 'tcnt' as
> test_planor test_count?

This is definitly something that hasn't been finished yet...

> * also, why bitfields in the struct tst_test have different number of 
> storage bits?
> 
>    int needs_tmpdir:1;
>    int needs_root:2;
>    int forks_child:3;
>    int needs_device:4;
>    int needs_checkpoints:5;
> 
> if their value is just '0' or '1', ':1' - should be instead.

Good catch. This is silly mistake of mine.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
> > > 2. Can we keep ltp_syscall() and call correct brk func with some magic?
> > 
> > Well we can split the header as we did with the rest of them, do you
> > think that it's worth of it?
> 
> I was thinking some ifdef magic. It has same signature in both
> versions of API, so adding new function with different name,
> that does pretty much the same seems like unnecessary complication.

My reasoning was different. There are only two functions that start with
ltp_ in the library the ltp_syscall and ltp_clone. So the reason for
rename is unifying on tst_ and it was convinient in this case as well.
But I do not care that much about this. If you think keeping
ltp_syscall() is better, then we can go for it.

> > > 5c) What if we stored ipc path to env variable?
> > > 
> > > setup_ipc
> > >   generates tmp name based on test name: ltp_ipc_path
> > >   for convenience will initialize also envp array:
> > >     ltp_only_ipc_env[] = { "LTP_IPC_PATH="$ltp_ipc_path, NULL }
> > >   creates ipc file
> > 
> > Hmm, that way the test would have to explicitly pass it to the execve().
> 
> True, but it would be rare, as you said it's for ~10 testcases.
> 
> > 
> > I would rather make it reasonably unique but decideable without
> > explicitly passing variables around.
> 
> Should we consider multiple instances running at a time? I do
> recall that tools/pounder21 allows running things in parallel.
> (Not sure if anyone runs more instances of same test though)

I think that disabling the possibility just to make writing the test
library a bit easier is pretty bad idea. Most of the testcases we have
can run in parallel just fine. There are only a few that stress the
system to the limit or use global resources (devices, IPC, change system
time, ...) and if we anotate these tests we can easily speed up the test
run five times just by running most of the testcases in parallel.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Thursday, 18 February, 2016 12:07:53 PM
> Subject: Re: [LTP] Test library API changes
> 
> Hi!
> > > > 2. Can we keep ltp_syscall() and call correct brk func with some magic?
> > > 
> > > Well we can split the header as we did with the rest of them, do you
> > > think that it's worth of it?
> > 
> > I was thinking some ifdef magic. It has same signature in both
> > versions of API, so adding new function with different name,
> > that does pretty much the same seems like unnecessary complication.
> 
> My reasoning was different. There are only two functions that start with
> ltp_ in the library the ltp_syscall and ltp_clone. So the reason for
> rename is unifying on tst_ and it was convinient in this case as well.
> But I do not care that much about this. If you think keeping
> ltp_syscall() is better, then we can go for it.
> 
> > > > 5c) What if we stored ipc path to env variable?
> > > > 
> > > > setup_ipc
> > > >   generates tmp name based on test name: ltp_ipc_path
> > > >   for convenience will initialize also envp array:
> > > >     ltp_only_ipc_env[] = { "LTP_IPC_PATH="$ltp_ipc_path, NULL }
> > > >   creates ipc file
> > > 
> > > Hmm, that way the test would have to explicitly pass it to the execve().
> > 
> > True, but it would be rare, as you said it's for ~10 testcases.
> > 
> > > 
> > > I would rather make it reasonably unique but decideable without
> > > explicitly passing variables around.
> > 
> > Should we consider multiple instances running at a time? I do
> > recall that tools/pounder21 allows running things in parallel.
> > (Not sure if anyone runs more instances of same test though)
> 
> I think that disabling the possibility just to make writing the test
> library a bit easier is pretty bad idea. Most of the testcases we have
> can run in parallel just fine. There are only a few that stress the
> system to the limit or use global resources (devices, IPC, change system
> time, ...) and if we anotate these tests we can easily speed up the test
> run five times just by running most of the testcases in parallel.

I wasn't suggesting we do that. I was thinking about making ipc filename
more unique for each instance, in case we wouldn't have /proc and test
cleanup doesn't run for whatever reason. That of course would make 
ipc file names less predictable.

Regards,
Jan

> 
> --
> Cyril Hrubis
> chrubis@suse.cz
> 

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
> > I think that disabling the possibility just to make writing the test
> > library a bit easier is pretty bad idea. Most of the testcases we have
> > can run in parallel just fine. There are only a few that stress the
> > system to the limit or use global resources (devices, IPC, change system
> > time, ...) and if we anotate these tests we can easily speed up the test
> > run five times just by running most of the testcases in parallel.
> 
> I wasn't suggesting we do that. I was thinking about making ipc filename
> more unique for each instance, in case we wouldn't have /proc and test
> cleanup doesn't run for whatever reason. That of course would make
> ipc file names less predictable.

If we decide to pass the filename around we can just add arbitrarily
large random string to the name. But the more I think about it the more
it looks like classical chicken egg problem. We cannot pass it without
IPC but we cannot do that easily before IPC is initialized and the
environment variable is probably the best bet.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
The up to date version is at usuall place at:

https://github.com/metan-ucw/ltp


The changes are:

* acnt was removed completly

* the bitflags in struct tst_test were fixed to be 1 bit wide

* the headers were changed so that most of the old headers starts with
  old_ prefix

  The safe_macros.h and test.h were left intact at the moment since
  these are included in numerous testcases. We can rename them in a
  separate patch if needed.

* Added macro TST_TEST_TCONF() to ease writing testcases that are
  guarded by #ifdefs, see newly converted llistxattr testcases

* Few more converted testcases

* Started to update Test-Writing-Guidelines for new API


The open issues:

* The SHM base file path propagation
  - we will probably go with the env variable

* Finishing rewrite of Test Writing Guidelines
  - needs just a bit more work

* Should we allow SAFE_MACROS() in cleanup?

  - we can easily do that since cleanup is executed from the test
    library and we can change tst_brk() to return to the caller
    if cleanup was entered

  - plus point is that writing cleanup is bit easier

  - minus point is that tst_brk() can now return and as a side effect
    tst_brkm() has to return as well (since tst_brkm() from safe macros
    is redirected to tst_brk()) and because of that we have to drop
    the __attribute__((noreturn)) since that causes gcc to optimize out
    return statements from the function which generates compilation
    warnings in some cases


Is there anything else?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>, "Alexey Kodanev" <alexey.kodanev@oracle.com>
> Cc: ltp@lists.linux.it
> Sent: Wednesday, 2 March, 2016 3:44:21 PM
> Subject: Re: [LTP] Test library API changes
> 
> Hi!
> The up to date version is at usuall place at:
> 
> https://github.com/metan-ucw/ltp
> 
> 
> The changes are:
> 
> * acnt was removed completly
> 
> * the bitflags in struct tst_test were fixed to be 1 bit wide
> 
> * the headers were changed so that most of the old headers starts with
>   old_ prefix
> 
>   The safe_macros.h and test.h were left intact at the moment since
>   these are included in numerous testcases. We can rename them in a
>   separate patch if needed.
> 
> * Added macro TST_TEST_TCONF() to ease writing testcases that are
>   guarded by #ifdefs, see newly converted llistxattr testcases
> 
> * Few more converted testcases
> 
> * Started to update Test-Writing-Guidelines for new API
> 
> 
> The open issues:
> 
> * The SHM base file path propagation
>   - we will probably go with the env variable
> 
> * Finishing rewrite of Test Writing Guidelines
>   - needs just a bit more work
> 
> * Should we allow SAFE_MACROS() in cleanup?
> 
>   - we can easily do that since cleanup is executed from the test
>     library and we can change tst_brk() to return to the caller
>     if cleanup was entered

In that case, tst_vbrk_() should also call update_results(), so TBROK
during cleanup doesn't get lost/ignored.

> 
>   - plus point is that writing cleanup is bit easier
> 
>   - minus point is that tst_brk() can now return and as a side effect
>     tst_brkm() has to return as well (since tst_brkm() from safe macros
>     is redirected to tst_brk()) and because of that we have to drop
>     the __attribute__((noreturn)) since that causes gcc to optimize out
>     return statements from the function which generates compilation
>     warnings in some cases

I assume if we drop noreturn, we still have a different way to suppress
those warnings, right?

> 
> 
> Is there anything else?

I ran into number of compilation errors on RHEL5.6, but they will
likely happen on other distros too:

1. lib/ltp_priv.h and va_list
Adding "#include <stdarg.h>" fixed it.

2. preadv/preadv.h:27: undefined reference to `tst_brkm'
pread01 and pread02 share predv.h, which is using ltp_syscall

3. recvmsg02.c:68: error: ‘SOCK_CLOEXEC’ undeclared (first use in this function)
I'm guessing some old api header used to include also lapi/fcntl.h

Regards,
Jan

> 
> --
> Cyril Hrubis
> chrubis@suse.cz
> 

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
> In that case, tst_vbrk_() should also call update_results(), so TBROK
> during cleanup doesn't get lost/ignored.

Yes, that is what I did when I was experimenting with this change.

> > 
> >   - plus point is that writing cleanup is bit easier
> > 
> >   - minus point is that tst_brk() can now return and as a side effect
> >     tst_brkm() has to return as well (since tst_brkm() from safe macros
> >     is redirected to tst_brk()) and because of that we have to drop
> >     the __attribute__((noreturn)) since that causes gcc to optimize out
> >     return statements from the function which generates compilation
> >     warnings in some cases
> 
> I assume if we drop noreturn, we still have a different way to suppress
> those warnings, right?

All I can come up is to fix all occurences. I've recompiled LTP with the
changes and I got 1090 instances of:

"warning: control reaches end of non-void function [-Wreturn-type]"

Fortunately 1037 seems to come from including compat_16.h header and
it's syscall wrappers. Then there is still 53 cases scattered around the
code base.

This is why I'm not 100% sure we should do the change...

> > 
> > 
> > Is there anything else?
> 
> I ran into number of compilation errors on RHEL5.6, but they will
> likely happen on other distros too:
> 
> 1. lib/ltp_priv.h and va_list
> Adding "#include <stdarg.h>" fixed it.

Ack, will add it.

> 2. preadv/preadv.h:27: undefined reference to `tst_brkm'
> pread01 and pread02 share predv.h, which is using ltp_syscall

Ack, will fix that once second test is converted.

> 3. recvmsg02.c:68: error: ???SOCK_CLOEXEC??? undeclared (first use in this function)
> I'm guessing some old api header used to include also lapi/fcntl.h

Fixed in main git repo meanwhile, will be fixed once I rebase the
patchset.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
Up to date version is at usuall place:

https://github.com/metan-ucw/ltp


Changes are:

* Now we use env variable for SHM path

  - I was thinking about setting it to the /proc path if proc is mounted
    but that way we would have to keep the fd open (and it would occupy
    fd=3) and I think that the less resources the library occupy the
    better

* The rest of the default test options -I and -C are now working

* I've given up on SAFE_MACROS in cleaup for now

  - We may add them later, but I would like to get this merged ideally
    before we do another release and doing this would take some time

* All relevant parts of test-writing-guidelines should be now rewritten

  - There are couple of functions that still take cleanup as fist
    parameter and that are not exported to the new library but that
    should be all that is missing


Anything else that should be done before we can merge it?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Thursday, 10 March, 2016 5:57:21 PM
> Subject: Re: [LTP] Test library API changes
> 
> Hi!
> Up to date version is at usuall place:
> 
> https://github.com/metan-ucw/ltp
> 
> 
> Changes are:
> 
> * Now we use env variable for SHM path
> 
>   - I was thinking about setting it to the /proc path if proc is mounted
>     but that way we would have to keep the fd open (and it would occupy
>     fd=3) and I think that the less resources the library occupy the
>     better
> 
> * The rest of the default test options -I and -C are now working
> 
> * I've given up on SAFE_MACROS in cleaup for now
> 
>   - We may add them later, but I would like to get this merged ideally
>     before we do another release and doing this would take some time

I think we have this problem already. tst_brk() will call tst_vbrk_(),
which calls cleanup_ipc(), which is using SAFE_* macros. It looks like
we'll get infinite loop if any of those fail.

> 
> * All relevant parts of test-writing-guidelines should be now rewritten
> 
>   - There are couple of functions that still take cleanup as fist
>     parameter and that are not exported to the new library but that
>     should be all that is missing

"Child processes started via exec*, or any other processes must initialize the
checkpoint by calling 'TST_CHECKPOINT_INIT()' before usage."
This doesn't sound correct for newlib. We now have "tst_checkpoint_open()".

---

'To make sure cleanup() was called only once LTP has a helper
"TST_DECLARE_ONCE_FN"'
TST_DECLARE_ONCE_FN is declared in test.h, it doesn't look like newlib
can use it at the moment.

---

I'm not sure it is enough to make cleanup() call only once. It looks like
we still can get into situation where we call "do_cleanup();cleanup_ipc();"
more than once (if we hit tst_brk in multiple threads with newlib)

---

Can we remove "SAFE_CLOSE(ipc_fd);" from cleanup_ipc()? This fd
is already closed in setup_ipc().

---

Should "struct tst_test *tst_test" be initialized to NULL?
Some tst_res.c oldlib functions are using it, but it is initialized only
if you use newlib, at tst_run_tcases().

> 
> 
> Anything else that should be done before we can merge it?

I only made a compile test on some old/recent RHEL distros
and ran some newlib testcases by hand. This worked OK.

Have you tried running all of the (oldlib) tests on
various arches/releases to see if we haven't regressed?

I could run it against various RHEL majors, it
would take couple days.

Regards,
Jan

> 
> --
> Cyril Hrubis
> chrubis@suse.cz
> 

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
> >   - We may add them later, but I would like to get this merged ideally
> >     before we do another release and doing this would take some time
> 
> I think we have this problem already. tst_brk() will call tst_vbrk_(),
> which calls cleanup_ipc(), which is using SAFE_* macros. It looks like
> we'll get infinite loop if any of those fail.

Hrm, and we also call tst_release_device() with calls SAFE_OPEN() in
do_cleanup().

So either we:

1. fix all cleanup codepaths not to call tst_brk/tst_brkm
or
2. make tst_brk usable in cleanups.

For short term I would go for the 1. then try for 2. in longterm.

> > * All relevant parts of test-writing-guidelines should be now rewritten
> > 
> >   - There are couple of functions that still take cleanup as fist
> >     parameter and that are not exported to the new library but that
> >     should be all that is missing
> 
> "Child processes started via exec*, or any other processes must initialize the
> checkpoint by calling 'TST_CHECKPOINT_INIT()' before usage."
> This doesn't sound correct for newlib. We now have "tst_checkpoint_open()".

Ah, right. I was thinking of calling it tst_reinit() or similar so that
we can initialize the struct results in it if needed as well.

> ---
> 
> 'To make sure cleanup() was called only once LTP has a helper
> "TST_DECLARE_ONCE_FN"'
> TST_DECLARE_ONCE_FN is declared in test.h, it doesn't look like newlib
> can use it at the moment.

Right. Should be moved to a common header.

> ---
> 
> I'm not sure it is enough to make cleanup() call only once. It looks like
> we still can get into situation where we call "do_cleanup();cleanup_ipc();"
> more than once (if we hit tst_brk in multiple threads with newlib)

Right this macro is purely for guarding the user supplied callback.

Unfortunately the pthread_once() does not compile to a stub without
-pthread, otherwise we could just use it on the do_cleanup() in library
and get rid of that part in the testcase.

I guess that we can emulate it with mutexes as well and guard the
library cleanup hence the user supplied cleanup would end up thread safe
as well without any additional work.

> ---
> 
> Can we remove "SAFE_CLOSE(ipc_fd);" from cleanup_ipc()? This fd
> is already closed in setup_ipc().

Not if the SAFE_FTRUNCATE() or SAFE_MMAP() in setup_ipc() fails.

> ---
> 
> Should "struct tst_test *tst_test" be initialized to NULL?
> Some tst_res.c oldlib functions are using it, but it is initialized only
> if you use newlib, at tst_run_tcases().

It's a global variable so if you don't call tst_run_tcases() it is.

> > 
> > 
> > Anything else that should be done before we can merge it?
> 
> I only made a compile test on some old/recent RHEL distros
> and ran some newlib testcases by hand. This worked OK.
> 
> Have you tried running all of the (oldlib) tests on
> various arches/releases to see if we haven't regressed?

Not yet, will do a couple runs once we consider it finished enough.

> I could run it against various RHEL majors, it
> would take couple days.

I will do that as preparations for next LTP release since I do numerous
runs for several SLES and openSUSE versions at that point anyway.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
> > I'm not sure it is enough to make cleanup() call only once. It looks like
> > we still can get into situation where we call "do_cleanup();cleanup_ipc();"
> > more than once (if we hit tst_brk in multiple threads with newlib)
> 
> Right this macro is purely for guarding the user supplied callback.
> 
> Unfortunately the pthread_once() does not compile to a stub without
> -pthread, otherwise we could just use it on the do_cleanup() in library
> and get rid of that part in the testcase.
> 
> I guess that we can emulate it with mutexes as well and guard the
> library cleanup hence the user supplied cleanup would end up thread safe
> as well without any additional work.

Thinking of thread safe callback it gets more complicated than that.

We should not execute the cleanup while other threads are still running
as well since the cleanup may remove resources these threads are using
and we may end up segfaulting.

The ideal solution would be killing/stopping rest of the threads at this
point. I guess that we can kill them looping over /proc/self/task/
directory.

I.e.:

mutex_lock()

for tid in /proc/self/task/* {
	if tid != mytid
		tgkill(pid, tid, 9);
}

mutex_unlock()

do_cleanup()
do_exit();


What do you think?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
> > * I've given up on SAFE_MACROS in cleaup for now
> > 
> >   - We may add them later, but I would like to get this merged ideally
> >     before we do another release and doing this would take some time
> 
> I think we have this problem already. tst_brk() will call tst_vbrk_(),
> which calls cleanup_ipc(), which is using SAFE_* macros. It looks like
> we'll get infinite loop if any of those fail.

This part has been fixed. Now as far as I can see tst_brk() is no longer
called from the test library cleanup code.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Monday, 14 March, 2016 5:00:37 PM
> Subject: Re: [LTP] Test library API changes
> 
> Hi!
> > > I'm not sure it is enough to make cleanup() call only once. It looks like
> > > we still can get into situation where we call
> > > "do_cleanup();cleanup_ipc();"
> > > more than once (if we hit tst_brk in multiple threads with newlib)
> > 
> > Right this macro is purely for guarding the user supplied callback.
> > 
> > Unfortunately the pthread_once() does not compile to a stub without
> > -pthread, otherwise we could just use it on the do_cleanup() in library
> > and get rid of that part in the testcase.
> > 
> > I guess that we can emulate it with mutexes as well and guard the
> > library cleanup hence the user supplied cleanup would end up thread safe
> > as well without any additional work.
> 
> Thinking of thread safe callback it gets more complicated than that.
> 
> We should not execute the cleanup while other threads are still running
> as well since the cleanup may remove resources these threads are using
> and we may end up segfaulting.
> 
> The ideal solution would be killing/stopping rest of the threads at this
> point. I guess that we can kill them looping over /proc/self/task/
> directory.
> 
> I.e.:
> 
> mutex_lock()
> 
> for tid in /proc/self/task/* {
> 	if tid != mytid
> 		tgkill(pid, tid, 9);
> }
> 
> mutex_unlock()
> 
> do_cleanup()
> do_exit();
> 
> 
> What do you think?

That it will only cause signal to be delivered to that thread,
but handler will still kill entire process.

We could use different signal and define a signal handler,
that would just loop, but that still doesn't guarantee
when pending signal is delivered. Also threads could mask
the signal.

Then I was thinking about blocking threads using affinity,
but if test changed priority we could lock ourselves out.

We do say in style guide, that cleanup can be called at any
time. Should we leave it up to test author? We had the same
problem with oldlib, right?

Regards,
Jan

> 
> --
> Cyril Hrubis
> chrubis@suse.cz
> 

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
> That it will only cause signal to be delivered to that thread,
> but handler will still kill entire process.

Ah, right. I tend to forget that part.

> We could use different signal and define a signal handler,
> that would just loop, but that still doesn't guarantee
> when pending signal is delivered. Also threads could mask
> the signal.

We can call a pause() in a loop from the signal handler and check that
the thread is sleeping in the kernel from the main thread. But this is
getting too complicated.

> Then I was thinking about blocking threads using affinity,
> but if test changed priority we could lock ourselves out.
> 
> We do say in style guide, that cleanup can be called at any
> time. Should we leave it up to test author? We had the same
> problem with oldlib, right?

Yes, and technically we have the same problem with the forked processes
as well. When the main process calls tst_brk() the child processes may
still be running. But killing/waiting children in tst_brk() could be
done quite easily.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
I've rethinked the thread safety and LTP library and rewrote the
documentation, there are no changes to the library as the ideal place to
deal with thread safety is the user supplied cleanup.

Can you please review:

https://github.com/metan-ucw/ltp/blob/master/doc/test-writing-guidelines.txt#L732

There is also a new test at:

https://github.com/metan-ucw/ltp/blob/master/lib/newlib_tests/test08.c

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Thursday, 17 March, 2016 5:06:06 PM
> Subject: Re: [LTP] Test library API changes
> 
> Hi!
> I've rethinked the thread safety and LTP library and rewrote the
> documentation, there are no changes to the library as the ideal place to
> deal with thread safety is the user supplied cleanup.
> 
> Can you please review:
> 
> https://github.com/metan-ucw/ltp/blob/master/doc/test-writing-guidelines.txt#L732

Looks good to me.

"Only the main thread must return from the 'test()' function to the test
library"

We could also use an atomic flag in library to detect that
more threads escaped.
- in run_tests() after test_all() / test()
- in do_cleanup() after cleanup()

If flag is more then 1 then we know something is wrong,
e.g. more than 1 thread escaped cleanup() or test().

Regards,
Jan

> 
> There is also a new test at:
> 
> https://github.com/metan-ucw/ltp/blob/master/lib/newlib_tests/test08.c
> 
> --
> Cyril Hrubis
> chrubis@suse.cz
> 

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
Latest version is at usuall place:

https://github.com/metan-ucw/ltp

Now rest of the library calls should be wired to the new library as
well, see for example this header:

https://github.com/metan-ucw/ltp/blob/master/include/tst_fs.h

The documentation was updated as well (I've even added a few functions
that were not documented previously).

I've also compared build logs to make sure that the changes haven't
introduced new warnings and did a few syscall testruns to make sure
everything works fine. Both looks good to me.

We are not that far from next LTP release. As a matter of fact we should
start preparing for it anytime soon. Ideally I would like to get this
merged before the release freeze and testing. What do you think?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Thursday, 31 March, 2016 12:01:46 PM
> Subject: Re: [LTP] Test library API changes
> 
> Hi!
> Latest version is at usuall place:
> 
> https://github.com/metan-ucw/ltp
> 
> Now rest of the library calls should be wired to the new library as
> well, see for example this header:
> 
> https://github.com/metan-ucw/ltp/blob/master/include/tst_fs.h

Isn't it the other way around? Newlib interface wired to old lib implementation?
Do the names with "_" at the end have any special meaning?

Was "#ifdef TST_TEST_H__" used in previous versions? Is there a difference
between ifdef approach and splitting interface to old_* header?

> 
> The documentation was updated as well (I've even added a few functions
> that were not documented previously).

"resource_files" still appears to be undocumented.

> 
> I've also compared build logs to make sure that the changes haven't
> introduced new warnings and did a few syscall testruns to make sure
> everything works fine. Both looks good to me.
> 
> We are not that far from next LTP release. As a matter of fact we should
> start preparing for it anytime soon. Ideally I would like to get this
> merged before the release freeze and testing. What do you think?

I think technically it's going to work fine. My only worry is
how much are old/new API mixed together when I'm looking at
ltp/include directory. And I keep thinking: "As someone editing
testcase that is using old/new API, what headers should/shouldn't
I use?"

Regards,
Jan

> 
> --
> Cyril Hrubis
> chrubis@suse.cz
> 

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
> > Now rest of the library calls should be wired to the new library as
> > well, see for example this header:
> > 
> > https://github.com/metan-ucw/ltp/blob/master/include/tst_fs.h
> 
> Isn't it the other way around? Newlib interface wired to old lib implementation?
> Do the names with "_" at the end have any special meaning?

What I was trying to describe is that the rest of the functionality is
available there i.e. wired to the new library.

The names with _ are the original prototypes so that we can have
functions with and without the cleanup parameter depending on which
proxy static inline function is selected.

> Was "#ifdef TST_TEST_H__" used in previous versions? Is there a difference
> between ifdef approach and splitting interface to old_* header?

Well the old headers are there for two cases. Either the functions does
not need to be exported to the testcases as the functionality is
provided by the library, or the header contains some kind of SAFE_MACROS
(minus the safe_macros.h that wasn't renamed beacause it's included
directly in many testcases). I could have split the rest of the headers
the very same way but we would need three files for each. One with the
internal functions with underscore and one for oldlib and one for newlib
inline functions, which looked to me like unnecessary many files.

Looking at it now we can easily stick the static inline functions for
oldlib to the test.h. But we would still need to split the tst_ headers
into two, one for the prototypes with underscore ending in _fn.h and one
with the static inline for new library.

Or do you have a better idea how to proceed?

> > The documentation was updated as well (I've even added a few functions
> > that were not documented previously).
> 
> "resource_files" still appears to be undocumented.

Right. Will fix that.

> > I've also compared build logs to make sure that the changes haven't
> > introduced new warnings and did a few syscall testruns to make sure
> > everything works fine. Both looks good to me.
> > 
> > We are not that far from next LTP release. As a matter of fact we should
> > start preparing for it anytime soon. Ideally I would like to get this
> > merged before the release freeze and testing. What do you think?
> 
> I think technically it's going to work fine. My only worry is
> how much are old/new API mixed together when I'm looking at
> ltp/include directory. And I keep thinking: "As someone editing
> testcase that is using old/new API, what headers should/shouldn't
> I use?"

Currently everything test needs is included in tst_test.h which is all
that should be included in the newlib testcase. Which is IMHO better
than requiring to include new header for each couple of functions. And
given that everything that the library exports should start either with
TST_ tst_ SAFE_ or safe_ there shouldn't be any unexpected collisions
either.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Monday, 4 April, 2016 2:04:45 PM
> Subject: Re: [LTP] Test library API changes
> 
> Hi!
> > > Now rest of the library calls should be wired to the new library as
> > > well, see for example this header:
> > > 
> > > https://github.com/metan-ucw/ltp/blob/master/include/tst_fs.h
> > 
> > Isn't it the other way around? Newlib interface wired to old lib
> > implementation?
> > Do the names with "_" at the end have any special meaning?
> 
> What I was trying to describe is that the rest of the functionality is
> available there i.e. wired to the new library.
> 
> The names with _ are the original prototypes so that we can have
> functions with and without the cleanup parameter depending on which
> proxy static inline function is selected.
> 
> > Was "#ifdef TST_TEST_H__" used in previous versions? Is there a difference
> > between ifdef approach and splitting interface to old_* header?
> 
> Well the old headers are there for two cases. Either the functions does
> not need to be exported to the testcases as the functionality is
> provided by the library, or the header contains some kind of SAFE_MACROS
> (minus the safe_macros.h that wasn't renamed beacause it's included
> directly in many testcases). I could have split the rest of the headers
> the very same way but we would need three files for each. One with the
> internal functions with underscore and one for oldlib and one for newlib
> inline functions, which looked to me like unnecessary many files.
> 
> Looking at it now we can easily stick the static inline functions for
> oldlib to the test.h. But we would still need to split the tst_ headers
> into two, one for the prototypes with underscore ending in _fn.h and one
> with the static inline for new library.
> 
> Or do you have a better idea how to proceed?

I don't think I have. I was thinking about macros to generate
both prototypes, but that would probably get messy soon.

One related idea I had for more "visual separation", was to
create subdir, e.g. include/oldlib and move all oldlib exclusive
headers there. And then add  that dir to include dirs in CFLAGS,
so nothing changes for tests.

Then (if I get the big picture), we could tell: "if you're looking
for a function for your newlib test, just avoid _fn headers
and anything in oldlib subdir". And same would apply for adding new
functions to newlib.

What I image people do to discover these functions is search in ltp/include,
and I'm guessing they'll get up to 3 hits: old header (which may or may not
have old_ in name), _fn header and tst_ header.

This is what made me think if we can somehow steer them to look
at newlib headers first.

> 
> > > The documentation was updated as well (I've even added a few functions
> > > that were not documented previously).
> > 
> > "resource_files" still appears to be undocumented.
> 
> Right. Will fix that.
> 
> > > I've also compared build logs to make sure that the changes haven't
> > > introduced new warnings and did a few syscall testruns to make sure
> > > everything works fine. Both looks good to me.
> > > 
> > > We are not that far from next LTP release. As a matter of fact we should
> > > start preparing for it anytime soon. Ideally I would like to get this
> > > merged before the release freeze and testing. What do you think?
> > 
> > I think technically it's going to work fine. My only worry is
> > how much are old/new API mixed together when I'm looking at
> > ltp/include directory. And I keep thinking: "As someone editing
> > testcase that is using old/new API, what headers should/shouldn't
> > I use?"
> 
> Currently everything test needs is included in tst_test.h which is all
> that should be included in the newlib testcase. Which is IMHO better
> than requiring to include new header for each couple of functions.

I agree.

Regards,
Jan

> And
> given that everything that the library exports should start either with
> TST_ tst_ SAFE_ or safe_ there shouldn't be any unexpected collisions
> either.
> 
> --
> Cyril Hrubis
> chrubis@suse.cz
> 

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
> > Or do you have a better idea how to proceed?
> 
> I don't think I have. I was thinking about macros to generate
> both prototypes, but that would probably get messy soon.
> 
> One related idea I had for more "visual separation", was to
> create subdir, e.g. include/oldlib and move all oldlib exclusive
> headers there. And then add  that dir to include dirs in CFLAGS,
> so nothing changes for tests.
> 
> Then (if I get the big picture), we could tell: "if you're looking
> for a function for your newlib test, just avoid _fn headers
> and anything in oldlib subdir". And same would apply for adding new
> functions to newlib.
> 
> What I image people do to discover these functions is search in ltp/include,
> and I'm guessing they'll get up to 3 hits: old header (which may or may not
> have old_ in name), _fn header and tst_ header.
> 
> This is what made me think if we can somehow steer them to look
> at newlib headers first.

I've added a patch on the top of the one that adds the new library that
moves the old headers to include/old. Now only config.h, tst_* headers
and safe_*fn.h are directly in include.

The resource files are documented in the test-writing-guidelines as well
and the patchset is rebased on the top of the latest ltp git.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Test library API changes

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Tuesday, 5 April, 2016 4:16:28 PM
> Subject: Re: [LTP] Test library API changes
> 
> Hi!
> > > Or do you have a better idea how to proceed?
> > 
> > I don't think I have. I was thinking about macros to generate
> > both prototypes, but that would probably get messy soon.
> > 
> > One related idea I had for more "visual separation", was to
> > create subdir, e.g. include/oldlib and move all oldlib exclusive
> > headers there. And then add  that dir to include dirs in CFLAGS,
> > so nothing changes for tests.
> > 
> > Then (if I get the big picture), we could tell: "if you're looking
> > for a function for your newlib test, just avoid _fn headers
> > and anything in oldlib subdir". And same would apply for adding new
> > functions to newlib.
> > 
> > What I image people do to discover these functions is search in
> > ltp/include,
> > and I'm guessing they'll get up to 3 hits: old header (which may or may not
> > have old_ in name), _fn header and tst_ header.
> > 
> > This is what made me think if we can somehow steer them to look
> > at newlib headers first.
> 
> I've added a patch on the top of the one that adds the new library that
> moves the old headers to include/old. Now only config.h, tst_* headers
> and safe_*fn.h are directly in include.

Thanks, include dir looks more tidy to me now.

I don't have any other ideas / comments / questions. I went mostly through
lib changes, I checked converted testcases only briefly.

Overall, I think we can merge it. I'd suggest to extend our usual
test/freeze time before release to make sure people have enough
time to try it. 

Regards,
Jan

[LTP] Test library API changes

[Cyril Hrubis]

Hi!
> Overall, I think we can merge it. I'd suggest to extend our usual
> test/freeze time before release to make sure people have enough
> time to try it.

I've wrote a bit longer changelog for the patch that adds new library
and pushed. Many thanks for you help :).

I will also write an email about the changes to the ML and ask everybody
to test it.

-- 
Cyril Hrubis
chrubis@suse.cz