From: Markus Armbruster <armbru@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: Peter Xu <peterx@redhat.com>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] monitor: enable OOB by default
Date: Wed, 27 Jun 2018 10:35:15 +0200 [thread overview]
Message-ID: <871scsaapo.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <87po0cad8n.fsf_-_@dusky.pond.sub.org> (Markus Armbruster's message of "Wed, 27 Jun 2018 09:40:40 +0200")
Markus Armbruster <armbru@redhat.com> writes:
> Another lose end: event COMMAND_DROPPED seems to lack test coverage.
Hmm, dropping commands serves to limit the request queue. What limits
the response queue?
Before OOB, the monitor read at most one command, and wrote its response
with monitor_puts().
For input, this leaves queueing to the kernel: if the client sends
commands faster than the server can execute them, eventually the kernel
refuses to buffer more, and the client's send either blocks or fails
with EAGAIN.
Output is a mess. monitor_puts() uses an output buffer. It tries to
flush at newline. Issues:
* If flushing runs into a partial write, the unwritten remainder remains
in the output buffer until the next newline. That newline may take
its own sweet time to arrive. Could even lead to deadlocks, where a
client awaits complete output before it sends more input. Bug,
predates OOB, doesn't block this series.
* If the client fails to read, the output buffer can grow without bound.
Not a security issue; the client is trusted. Just bad workmanship.
OOB doesn't change this for monitors running in the main thread. Only
mux chardevs run there.
Aside: keeping special case code around just for mux is a really bad
idea. We need to get rid of it.
For monitors running in an I/O thread, we add another buffer: the
response queue. It's drained by monitor_qmp_bh_responder(). I guess
that means the response queue is effectively bounded by timely draining.
Correct?
Buffering twice seems silly, but that could be addressed in follow-up
patches.
next prev parent reply other threads:[~2018-06-27 8:35 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
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 [this message]
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=871scsaapo.fsf@dusky.pond.sub.org \
--to=armbru@redhat.com \
--cc=peterx@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.