Re: [Qemu-devel] [PATCH V11 13/17] block: dump to buffer for bdrv_snapshot_dump() and bdrv_image_info_dump()

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 2013-4-11 20:05, Markus Armbruster 写道:
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> 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