Re: [Qemu-devel] [RFC v2 2/8] monitor: allow monitor to create thread to poll

[Markus Armbruster <armbru@redhat.com>]

Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi
>
> On Mon, Aug 28, 2017 at 1:08 PM Markus Armbruster <armbru@redhat.com> wrote:
>
>> Marc-André Lureau <marcandre.lureau@gmail.com> writes:
>>
>> > On Fri, Aug 25, 2017 at 5:33 PM Dr. David Alan Gilbert <
>> dgilbert@redhat.com>
>> > wrote:
>> >
>> >> * Marc-André Lureau (marcandre.lureau@gmail.com) wrote:
[...]
>> >> > Could the BQL be pushed down to the monitor commands level instead? That
>> >> > way we wouldn't need a seperate thread to solve the hang on commands that
>> >> > do not need BQL.
>> >>
>> >> If the main thread is stuck though I don't see how that helps you; you
>> >> have to be able to run these commands on another thread.
>> >>
>> >
>> > Why would the main thread be stuck? In (1) If the vcpu thread takes the BQL
>> > and the command doesn't need it, it would work.  In (2),  info cpus
>> > shouldn't keep the BQL (my qapi-async series would probably help here)
>>
>> This has been discussed several times[*], but of course not with
>> everybody, so I'll summarize once more: asynchronous commands are not a
>> actually *required* for anything.  They are *one* way to package the
>> "kick off task, receive an asynchronous message when it's done" pattern.
>> Another way is synchronous command for the kick off, event for the
>> "done".
>>
>
> But you would have to break or duplicate the QMP APIs. My proposal doesn't
> need that, a command can reenter the main loop, and keep current QMP API.

Changing an existing command from synchronous to asynchronous is
definitely an API change, as discussed before.

>> For better or worse, synchronous command + event is what we have today.
>> Whether adding another way to package the the same thing improves the
>> QMP interface is doubtful.
>>
>
> I would argue my series is mostly about internal refactoring for the
> benefit mentionned above. The fact that you can do (optionnaly) concurrent
> QMP commands is a nice bonus. Furthermore, it simplifies the API compared
> to CMD / dummy reply + EVENT. And it gives a meaning to the exisiting
> command "id"..

Call a change "mostly internal" when it fundamentally extends the QMP
protocol makes as much sense to me as "mostly not pregnant".

The non-dummy nature of the command reply has also been discussed
several times.  So has been the relative complexity of "synchronous
commands + events" vs. "asynchronous commands" vs. "both".  In
considerable detail, in fact:

    Message-ID: <87mvaszbsu.fsf@dusky.pond.sub.org>
    http://lists.gnu.org/archive/html/qemu-devel/2017-05/msg01090.html

Let me quote its last few lines:

    Bottom line:

    1. I still don't want to merge this.

    2. I want us to tackle jobs sooner rather than later.

    3. Once we got at least a jobs prototype, I'm willing to reconsider
       asynchronous commands implemented as special case of jobs.

    One of the most important maintainer duties is saying "no".  It's also
    one of the least fun duties.

I'm happy to reconsider this conclusion when presented with new
evidence.

Asynchronous commands vs. synchronous commands + events are different
packaging of the same thing: neither can do anything the other could not
do.  If we want to make progress on the monitor hang problem (this
thread's topic), we should focus on the *concepts*, not how to best
package them for QMP.

>> [*] Try this one:
>> Message-ID: <87o9yv890z.fsf@dusky.pond.sub.org>
>> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05483.html