Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()

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

于 2013-5-16 20:17, Luiz Capitulino 写道:
> On Thu, 16 May 2013 10:22:09 +0800
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>
>> 于 2013-5-15 20:28, Luiz Capitulino 写道:
>>> On Wed, 15 May 2013 10:10:37 +0800
>>> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>>
>>>> 于 2013-5-6 21:22, Luiz Capitulino 写道:
>>>>> On Mon, 06 May 2013 10:09:43 +0800
>>>>> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>>>>
>>>>>> 于 2013-5-3 10:51, Wenchao Xia 写道:
>>>>>>> 于 2013-5-2 20:02, Luiz Capitulino 写道:
>>>>>>>> On Thu, 02 May 2013 10:05:08 +0800
>>>>>>>> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>>>>>>>
>>>>>>>>> 于 2013-4-30 3:05, Luiz Capitulino 写道:
>>>>>>>>>> On Fri, 26 Apr 2013 16:46:57 +0200
>>>>>>>>>> Stefan Hajnoczi <stefanha@gmail.com> wrote:
>>>>>>>>>>
>>>>>>>>>>> On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:
>>>>>>>>>>>> @@ -2586,10 +2585,12 @@ void do_info_snapshots(Monitor *mon, const
>>>>>>>>>>>> QDict *qdict)
>>>>>>>>>>>>           }
>>>>>>>>>>>>
>>>>>>>>>>>>           if (total > 0) {
>>>>>>>>>>>> -        monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf,
>>>>>>>>>>>> sizeof(buf), NULL));
>>>>>>>>>>>> +        bdrv_snapshot_dump(NULL);
>>>>>>>>>>>> +        monitor_printf(mon, "\n");
>>>>>>>>>>>
>>>>>>>>>>> Luiz: any issue with mixing monitor_printf(mon) and
>>>>>>>>>>> monitor_vprintf(cur_mon) calls?  I guess there was a reason for
>>>>>>>>>>> explicitly passing mon instead of relying on cur_mon.
>>>>>>>>>>
>>>>>>>>>> where are they being mixed?
>>>>>>>>>>
>>>>>>>>>        bdrv_snapshot_dump() used a global variable "cur_mon" inside,
>>>>>>>>> instead
>>>>>>>>> of let caller pass in a explicit montior* "mon", I guess that is the
>>>>>>>>> question.
>>>>>>>>
>>>>>>>> I'd have to see the code to tell, but yes, what Stefan described is the
>>>>>>>> best practice for the Monitor.
>>>>>>>>
>>>>>>>       I think this would not be a problem until qemu wants more than one
>>>>>>> human monitor console, and then we may require a data structure to tell
>>>>>>> where to output the string: stdout, *mon, or even stderr, and
>>>>>>> error_printf() also need to be changed.
>>>>>>>
>>>>>>       Luiz, what is your idea? I'd like to respin v2 if no issues for it.
>>>>>
>>>>> As I said before, I'd have to see the code to tell. But answering your comment,
>>>>> the code does support multiple monitors.
>>>>>
>>>> Hi Luiz,
>>>>      Sorry to ask again, do you think method above is OK now, waiting for
>>>> your confirm.
>>>
>>> Can you point me to the code in question?
>>>
>> Sure, it is
>>
>> +
>> +/*
>> + * Print to current monitor if we have one, else to stdout. It is
>> similar with
>> + * error_printf().
>> + * TODO just like error_vprintf()
>> + */
>> +void message_printf(const char *fmt, ...)
>> +{
>> +    va_list ap;
>> +
>> +    va_start(ap, fmt);
>> +    if (cur_mon) {
>> +        monitor_vprintf(cur_mon, fmt, ap);
>> +    } else {
>> +        vfprintf(stdout, fmt, ap);
>> +    }
>> +    va_end(ap);
>> +}
>>
>>     This function used global variable cur_mon instead of input parameter,
>> similar to error_printf().
>
> Why do you need it? Why can't you you use error_printf() for example?
>
   error_printf() will print out to stderr in qemu-img, but stdout is
wanted for those dump info function.

-- 
Best Regards

Wenchao Xia