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
