Re: [Qemu-devel] [RFC][PATCH 01/16 v6] monitor: introduce qemu_suspend_monitor()/qemu_resume_monitor()

[Jan Kiszka]

On 2012-02-09 04:19, Wen Congyang wrote:
> Sync command needs these two APIs to suspend/resume monitor.
> 
> Signed-off-by: Wen Congyang <address@hidden>
> ---
>  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.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux