From: Peter Xu <peterx@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] monitor: enable OOB by default
Date: Wed, 27 Jun 2018 21:34:42 +0800 [thread overview]
Message-ID: <20180627133442.GB2455@xz-mi> (raw)
In-Reply-To: <87h8lo74oa.fsf@dusky.pond.sub.org>
On Wed, Jun 27, 2018 at 03:13:57PM +0200, Markus Armbruster wrote:
> Monitor behavior changes even when the client rejects capability "oob".
>
> Traditionally, the monitor reads, executes and responds to one command
> after the other. If the client sends in-band commands faster than the
> server can execute them, the kernel will eventually refuse to buffer
> more, and sending blocks or fails with EAGAIN.
>
> To make OOB possible, we need to read and queue commands as we receive
> them. 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.
>
> However, we get the new behavior even when the client rejects capability
> "oob". We get the traditional behavior only when the server doesn't
> offer "oob".
>
> Is this what we want?
Not really. But I thought we kept that old behavior, haven't we?
In handle_qmp_command() we have this:
/*
* If OOB is not enabled on the current monitor, we'll emulate the
* old behavior that we won't process the current monitor any more
* until it has responded. This helps make sure that as long as
* OOB is not enabled, the server will never drop any command.
*/
if (!qmp_oob_enabled(mon)) {
monitor_suspend(mon);
req_obj->need_resume = true;
} else {
/* Drop the request if queue is full. */
if (mon->qmp.qmp_requests->length >= QMP_REQ_QUEUE_LEN_MAX) {
qemu_mutex_unlock(&mon->qmp.qmp_queue_lock);
qapi_event_send_command_dropped(id,
COMMAND_DROP_REASON_QUEUE_FULL,
&error_abort);
qmp_request_free(req_obj);
return;
}
}
Here assuming that we are not enabling OOB, then since we will suspend
the monitor when we receive one command, then monitor_can_read() later
will check with a result that "we should not read the chardev", then
it'll leave all the data in the input buffer. Then AFAIU the QMP
client that didn't enable OOB will have similar behavior as before.
Also, we will never receive a COMMAND_DROPPED event in that legacy
mode as well since it's in the "else" block. Am I right?
Regards,
--
Peter Xu
next prev parent reply other threads:[~2018-06-27 13:34 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-20 7:32 [Qemu-devel] [PATCH v5 0/7] monitor: enable OOB by default Peter Xu
2018-06-20 7:32 ` [Qemu-devel] [PATCH v5 1/7] chardev: comment details for CLOSED event Peter Xu
2018-06-20 7:32 ` [Qemu-devel] [PATCH v5 2/7] monitor: rename *_pop_one to *_pop_any Peter Xu
2018-06-20 7:32 ` [Qemu-devel] [PATCH v5 3/7] monitor: flush qmp responses when CLOSED Peter Xu
2018-06-20 8:33 ` Markus Armbruster
2018-06-20 8:38 ` Peter Xu
2018-06-20 9:50 ` Markus Armbruster
2018-06-20 7:32 ` [Qemu-devel] [PATCH v5 4/7] tests: iotests: drop some stderr line Peter Xu
2018-06-20 8:35 ` Markus Armbruster
2018-06-20 7:32 ` [Qemu-devel] [PATCH v5 5/7] docs: mention shared state protect for OOB Peter Xu
2018-06-20 7:32 ` [Qemu-devel] [PATCH v5 6/7] monitor: remove "x-oob", turn oob on by default Peter Xu
2018-06-20 7:32 ` [Qemu-devel] [PATCH v5 7/7] Revert "tests: Add parameter to qtest_init_without_qmp_handshake" Peter Xu
2018-06-26 17:21 ` [Qemu-devel] (no subject) Markus Armbruster
2018-06-27 7:38 ` [Qemu-devel] monitor: enable OOB by default Markus Armbruster
2018-06-27 8:41 ` Markus Armbruster
2018-06-27 10:20 ` Daniel P. Berrangé
2018-06-27 11:23 ` Markus Armbruster
2018-06-27 12:07 ` Peter Xu
2018-06-27 12:37 ` Eric Blake
2018-06-28 7:04 ` Markus Armbruster
2018-06-29 7:20 ` Peter Xu
2018-06-28 6:55 ` Markus Armbruster
2018-06-28 11:43 ` Eric Blake
2018-06-29 8:18 ` Peter Xu
2018-06-27 13:13 ` Markus Armbruster
2018-06-27 13:28 ` Daniel P. Berrangé
2018-06-28 13:02 ` Markus Armbruster
2018-06-27 13:34 ` Peter Xu [this message]
2018-06-28 13:20 ` Markus Armbruster
2018-06-29 9:01 ` Peter Xu
2018-07-18 15:08 ` Markus Armbruster
2018-07-19 13:00 ` Peter Xu
2018-06-27 7:40 ` Markus Armbruster
2018-06-27 8:35 ` Markus Armbruster
2018-06-27 12:32 ` Peter Xu
2018-06-28 9:29 ` Markus Armbruster
2018-06-29 9:42 ` Peter Xu
2018-06-27 13:36 ` Peter Xu
2018-06-27 11:59 ` [Qemu-devel] your mail Peter Xu
2018-06-28 8:31 ` Markus Armbruster
2018-06-28 11:51 ` Eric Blake
2018-06-28 12:00 ` Daniel P. Berrangé
2018-06-29 9:57 ` Peter Xu
2018-06-29 15:40 ` Eric Blake
2018-07-02 5:43 ` [Qemu-devel] monitor: enable OOB by default Markus Armbruster
2018-07-04 5:44 ` Peter Xu
2018-07-04 7:03 ` Markus Armbruster
2018-06-30 16:26 ` [Qemu-devel] [PATCH v5 0/7] " Markus Armbruster
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180627133442.GB2455@xz-mi \
--to=peterx@redhat.com \
--cc=armbru@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).