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

[Marcel Apfelbaum <marcel@redhat.com>]

On 05/03/2016 12:22 PM, Dr. David Alan Gilbert wrote:
> * Marcel Apfelbaum (marcel@redhat.com) wrote:
>> Hi David,
>>
>> On 04/29/2016 05:47 PM, Dr. David Alan Gilbert (git) wrote:
>>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> <snip>
>
>>> and that can be assembled by the following magic:
>>>       as --32 -march=i486 fill.s -o fill.o
>>>       objcopy -O binary fill.o fill.boot
>>>       dd if=fill.boot of=bootsect bs=256 count=2 skip=124
>>>       xxd -i bootsect
>>>
>>
>> I suppose you can use this a source file and compile it
>> as part of make check, but I am not sure if is worth it.
>
> Yeh, I thought it was easier just to include the blob for that.
>
>>> +        default:
>>> +            fprintf(stderr, "Unexpected %d on %s serial\n", readvalue, side);
>>> +            assert(0);
>>
>> Maybe g_assert_not_reached ? just to be consistent.
>
> Fixed.
>
>>> +        }
>>> +    } while (true);
>>> +}
>>> +
>>> +/*
>>> + * Events can get in the way of responses we are actually waiting for.
>>> + */
>>> +static QDict *return_or_event(QDict *response)
>>> +{
>>> +    const char *event_string;
>>> +    if (!qdict_haskey(response, "event")) {
>>> +        return response;
>>> +    }
>>> +
>>> +    /* OK, it was an event */
>>> +    event_string = qdict_get_str(response, "event");
>>> +    if (!strcmp(event_string, "STOP")) {
>>> +        got_stop = true;
>>> +    }
>>> +    QDECREF(response);
>>> +    return return_or_event(qtest_qmp_receive(global_qtest));
>>> +}
>>> +
>>> +
>>> +/*
>>> + * It's tricky to use qemu's migration event capability with qtest,
>>> + * events suddenly appearing confuse the qmp()/hmp() responses.
>>> + * so wait for a couple of passes to have happened before
>>> + * going postcopy.
>>> + */
>>> +
>>> +static uint64_t get_migration_pass(void)
>>> +{
>>> +    QDict *rsp, *rsp_return, *rsp_ram;
>>> +    uint64_t result;
>>> +
>>> +    rsp = return_or_event(qmp("{ 'execute': 'query-migrate' }"));
>>> +    rsp_return = qdict_get_qdict(rsp, "return");
>>> +    if (!qdict_haskey(rsp_return, "ram")) {
>>> +        /* Still in setup */
>>> +        result = 0;
>>> +    } else {
>>> +        rsp_ram = qdict_get_qdict(rsp_return, "ram");
>>> +        result = qdict_get_try_int(rsp_ram, "dirty-sync-count", 0);
>>> +        QDECREF(rsp);
>>> +    }
>>> +    return result;
>>> +}
>>> +
>>> +static void wait_for_migration_complete(void)
>>> +{
>>> +    QDict *rsp, *rsp_return;
>>> +    bool completed;
>>> +
>>> +    do {
>>> +        const char *status;
>>> +
>>> +        rsp = return_or_event(qmp("{ 'execute': 'query-migrate' }"));
>>> +        rsp_return = qdict_get_qdict(rsp, "return");
>>> +        status = qdict_get_str(rsp_return, "status");
>>> +        completed = strcmp(status, "completed") == 0;
>>> +        assert(strcmp(status, "failed"));
>>
>> maybe g_assert/g_assert_cmpstr()
>
> Done.
>
>>
>>> +        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.

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

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

Thanks,
Marcel


> Dave
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>