From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MJ6Rs-0001tQ-OZ for qemu-devel@nongnu.org; Tue, 23 Jun 2009 09:54:08 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MJ6Rn-0001or-AM for qemu-devel@nongnu.org; Tue, 23 Jun 2009 09:54:08 -0400 Received: from [199.232.76.173] (port=39307 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MJ6Rn-0001of-4Y for qemu-devel@nongnu.org; Tue, 23 Jun 2009 09:54:03 -0400 Received: from mx2.redhat.com ([66.187.237.31]:45381) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MJ6Rm-0006zU-IX for qemu-devel@nongnu.org; Tue, 23 Jun 2009 09:54:02 -0400 Date: Tue, 23 Jun 2009 10:53:53 -0300 From: Eduardo Habkost Subject: Re: [Qemu-devel] [PATCH 03/11] QMP: Introduce protocol print functions Message-ID: <20090623135353.GD6462@blackpad> References: <20090623012838.764dd37a@doriath> <4A40DC62.9030407@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4A40DC62.9030407@codemonkey.ws> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: aliguori@us.ibm.com, jan.kiszka@siemens.com, dlaor@redhat.com, qemu-devel@nongnu.org, Luiz Capitulino , avi@redhat.com On Tue, Jun 23, 2009 at 08:45:06AM -0500, Anthony Liguori wrote: > Luiz Capitulino wrote: >> This change introduces the functions that will be used by the Monitor >> to output data in the format defined by the protocol specification. >> >> There are four functions: >> >> o monitor_printf_bad() signals a protocol error >> o monitor_print_ok() signals successful execution >> o monitor_printf_err() used by commands, to signal execution errors >> o monitor_printf_data() used by commands, to output data >> >> For now monitor_print_ok() compilation is disabled to avoid breaking >> the build as the function is not currently used. It will be enabled >> by a later commit. >> >> Signed-off-by: Luiz Capitulino >> --- >> monitor.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> monitor.h | 3 ++ >> qemu-tool.c | 12 +++++++++++ >> 3 files changed, 78 insertions(+), 0 deletions(-) >> >> diff --git a/monitor.c b/monitor.c >> index 514db00..dfa777d 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -199,6 +199,69 @@ static int monitor_fprintf(FILE *stream, const char *fmt, ...) >> return 0; >> } >> +/* >> + * QEMU Monitor Control print functions >> + */ >> + >> +/* Protocol errors */ >> +void monitor_printf_bad(Monitor *mon, const char *fmt, ...) >> +{ >> + if (monitor_ctrl_mode(mon)) { >> + va_list ap; >> + >> + monitor_puts(mon, "- BAD "); >> + >> + va_start(ap, fmt); >> + monitor_vprintf(mon, fmt, ap); >> + va_end(ap); >> + } >> +} >> + >> +#if 0 >> +/* OK command completion, 'info' commands are special */ >> +static void monitor_print_ok(Monitor *mon, const char *cmd, const char *arg) >> +{ >> + if (!monitor_ctrl_mode(mon)) >> + return; >> + >> + monitor_printf(mon, "+ OK %s", cmd); >> + if (!strcmp(cmd, "info")) >> + monitor_printf(mon, " %s", arg); >> + monitor_puts(mon, " completed\n"); >> +} >> +#endif >> + >> +static void monitor_ctrl_pprintf(Monitor *mon, const char *prefix, >> + const char *fmt, va_list ap) >> +{ >> + if (monitor_ctrl_mode(mon)) >> + monitor_puts(mon, prefix); >> + >> + monitor_vprintf(mon, fmt, ap); >> +} >> + >> +/* Should be used by commands to signal errors, will add the prefix >> + * '- ERR ' if in control mode */ >> +void monitor_printf_err(Monitor *mon, const char *fmt, ...) >> +{ >> + va_list ap; >> + >> + va_start(ap, fmt); >> + monitor_ctrl_pprintf(mon, "- ERR ", fmt, ap); >> + va_end(ap); >> +} >> + >> +/* Should be used by commands to print any output which is not >> + * an error, will add the prefix '= ' if in control mode */ >> +void monitor_printf_data(Monitor *mon, const char *fmt, ...) >> +{ >> + va_list ap; >> + >> + va_start(ap, fmt); >> + monitor_ctrl_pprintf(mon, "= ", fmt, ap); >> + va_end(ap); >> +} >> + >> static int compare_cmd(const char *name, const char *list) >> { >> const char *p, *pstart; >> diff --git a/monitor.h b/monitor.h >> index 48bc056..8b054eb 100644 >> --- a/monitor.h >> +++ b/monitor.h >> @@ -24,6 +24,9 @@ void monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs, >> void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap); >> void monitor_printf(Monitor *mon, const char *fmt, ...) >> __attribute__ ((__format__ (__printf__, 2, 3))); >> +void monitor_printf_bad(Monitor *mon, const char *fmt, ...); >> +void monitor_printf_err(Monitor *mon, const char *fmt, ...); >> +void monitor_printf_data(Monitor *mon, const char *fmt, ...); >> > > Probably could just introduce a flag to monitor_printf(). In fact, you > could go kernel style and have: > > monitor_printf(mon, MON_BAD "bad monitor command\n"); > monitor_printf(mon, MON_DATA "some monitor data\n"); > > With MON_DATA being explicit. > > If not, make sure to add the format attribute to the printfs. I think different functions make sense because some messages may have additional attributes (not only a text message), that may be arguments to the function. e.g.: void monitor_printf_event(Monitor *mon, qemu_event_t event, const char *fmt, ...); void monitor_printf_err(Monitor *mon, int status_code, const char *fmt, ...); (or maybe we can just call it monitor_printf_result(), being applicable to both success and failure, depending on the status code). -- Eduardo