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

[Luiz Capitulino]

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.

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