From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:45568) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UfhpW-0002Ei-Um for qemu-devel@nongnu.org; Thu, 23 May 2013 22:34:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UfhpV-0000Da-La for qemu-devel@nongnu.org; Thu, 23 May 2013 22:34:06 -0400 Received: from e23smtp03.au.ibm.com ([202.81.31.145]:47613) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UfhpV-00009I-1u for qemu-devel@nongnu.org; Thu, 23 May 2013 22:34:05 -0400 Received: from /spool/local by e23smtp03.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 24 May 2013 12:25:29 +1000 Received: from d23relay05.au.ibm.com (d23relay05.au.ibm.com [9.190.235.152]) by d23dlp01.au.ibm.com (Postfix) with ESMTP id 02B222CE804D for ; Fri, 24 May 2013 12:34:00 +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 r4O2Je7l23658618 for ; Fri, 24 May 2013 12:19:40 +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 r4O2XxnY022089 for ; Fri, 24 May 2013 12:33:59 +1000 Message-ID: <519ED114.5020806@linux.vnet.ibm.com> Date: Fri, 24 May 2013 10:31:48 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1369298836-17416-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1369298836-17416-6-git-send-email-xiawenc@linux.vnet.ibm.com> <519E3634.30107@redhat.com> <519EC6D1.6040605@linux.vnet.ibm.com> In-Reply-To: <519EC6D1.6040605@linux.vnet.ibm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V2 5/5] block: dump to specified output for bdrv_snapshot_dump() and bdrv_image_info_dump() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: kwolf@redhat.com, phrdina@redhat.com, stefanha@gmail.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, pbonzini@redhat.com, armbru@redhat.com 于 2013-5-24 9:48, Wenchao Xia 写道: > 于 2013-5-23 23:31, Eric Blake 写道: >> On 05/23/2013 02:47 AM, Wenchao Xia wrote: >>> Buffer is not used now so the string would not be truncated any more. >>> They can be used >>> by both qemu and qemu-img with correct parameter specified. >>> >>> Signed-off-by: Wenchao Xia >>> --- >>> block/qapi.c | 65 >>> +++++++++++++++++++++++++++----------------------- >>> include/block/qapi.h | 5 ++- >>> qemu-img.c | 15 +++++++---- >>> savevm.c | 11 ++++++-- >>> 4 files changed, 55 insertions(+), 41 deletions(-) >>> >> >>> @@ -282,17 +282,17 @@ char *bdrv_snapshot_dump(char *buf, int >>> buf_size, QEMUSnapshotInfo *sn) >>> (int)((secs / 60) % 60), >>> (int)(secs % 60), >>> (int)((sn->vm_clock_nsec / 1000000) % 1000)); >>> - snprintf(buf, buf_size, >>> - "%-10s%-20s%7s%20s%15s", >>> - sn->id_str, sn->name, >>> - get_human_readable_size(buf1, sizeof(buf1), >>> sn->vm_state_size), >>> - date_buf, >>> - clock_buf); >>> + message_printf(output, >> >> You got rid of ONE buffer... >> >>> + "%-10s%-20s%7s%20s%15s", >>> + sn->id_str, sn->name, >>> + get_human_readable_size(buf1, sizeof(buf1), >> >> ...but what is this other buffer still doing? get_human_readable_size >> needs to be converted to use QemuOutput. >> >>> +void bdrv_image_info_dump(const QemuOutput *output, ImageInfo *info) >>> { >>> char size_buf[128], dsize_buf[128]; >> >> Why do we still need size_buf and dsize_buf? >> >>> if (!info->has_actual_size) { >>> @@ -302,43 +302,47 @@ void bdrv_image_info_dump(ImageInfo *info) >>> info->actual_size); >>> } >>> get_human_readable_size(size_buf, sizeof(size_buf), >>> info->virtual_size); >> >> Again, get_human_readable_size should be converted to use QemuOutput. >> > They do not likely have a chance to be truncated, so have not change > them. I will convert them also in next version. > Just found this buffer is used in format control: message_printf(output, "%-10s%-20s%7s%20s%15s", sn->id_str, sn->name, get_human_readable_size(buf1, sizeof(buf1), sn->vm_state_size), date_buf, clock_buf); if get_human_readable_size() is converted, a manual control is needed. Same thing happens to clock_buf. It seems better to keep what it is. >>> +++ b/qemu-img.c >>> @@ -1554,16 +1554,18 @@ static void dump_snapshots(BlockDriverState *bs) >>> { >>> QEMUSnapshotInfo *sn_tab, *sn; >>> int nb_sns, i; >>> - char buf[256]; >>> + QemuOutput output = {OUTPUT_STREAM, {stdout,} }; >> >> This is relying on C99's rule that a union is initialized by its first >> named member. But I think it might be more readable as: >> >> output = { .kind = OUTPUT_STREAM, .stream = stdout }; >> >> not to mention that you will HAVE to use a designator to ever initialize >> the monitor element of the union in any parallel code that favors the >> monitor. > This solve the initialization issue, will use it, thank u for the tip. > >> >> Hmm, does C99 even allow anonymous unions, or is that a gcc extension? >> >> Overall, I like the direction this is headed. The conversion looks >> reasonable, although it didn't quite go far enough for getting rid of >> buffers. >> > > -- Best Regards Wenchao Xia