qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: John Snow <jsnow@redhat.com>
Cc: marc.mari.barcelo@gmail.com, pbonzini@redhat.com,
	qemu-block@nongnu.org, qemu-devel@nongnu.org,
	stefanha@redhat.com
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v3 3/9] libqos: Add migration helpers
Date: Tue, 5 May 2015 13:35:50 +0200	[thread overview]
Message-ID: <20150505113550.GC3866@noname.redhat.com> (raw)
In-Reply-To: <5547B1CE.1020308@redhat.com>

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

  reply	other threads:[~2015-05-05 11:36 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-30 18:07 [Qemu-devel] [PATCH v3 0/9] ahci: enable migration John Snow
2015-04-30 18:07 ` [Qemu-devel] [PATCH v3 1/9] libqos/ahci: Add halted command helpers John Snow
2015-04-30 18:07 ` [Qemu-devel] [PATCH v3 2/9] libqos/ahci: Fix sector set method John Snow
2015-04-30 18:07 ` [Qemu-devel] [PATCH v3 3/9] libqos: Add migration helpers John Snow
2015-05-04 12:07   ` [Qemu-devel] [Qemu-block] " Kevin Wolf
2015-05-04 17:52     ` John Snow
2015-05-05 11:35       ` Kevin Wolf [this message]
2015-05-05 15:50         ` John Snow
2015-04-30 18:07 ` [Qemu-devel] [PATCH v3 4/9] ich9/ahci: Enable Migration John Snow
2015-04-30 18:07 ` [Qemu-devel] [PATCH v3 5/9] qtest/ahci: Add migration test John Snow
2015-04-30 18:07 ` [Qemu-devel] [PATCH v3 6/9] qtest/ahci: add migrate dma test John Snow
2015-04-30 18:07 ` [Qemu-devel] [PATCH v3 7/9] qtest/ahci: add flush migrate test John Snow
2015-04-30 18:07 ` [Qemu-devel] [PATCH v3 8/9] qtest/ahci: add halted dma test John Snow
2015-04-30 18:07 ` [Qemu-devel] [PATCH v3 9/9] qtest/ahci: add migrate " John Snow
2015-05-04 12:29 ` [Qemu-devel] [Qemu-block] [PATCH v3 0/9] ahci: enable migration Kevin Wolf
2015-05-04 15:40   ` John Snow
2015-05-05 22:45 ` [Qemu-devel] " John Snow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150505113550.GC3866@noname.redhat.com \
    --to=kwolf@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=marc.mari.barcelo@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).