From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42037) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T8ZJB-0000Fg-8w for qemu-devel@nongnu.org; Mon, 03 Sep 2012 12:15:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T8ZJ9-0008Qi-VB for qemu-devel@nongnu.org; Mon, 03 Sep 2012 12:15:29 -0400 Received: from e23smtp09.au.ibm.com ([202.81.31.142]:46628) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T8ZJ9-0007wl-Cj for qemu-devel@nongnu.org; Mon, 03 Sep 2012 12:15:27 -0400 Received: from /spool/local by e23smtp09.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 4 Sep 2012 02:13:26 +1000 Received: from d23av02.au.ibm.com (d23av02.au.ibm.com [9.190.235.138]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q83G5f4u28442650 for ; Tue, 4 Sep 2012 02:05:42 +1000 Received: from d23av02.au.ibm.com (loopback [127.0.0.1]) by d23av02.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q83GEbEI015423 for ; Tue, 4 Sep 2012 02:14:37 +1000 Message-ID: <5044D74B.60204@linux.vnet.ibm.com> Date: Tue, 04 Sep 2012 00:14:03 +0800 From: Lei Li MIME-Version: 1.0 References: <1345698866-19794-1-git-send-email-lilei@linux.vnet.ibm.com> <1345698866-19794-3-git-send-email-lilei@linux.vnet.ibm.com> <20120830155146.718ccd30@doriath.home> In-Reply-To: <20120830155146.718ccd30@doriath.home> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/6] monitor: Adjust qmp_human_monitor_command to new MemCharDriver List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: aliguori@us.ibm.com, eblake@redhat.com, qemu-devel@nongnu.org On 08/31/2012 02:51 AM, Luiz Capitulino wrote: > On Thu, 23 Aug 2012 13:14:22 +0800 > Lei Li wrote: > >> Signed-off-by: Lei Li >> --- >> 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