[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V11 13/17] block: dump to buffer for bdrv_snapsh
From: |
Wenchao Xia |
Subject: |
Re: [Qemu-devel] [PATCH V11 13/17] block: dump to buffer for bdrv_snapshot_dump() and bdrv_image_info_dump() |
Date: |
Fri, 12 Apr 2013 12:51:27 +0800 |
User-agent: |
Mozilla/5.0 (Windows NT 5.1; rv:17.0) Gecko/20130328 Thunderbird/17.0.5 |
δΊ 2013-4-11 20:05, Markus Armbruster ει:
> Wenchao Xia <address@hidden> 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
- Re: [Qemu-devel] [PATCH V11 11/17] qmp: add ImageInfo in BlockDeviceInfo used by query-block, (continued)
Re: [Qemu-devel] [PATCH V11 11/17] qmp: add ImageInfo in BlockDeviceInfo used by query-block, Markus Armbruster, 2013/04/10
[Qemu-devel] [PATCH V11 13/17] block: dump to buffer for bdrv_snapshot_dump() and bdrv_image_info_dump(), Wenchao Xia, 2013/04/02
[Qemu-devel] [PATCH V11 16/17] hmp: show ImageInfo in 'info block', Wenchao Xia, 2013/04/02
[Qemu-devel] [PATCH V11 15/17] hmp: switch snapshot info function to qmp based one, Wenchao Xia, 2013/04/02
[Qemu-devel] [PATCH V11 17/17] hmp: add parameters device and -v for info block, Wenchao Xia, 2013/04/02
[Qemu-devel] [PATCH V11 14/17] hmp: add function hmp_info_snapshots(), Wenchao Xia, 2013/04/02
Re: [Qemu-devel] [PATCH V11 00/17] qmp/hmp interfaces for internal snapshot info, Wenchao Xia, 2013/04/07
Re: [Qemu-devel] [PATCH V11 00/17] qmp/hmp interfaces for internal snapshot info, Kevin Wolf, 2013/04/08
Re: [Qemu-devel] [PATCH V11 00/17] qmp/hmp interfaces for internal snapshot info, Stefan Hajnoczi, 2013/04/08
Re: [Qemu-devel] [PATCH V11 00/17] qmp/hmp interfaces for internal snapshot info, Markus Armbruster, 2013/04/11