[Qemu-devel] Re: [PATCH 09/11] QMP: Port 'info blockstats' command

[Jan Kiszka <jan.kiszka@siemens.com>]

Avi Kivity wrote:
> On 06/23/2009 12:59 PM, Jan Kiszka wrote:
>>>> --- a/block.c
>>>> +++ b/block.c
>>>> @@ -1102,15 +1102,15 @@ void bdrv_info_stats(Monitor *mon)
>>>>        BlockDriverState *bs;
>>>>
>>>>        for (bs = bdrv_first; bs != NULL; bs = bs->next) {
>>>> -        monitor_printf(mon, "%s:"
>>>> -                       " rd_bytes=%" PRIu64
>>>> -                       " wr_bytes=%" PRIu64
>>>> -                       " rd_operations=%" PRIu64
>>>> -                       " wr_operations=%" PRIu64
>>>> -                       "\n",
>>>> -                       bs->device_name,
>>>> -                       bs->rd_bytes, bs->wr_bytes,
>>>> -                       bs->rd_ops, bs->wr_ops);
>>>> +        monitor_printf_data(mon, "%s:"
>>>> +                            " rd_bytes=%" PRIu64
>>>> +                            " wr_bytes=%" PRIu64
>>>> +                            " rd_operations=%" PRIu64
>>>> +                            " wr_operations=%" PRIu64
>>>> +                            "\n",
>>>> +                            bs->device_name,
>>>> +                            bs->rd_bytes, bs->wr_bytes,
>>>> +                            bs->rd_ops, bs->wr_ops);
>>>>
>>>>        
>>> It may be easier for control mode to drop the variable= prefixes (they
>>> are described in the spec anyway).
>>>
>>>      
>>
>> Isn't monitor_printf_data also used for pretty-printing in case of
>> human-operated monitors?
> 
> It is.
> 
>> I think we need to keep this - or provide two
>> output formats which are filtered by the monitor based on the the target
>> terminal's mode. But that sounds like a lot of work /wrt conversion and
>> long-term maintenance.
>>    
> 
> I prefer separate printfs.  We don't want to allow choices made for the
> human protocol to influence the machine protocol, the whole point of the
> machine protocol was to have a clean break.

OK... yeah, makes sense. Then the above hunk must not remove the
existing monitor_printf, just add new monitor_printf_data.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux