Marc-André Lureau <marcandre.lur...@gmail.com> writes: > Hi > > On Tue, Jan 7, 2020 at 9:17 AM Kevin Wolf <kw...@redhat.com> wrote: >> >> Am 06.01.2020 um 19:21 hat Marc-André Lureau geschrieben: >> > > What my patch does is moving everything into a coroutine. This is wrong >> > > because not everything can be run in a coroutine, so it needs to be made >> > > optional (like you did with your async flag). >> > >> > "everything" is a bit too much ;) You proposal is to replace >> > qmp_dispatch_bh by a coroutine version (except for OOB commands). This >> > is nice because indeed, it allows to reenter the mainloop with a >> > simple yield in QMP commands. It is also simpler than my "async" >> > proposal, because some of the state is part of the coroutine, and >> > because it doesn't allow QMP commands concurrency (beside existing >> > OOB). >> > >> > Iow, coroutine (for async) + oob (for concurrency) make my proposal >> > kinda obsolete. I can only regret that a simple callback-based >> > solution looked simpler to me than one that mixes both threads & >> > coroutines, but I don't mind if everybody likes it better :) I can >> > definitely see the point for block commands, which rely on coroutines >> > anyway, and qemu is already that complex in general. >> >> Callbacks are indeed simple enough for implementing the infrastructure, >> but for the users they only look simple as long as they do trivial >> things. :-) >> >> Anyway, now that you have seen my POC hack, do you agree that this >> should help solving the screendump problem, too? > > Yes, and I will work on it as soon as you have a working patch series > or branch :) > >> >> > > The problem isn't with completely coroutine-unaware code, though: That >> > > one would just work, even if not taking advantage from the coroutine. A >> > > potential problem exists with code that behaves differently when run in >> > > a coroutine or outside of coroutine context (generally by checking >> > > qemu_in_coroutine())), or calls of coroutine-unaware code into such >> > > functions. >> > > >> > > Running some command handlers outside of coroutine context wouldn't be >> > > hard to add to my patch (basically just a BH), but I haven't looked into >> > > the QAPI side of making it an option. >> > >> > Yes, I think we should have a 'coroutine': true, for commands that >> > should be run with a coroutine. >> > >> > Or perhaps replace existing allow-oob with 'dispatch': >> > - 'bh' (default) >> > - 'coroutine' >> > - 'allow-oob' (oob + bh fallback, since oob don't have coroutine - at >> > this point) >> >> If it's "at this point", then making it two separate bools would make >> more sense. But I seem to remember that OOB handlers are fundamentally >> not supposed to block, so coroutine support would be pointless for them >> and an enum could work. > > I think so too > >> >> I'll defer to Markus on this one. > > Yup, Markus should take a look at your proposal and give some > guidance. And hopefully, it won't take >2y.
Is it "[PATCH 0/4] qmp: Optionally run handlers in coroutines"? [...]
