At 02/15/2012 09:01 PM, Luiz Capitulino Wrote:
> On Wed, 15 Feb 2012 09:51:04 +0100
> Jan Kiszka <jan.kis...@siemens.com> wrote:
>
>> On 2012-02-15 03:54, Wen Congyang wrote:
>>> At 02/15/2012 12:19 AM, Jan Kiszka Wrote:
>>>> On 2012-02-09 04:19, Wen Congyang wrote:
>>>>> Sync command needs these two APIs to suspend/resume monitor.
>>>>>
>>>>> Signed-off-by: Wen Congyang <we...@cn.fujitsu.com>
>>>>> ---
>>>>> monitor.c | 27 +++++++++++++++++++++++++++
>>>>> monitor.h | 2 ++
>>>>> 2 files changed, 29 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/monitor.c b/monitor.c
>>>>> index 11639b1..7e72739 100644
>>>>> --- a/monitor.c
>>>>> +++ b/monitor.c
>>>>> @@ -4442,6 +4442,26 @@ static void monitor_command_cb(Monitor *mon, const
>>>>> char *cmdline, void *opaque)
>>>>> monitor_resume(mon);
>>>>> }
>>>>>
>>>>> +int qemu_suspend_monitor(const char *fmt, ...)
>>>>> +{
>>>>> + int ret;
>>>>> +
>>>>> + if (cur_mon) {
>>>>> + ret = monitor_suspend(cur_mon);
>>>>> + } else {
>>>>> + ret = -ENOTTY;
>>>>> + }
>>>>> +
>>>>> + if (ret < 0 && fmt) {
>>>>> + va_list ap;
>>>>> + va_start(ap, fmt);
>>>>> + monitor_vprintf(cur_mon, fmt, ap);
>>>>> + va_end(ap);
>>>>> + }
>>>>> +
>>>>> + return ret;
>>>>> +}
>>>>> +
>>>>> int monitor_suspend(Monitor *mon)
>>>>> {
>>>>> if (!mon->rs)
>>>>> @@ -4450,6 +4470,13 @@ int monitor_suspend(Monitor *mon)
>>>>> return 0;
>>>>> }
>>>>>
>>>>> +void qemu_resume_monitor(void)
>>>>> +{
>>>>> + if (cur_mon) {
>>>>> + monitor_resume(cur_mon);
>>>>> + }
>>>>> +}
>>>>> +
>>>>> void monitor_resume(Monitor *mon)
>>>>> {
>>>>> if (!mon->rs)
>>>>> diff --git a/monitor.h b/monitor.h
>>>>> index 58109af..60a1e17 100644
>>>>> --- a/monitor.h
>>>>> +++ b/monitor.h
>>>>> @@ -46,7 +46,9 @@ int monitor_cur_is_qmp(void);
>>>>> void monitor_protocol_event(MonitorEvent event, QObject *data);
>>>>> void monitor_init(CharDriverState *chr, int flags);
>>>>>
>>>>> +int qemu_suspend_monitor(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>>>>> int monitor_suspend(Monitor *mon);
>>>>> +void qemu_resume_monitor(void);
>>>>> void monitor_resume(Monitor *mon);
>>>>>
>>>>> int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs,
>>>>
>>>> I don't see any added value in this API, specifically as it is built on
>>>> top of cur_mon. Just use the existing services like the migration code
>>>> does. If you properly pass down the monitor reference from the command
>>>> to the suspend and store what monitor you suspended, all should be fine.
>>>
>>> This API is like qemu_get_fd() which is not merged into upstream qemu.
>>> I need this API because I cannot use monitor in qapi command.
>>
>> OK, then I need to comment on that approach. QMP looks flawed here.
>> Either you have a need for a Monitor object (or a generic HMP/QMP
>> context), then you also have a handle. Or your don't, then you do not
>> need monitor suspend/resume or get_fd as well.
>
> The getfd one is explained in the other thread, but suspend/resume should
> be done from HMP only.
I have read the newest migration code. I will change the code like that and
remove these two APIs.
Please ignore this patch
Thanks
Wen Congyang
>
> PS: Haven't reviewed this series yet.
>
