Re: [Qemu-devel] [Qemu-block] [PATCH v3 3/9] libqos: Add migration helpers

[John Snow <jsnow@redhat.com>]

On 05/04/2015 08:07 AM, Kevin Wolf wrote:
> Am 30.04.2015 um 20:07 hat John Snow geschrieben:
>> libqos.c:
>>      -set_context for addressing which commands go where
>>      -migrate performs the actual migration
>>
>> malloc.c:
>>      - Structure of the allocator is adjusted slightly with
>>        a second-tier malloc to make swapping around the allocators
>>        easy when we "migrate" the lists from the source to the destination.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   tests/libqos/libqos.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/libqos/libqos.h |  2 ++
>>   tests/libqos/malloc.c | 74 ++++++++++++++++++++++++++++++++++-----------
>>   tests/libqos/malloc.h |  1 +
>>   4 files changed, 144 insertions(+), 17 deletions(-)
>>
>> diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
>> index 7e72078..ac1bae1 100644
>> --- a/tests/libqos/libqos.c
>> +++ b/tests/libqos/libqos.c
>> @@ -1,5 +1,6 @@
>>   #include <stdio.h>
>>   #include <stdlib.h>
>> +#include <string.h>
>>   #include <glib.h>
>>   #include <unistd.h>
>>   #include <fcntl.h>
>> @@ -62,6 +63,89 @@ void qtest_shutdown(QOSState *qs)
>>       g_free(qs);
>>   }
>>
>> +void set_context(QOSState *s)
>> +{
>> +    global_qtest = s->qts;
>> +}
>> +
>> +static QDict *qmp_execute(const char *command)
>> +{
>> +    char *fmt;
>> +    QDict *rsp;
>> +
>> +    fmt = g_strdup_printf("{ 'execute': '%s' }", command);
>> +    rsp = qmp(fmt);
>> +    g_free(fmt);
>> +
>> +    return rsp;
>> +}
>> +
>> +void migrate(QOSState *from, QOSState *to, const char *uri)
>> +{
>> +    const char *st;
>> +    char *s;
>> +    QDict *rsp, *sub;
>> +    bool running;
>> +
>> +    set_context(from);
>> +
>> +    /* Is the machine currently running? */
>> +    rsp = qmp_execute("query-status");
>> +    g_assert(qdict_haskey(rsp, "return"));
>> +    sub = qdict_get_qdict(rsp, "return");
>> +    g_assert(qdict_haskey(sub, "running"));
>> +    running = qdict_get_bool(sub, "running");
>> +    QDECREF(rsp);
>> +
>> +    /* Issue the migrate command. */
>> +    s = g_strdup_printf("{ 'execute': 'migrate',"
>> +                        "'arguments': { 'uri': '%s' } }",
>> +                        uri);
>> +    rsp = qmp(s);
>> +    g_free(s);
>> +    g_assert(qdict_haskey(rsp, "return"));
>> +    QDECREF(rsp);
>> +
>> +    /* Wait for STOP event, but only if we were running: */
>> +    if (running) {
>> +        qmp_eventwait("STOP");
>> +    }
>> +
>> +    /* If we were running, we can wait for an event. */
>> +    if (running) {
>> +        migrate_allocator(from->alloc, to->alloc);
>> +        set_context(to);
>> +        qmp_eventwait("RESUME");
>> +        return;
>> +    }
>> +
>> +    /* Otherwise, we need to wait: poll until migration is completed. */
>> +    while (1) {
>> +        rsp = qmp_execute("query-migrate");
>> +        g_assert(qdict_haskey(rsp, "return"));
>> +        sub = qdict_get_qdict(rsp, "return");
>> +        g_assert(qdict_haskey(sub, "status"));
>> +        st = qdict_get_str(sub, "status");
>> +
>> +        /* "setup", "active", "completed", "failed", "cancelled" */
>> +        if (strcmp(st, "completed") == 0) {
>> +            QDECREF(rsp);
>> +            break;
>> +        }
>> +
>> +        if ((strcmp(st, "setup") == 0) || (strcmp(st, "active") == 0)) {
>> +            QDECREF(rsp);
>> +            continue;
>
> Wouldn't it be nicer to sleep a bit before retrying?
>

I actually figured that all the string and stream manipulation for 
sending and receiving QMP queries was "enough sleep" because of how 
quick a migration without any guest should complete -- in practice this 
loop doesn't ever seem to trigger more than once.

If you still think sleep is necessary, I can add some very small sleep 
in a separate patch, or when I merge the tree. Something like:

g_usleep(5000) /* 5 msec */

>> +        }
>> +
>> +        fprintf(stderr, "Migration did not complete, status: %s\n", st);
>> +        g_assert_not_reached();
>> +    }
>> +
>> +    migrate_allocator(from->alloc, to->alloc);
>> +    set_context(to);
>> +}
>
> Kevin
>