Re: [Qemu-devel] [PATCH v7 00/23] QMP: out-of-band (OOB) execution support
On Wed, Jan 24, 2018 at 01:39:34PM +0800, Peter Xu wrote: > This version should have addressed all comments in previous one, also > fixed another race condition after I addressed all the comments (a new > race condition introduced by addressing the comments...). For some > more details of the race condition, please see the last entry of > change log, and please refer to patch 9 for the code change. > > I removed RFC tag from this version. Please review. Thanks. I have posted comments on a few patches. Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v7 00/23] QMP: out-of-band (OOB) execution support
On Wed, Jan 24, 2018 at 01:39:34PM +0800, Peter Xu wrote: > This version should have addressed all comments in previous one, also > fixed another race condition after I addressed all the comments (a new > race condition introduced by addressing the comments...). For some > more details of the race condition, please see the last entry of > change log, and please refer to patch 9 for the code change. > > I removed RFC tag from this version. Please review. Thanks. It's still a month there until the soft freeze of 2.12, but I would still like to have a gentle ping for this series in case there will be further surprises. Hopefully, this series can catch the train for 2.12. I'll respin the postcopy recovery series very soon. Thanks, -- Peter Xu
Re: [Qemu-devel] [PATCH v7 00/23] QMP: out-of-band (OOB) execution support
* Peter Xu (pet...@redhat.com) wrote: > On Wed, Jan 24, 2018 at 01:39:34PM +0800, Peter Xu wrote: > > This version should have addressed all comments in previous one, also > > fixed another race condition after I addressed all the comments (a new > > race condition introduced by addressing the comments...). For some > > more details of the race condition, please see the last entry of > > change log, and please refer to patch 9 for the code change. > > > > I removed RFC tag from this version. Please review. Thanks. > > > > v7: > > - add some r-bs, and remove some. > > - remove the chardev fix since already queued by Paolo > > - use local var in qemu_chr_fe_add_watch [Stefan] > > - move doc patch to front, mention it in some patches [Eric] > > - Quite a few of English fixes [Eric] > > - fix unlock missing in handle_qmp_command [Stefan] > > - squash some patches according to the review comments > > - don't break gdbserver usage on HMP non-interactive mode by fixing up > > the suspend/resume logic [Fam, Stefan] > > - move the qemu_chr_fe_set_handlers() call in monitor_init() into a > > bottom half to avoid race between the call itself and > > iothread. [Stefan] > > - spent quite a lot of time debugging another assertion failure in > > io_watch_poll_finalize() after above change is made (ouch! I really > > hoped we always have the latest glib): when QEMU inits chardevs in > > chardev_init_func() it's possible that QEMU registers the chardev > > handlers there, even before CharBackend is connected to that chardev > > in monitor_init(). Then, when we reach monitor_init() we must make > > sure we unregister that old one first, or there can have one orphan > > GSource still in default gcontext (note that this can really happen > > when we start to use QEMUBH to setup chardev frontends, which is > > above change). > > Online repository updated for v7: > > https://github.com/xzpeter/qemu/tree/monitor-oob This version seems to have fixed the problem I had where after an error I couldn't reconnect (the one I mentioned on the 12th). Dave > Thanks, > > -- > Peter Xu -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v7 00/23] QMP: out-of-band (OOB) execution support
On Wed, Jan 24, 2018 at 01:39:34PM +0800, Peter Xu wrote: > This version should have addressed all comments in previous one, also > fixed another race condition after I addressed all the comments (a new > race condition introduced by addressing the comments...). For some > more details of the race condition, please see the last entry of > change log, and please refer to patch 9 for the code change. > > I removed RFC tag from this version. Please review. Thanks. > > v7: > - add some r-bs, and remove some. > - remove the chardev fix since already queued by Paolo > - use local var in qemu_chr_fe_add_watch [Stefan] > - move doc patch to front, mention it in some patches [Eric] > - Quite a few of English fixes [Eric] > - fix unlock missing in handle_qmp_command [Stefan] > - squash some patches according to the review comments > - don't break gdbserver usage on HMP non-interactive mode by fixing up > the suspend/resume logic [Fam, Stefan] > - move the qemu_chr_fe_set_handlers() call in monitor_init() into a > bottom half to avoid race between the call itself and > iothread. [Stefan] > - spent quite a lot of time debugging another assertion failure in > io_watch_poll_finalize() after above change is made (ouch! I really > hoped we always have the latest glib): when QEMU inits chardevs in > chardev_init_func() it's possible that QEMU registers the chardev > handlers there, even before CharBackend is connected to that chardev > in monitor_init(). Then, when we reach monitor_init() we must make > sure we unregister that old one first, or there can have one orphan > GSource still in default gcontext (note that this can really happen > when we start to use QEMUBH to setup chardev frontends, which is > above change). Online repository updated for v7: https://github.com/xzpeter/qemu/tree/monitor-oob Thanks, -- Peter Xu
[Qemu-devel] [PATCH v7 00/23] QMP: out-of-band (OOB) execution support
This version should have addressed all comments in previous one, also fixed another race condition after I addressed all the comments (a new race condition introduced by addressing the comments...). For some more details of the race condition, please see the last entry of change log, and please refer to patch 9 for the code change. I removed RFC tag from this version. Please review. Thanks. v7: - add some r-bs, and remove some. - remove the chardev fix since already queued by Paolo - use local var in qemu_chr_fe_add_watch [Stefan] - move doc patch to front, mention it in some patches [Eric] - Quite a few of English fixes [Eric] - fix unlock missing in handle_qmp_command [Stefan] - squash some patches according to the review comments - don't break gdbserver usage on HMP non-interactive mode by fixing up the suspend/resume logic [Fam, Stefan] - move the qemu_chr_fe_set_handlers() call in monitor_init() into a bottom half to avoid race between the call itself and iothread. [Stefan] - spent quite a lot of time debugging another assertion failure in io_watch_poll_finalize() after above change is made (ouch! I really hoped we always have the latest glib): when QEMU inits chardevs in chardev_init_func() it's possible that QEMU registers the chardev handlers there, even before CharBackend is connected to that chardev in monitor_init(). Then, when we reach monitor_init() we must make sure we unregister that old one first, or there can have one orphan GSource still in default gcontext (note that this can really happen when we start to use QEMUBH to setup chardev frontends, which is above change). v6: - add r-bs - s/negociate/negotiate/ [Dave] - let mon_global be anonymous struct [Stefan] - in monitor_init(): call qemu_chr_fe_set_handlers() later than init parser, then it's safe [Stefan] - drop patch "qjson: add "opaque" field to JSONMessageParser", re-write the following one to use container_of(). [Stefan] - keep get_qmp_greeting() the old way, add cap list [Stefan, Fam] - fix the iothread_stop() comment since that's not really related to the glib bug [Stefan] - when do qmp greeting, don't expose oob capability if iothread not used [Stefan, Fam] - squash "qmp: introduce some capability helpers" into patch "monitor: separate QMP parser and dispatcher" [Fam] - tune comments for monitor_qmp_respond(). [Stefan] - use atomic_mb_{read|set}() where proper instead of no-mb ones [Stefan] - add patch to let monitor suspend/resume really support QMP. Notify iothread when needed, in both suspend/resume. [Stefan] - add comment for qmp_queue_lock on lock taking ordering. [Stefan] - refactor monitor_qmp_bh_dispatcher() to not loop, instead queue itself after each request. [Stefan] - rename "request" into "command" in the new qmp event [Stefan] - in handle_qmp_command() protect the queue length fetch with qmp_queue_lock [Stefan] - one more s/2.11/2.12/. [Fam] - when isolating response queue, not only flush the queue, but also flush the monitor out buffer [Stefan] - document rewrite [Stefan] - some other touch-ups that I forgot to mention and some tiny new patches... anyway, rbs will be gone if any of the patch is touched up. Please have a look! v5 - rename "monitor_iothread" to "mon_iothread" [Dave] - add comment in monitor_cleanup(), note that when the hacks can be removed. [Dan] - add a note section in qmp-spec.txt, mentioning about how to migrate existing QMP command to oob-capable command. [Dave] - drop patch "qmp: let migrate-incoming allow out-of-band". All migration related changes will all be put into postcopy-recovery series. v4: - drop first patch to fix IOWatchPool [Stefan, Dan] - add s-o-b where missing, and newly got r-bs - fix English error in commit msg [Fam] - some tunes on request-dropped event: [Stefan] - firstly let 'id' be any type, meanwhile make sure "id" is there as long as OOB is enabled for the monitor. - some comments fix - add new command "x-oob-test" for testing oob commands [Stefan] - simplify the test codes to use new x-oob-test - flush response queue before cleanup monitors This series was born from this one: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg04310.html The idea comes from Markus Armbruster and the discussion we had in the thread. It's not a super ideal solution (I believe Markus had been thinking hard to keep everything in order meanwhile trying to satisfy the migration requirement), but AFAIU it's currently the best. What is OOB? It's the shortcut of Out-Of-Band execution, its name is given by Markus. It's a way to quickly execute a QMP request. Say, originally QMP is going throw these steps: JSON Parser --> QMP Dispatcher --> Respond /|\(2)(3) | (1) | \|/ (4) +- main thread + The requests are executed by the so-called QMP-dispatcher after the JSON is parsed. If OOB is