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

[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.

[...]