Re: [Qemu-devel] [PATCH 2/6] monitor: Adjust qmp_human_monitor_command to new MemCharDriver

[Lei Li]

On 08/31/2012 02:51 AM, Luiz Capitulino wrote:
> On Thu, 23 Aug 2012 13:14:22 +0800
> Lei Li <address@hidden> wrote:
>
> > Signed-off-by: Lei Li <address@hidden>
> > ---
> >   monitor.c |    8 +++++++-
> >   1 files changed, 7 insertions(+), 1 deletions(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index 480f583..ab4650b 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -642,7 +642,13 @@ char *qmp_human_monitor_command(const char *command_line, 
> > bool has_cpu_index,
> >       CharDriverState mchar;
> >   
> >       memset(&hmp, 0, sizeof(hmp));
> > -    qemu_chr_init_mem(&mchar);
> > +
> > +    /* Since the backend of MemCharDriver convert to a circular
> > +     * buffer with fixed size, so should indicate the init memory
> > +     * size.
> > +     *
> > +     * XXX:  is 4096 as init memory enough for this? */
> > +    qemu_chr_init_mem(&mchar, 4096);
> I'm not sure I like this. The end result will be that hmp commands writing
> more than 4096 bytes will simply fail or return garbage (if the circular buffer
> is changed to allow writing more than it supports) today they would just work.
>
> Although it's always possible to increase the buffer size, we would only realize
> this is needed when the bug is triggered, which means it has a high chance
> of happening in production. IOW, this would be a regression.
>
> The only solution I can think of is to make the circular buffer and the
> current MemoryDriver live in parallel. Actually, you really seem to be
> adding something else.

Hi Luiz,

Thanks for your review. Yes, I agree with that it's not a good solution for the
old user human command. Make the circular buffer and the current MemoryDriver
live in parallel, do you mean don't change the current MemoryDriver, choose to
expose a new char device backend with circular buffer?

> >       hmp.chr = &mchar;
> >   
> >       old_mon = cur_mon;


--
Lei