| Message ID | 1430417242-11859-4-git-send-email-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
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? > + } > + > + fprintf(stderr, "Migration did not complete, status: %s\n", st); > + g_assert_not_reached(); > + } > + > + migrate_allocator(from->alloc, to->alloc); > + set_context(to); > +} Kevin
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 >
Am 04.05.2015 um 19:52 hat John Snow geschrieben: > > > On 05/04/2015 08:07 AM, Kevin Wolf wrote: > >Am 30.04.2015 um 20:07 hat John Snow geschrieben: > >>+ /* 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. This surprised me a bit at first because there's no way that string operations are _that_ slow. You would definitely spin a while in this loop (and potentially slow down the migration by that). I think what saves you is that you wait for the STOP event first, and when qemu's migration thread sends that event, it happens to have already taken the global mutex. This means that you get your "enough sleep" from the qemu monitor, which won't respond before migration has completed. > 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 */ If I were you, I'd add it just to be nice (just applying it to your tree instead of sending out a new version would be okay). If you don't want to, I won't insist, though. I mean, I already gave my R-b... Kevin
On 05/05/2015 07:35 AM, Kevin Wolf wrote: > Am 04.05.2015 um 19:52 hat John Snow geschrieben: >> >> >> On 05/04/2015 08:07 AM, Kevin Wolf wrote: >>> Am 30.04.2015 um 20:07 hat John Snow geschrieben: >>>> + /* 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. > > This surprised me a bit at first because there's no way that string > operations are _that_ slow. You would definitely spin a while in this > loop (and potentially slow down the migration by that). > > I think what saves you is that you wait for the STOP event first, and > when qemu's migration thread sends that event, it happens to have > already taken the global mutex. This means that you get your "enough > sleep" from the qemu monitor, which won't respond before migration has > completed. > >> 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 */ > > If I were you, I'd add it just to be nice (just applying it to your tree > instead of sending out a new version would be okay). If you don't want > to, I won't insist, though. I mean, I already gave my R-b... > > Kevin > It's worth finding out if my reasoning is sane, and you cared enough to comment. I'll add the sleep when I merge, no problem :) Thanks! --js
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; + } + + fprintf(stderr, "Migration did not complete, status: %s\n", st); + g_assert_not_reached(); + } + + migrate_allocator(from->alloc, to->alloc); + set_context(to); +} + void mkimg(const char *file, const char *fmt, unsigned size_mb) { gchar *cli; diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h index f57362b..e1f14ea 100644 --- a/tests/libqos/libqos.h +++ b/tests/libqos/libqos.h @@ -21,6 +21,8 @@ QOSState *qtest_boot(QOSOps *ops, const char *cmdline_fmt, ...); void qtest_shutdown(QOSState *qs); void mkimg(const char *file, const char *fmt, unsigned size_mb); void mkqcow2(const char *file, unsigned size_mb); +void set_context(QOSState *s); +void migrate(QOSState *from, QOSState *to, const char *uri); void prepare_blkdebug_script(const char *debug_fn, const char *event); static inline uint64_t qmalloc(QOSState *q, size_t bytes) diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c index 67f3190..8276130 100644 --- a/tests/libqos/malloc.c +++ b/tests/libqos/malloc.c @@ -30,8 +30,8 @@ struct QGuestAllocator { uint64_t end; uint32_t page_size; - MemList used; - MemList free; + MemList *used; + MemList *free; }; #define DEFAULT_PAGE_SIZE 4096 @@ -150,7 +150,7 @@ static uint64_t mlist_fulfill(QGuestAllocator *s, MemBlock *freenode, addr = freenode->addr; if (freenode->size == size) { /* re-use this freenode as our used node */ - QTAILQ_REMOVE(&s->free, freenode, MLIST_ENTNAME); + QTAILQ_REMOVE(s->free, freenode, MLIST_ENTNAME); usednode = freenode; } else { /* adjust the free node and create a new used node */ @@ -159,7 +159,7 @@ static uint64_t mlist_fulfill(QGuestAllocator *s, MemBlock *freenode, usednode = mlist_new(addr, size); } - mlist_sort_insert(&s->used, usednode); + mlist_sort_insert(s->used, usednode); return addr; } @@ -171,7 +171,7 @@ static void mlist_check(QGuestAllocator *s) uint64_t addr = s->start > 0 ? s->start - 1 : 0; uint64_t next = s->start; - QTAILQ_FOREACH(node, &s->free, MLIST_ENTNAME) { + QTAILQ_FOREACH(node, s->free, MLIST_ENTNAME) { g_assert_cmpint(node->addr, >, addr); g_assert_cmpint(node->addr, >=, next); addr = node->addr; @@ -180,7 +180,7 @@ static void mlist_check(QGuestAllocator *s) addr = s->start > 0 ? s->start - 1 : 0; next = s->start; - QTAILQ_FOREACH(node, &s->used, MLIST_ENTNAME) { + QTAILQ_FOREACH(node, s->used, MLIST_ENTNAME) { g_assert_cmpint(node->addr, >, addr); g_assert_cmpint(node->addr, >=, next); addr = node->addr; @@ -192,7 +192,7 @@ static uint64_t mlist_alloc(QGuestAllocator *s, uint64_t size) { MemBlock *node; - node = mlist_find_space(&s->free, size); + node = mlist_find_space(s->free, size); if (!node) { fprintf(stderr, "Out of guest memory.\n"); g_assert_not_reached(); @@ -208,7 +208,7 @@ static void mlist_free(QGuestAllocator *s, uint64_t addr) return; } - node = mlist_find_key(&s->used, addr); + node = mlist_find_key(s->used, addr); if (!node) { fprintf(stderr, "Error: no record found for an allocation at " "0x%016" PRIx64 ".\n", @@ -217,9 +217,9 @@ static void mlist_free(QGuestAllocator *s, uint64_t addr) } /* Rip it out of the used list and re-insert back into the free list. */ - QTAILQ_REMOVE(&s->used, node, MLIST_ENTNAME); - mlist_sort_insert(&s->free, node); - mlist_coalesce(&s->free, node); + QTAILQ_REMOVE(s->used, node, MLIST_ENTNAME); + mlist_sort_insert(s->free, node); + mlist_coalesce(s->free, node); } /* @@ -233,7 +233,7 @@ void alloc_uninit(QGuestAllocator *allocator) QAllocOpts mask; /* Check for guest leaks, and destroy the list. */ - QTAILQ_FOREACH_SAFE(node, &allocator->used, MLIST_ENTNAME, tmp) { + QTAILQ_FOREACH_SAFE(node, allocator->used, MLIST_ENTNAME, tmp) { if (allocator->opts & (ALLOC_LEAK_WARN | ALLOC_LEAK_ASSERT)) { fprintf(stderr, "guest malloc leak @ 0x%016" PRIx64 "; " "size 0x%016" PRIx64 ".\n", @@ -248,7 +248,7 @@ void alloc_uninit(QGuestAllocator *allocator) /* If we have previously asserted that there are no leaks, then there * should be only one node here with a specific address and size. */ mask = ALLOC_LEAK_ASSERT | ALLOC_PARANOID; - QTAILQ_FOREACH_SAFE(node, &allocator->free, MLIST_ENTNAME, tmp) { + QTAILQ_FOREACH_SAFE(node, allocator->free, MLIST_ENTNAME, tmp) { if ((allocator->opts & mask) == mask) { if ((node->addr != allocator->start) || (node->size != allocator->end - allocator->start)) { @@ -260,6 +260,8 @@ void alloc_uninit(QGuestAllocator *allocator) g_free(node); } + g_free(allocator->used); + g_free(allocator->free); g_free(allocator); } @@ -297,11 +299,13 @@ QGuestAllocator *alloc_init(uint64_t start, uint64_t end) s->start = start; s->end = end; - QTAILQ_INIT(&s->used); - QTAILQ_INIT(&s->free); + s->used = g_malloc(sizeof(MemList)); + s->free = g_malloc(sizeof(MemList)); + QTAILQ_INIT(s->used); + QTAILQ_INIT(s->free); node = mlist_new(s->start, s->end - s->start); - QTAILQ_INSERT_HEAD(&s->free, node, MLIST_ENTNAME); + QTAILQ_INSERT_HEAD(s->free, node, MLIST_ENTNAME); s->page_size = DEFAULT_PAGE_SIZE; @@ -319,7 +323,7 @@ QGuestAllocator *alloc_init_flags(QAllocOpts opts, void alloc_set_page_size(QGuestAllocator *allocator, size_t page_size) { /* Can't alter the page_size for an allocator in-use */ - g_assert(QTAILQ_EMPTY(&allocator->used)); + g_assert(QTAILQ_EMPTY(allocator->used)); g_assert(is_power_of_2(page_size)); allocator->page_size = page_size; @@ -329,3 +333,39 @@ void alloc_set_flags(QGuestAllocator *allocator, QAllocOpts opts) { allocator->opts |= opts; } + +void migrate_allocator(QGuestAllocator *src, + QGuestAllocator *dst) +{ + MemBlock *node, *tmp; + MemList *tmpused, *tmpfree; + + /* The general memory layout should be equivalent, + * though opts can differ. */ + g_assert_cmphex(src->start, ==, dst->start); + g_assert_cmphex(src->end, ==, dst->end); + + /* Destroy (silently, regardless of options) the dest-list: */ + QTAILQ_FOREACH_SAFE(node, dst->used, MLIST_ENTNAME, tmp) { + g_free(node); + } + QTAILQ_FOREACH_SAFE(node, dst->free, MLIST_ENTNAME, tmp) { + g_free(node); + } + + tmpused = dst->used; + tmpfree = dst->free; + + /* Inherit the lists of the source allocator: */ + dst->used = src->used; + dst->free = src->free; + + /* Source is now re-initialized, the source memory is 'invalid' now: */ + src->used = tmpused; + src->free = tmpfree; + QTAILQ_INIT(src->used); + QTAILQ_INIT(src->free); + node = mlist_new(src->start, src->end - src->start); + QTAILQ_INSERT_HEAD(src->free, node, MLIST_ENTNAME); + return; +} diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h index 71ac407..0c6c9b7 100644 --- a/tests/libqos/malloc.h +++ b/tests/libqos/malloc.h @@ -31,6 +31,7 @@ void alloc_uninit(QGuestAllocator *allocator); /* Always returns page aligned values */ uint64_t guest_alloc(QGuestAllocator *allocator, size_t size); void guest_free(QGuestAllocator *allocator, uint64_t addr); +void migrate_allocator(QGuestAllocator *src, QGuestAllocator *dst); QGuestAllocator *alloc_init(uint64_t start, uint64_t end); QGuestAllocator *alloc_init_flags(QAllocOpts flags,
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(-)