Re: [RFC PATCH-for-5.1?] tests/tcg/multiarch/linux-test: Skip test if nanosleep missing (Travis)

[Philippe Mathieu-Daudé]

On 7/21/20 2:24 PM, Laurent Vivier wrote:
> Le 21/07/2020 à 13:38, Laurent Vivier a écrit :
>> Le 21/07/2020 à 10:57, Philippe Mathieu-Daudé a écrit :
>>> The time test sometimes fails on Travis-CI [*]:
>>>
>>>     TEST    linux-test on aarch64
>>>   tests/tcg/multiarch/linux-test.c:237: nanosleep
>>>   make[2]: *** [run-linux-test] Error 1
>>>   make: *** [run-tcg-tests-aarch64-linux-user] Error 2
>>>
>>> As this seems due to a container limitation on Travis-CI,
>>> simply skip the test there.
>>>
>>> [*] https://travis-ci.org/github/qemu/qemu/jobs/710005078#L3706
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>> RFC because per Laurent Vivier we are not using the correct libc
>>>     while cross-linking the test (maybe change in the container
>>>     packages?)
>>> ---
>>>  tests/tcg/multiarch/linux-test.c | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/tcg/multiarch/linux-test.c 
>>> b/tests/tcg/multiarch/linux-test.c
>>> index 8a7c15cd31..c7dfdec9ec 100644
>>> --- a/tests/tcg/multiarch/linux-test.c
>>> +++ b/tests/tcg/multiarch/linux-test.c
>>> @@ -233,8 +233,13 @@ static void test_time(void)
>>>      ts.tv_sec = 0;
>>>      ts.tv_nsec = 20 * 1000000;
>>>      chk_error(nanosleep(&ts, &rem));
>>> -    if (rem.tv_sec != 1)
>>> +    if (rem.tv_sec != 1) {
>>> +        if (getenv("TRAVIS_ARCH")) {
>>> +            printf("nanosleep missing? skipping 'time' test\n");
>>> +            return;
>>> +        }
>>>          error("nanosleep");
>>> +    }
>>>      chk_error(gettimeofday(&tv2, NULL));
>>>      ti = tv2.tv_sec - tv.tv_sec;
>>>      if (ti >= 2)
>>>
>>
>> Well, in the end I think the problem is in linux-user:
>>
>> We copy the "rem" structure even if there is no error, so "1" is
>> overwritten.
>>
>> We don't have the problem on all architectures because some use
>> nanosleep() syscall (that is correct) others use clock_nanosleep()
>> syscall that is not correct.
>>
>> This should fix the problem:
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 1211e759c26c..130005716ece 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -11831,7 +11831,7 @@ static abi_long do_syscall1(void *cpu_env, int
>> num, abi_long arg1,
>>          target_to_host_timespec(&ts, arg3);
>>          ret = get_errno(safe_clock_nanosleep(arg1, arg2,
>>                                               &ts, arg4 ? &ts : NULL));
>> -        if (arg4)
>> +        if (is_error(ret) && arg4)
>>              host_to_target_timespec(arg4, &ts);
>>
>>  #if defined(TARGET_PPC)
> 
> According to clock_nanosleep(2) it should be in fact:
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1211e759c26c..63e7cd8947e5 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -11831,8 +11831,9 @@ static abi_long do_syscall1(void *cpu_env, int
> num, abi_long arg1,
>          target_to_host_timespec(&ts, arg3);
>          ret = get_errno(safe_clock_nanosleep(arg1, arg2,
>                                               &ts, arg4 ? &ts : NULL));
> -        if (arg4)
> +        if (ret == -TARGET_EINTR && arg4 && arg2 != TIMER_ABSTIME) {
>              host_to_target_timespec(arg4, &ts);
> +        }

I tested this hunk and couldn't reproduce it on Travis-CI. Since it is
intermittent, that doesn't mean much, but still if you send a proper
path you can add "Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>".

> 
>  #if defined(TARGET_PPC)
>          /* clock_nanosleep is odd in that it returns positive errno values.
> 
>