Patchwork [09/18] QMP: Introduce command dispatch table

login
register
mail settings
Submitter Luiz Capitulino
Date Sept. 16, 2010, 8:20 p.m.
Message ID <1284668464-15981-10-git-send-email-lcapitulino@redhat.com>
Download mbox | patch
Permalink /patch/65008/
State New
Headers show

Comments

Luiz Capitulino - Sept. 16, 2010, 8:20 p.m.
Also update QMP functions to use it. The table is generated
from the qemu-monitor-qmp.hx file.

From now on, QMP and HMP have different command dispatch
tables.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 Makefile.target |    7 +++++--
 monitor.c       |   11 +++++++++--
 2 files changed, 14 insertions(+), 4 deletions(-)
Markus Armbruster - Sept. 29, 2010, 11:31 a.m.
Luiz Capitulino <lcapitulino@redhat.com> writes:

> Also update QMP functions to use it. The table is generated
> from the qemu-monitor-qmp.hx file.
>
>>From now on, QMP and HMP have different command dispatch
> tables.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  Makefile.target |    7 +++++--
>  monitor.c       |   11 +++++++++--
>  2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile.target b/Makefile.target
> index a4e80b1..7fab81b 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -299,7 +299,7 @@ obj-alpha-y = alpha_palcode.o
>  
>  main.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
>  
> -monitor.o: qemu-monitor.h
> +monitor.o: qemu-monitor.h qemu-monitor-qmp.h
>  
>  $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y): $(GENERATED_HEADERS)
>  
> @@ -323,10 +323,13 @@ gdbstub-xml.c: $(TARGET_XML_FILES) $(SRC_PATH)/feature_to_c.sh
>  qemu-monitor.h: $(SRC_PATH)/qemu-monitor.hx
>  	$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@,"  GEN   $(TARGET_DIR)$@")
>  
> +qemu-monitor-qmp.h: $(SRC_PATH)/qemu-monitor-qmp.hx
> +	$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@,"  GEN   $(TARGET_DIR)$@")
> +
>  clean:
>  	rm -f *.o *.a *~ $(PROGS) nwfpe/*.o fpu/*.o
>  	rm -f *.d */*.d tcg/*.o ide/*.o
> -	rm -f qemu-monitor.h gdbstub-xml.c
> +	rm -f qemu-monitor.h qmp-monitor-qmp.h gdbstub-xml.c

Typo: qmp-monitor-qmp.h instead of qemu-monitor-qmp.h.

I'm not thrilled about qemu-monitor-qmp; because if I expand the
abbreviation, I get "QEMU monitor QEMU Monitor Protocol".

What about just qmp?

[...]
>  install: all
>  ifneq ($(PROGS),)
> diff --git a/monitor.c b/monitor.c
> index 7b699db..fe1a29a 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -189,6 +189,8 @@ static QLIST_HEAD(mon_list, Monitor) mon_list;
>  static const mon_cmd_t mon_cmds[];
>  static const mon_cmd_t info_cmds[];
>  
> +static const mon_cmd_t qmp_cmds[];
> +
>  Monitor *cur_mon;
>  Monitor *default_mon;
>  
> @@ -742,7 +744,7 @@ static void do_info_commands(Monitor *mon, QObject **ret_data)
>  
>      cmd_list = qlist_new();
>  
> -    for (cmd = mon_cmds; cmd->name != NULL; cmd++) {
> +    for (cmd = qmp_cmds; cmd->name != NULL; cmd++) {
>          if (monitor_handler_ported(cmd) && !monitor_cmd_user_only(cmd) &&
>              !compare_cmd(cmd->name, "info")) {
>              qlist_append_obj(cmd_list, get_cmd_dict(cmd->name));

We need to switch here, because "info commands" is about QMP commands,
not about HMP commands.  Curious, isn't it?

> @@ -2632,6 +2634,11 @@ static const mon_cmd_t info_cmds[] = {
>      },
>  };
>  
> +static const mon_cmd_t qmp_cmds[] = {
> +#include "qemu-monitor-qmp.h"
> +    { /* NULL */ },
> +};
> +
>  /*******************************************************************/
>  
>  static const char *pch;
> @@ -3364,7 +3371,7 @@ static const mon_cmd_t *qmp_find_query_cmd(const char *info_item)
>  
>  static const mon_cmd_t *qmp_find_cmd(const char *cmdname)
>  {
> -    return find_dispatch_table(mon_cmds, cmdname);
> +    return find_dispatch_table(qmp_cmds, cmdname);
>  }
>  
>  static const mon_cmd_t *monitor_parse_command(Monitor *mon,
Luiz Capitulino - Sept. 29, 2010, 1:47 p.m.
On Wed, 29 Sep 2010 13:31:13 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > Also update QMP functions to use it. The table is generated
> > from the qemu-monitor-qmp.hx file.
> >
> >>From now on, QMP and HMP have different command dispatch
> > tables.
> >
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  Makefile.target |    7 +++++--
> >  monitor.c       |   11 +++++++++--
> >  2 files changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/Makefile.target b/Makefile.target
> > index a4e80b1..7fab81b 100644
> > --- a/Makefile.target
> > +++ b/Makefile.target
> > @@ -299,7 +299,7 @@ obj-alpha-y = alpha_palcode.o
> >  
> >  main.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
> >  
> > -monitor.o: qemu-monitor.h
> > +monitor.o: qemu-monitor.h qemu-monitor-qmp.h
> >  
> >  $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y): $(GENERATED_HEADERS)
> >  
> > @@ -323,10 +323,13 @@ gdbstub-xml.c: $(TARGET_XML_FILES) $(SRC_PATH)/feature_to_c.sh
> >  qemu-monitor.h: $(SRC_PATH)/qemu-monitor.hx
> >  	$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@,"  GEN   $(TARGET_DIR)$@")
> >  
> > +qemu-monitor-qmp.h: $(SRC_PATH)/qemu-monitor-qmp.hx
> > +	$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@,"  GEN   $(TARGET_DIR)$@")
> > +
> >  clean:
> >  	rm -f *.o *.a *~ $(PROGS) nwfpe/*.o fpu/*.o
> >  	rm -f *.d */*.d tcg/*.o ide/*.o
> > -	rm -f qemu-monitor.h gdbstub-xml.c
> > +	rm -f qemu-monitor.h qmp-monitor-qmp.h gdbstub-xml.c
> 
> Typo: qmp-monitor-qmp.h instead of qemu-monitor-qmp.h.
> 
> I'm not thrilled about qemu-monitor-qmp; because if I expand the
> abbreviation, I get "QEMU monitor QEMU Monitor Protocol".
> 
> What about just qmp?

I think qmp-commands is better, but then I'll also change qemu-monitor.hx to
hmp-commands.hx.

Objections?

> [...]
> >  install: all
> >  ifneq ($(PROGS),)
> > diff --git a/monitor.c b/monitor.c
> > index 7b699db..fe1a29a 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -189,6 +189,8 @@ static QLIST_HEAD(mon_list, Monitor) mon_list;
> >  static const mon_cmd_t mon_cmds[];
> >  static const mon_cmd_t info_cmds[];
> >  
> > +static const mon_cmd_t qmp_cmds[];
> > +
> >  Monitor *cur_mon;
> >  Monitor *default_mon;
> >  
> > @@ -742,7 +744,7 @@ static void do_info_commands(Monitor *mon, QObject **ret_data)
> >  
> >      cmd_list = qlist_new();
> >  
> > -    for (cmd = mon_cmds; cmd->name != NULL; cmd++) {
> > +    for (cmd = qmp_cmds; cmd->name != NULL; cmd++) {
> >          if (monitor_handler_ported(cmd) && !monitor_cmd_user_only(cmd) &&
> >              !compare_cmd(cmd->name, "info")) {
> >              qlist_append_obj(cmd_list, get_cmd_dict(cmd->name));
> 
> We need to switch here, because "info commands" is about QMP commands,
> not about HMP commands.  Curious, isn't it?

Drop the compare_cmd() and change the function's name, you mean?

> > @@ -2632,6 +2634,11 @@ static const mon_cmd_t info_cmds[] = {
> >      },
> >  };
> >  
> > +static const mon_cmd_t qmp_cmds[] = {
> > +#include "qemu-monitor-qmp.h"
> > +    { /* NULL */ },
> > +};
> > +
> >  /*******************************************************************/
> >  
> >  static const char *pch;
> > @@ -3364,7 +3371,7 @@ static const mon_cmd_t *qmp_find_query_cmd(const char *info_item)
> >  
> >  static const mon_cmd_t *qmp_find_cmd(const char *cmdname)
> >  {
> > -    return find_dispatch_table(mon_cmds, cmdname);
> > +    return find_dispatch_table(qmp_cmds, cmdname);
> >  }
> >  
> >  static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>
Markus Armbruster - Sept. 29, 2010, 2:23 p.m.
Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Wed, 29 Sep 2010 13:31:13 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Luiz Capitulino <lcapitulino@redhat.com> writes:
>> 
>> > Also update QMP functions to use it. The table is generated
>> > from the qemu-monitor-qmp.hx file.
>> >
>> >>From now on, QMP and HMP have different command dispatch
>> > tables.
>> >
>> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
>> > ---
>> >  Makefile.target |    7 +++++--
>> >  monitor.c       |   11 +++++++++--
>> >  2 files changed, 14 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/Makefile.target b/Makefile.target
>> > index a4e80b1..7fab81b 100644
>> > --- a/Makefile.target
>> > +++ b/Makefile.target
>> > @@ -299,7 +299,7 @@ obj-alpha-y = alpha_palcode.o
>> >  
>> >  main.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
>> >  
>> > -monitor.o: qemu-monitor.h
>> > +monitor.o: qemu-monitor.h qemu-monitor-qmp.h
>> >  
>> >  $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y): $(GENERATED_HEADERS)
>> >  
>> > @@ -323,10 +323,13 @@ gdbstub-xml.c: $(TARGET_XML_FILES) $(SRC_PATH)/feature_to_c.sh
>> >  qemu-monitor.h: $(SRC_PATH)/qemu-monitor.hx
>> >  	$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@,"  GEN   $(TARGET_DIR)$@")
>> >  
>> > +qemu-monitor-qmp.h: $(SRC_PATH)/qemu-monitor-qmp.hx
>> > +	$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@,"  GEN   $(TARGET_DIR)$@")
>> > +
>> >  clean:
>> >  	rm -f *.o *.a *~ $(PROGS) nwfpe/*.o fpu/*.o
>> >  	rm -f *.d */*.d tcg/*.o ide/*.o
>> > -	rm -f qemu-monitor.h gdbstub-xml.c
>> > +	rm -f qemu-monitor.h qmp-monitor-qmp.h gdbstub-xml.c
>> 
>> Typo: qmp-monitor-qmp.h instead of qemu-monitor-qmp.h.
>> 
>> I'm not thrilled about qemu-monitor-qmp; because if I expand the
>> abbreviation, I get "QEMU monitor QEMU Monitor Protocol".
>> 
>> What about just qmp?
>
> I think qmp-commands is better, but then I'll also change qemu-monitor.hx to
> hmp-commands.hx.
>
> Objections?

Fine with me.

>> [...]
>> >  install: all
>> >  ifneq ($(PROGS),)
>> > diff --git a/monitor.c b/monitor.c
>> > index 7b699db..fe1a29a 100644
>> > --- a/monitor.c
>> > +++ b/monitor.c
>> > @@ -189,6 +189,8 @@ static QLIST_HEAD(mon_list, Monitor) mon_list;
>> >  static const mon_cmd_t mon_cmds[];
>> >  static const mon_cmd_t info_cmds[];
>> >  
>> > +static const mon_cmd_t qmp_cmds[];
>> > +
>> >  Monitor *cur_mon;
>> >  Monitor *default_mon;
>> >  
>> > @@ -742,7 +744,7 @@ static void do_info_commands(Monitor *mon, QObject **ret_data)
>> >  
>> >      cmd_list = qlist_new();
>> >  
>> > -    for (cmd = mon_cmds; cmd->name != NULL; cmd++) {
>> > +    for (cmd = qmp_cmds; cmd->name != NULL; cmd++) {
>> >          if (monitor_handler_ported(cmd) && !monitor_cmd_user_only(cmd) &&
>> >              !compare_cmd(cmd->name, "info")) {
>> >              qlist_append_obj(cmd_list, get_cmd_dict(cmd->name));
>> 
>> We need to switch here, because "info commands" is about QMP commands,
>> not about HMP commands.  Curious, isn't it?
>
> Drop the compare_cmd() and change the function's name, you mean?

No.  The reason for this change wasn't obvious to me, so I wrote it
down, in case it's not obvious to others, either.  I didn't mean to
suggest you change anything here.

[...]

Patch

diff --git a/Makefile.target b/Makefile.target
index a4e80b1..7fab81b 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -299,7 +299,7 @@  obj-alpha-y = alpha_palcode.o
 
 main.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
 
-monitor.o: qemu-monitor.h
+monitor.o: qemu-monitor.h qemu-monitor-qmp.h
 
 $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y): $(GENERATED_HEADERS)
 
@@ -323,10 +323,13 @@  gdbstub-xml.c: $(TARGET_XML_FILES) $(SRC_PATH)/feature_to_c.sh
 qemu-monitor.h: $(SRC_PATH)/qemu-monitor.hx
 	$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@,"  GEN   $(TARGET_DIR)$@")
 
+qemu-monitor-qmp.h: $(SRC_PATH)/qemu-monitor-qmp.hx
+	$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@,"  GEN   $(TARGET_DIR)$@")
+
 clean:
 	rm -f *.o *.a *~ $(PROGS) nwfpe/*.o fpu/*.o
 	rm -f *.d */*.d tcg/*.o ide/*.o
-	rm -f qemu-monitor.h gdbstub-xml.c
+	rm -f qemu-monitor.h qmp-monitor-qmp.h gdbstub-xml.c
 
 install: all
 ifneq ($(PROGS),)
diff --git a/monitor.c b/monitor.c
index 7b699db..fe1a29a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -189,6 +189,8 @@  static QLIST_HEAD(mon_list, Monitor) mon_list;
 static const mon_cmd_t mon_cmds[];
 static const mon_cmd_t info_cmds[];
 
+static const mon_cmd_t qmp_cmds[];
+
 Monitor *cur_mon;
 Monitor *default_mon;
 
@@ -742,7 +744,7 @@  static void do_info_commands(Monitor *mon, QObject **ret_data)
 
     cmd_list = qlist_new();
 
-    for (cmd = mon_cmds; cmd->name != NULL; cmd++) {
+    for (cmd = qmp_cmds; cmd->name != NULL; cmd++) {
         if (monitor_handler_ported(cmd) && !monitor_cmd_user_only(cmd) &&
             !compare_cmd(cmd->name, "info")) {
             qlist_append_obj(cmd_list, get_cmd_dict(cmd->name));
@@ -2632,6 +2634,11 @@  static const mon_cmd_t info_cmds[] = {
     },
 };
 
+static const mon_cmd_t qmp_cmds[] = {
+#include "qemu-monitor-qmp.h"
+    { /* NULL */ },
+};
+
 /*******************************************************************/
 
 static const char *pch;
@@ -3364,7 +3371,7 @@  static const mon_cmd_t *qmp_find_query_cmd(const char *info_item)
 
 static const mon_cmd_t *qmp_find_cmd(const char *cmdname)
 {
-    return find_dispatch_table(mon_cmds, cmdname);
+    return find_dispatch_table(qmp_cmds, cmdname);
 }
 
 static const mon_cmd_t *monitor_parse_command(Monitor *mon,