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

[Luiz Capitulino]

On Fri, 02 Sep 2011 11:00:50 -0500
Anthony Liguori <address@hidden> wrote:

> On 08/25/2011 11:24 AM, Michael Roth wrote:
> > On 08/24/2011 01:42 PM, Anthony Liguori 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);
> >> + } 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);
> >> }
> >> }
> >>
> >
> > If we instead use the new dispatch stuff in qapi/qmp-dispatch.c, and use
> > that dispatch table as a fallback for qmp_find_command(), I think we
> > could drop most of the code generator additions in patch #2.
> >
> > We'd also be able to completely remove any old-style definitions in
> > qmp-commands.hx, so once the conversion was complete we'd basically have
> > an empty file.
> >
> > The only downside I can think of would be that documentation for
> > commands would be split into 2 locations until the conversion was complete.
> 
> This ended up being a bit harder than I initially thought so I'd prefer 
> to delay this to another series so we can start converting commands to 
> QAPI ASAP.

Fine with me.