* Stefan Hajnoczi (stefa...@gmail.com) wrote: > On Thu, Sep 7, 2017 at 9:13 AM, Peter Xu <pet...@redhat.com> 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 (berra...@redhat.com) wrote: > >> > > This does imply that you need a separate monitor I/O processing, from > >> > > the > >> > > command execution thread, but I see no need for all commands to > >> > > suddenly > >> > > become async. Just allowing interleaved replies is sufficient from the > >> > > POV of the protocol definition. This interleaving is easy to handle > >> > > from > >> > > the client POV - just requires a unique 'serial' in the request by the > >> > > client, that is copied into the reply by QEMU. > >> > > >> > OK, so for that we can just take Marc-André'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 executing > >> 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).
But it has the pc as well, so that's actual state. Dave > The next step after that would be to audit other monitor commands for > unnecessary vcpu synchronization. > > Stefan -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
