From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59231) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T8ZNz-0001ou-5x for qemu-devel@nongnu.org; Mon, 03 Sep 2012 12:20:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T8ZNv-0001Y0-06 for qemu-devel@nongnu.org; Mon, 03 Sep 2012 12:20:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37290) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T8ZNu-0001W1-NL for qemu-devel@nongnu.org; Mon, 03 Sep 2012 12:20:22 -0400 Date: Mon, 3 Sep 2012 13:21:00 -0300 From: Luiz Capitulino Message-ID: <20120903132100.349ff426@doriath.home> In-Reply-To: <5044D74B.60204@linux.vnet.ibm.com> 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> <5044D74B.60204@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII 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: Lei Li Cc: aliguori@us.ibm.com, eblake@redhat.com, qemu-devel@nongnu.org On Tue, 04 Sep 2012 00:14:03 +0800 Lei Li wrote: > 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? Yes, you could add CircularMemoryDriver for example. > > >> hmp.chr = &mchar; > >> > >> old_mon = cur_mon; > > > >