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

[Marc-André Lureau]

Hi

On Mon, Aug 28, 2017 at 6:24 PM Markus Armbruster <address@hidden> wrote:

> Marc-André Lureau <address@hidden> writes:
>
> > Hi
> >
> > On Mon, Aug 28, 2017 at 1:08 PM Markus Armbruster <address@hidden>
> wrote:
> >
> >> Marc-André Lureau <address@hidden> writes:
> >>
> >> > On Fri, Aug 25, 2017 at 5:33 PM Dr. David Alan Gilbert <
> >> address@hidden>
> >> > wrote:
> >> >
> >> >> * Marc-André Lureau (address@hidden) 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.
>

We are getting of topic, but I really feel there is a misunderstanding and
a wrong evaluation of my -async proposal.

The command is in no way asynchronous from an external point of view, as
long as the QMP user doesn't declare async capability.

There is no external API change. The "internal API" change is made optional
too.


> >> 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: <address@hidden>
>     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.
>

This is not being fair tbh, my proposal is RFC for 2y, and has been ready
for a while. I have no clear clue what the "jobs" API would look like, or
what are the requirements, so I can't work on it or defend my work. But it
will almost certainly benefit from my low-level -async work.


>     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.
>

 I am still convince my proposal has many merits, so I will keep proposing
it when I see conflicts with what I have, even if you said "no" to an
earlier merge request.


> 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.
>

Right, please ignore  the optional QMP protocol -async capability and look
at it: one way is to make long-lived/blocking QMP commands -async
internally, so other work can happen. This is 90% of my -async proposal.


> >> [*] Try this one:
> >> Message-ID: <address@hidden>
> >> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05483.html
>
-- 
Marc-André Lureau