From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:49843) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QzWew-0005vB-Ui for qemu-devel@nongnu.org; Fri, 02 Sep 2011 12:32:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QzWet-0005AI-E3 for qemu-devel@nongnu.org; Fri, 02 Sep 2011 12:32:02 -0400 Received: from e31.co.us.ibm.com ([32.97.110.149]:46326) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QzWes-00058C-RZ for qemu-devel@nongnu.org; Fri, 02 Sep 2011 12:31:59 -0400 Received: from /spool/local by us.ibm.com with XMail ESMTP for from ; Fri, 2 Sep 2011 10:31:56 -0600 Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by d03relay03.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p82GVpdj163064 for ; Fri, 2 Sep 2011 10:31:52 -0600 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p82AVOQU001182 for ; Fri, 2 Sep 2011 04:31:24 -0600 Message-ID: <4E6104F5.6010202@linux.vnet.ibm.com> Date: Fri, 02 Sep 2011 11:31:49 -0500 From: Michael Roth MIME-Version: 1.0 References: <1314211389-28915-1-git-send-email-aliguori@us.ibm.com> <1314211389-28915-4-git-send-email-aliguori@us.ibm.com> <4E567724.7020601@linux.vnet.ibm.com> <4E60FDB2.7060708@codemonkey.ws> <20110902130931.50b0f63a@doriath> In-Reply-To: <20110902130931.50b0f63a@doriath> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 03/14] qapi: use middle mode in QMP server List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Kevin Wolf , Anthony Liguori , qemu-devel@nongnu.org On 09/02/2011 11:09 AM, Luiz Capitulino wrote: > On Fri, 02 Sep 2011 11:00:50 -0500 > Anthony Liguori 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 >>>> --- >>>> 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. Same. Curious what the complications are though, I wouldn't mind looking at it.