Re: [Qemu-devel] [PATCH 03/14] qapi: use middle mode in QMP server

[Anthony Liguori]

On 08/24/2011 03:20 PM, Luiz Capitulino wrote:
> On Wed, 24 Aug 2011 13:42:58 -0500
> Anthony Liguori<address@hidden>  wrote:
>
> > Use the new middle mode within the existing QMP server.
> >
> > Signed-off-by: Anthony Liguori<address@hidden>
> > ---
> >   Makefile         |   11 +++++++++++
> >   Makefile.objs    |    2 ++
> >   Makefile.target  |    6 +++---
> >   monitor.c        |   11 ++++++++---
> >   qapi-schema.json |    3 +++
> >   5 files changed, 27 insertions(+), 6 deletions(-)
> >   create mode 100644 qapi-schema.json
> >
> > diff --git a/Makefile b/Makefile
> > index 8606849..23ee7e0 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -184,9 +184,20 @@ $(qapi-dir)/qga-qapi-types.h: 
> > $(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scr
> >   $(qapi-dir)/qga-qapi-visit.c: $(qapi-dir)/qga-qapi-visit.h
> >   $(qapi-dir)/qga-qapi-visit.h: $(SRC_PATH)/qapi-schema-guest.json 
> > $(SRC_PATH)/scripts/qapi-visit.py
> >         $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py -o "$(qapi-dir)" -p 
> > "qga-"<  $<, "  GEN   $@")
> > +$(qapi-dir)/qga-qmp-commands.h: $(qapi-dir)/qga-qmp-marshal.c
> >   $(qapi-dir)/qga-qmp-marshal.c: $(SRC_PATH)/qapi-schema-guest.json 
> > $(SRC_PATH)/scripts/qapi-commands.py
> >         $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py -o "$(qapi-dir)" -p 
> > "qga-"<  $<, "  GEN   $@")
> >
> > +qapi-types.c: qapi-types.h
> > +qapi-types.h: $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py
> > +       $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py -o "."<  $<, 
> > "  GEN   $@")
> > +qapi-visit.c: qapi-visit.h
> > +qapi-visit.h: $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py
> > +       $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py -o "."<  $<, 
> > "  GEN   $@")
> > +qmp-commands.h: qmp-marshal.c
> > +qmp-marshal.c: $(SRC_PATH)/qapi-schema.json 
> > $(SRC_PATH)/scripts/qapi-commands.py
> > +       $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py -m -o "."<  
> > $<, "  GEN   $@")
> > +
> >   test-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h 
> > test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y)
> >   test-visitor: test-visitor.o qfloat.o qint.o qdict.o qstring.o qlist.o 
> > qbool.o $(qapi-obj-y) error.o osdep.o $(oslib-obj-y) qjson.o json-streamer.o 
> > json-lexer.o json-parser.o qerror.o qemu-error.o qemu-tool.o 
> > $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o
> >
> > diff --git a/Makefile.objs b/Makefile.objs
> > index d1f3e5d..c02431f 100644
> > --- a/Makefile.objs
> > +++ b/Makefile.objs
> > @@ -396,6 +396,8 @@ qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o 
> > qmp-output-visitor.o qapi-
> >   qapi-nested-y += qmp-registry.o qmp-dispatch.o
> >   qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
> >
> > +common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o $(qapi-obj-y)
> > +
> >   ######################################################################
> >   # guest agent
> >
> > diff --git a/Makefile.target b/Makefile.target
> > index e280bf6..2cd0ec5 100644
> > --- a/Makefile.target
> > +++ b/Makefile.target
> > @@ -375,7 +375,7 @@ obj-alpha-y += vga.o cirrus_vga.o
> >
> >   main.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
> >
> > -monitor.o: hmp-commands.h qmp-commands.h
> > +monitor.o: hmp-commands.h qmp-commands-old.h
> >
> >   $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y): $(GENERATED_HEADERS)
> >
> > @@ -405,13 +405,13 @@ gdbstub-xml.c: $(TARGET_XML_FILES) 
> > $(SRC_PATH)/scripts/feature_to_c.sh
> >   hmp-commands.h: $(SRC_PATH)/hmp-commands.hx
> >         $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h<  $<  >  $@,"  GEN   
> > $(TARGET_DIR)$@")
> >
> > -qmp-commands.h: $(SRC_PATH)/qmp-commands.hx
> > +qmp-commands-old.h: $(SRC_PATH)/qmp-commands.hx
> >         $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h<  $<  >  $@,"  GEN   
> > $(TARGET_DIR)$@")
> >
> >   clean:
> >         rm -f *.o *.a *~ $(PROGS) nwfpe/*.o fpu/*.o
> >         rm -f *.d */*.d tcg/*.o ide/*.o 9pfs/*.o
> > -       rm -f hmp-commands.h qmp-commands.h gdbstub-xml.c
> > +       rm -f hmp-commands.h qmp-commands-old.h gdbstub-xml.c
> >   ifdef CONFIG_SYSTEMTAP_TRACE
> >         rm -f *.stp
> >   endif
> > diff --git a/monitor.c b/monitor.c
> > index ada51d0..ef204c0 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -119,6 +119,7 @@ typedef struct mon_cmd_t {
> >           int  (*cmd_async)(Monitor *mon, const QDict *params,
> >                             MonitorCompletion *cb, void *opaque);
> >       } mhandler;
> > +    bool qapi;
> >       int flags;
> >   } mon_cmd_t;
> >
> > @@ -3157,7 +3158,7 @@ static const mon_cmd_t info_cmds[] = {
> >   };
> >
> >   static const mon_cmd_t qmp_cmds[] = {
> > -#include "qmp-commands.h"
> > +#include "qmp-commands-old.h"
> >       { /* NULL */ },
> >   };
> >
> > @@ -5027,10 +5028,14 @@ static void qmp_call_query_cmd(Monitor *mon, const 
> > mon_cmd_t *cmd)
> >           if (monitor_has_error(mon)) {
> >               monitor_protocol_emitter(mon, NULL);
> >           }
> > -    } else {
> > -        cmd->mhandler.info_new(mon,&ret_data);
>
> This breaks query- commands not converted to the QAPI yet.

Doh!  Good catch.

> > +    } else if (cmd->qapi) {
> > +        QDict *args = qdict_new();
> > +
> > +        cmd->mhandler.cmd_new(mon, args,&ret_data);
> >           monitor_protocol_emitter(mon, ret_data);
> >           qobject_decref(ret_data);
> > +
> > +        QDECREF(args);
> >       }
>
> I think it would also be a good idea to start moving query- commands from
> qmp_query_cmds[] to qmp_cmds[]. So that we have only one dispatch table.
>
> If we do this, we don't need the hunk above. We need this:
>
> diff --git a/monitor.c b/monitor.c
> index 6a3a3d2..01e141c 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -5070,12 +5070,10 @@ static void handle_qmp_command(JSONMessageParser 
> *parser, QList *tokens)
>           goto err_out;
>       }
>
> -    if (strstart(cmd_name, "query-",&query_cmd)) {
> +    cmd = qmp_find_cmd(cmd_name);
> +    if (!cmd&&  strstart(cmd_name, "query-",&query_cmd)) {
>           cmd = qmp_find_query_cmd(query_cmd);
> -    } else {
> -        cmd = qmp_find_cmd(cmd_name);
>       }
> -
>       if (!cmd) {
>           qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
>           goto err_out;
>
> and name the command appropriately in the qmp-commands.hx struct, like
> "query-name".

Agreed.  I'll update for v2.

Regards,

Anthony Liguori

> >   }
> >
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > new file mode 100644
> > index 0000000..7fcefdb
> > --- /dev/null
> > +++ b/qapi-schema.json
> > @@ -0,0 +1,3 @@
> > +# -*- Mode: Python -*-
> > +#
> > +# QAPI Schema