Re: [Qemu-devel] [PATCH v2 4/5] test: Postcopy

[Marcel Apfelbaum <marcel@redhat.com>]

On 05/03/2016 01:34 PM, Dr. David Alan Gilbert wrote:
> * Marcel Apfelbaum (marcel@redhat.com) wrote:
>> On 05/03/2016 12:22 PM, Dr. David Alan Gilbert wrote:
>
>>>>> +        QDECREF(rsp);
>>>>> +        usleep(1000 * 100);
>>>>> +    } while (!completed);
>>>>> +}
>>>>
>>>> It is possible that the migration will not finish (from some reason) ?
>>>> Do you expect the test runner to stop the test?
>>>
>>> The migration should always complete in postcopy; failure to complete
>>> is failure of the test;   although I've not explicitly added any timeouts.
>>>
>>
>> OK, so on a failure I run make-check and it never ends ? :)
>> For build-bots there is no problem since they kill tests on timeout,
>> but we are running it manually.
>> However, we can add the test as is and if it 'behaves' is OK.
>
> Most of the failure cases will cause an assert/crash - a hang should be
> unlikely.

Agreed

   I'm assuming a bunch of tests can in principal hang if they go wrong;

Not sure about that.

> if there's some qtest structure for timeouts I can add it.

Not that I know of.

But again, let's see how it behaves, maybe there is no problem at all.

>
>>> <snip>
>>>
>>>>> +int main(int argc, char **argv)
>>>>> +{
>>>>> +    char template[] = "/tmp/postcopy-test-XXXXXX";
>>>>
>>>> I would not explicitly use /tmp/
>>>
>>> The ivshmem-test, vhost-user-test and test-qga seem to do it this way;
>>> what's your preferred fix?
>>
>> You could use the P_tmpdir macro instead of '/tmp', but again,
>> if all the tests assume /tmp existence maybe is not an issue.
>
> I don't see any use of P_tmpdir in qemu at all; we do have one use of
> g_get_tmp_dir (in util/memfd.c).

Even better! I like the way it look for it:
     On UNIX, this is taken from the TMPDIR environment variable.
     If the variable is not set, P_tmpdir is used, as defined by the system C library.
     Failing that, a hard-coded default of "/tmp" is returned.

>
>>>>> +    int ret;
>>>>> +
>>>>> +    g_test_init(&argc, &argv, NULL);
>>>>> +
>>>>> +    if (!ufd_version_check()) {
>>>>> +        return 0;
>>>>> +    }
>>>>> +
>>>>> +    tmpfs = mkdtemp(template);
>>>>> +    if (!tmpfs) {
>>>>> +        g_test_message("mkdtemp on path (%s): %s\n", template, strerror(errno));
>>>>> +    }
>>>>> +    g_assert(tmpfs);
>>>>> +
>>>>> +    module_call_init(MODULE_INIT_QOM);
>>>>> +
>>>>> +    qtest_add_func("/postcopy", test_migrate);
>>>>> +
>>>>
>>>> How much time does this test takes? If is too long, maybe we should not run it
>>>> automatically as part of make check, but using an environment variable.
>>>
>>> 4 seconds on my laptop; it seems reasonable.
>>>
>>
>> make-check takes about 1 and a half minute for x86_64 configuration, 4 seconds
>> more seems reasonable indeed.
>
> Dave
>
>> Thanks,
>> Marcel
>>
>>
>>> Dave
>>> --
>>> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>>>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>