On Wed, 27 Mar 2013 14:45:53 +0800
Wenchao Xia <xiaw...@linux.vnet.ibm.com> wrote:
> Hi, Luiz
> Personally I hope reduce the dynamic allocated buffer which brings
> fragments and unexpected memory grow. Instead, how about sacrifice
> some time to wait output complete, since monitor is not time critical?
> in this case static buffer's size can decide how many work can be
> postponded. Following is my suggestion:
I'd be fine with it if you find a good way to postpone work, but sleeping
on random timers is not a good way.
Also, QEMU allocates fragments of memory in so many places that I doubt
this is an valid argument. You're right that long QMP output can cause
unexpected memory growth, but I expected this to be really rare and if
this really bother us, we can ask monitor_puts() to flush at, say, every
4096 bytes.
>
> --- a/monitor.c
> +++ b/monitor.c
> @@ -293,17 +293,28 @@ static void monitor_puts(Monitor *mon, const char
> *str)
> {
> char c;
>
> + /* if mux do not put in any thing to buffer */
> + if (mon->mux_out) {
> + return;
> + }
> +
> for(;;) {
> - assert(mon->outbuf_index < sizeof(mon->outbuf) - 1);
> + if (mon->outbuf_index >= sizeof(mon->outbuf) - 1) {
> + /* when buffer is full, flush it and retry. If buffer is
> bigger, more
> + work can be postponed. */
> + monitor_flush(mon);
> + usleep(1);
> + continue;
> + }
> c = *str++;
> if (c == '\0')
> break;
> if (c == '\n')
> mon->outbuf[mon->outbuf_index++] = '\r';
> mon->outbuf[mon->outbuf_index++] = c;
> - if (mon->outbuf_index >= (sizeof(mon->outbuf) - 1)
> - || c == '\n')
> + if (c == '\n') {
> monitor_flush(mon);
> + }
> }
> }
>
> > Commit f628926bb423fa8a7e0b114511400ea9df38b76a changed monitor_flush()
> > to retry on qemu_chr_fe_write() errors. However, the Monitor's output
> > buffer can keep growing while the retry is not issued and this can
> > cause the buffer to overflow.
> >
> > To reproduce this issue, just start qemu and type on the Monitor:
> >
> > (qemu) ?
> >
> > This will cause the assertion to trig.
> >
> > To fix this problem this commit makes the Monitor buffer dynamic,
> > which means that it can grow as much as needed.
> >
> > Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com>
> > ---
> > monitor.c | 42 +++++++++++++++++++++++++-----------------
> > 1 file changed, 25 insertions(+), 17 deletions(-)
> >
>
