From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41607) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UQVyX-0000zD-9Q for qemu-devel@nongnu.org; Fri, 12 Apr 2013 00:52:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UQVyV-0005ns-Fy for qemu-devel@nongnu.org; Fri, 12 Apr 2013 00:52:37 -0400 Received: from e23smtp04.au.ibm.com ([202.81.31.146]:32815) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UQVyU-0005nl-TD for qemu-devel@nongnu.org; Fri, 12 Apr 2013 00:52:35 -0400 Received: from /spool/local by e23smtp04.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 12 Apr 2013 14:41:07 +1000 Received: from d23relay05.au.ibm.com (d23relay05.au.ibm.com [9.190.235.152]) by d23dlp01.au.ibm.com (Postfix) with ESMTP id 136612CE8051 for ; Fri, 12 Apr 2013 14:52:25 +1000 (EST) Received: from d23av01.au.ibm.com (d23av01.au.ibm.com [9.190.234.96]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r3C4csFC8585560 for ; Fri, 12 Apr 2013 14:38:55 +1000 Received: from d23av01.au.ibm.com (loopback [127.0.0.1]) by d23av01.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r3C4qN03031404 for ; Fri, 12 Apr 2013 14:52:23 +1000 Message-ID: <516792CF.9020202@linux.vnet.ibm.com> Date: Fri, 12 Apr 2013 12:51:27 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1364903250-10429-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1364903250-10429-14-git-send-email-xiawenc@linux.vnet.ibm.com> <87fvyytixe.fsf@blackfin.pond.sub.org> <516659A9.3010101@linux.vnet.ibm.com> <87fvyxff1y.fsf@blackfin.pond.sub.org> In-Reply-To: <87fvyxff1y.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=GB2312 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V11 13/17] block: dump to buffer for bdrv_snapshot_dump() and bdrv_image_info_dump() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kwolf@redhat.com, stefanha@gmail.com, pbonzini@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com ÓÚ 2013-4-11 20:05, Markus Armbruster дµÀ: > Wenchao Xia writes: > >>>> >>>> -void bdrv_image_info_dump(ImageInfo *info) >>>> +void bdrv_image_info_dump(GString *buf, ImageInfo *info) >>>> { >>>> char size_buf[128], dsize_buf[128]; >>>> if (!info->has_actual_size) { >>>> @@ -370,43 +369,48 @@ void bdrv_image_info_dump(ImageInfo *info) >>> >>> I don't like this change, because it introduces buffering for no >>> discernible reason. Unless you can show me one, I'd like you to keep >>> printing directly. >>> >> HMP code later need to call this function, and then print buf to >> monitor console, which is the goal of this patch. > > Aha, the actual problem is "print either to stdout or to current > monitor", so that we can share code among qemu-img and monitor commands. > Such sharing is good, and the problem is real. > > bdrv_snapshot_dump() solves it this way: print to a fixed-size buffer, > which the caller either prints to stdout (qemu-img) or to the current > monitor (info snapshots). > > Your patch makes the buffer flexible. Improvement. > > But in my opinion, the detour through the buffer is silly. Why not > simply use a print function that does the right thing? > > We already have such a function for "print either to stderr or to > current monitor": error_printf(). Could easily add one for stdout. > > [...] > Indeed it is a better way, how about: void message_printf(const char *fmt, ...); I am not sure where should this function be placed, hmp.c? -- Best Regards Wenchao Xia