From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:50757)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jan.kiszka@siemens.com>) id 1RxL6I-0001ic-NT
	for qemu-devel@nongnu.org; Tue, 14 Feb 2012 11:19:36 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <jan.kiszka@siemens.com>) id 1RxL6D-0002xU-OZ
	for qemu-devel@nongnu.org; Tue, 14 Feb 2012 11:19:30 -0500
Received: from thoth.sbs.de ([192.35.17.2]:20601)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jan.kiszka@siemens.com>) id 1RxL6D-0002x1-E4
	for qemu-devel@nongnu.org; Tue, 14 Feb 2012 11:19:25 -0500
Message-ID: <4F3A897B.5060709@siemens.com>
Date: Tue, 14 Feb 2012 17:19:07 +0100
From: Jan Kiszka <jan.kiszka@siemens.com>
MIME-Version: 1.0
References: <4F333AAA.1070601@cn.fujitsu.com> <4F333B26.5050502@cn.fujitsu.com>
In-Reply-To: <4F333B26.5050502@cn.fujitsu.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [RFC][PATCH 01/16 v6] monitor: introduce
	qemu_suspend_monitor()/qemu_resume_monitor()
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Wen Congyang <wency@cn.fujitsu.com>
Cc: Eric Blake <eblake@redhat.com>, HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>, Dave Anderson <anderson@redhat.com>, qemu-devel <qemu-devel@nongnu.org>, Luiz Capitulino <lcapitulino@redhat.com>

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

Jan

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