From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34464) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ypf7m-0002Au-Ho for qemu-devel@nongnu.org; Tue, 05 May 2015 11:51:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ypf7f-0008O6-1W for qemu-devel@nongnu.org; Tue, 05 May 2015 11:51:10 -0400 Message-ID: <5548E6D7.4040400@redhat.com> Date: Tue, 05 May 2015 11:50:47 -0400 From: John Snow MIME-Version: 1.0 References: <1430417242-11859-1-git-send-email-jsnow@redhat.com> <1430417242-11859-4-git-send-email-jsnow@redhat.com> <20150504120727.GF3676@noname.str.redhat.com> <5547B1CE.1020308@redhat.com> <20150505113550.GC3866@noname.redhat.com> In-Reply-To: <20150505113550.GC3866@noname.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v3 3/9] libqos: Add migration helpers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: marc.mari.barcelo@gmail.com, pbonzini@redhat.com, qemu-block@nongnu.org, qemu-devel@nongnu.org, stefanha@redhat.com 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