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

[Lei Li <lilei@linux.vnet.ibm.com>]

On 08/31/2012 02:51 AM, Luiz Capitulino wrote:
> On Thu, 23 Aug 2012 13:14:22 +0800
> Lei Li <lilei@linux.vnet.ibm.com> wrote:
>
>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
>> ---
>>   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