Re: [Qemu-devel] [PATCH v7 4/7] qapi: remove COMMAND_DROPPED event

[Markus Armbruster]

Cc: Eric & Daniel for the libvirt point of view.

Peter Xu <address@hidden> writes:

> On Mon, Sep 03, 2018 at 09:49:13AM +0200, Markus Armbruster wrote:
>> Peter Xu <address@hidden> writes:
>> 
>> > Now it was not used any more, drop it.  We can still do that since
>> > out-of-band is still experimental, and this event is only used when
>> > out-of-band is enabled.
>> >
>> > Signed-off-by: Peter Xu <address@hidden>
>> > ---
>> >  docs/interop/qmp-spec.txt |  5 +++--
>> >  qapi/misc.json            | 40 ---------------------------------------
>> >  2 files changed, 3 insertions(+), 42 deletions(-)
>> >
>> > diff --git a/docs/interop/qmp-spec.txt b/docs/interop/qmp-spec.txt
>> > index 8f7da0245d..67e44a8120 100644
>> > --- a/docs/interop/qmp-spec.txt
>> > +++ b/docs/interop/qmp-spec.txt
>> > @@ -130,8 +130,9 @@ to pass "id" with out-of-band commands.  Passing it 
>> > with all commands
>> >  is recommended for clients that accept capability "oob".
>> >  
>> >  If the client sends in-band commands faster than the server can
>> > -execute them, the server will eventually drop commands to limit the
>> > -queue length.  The sever sends event COMMAND_DROPPED then.
>> > +execute them, the server will stop reading the requests from the QMP
>> > +channel until the request queue length is reduced to an acceptable
>> > +range.
>> 
>> The change described by this documentation update is in the previous
>> commit.  This commit merely cleans up unused code.  Instead of moving
>> the doc update to the previous commit, we can simply squash the two
>> commits.
>
> Sure, I'll squash.
>
>> 
>> Let's add a hint on managing flow of in-band commands to keep the
>> monitor available for out-of-band commands.  Here's my try:
>> 
>>     If the client sends in-band commands faster than the server can
>>     execute them, the server will stop reading the requests from the QMP
>>     channel until its request queue length falls below the limit.  To
>>     keep the monitor available for out-of-band commands, the client has
>>     to manage the flow of in-band commands.
>> 
>> Of course, anyone trying to put this hint to use will need to know the
>> actual request queue limit.  Options:
>> 
>> * Make it ABI by documenting it here
>> 
>> * Let the client configure it with a suitable command
>> 
>> * Let the introspect it with a suitable command
>
> I'm not sure whether we should expose this information to the client,
> considering that the client can after all detect that "full" by
> monitoring the status of its output buffer used (e.g., poll with
> POLLOUT) and assuming that should be enough for a client.  Or did I
> miss anything that you were trying to emphasize but I didn't notice
> (since after all you mentioned this too in the other thread)?

By the time the client gets "write would block", OOB has already become
unavailable, and will remain so until the logjam clears sufficiently.

Example:

    client sends in-band command #1
    QEMU reads and queues
    QEMU dequeues in-band command #1
    in-band command #1 starts executing, but it's slooow
    client sends in-band command #2
    QEMU reads and queues
    ...
    client sends in-band command #8
    QEMU reads, queues and suspends the monitor
    client sends out-of-band command
--> time passes...
    in-band command #1 completes, QEMU sends reply
    QEMU dequeues in-band command #2, resumes the monitor
    in-band command #2 starts executing
    QEMU reads and executes out-of-band command
    out-of-band command completes, QEMU sends reply
    in-band command #2 completes, QEMU sends reply
    ... same for remaining in-band commands ...

The out-of-band command gets stuck behind the in-band commands.

The client can avoid this by managing the flow of in-band commands: have
no more than 7 in flight, so the monitor never gets suspended.

This is a potentially useful thing to do for clients, isn't it?

Eric, Daniel, is it something libvirt would do?