From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:52545) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RxaaC-0006eu-Kk for qemu-devel@nongnu.org; Wed, 15 Feb 2012 03:51:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RxaaB-0007X7-1b for qemu-devel@nongnu.org; Wed, 15 Feb 2012 03:51:24 -0500 Received: from thoth.sbs.de ([192.35.17.2]:29458) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RxaaA-0007Wv-KD for qemu-devel@nongnu.org; Wed, 15 Feb 2012 03:51:23 -0500 Message-ID: <4F3B71F8.1080602@siemens.com> Date: Wed, 15 Feb 2012 09:51:04 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <4F333AAA.1070601@cn.fujitsu.com> <4F333B26.5050502@cn.fujitsu.com> <4F3A897B.5060709@siemens.com> <4F3B1E5B.2050205@cn.fujitsu.com> In-Reply-To: <4F3B1E5B.2050205@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: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wen Congyang Cc: Eric Blake , HATAYAMA Daisuke , Dave Anderson , qemu-devel , Luiz Capitulino 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 >>> --- >>> 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. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux