From: Markus Armbruster <armbru@redhat.com>
To: Yury Kotov <yury-kotov@yandex-team.ru>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>,
qemu-devel@nongnu.org,
"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
"Denis Plotnikov" <dplotnikov@virtuozzo.com>,
yc-core@yandex-team.ru,
"Marc-André Lureau" <marcandre.lureau@redhat.com>,
"Denis V. Lunev" <den@openvz.org>
Subject: Re: [PATCH] monitor: Fix slow reading
Date: Fri, 29 Nov 2019 09:21:47 +0100 [thread overview]
Message-ID: <87v9r3w0ro.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <20191122092347.28309-1-yury-kotov@yandex-team.ru> (Yury Kotov's message of "Fri, 22 Nov 2019 12:23:47 +0300")
Yury Kotov <yury-kotov@yandex-team.ru> writes:
> The monitor_can_read (as a callback of qemu_chr_fe_set_handlers)
> should return size of buffer which monitor_qmp_read or monitor_read
> can process.
> Currently, monitor_can_read returns 1 as a result of logical not.
> Thus, for each QMP command, len(QMD) iterations of the main loop
> are required to handle a command.
> In fact, these both functions can process any buffer size.
> So, return 1024 as a reasonable size which is enough to process
> the most QMP commands, but not too big to block the main loop for
> a long time.
>
> Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
> ---
> monitor/monitor.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 12898b6448..cac3f39727 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -50,6 +50,13 @@ typedef struct {
> int64_t rate; /* Minimum time (in ns) between two events */
> } MonitorQAPIEventConf;
>
> +/*
> + * The maximum buffer size which the monitor can process in one iteration
> + * of the main loop. We don't want to block the loop for a long time
> + * because of JSON parser, so use a reasonable value.
> + */
> +#define MONITOR_READ_LEN_MAX 1024
> +
> /* Shared monitor I/O thread */
> IOThread *mon_iothread;
>
> @@ -498,7 +505,7 @@ int monitor_can_read(void *opaque)
> {
> Monitor *mon = opaque;
>
> - return !atomic_mb_read(&mon->suspend_cnt);
> + return atomic_mb_read(&mon->suspend_cnt) ? 0 : MONITOR_READ_LEN_MAX;
> }
>
> void monitor_list_append(Monitor *mon)
Prior attempt:
[PATCH 1/1] monitor: increase amount of data for monitor to read
Message-Id: <1493732857-10721-1-git-send-email-den@openvz.org>
https://lists.nongnu.org/archive/html/qemu-devel/2017-05/msg00206.html
Review concluded that it breaks HMP command migrate without -d. QMP is
probably okay. Sadly, no v2.
Next one:
Subject: [PATCH] monitor: increase amount of data for monitor to read
Message-Id: <20190610105906.28524-1-dplotnikov@virtuozzo.com>
https://lists.nongnu.org/archive/html/qemu-devel/2019-06/msg01912.html
Same patch, with a second, suspicious-looking hunk thrown in. I didn't
make the connection to the prior attempt back then. I wrote "I think I
need to (re-)examine how QMP reads input, with special consideration to
its OOB feature."
This patch is a cleaner variation on the same theme. Its ramifications
are as unobvious as ever.
I figure the HMP situation is unchanged: not safe, although we could
probably make it safe if we wanted to (Daniel sketched how). My simpler
suggestion stands: separate f_can_read() callbacks for HMP and QMP
[PATCH 1], then change only the one for QMP [PATCH 2].
The QMP situation is also unchanged: we still need to think through how
this affects reading of QMP input, in particular OOB.
next prev parent reply other threads:[~2019-11-29 8:29 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-22 9:23 [PATCH] monitor: Fix slow reading Yury Kotov
2019-11-22 10:23 ` Philippe Mathieu-Daudé
2019-11-29 8:21 ` Markus Armbruster [this message]
2019-12-02 17:43 ` Yury Kotov
2019-12-02 20:49 ` Markus Armbruster
2019-12-03 7:40 ` Denis V. Lunev
2019-12-03 8:27 ` Yury Kotov
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=87v9r3w0ro.fsf@dusky.pond.sub.org \
--to=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=den@openvz.org \
--cc=dgilbert@redhat.com \
--cc=dplotnikov@virtuozzo.com \
--cc=marcandre.lureau@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=yc-core@yandex-team.ru \
--cc=yury-kotov@yandex-team.ru \
/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).