[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 03/11] QMP: Introduce protocol print functions
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH 03/11] QMP: Introduce protocol print functions |
Date: |
Tue, 23 Jun 2009 10:53:53 -0300 |
User-agent: |
Mutt/1.5.18 (2008-05-17) |
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 <address@hidden>
>> ---
>> 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