From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44742) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dpsVY-00044Y-VT for qemu-devel@nongnu.org; Thu, 07 Sep 2017 04:49:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dpsVX-00041m-Pl for qemu-devel@nongnu.org; Thu, 07 Sep 2017 04:49:57 -0400 Received: from mail-wr0-x231.google.com ([2a00:1450:400c:c0c::231]:36461) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dpsVX-00041L-I5 for qemu-devel@nongnu.org; Thu, 07 Sep 2017 04:49:55 -0400 Received: by mail-wr0-x231.google.com with SMTP id o42so7419638wrb.3 for ; Thu, 07 Sep 2017 01:49:55 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20170907081341.GA23040@pxdev.xzpeter.org> References: <1503471071-2233-1-git-send-email-peterx@redhat.com> <20170829110357.GG3783@redhat.com> <20170906094846.GA2215@work-vm> <20170906104603.GK15510@redhat.com> <20170906104850.GB2215@work-vm> <20170906105414.GL15510@redhat.com> <20170906105704.GC2215@work-vm> <20170906110629.GM15510@redhat.com> <20170906113157.GD2215@work-vm> <20170906115428.GP15510@redhat.com> <20170907081341.GA23040@pxdev.xzpeter.org> From: Stefan Hajnoczi Date: Thu, 7 Sep 2017 09:49:53 +0100 Message-ID: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC v2 0/8] monitor: allow per-monitor thread List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Xu Cc: "Daniel P. Berrange" , Laurent Vivier , Fam Zheng , Juan Quintela , qemu-devel , Markus Armbruster , Michael Roth , "Dr. David Alan Gilbert" , Paolo Bonzini On Thu, Sep 7, 2017 at 9:13 AM, Peter Xu wrote: > On Wed, Sep 06, 2017 at 12:54:28PM +0100, Daniel P. Berrange wrote: >> On Wed, Sep 06, 2017 at 12:31:58PM +0100, Dr. David Alan Gilbert wrote: >> > * Daniel P. Berrange (berrange@redhat.com) wrote: >> > > This does imply that you need a separate monitor I/O processing, fro= m the >> > > command execution thread, but I see no need for all commands to sudd= enly >> > > become async. Just allowing interleaved replies is sufficient from t= he >> > > POV of the protocol definition. This interleaving is easy to handle = from >> > > the client POV - just requires a unique 'serial' in the request by t= he >> > > client, that is copied into the reply by QEMU. >> > >> > OK, so for that we can just take Marc-Andr=C3=A9's syntax and call it = 'id': >> > https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg03634.html >> > >> > then it's upto the caller to ensure those id's are unique. >> >> Libvirt has in fact generated a unique 'id' for every monitor command >> since day 1 of supporting QMP. >> >> > I do worry about two things: >> > a) With this the caller doesn't really know which commands could be >> > in parallel - for example if we've got a recovery command that's >> > executed by this non-locking thread that's OK, we expect that >> > to be doable in parallel. If in the future though we do >> > what you initially suggested and have a bunch of commands get >> > routed to the migration thread (say) then those would suddenly >> > operate in parallel with other commands that we're previously >> > synchronous. >> >> We could still have an opt-in for async commands. eg default to executin= g >> all commands in the main thread, unless the client issues an explicit >> "make it async" command, to switch to allowing the migration thread to >> process it async. >> >> { "execute": "qmp_allow_async", >> "data": { "commands": [ >> "migrate_cancel", >> ] } } >> >> >> { "return": { "commands": [ >> "migrate_cancel", >> ] } } >> >> The server response contains the subset of commands from the request >> for which async is supported. >> >> That gives good negotiation ability going forward as we incrementally >> support async on more commands. > > I think this goes back to the discussion on which design we'd like to > choose. IMHO the whole async idea plus the per-command-id is indeed > cleaner and nicer, and I believe that can benefit not only libvirt, > but also other QMP users. The problem is, I have no idea how long > it'll take to let us have such a feature - I believe that will include > QEMU and Libvirt to both support that. And it'll be a pity if the > postcopy recovery cannot work only because we cannot guarantee a > stable monitor. Please don't rush in a hack, they often introduce new bugs that we have to support long-term when they are part of the QMP API. In your original email you mentioned "info cpus". Have you considered modifying this command so it does not sync the CPU? I'm not sure callers really need to sync the CPU, typically they just want to know the vcpu numbers, thread IDs, and current state (halted, running, etc). The next step after that would be to audit other monitor commands for unnecessary vcpu synchronization. Stefan