qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH V3 0/4] HMP: allow parsing for sub command
@ 2012-12-27  9:40 Wenchao Xia
  2012-12-27  9:40 ` [Qemu-devel] [PATCH V3 1/4] HMP: add QDict to info callback handler Wenchao Xia
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Wenchao Xia @ 2012-12-27  9:40 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, chenwj, armbru, lcapitulino, pbonzini, Wenchao Xia

  These patches enhance HMP to allow it parse 2nd level of commands, such
as info sub command list, which means now foldered command with parameter
is possible.

V2:
  Follow the way supposed by Markus, which make the infrastructure knows
there is possible a 2nd level of command exist, instead of a hack. In this
way extention of command folder level is easy.
  Moved some code and better doc according to comments.
  Removed the patch about info snapshots, which will goto another serial.
V3:
  Move changelog position.
  Split out code moving patch.

Wenchao Xia (4):
  HMP: add QDict to info callback handler
  HMP: add infrastructure for sub command
  HMP: move define of mon_cmds
  HMP: add sub command table to info

 hmp-commands.hx         |    3 +-
 hmp.c                   |   36 ++++++++--------
 hmp.h                   |   36 ++++++++--------
 hw/i8259.c              |    4 +-
 hw/lm32_pic.c           |    4 +-
 hw/lm32_pic.h           |    4 +-
 hw/loader.c             |    2 +-
 hw/loader.h             |    3 +-
 hw/pc.h                 |    4 +-
 hw/pcmcia.h             |    2 +-
 hw/qdev-monitor.c       |    4 +-
 hw/qdev-monitor.h       |    4 +-
 hw/sun4m.c              |    4 +-
 hw/sun4m.h              |    4 +-
 hw/usb.h                |    2 +-
 hw/usb/bus.c            |    2 +-
 hw/usb/host-bsd.c       |    2 +-
 hw/usb/host-linux.c     |    2 +-
 include/net/net.h       |    2 +-
 include/net/slirp.h     |    2 +-
 include/sysemu/sysemu.h |    4 +-
 monitor.c               |  108 +++++++++++++++++++++-------------------------
 net/net.c               |    2 +-
 net/slirp.c             |    2 +-
 savevm.c                |    2 +-
 vl.c                    |    2 +-
 26 files changed, 119 insertions(+), 127 deletions(-)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH V3 1/4] HMP: add QDict to info callback handler
  2012-12-27  9:40 [Qemu-devel] [PATCH V3 0/4] HMP: allow parsing for sub command Wenchao Xia
@ 2012-12-27  9:40 ` Wenchao Xia
  2012-12-27  9:40 ` [Qemu-devel] [PATCH V3 2/4] HMP: add infrastructure for sub command Wenchao Xia
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Wenchao Xia @ 2012-12-27  9:40 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, chenwj, armbru, lcapitulino, pbonzini, Wenchao Xia

  This patch change all info call back function to take
additional QDict * parameter, which allow those command
take parameter. Now it is set to NULL at default case.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
v2: rebased, and better doc.
v3: move changelog position.

 hmp.c                   |   36 ++++++++++++++++++------------------
 hmp.h                   |   36 ++++++++++++++++++------------------
 hw/i8259.c              |    4 ++--
 hw/lm32_pic.c           |    4 ++--
 hw/lm32_pic.h           |    4 ++--
 hw/loader.c             |    2 +-
 hw/loader.h             |    3 ++-
 hw/pc.h                 |    4 ++--
 hw/pcmcia.h             |    2 +-
 hw/qdev-monitor.c       |    4 ++--
 hw/qdev-monitor.h       |    4 ++--
 hw/sun4m.c              |    4 ++--
 hw/sun4m.h              |    4 ++--
 hw/usb.h                |    2 +-
 hw/usb/bus.c            |    2 +-
 hw/usb/host-bsd.c       |    2 +-
 hw/usb/host-linux.c     |    2 +-
 include/net/net.h       |    2 +-
 include/net/slirp.h     |    2 +-
 include/sysemu/sysemu.h |    4 ++--
 monitor.c               |   32 ++++++++++++++++----------------
 net/net.c               |    2 +-
 net/slirp.c             |    2 +-
 savevm.c                |    2 +-
 vl.c                    |    2 +-
 25 files changed, 84 insertions(+), 83 deletions(-)

diff --git a/hmp.c b/hmp.c
index 9e9e624..2465d9b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -31,7 +31,7 @@ static void hmp_handle_error(Monitor *mon, Error **errp)
     }
 }
 
-void hmp_info_name(Monitor *mon)
+void hmp_info_name(Monitor *mon, const QDict *qdict)
 {
     NameInfo *info;
 
@@ -42,7 +42,7 @@ void hmp_info_name(Monitor *mon)
     qapi_free_NameInfo(info);
 }
 
-void hmp_info_version(Monitor *mon)
+void hmp_info_version(Monitor *mon, const QDict *qdict)
 {
     VersionInfo *info;
 
@@ -55,7 +55,7 @@ void hmp_info_version(Monitor *mon)
     qapi_free_VersionInfo(info);
 }
 
-void hmp_info_kvm(Monitor *mon)
+void hmp_info_kvm(Monitor *mon, const QDict *qdict)
 {
     KvmInfo *info;
 
@@ -70,7 +70,7 @@ void hmp_info_kvm(Monitor *mon)
     qapi_free_KvmInfo(info);
 }
 
-void hmp_info_status(Monitor *mon)
+void hmp_info_status(Monitor *mon, const QDict *qdict)
 {
     StatusInfo *info;
 
@@ -89,7 +89,7 @@ void hmp_info_status(Monitor *mon)
     qapi_free_StatusInfo(info);
 }
 
-void hmp_info_uuid(Monitor *mon)
+void hmp_info_uuid(Monitor *mon, const QDict *qdict)
 {
     UuidInfo *info;
 
@@ -98,7 +98,7 @@ void hmp_info_uuid(Monitor *mon)
     qapi_free_UuidInfo(info);
 }
 
-void hmp_info_chardev(Monitor *mon)
+void hmp_info_chardev(Monitor *mon, const QDict *qdict)
 {
     ChardevInfoList *char_info, *info;
 
@@ -111,7 +111,7 @@ void hmp_info_chardev(Monitor *mon)
     qapi_free_ChardevInfoList(char_info);
 }
 
-void hmp_info_mice(Monitor *mon)
+void hmp_info_mice(Monitor *mon, const QDict *qdict)
 {
     MouseInfoList *mice_list, *mouse;
 
@@ -131,7 +131,7 @@ void hmp_info_mice(Monitor *mon)
     qapi_free_MouseInfoList(mice_list);
 }
 
-void hmp_info_migrate(Monitor *mon)
+void hmp_info_migrate(Monitor *mon, const QDict *qdict)
 {
     MigrationInfo *info;
     MigrationCapabilityStatusList *caps, *cap;
@@ -209,7 +209,7 @@ void hmp_info_migrate(Monitor *mon)
     qapi_free_MigrationCapabilityStatusList(caps);
 }
 
-void hmp_info_migrate_capabilities(Monitor *mon)
+void hmp_info_migrate_capabilities(Monitor *mon, const QDict *qdict)
 {
     MigrationCapabilityStatusList *caps, *cap;
 
@@ -228,13 +228,13 @@ void hmp_info_migrate_capabilities(Monitor *mon)
     qapi_free_MigrationCapabilityStatusList(caps);
 }
 
-void hmp_info_migrate_cache_size(Monitor *mon)
+void hmp_info_migrate_cache_size(Monitor *mon, const QDict *qdict)
 {
     monitor_printf(mon, "xbzrel cache size: %" PRId64 " kbytes\n",
                    qmp_query_migrate_cache_size(NULL) >> 10);
 }
 
-void hmp_info_cpus(Monitor *mon)
+void hmp_info_cpus(Monitor *mon, const QDict *qdict)
 {
     CpuInfoList *cpu_list, *cpu;
 
@@ -272,7 +272,7 @@ void hmp_info_cpus(Monitor *mon)
     qapi_free_CpuInfoList(cpu_list);
 }
 
-void hmp_info_block(Monitor *mon)
+void hmp_info_block(Monitor *mon, const QDict *qdict)
 {
     BlockInfoList *block_list, *info;
 
@@ -326,7 +326,7 @@ void hmp_info_block(Monitor *mon)
     qapi_free_BlockInfoList(block_list);
 }
 
-void hmp_info_blockstats(Monitor *mon)
+void hmp_info_blockstats(Monitor *mon, const QDict *qdict)
 {
     BlockStatsList *stats_list, *stats;
 
@@ -360,7 +360,7 @@ void hmp_info_blockstats(Monitor *mon)
     qapi_free_BlockStatsList(stats_list);
 }
 
-void hmp_info_vnc(Monitor *mon)
+void hmp_info_vnc(Monitor *mon, const QDict *qdict)
 {
     VncInfo *info;
     Error *err = NULL;
@@ -406,7 +406,7 @@ out:
     qapi_free_VncInfo(info);
 }
 
-void hmp_info_spice(Monitor *mon)
+void hmp_info_spice(Monitor *mon, const QDict *qdict)
 {
     SpiceChannelList *chan;
     SpiceInfo *info;
@@ -453,7 +453,7 @@ out:
     qapi_free_SpiceInfo(info);
 }
 
-void hmp_info_balloon(Monitor *mon)
+void hmp_info_balloon(Monitor *mon, const QDict *qdict)
 {
     BalloonInfo *info;
     Error *err = NULL;
@@ -570,7 +570,7 @@ static void hmp_info_pci_device(Monitor *mon, const PciDeviceInfo *dev)
     }
 }
 
-void hmp_info_pci(Monitor *mon)
+void hmp_info_pci(Monitor *mon, const QDict *qdict)
 {
     PciInfoList *info_list, *info;
     Error *err = NULL;
@@ -593,7 +593,7 @@ void hmp_info_pci(Monitor *mon)
     qapi_free_PciInfoList(info_list);
 }
 
-void hmp_info_block_jobs(Monitor *mon)
+void hmp_info_block_jobs(Monitor *mon, const QDict *qdict)
 {
     BlockJobInfoList *list;
     Error *err = NULL;
diff --git a/hmp.h b/hmp.h
index 21f3e05..5cab9c0 100644
--- a/hmp.h
+++ b/hmp.h
@@ -18,24 +18,24 @@
 #include "qapi-types.h"
 #include "qapi/qmp/qdict.h"
 
-void hmp_info_name(Monitor *mon);
-void hmp_info_version(Monitor *mon);
-void hmp_info_kvm(Monitor *mon);
-void hmp_info_status(Monitor *mon);
-void hmp_info_uuid(Monitor *mon);
-void hmp_info_chardev(Monitor *mon);
-void hmp_info_mice(Monitor *mon);
-void hmp_info_migrate(Monitor *mon);
-void hmp_info_migrate_capabilities(Monitor *mon);
-void hmp_info_migrate_cache_size(Monitor *mon);
-void hmp_info_cpus(Monitor *mon);
-void hmp_info_block(Monitor *mon);
-void hmp_info_blockstats(Monitor *mon);
-void hmp_info_vnc(Monitor *mon);
-void hmp_info_spice(Monitor *mon);
-void hmp_info_balloon(Monitor *mon);
-void hmp_info_pci(Monitor *mon);
-void hmp_info_block_jobs(Monitor *mon);
+void hmp_info_name(Monitor *mon, const QDict *qdict);
+void hmp_info_version(Monitor *mon, const QDict *qdict);
+void hmp_info_kvm(Monitor *mon, const QDict *qdict);
+void hmp_info_status(Monitor *mon, const QDict *qdict);
+void hmp_info_uuid(Monitor *mon, const QDict *qdict);
+void hmp_info_chardev(Monitor *mon, const QDict *qdict);
+void hmp_info_mice(Monitor *mon, const QDict *qdict);
+void hmp_info_migrate(Monitor *mon, const QDict *qdict);
+void hmp_info_migrate_capabilities(Monitor *mon, const QDict *qdict);
+void hmp_info_migrate_cache_size(Monitor *mon, const QDict *qdict);
+void hmp_info_cpus(Monitor *mon, const QDict *qdict);
+void hmp_info_block(Monitor *mon, const QDict *qdict);
+void hmp_info_blockstats(Monitor *mon, const QDict *qdict);
+void hmp_info_vnc(Monitor *mon, const QDict *qdict);
+void hmp_info_spice(Monitor *mon, const QDict *qdict);
+void hmp_info_balloon(Monitor *mon, const QDict *qdict);
+void hmp_info_pci(Monitor *mon, const QDict *qdict);
+void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
 void hmp_quit(Monitor *mon, const QDict *qdict);
 void hmp_stop(Monitor *mon, const QDict *qdict);
 void hmp_system_reset(Monitor *mon, const QDict *qdict);
diff --git a/hw/i8259.c b/hw/i8259.c
index 8fc6339..51b4b3c 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -407,7 +407,7 @@ static void pic_init(PICCommonState *s)
     qdev_init_gpio_in(&s->dev.qdev, pic_set_irq, 8);
 }
 
-void pic_info(Monitor *mon)
+void pic_info(Monitor *mon, const QDict *qdict)
 {
     int i;
     PICCommonState *s;
@@ -425,7 +425,7 @@ void pic_info(Monitor *mon)
     }
 }
 
-void irq_info(Monitor *mon)
+void irq_info(Monitor *mon, const QDict *qdict)
 {
 #ifndef DEBUG_IRQ_COUNT
     monitor_printf(mon, "irq statistic code not compiled.\n");
diff --git a/hw/lm32_pic.c b/hw/lm32_pic.c
index 42d5602..a327425 100644
--- a/hw/lm32_pic.c
+++ b/hw/lm32_pic.c
@@ -39,7 +39,7 @@ struct LM32PicState {
 typedef struct LM32PicState LM32PicState;
 
 static LM32PicState *pic;
-void lm32_do_pic_info(Monitor *mon)
+void lm32_do_pic_info(Monitor *mon, const QDict *qdict)
 {
     if (pic == NULL) {
         return;
@@ -49,7 +49,7 @@ void lm32_do_pic_info(Monitor *mon)
             pic->im, pic->ip, pic->irq_state);
 }
 
-void lm32_irq_info(Monitor *mon)
+void lm32_irq_info(Monitor *mon, const QDict *qdict)
 {
     int i;
     uint32_t count;
diff --git a/hw/lm32_pic.h b/hw/lm32_pic.h
index 14456f3..5556803 100644
--- a/hw/lm32_pic.h
+++ b/hw/lm32_pic.h
@@ -8,7 +8,7 @@ uint32_t lm32_pic_get_im(DeviceState *d);
 void lm32_pic_set_ip(DeviceState *d, uint32_t ip);
 void lm32_pic_set_im(DeviceState *d, uint32_t im);
 
-void lm32_do_pic_info(Monitor *mon);
-void lm32_irq_info(Monitor *mon);
+void lm32_do_pic_info(Monitor *mon, const QDict *qdict);
+void lm32_irq_info(Monitor *mon, const QDict *qdict);
 
 #endif /* QEMU_HW_LM32_PIC_H */
diff --git a/hw/loader.c b/hw/loader.c
index 3f59fcd..995edc3 100644
--- a/hw/loader.c
+++ b/hw/loader.c
@@ -778,7 +778,7 @@ void *rom_ptr(hwaddr addr)
     return rom->data + (addr - rom->addr);
 }
 
-void do_info_roms(Monitor *mon)
+void do_info_roms(Monitor *mon, const QDict *qdict)
 {
     Rom *rom;
 
diff --git a/hw/loader.h b/hw/loader.h
index 26480ad..5e61c95 100644
--- a/hw/loader.h
+++ b/hw/loader.h
@@ -1,5 +1,6 @@
 #ifndef LOADER_H
 #define LOADER_H
+#include "qapi/qmp/qdict.h"
 
 /* loader.c */
 int get_image_size(const char *filename);
@@ -30,7 +31,7 @@ int rom_load_all(void);
 void rom_set_fw(void *f);
 int rom_copy(uint8_t *dest, hwaddr addr, size_t size);
 void *rom_ptr(hwaddr addr);
-void do_info_roms(Monitor *mon);
+void do_info_roms(Monitor *mon, const QDict *qdict);
 
 #define rom_add_file_fixed(_f, _a, _i)          \
     rom_add_file(_f, NULL, _a, _i)
diff --git a/hw/pc.h b/hw/pc.h
index a73e3e7..fb6cbe2 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -40,8 +40,8 @@ qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq);
 qemu_irq *kvm_i8259_init(ISABus *bus);
 int pic_read_irq(DeviceState *d);
 int pic_get_output(DeviceState *d);
-void pic_info(Monitor *mon);
-void irq_info(Monitor *mon);
+void pic_info(Monitor *mon, const QDict *qdict);
+void irq_info(Monitor *mon, const QDict *qdict);
 
 /* Global System Interrupts */
 
diff --git a/hw/pcmcia.h b/hw/pcmcia.h
index aac1d77..f916693 100644
--- a/hw/pcmcia.h
+++ b/hw/pcmcia.h
@@ -14,7 +14,7 @@ typedef struct {
 
 void pcmcia_socket_register(PCMCIASocket *socket);
 void pcmcia_socket_unregister(PCMCIASocket *socket);
-void pcmcia_info(Monitor *mon);
+void pcmcia_info(Monitor *mon, const QDict *qdict);
 
 struct PCMCIACardState {
     void *state;
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index b739867..e0f0075 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -564,13 +564,13 @@ static void qbus_print(Monitor *mon, BusState *bus, int indent)
 }
 #undef qdev_printf
 
-void do_info_qtree(Monitor *mon)
+void do_info_qtree(Monitor *mon, const QDict *qdict)
 {
     if (sysbus_get_default())
         qbus_print(mon, sysbus_get_default(), 0);
 }
 
-void do_info_qdm(Monitor *mon)
+void do_info_qdm(Monitor *mon, const QDict *qdict)
 {
     object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false, NULL);
 }
diff --git a/hw/qdev-monitor.h b/hw/qdev-monitor.h
index fae1b1e..9ec4850 100644
--- a/hw/qdev-monitor.h
+++ b/hw/qdev-monitor.h
@@ -6,8 +6,8 @@
 
 /*** monitor commands ***/
 
-void do_info_qtree(Monitor *mon);
-void do_info_qdm(Monitor *mon);
+void do_info_qtree(Monitor *mon, const QDict *qdict);
+void do_info_qdm(Monitor *mon, const QDict *qdict);
 int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int qdev_device_help(QemuOpts *opts);
diff --git a/hw/sun4m.c b/hw/sun4m.c
index 0d84b37..db23671 100644
--- a/hw/sun4m.c
+++ b/hw/sun4m.c
@@ -216,13 +216,13 @@ static void nvram_init(M48t59State *nvram, uint8_t *macaddr,
 
 static DeviceState *slavio_intctl;
 
-void sun4m_pic_info(Monitor *mon)
+void sun4m_pic_info(Monitor *mon, const QDict *qdict)
 {
     if (slavio_intctl)
         slavio_pic_info(mon, slavio_intctl);
 }
 
-void sun4m_irq_info(Monitor *mon)
+void sun4m_irq_info(Monitor *mon, const QDict *qdict)
 {
     if (slavio_intctl)
         slavio_irq_info(mon, slavio_intctl);
diff --git a/hw/sun4m.h b/hw/sun4m.h
index 47eb945..0361eee 100644
--- a/hw/sun4m.h
+++ b/hw/sun4m.h
@@ -27,8 +27,8 @@ void slavio_pic_info(Monitor *mon, DeviceState *dev);
 void slavio_irq_info(Monitor *mon, DeviceState *dev);
 
 /* sun4m.c */
-void sun4m_pic_info(Monitor *mon);
-void sun4m_irq_info(Monitor *mon);
+void sun4m_pic_info(Monitor *mon, const QDict *qdict);
+void sun4m_irq_info(Monitor *mon, const QDict *qdict);
 
 /* sparc32_dma.c */
 #include "sparc32_dma.h"
diff --git a/hw/usb.h b/hw/usb.h
index 81e265c..a24e9e0 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -429,7 +429,7 @@ int set_usb_string(uint8_t *buf, const char *str);
 /* usb-linux.c */
 USBDevice *usb_host_device_open(USBBus *bus, const char *devname);
 int usb_host_device_close(const char *devname);
-void usb_host_info(Monitor *mon);
+void usb_host_info(Monitor *mon, const QDict *qdict);
 
 /* usb-bt.c */
 USBDevice *usb_bt_init(USBBus *bus, HCIInfo *hci);
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 10260a1..b58e421 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -534,7 +534,7 @@ static char *usb_get_fw_dev_path(DeviceState *qdev)
     return fw_path;
 }
 
-void usb_info(Monitor *mon)
+void usb_info(Monitor *mon, const QDict *qdict)
 {
     USBBus *bus;
     USBDevice *dev;
diff --git a/hw/usb/host-bsd.c b/hw/usb/host-bsd.c
index 340c21a..bed54c6 100644
--- a/hw/usb/host-bsd.c
+++ b/hw/usb/host-bsd.c
@@ -633,7 +633,7 @@ static int usb_host_info_device(void *opaque,
     return 0;
 }
 
-void usb_host_info(Monitor *mon)
+void usb_host_info(Monitor *mon, const QDict *qdict)
 {
     usb_host_scan(mon, usb_host_info_device);
 }
diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c
index 669fbd2..cdacc54 100644
--- a/hw/usb/host-linux.c
+++ b/hw/usb/host-linux.c
@@ -1998,7 +1998,7 @@ static void hex2str(int val, char *str, size_t size)
     }
 }
 
-void usb_host_info(Monitor *mon)
+void usb_host_info(Monitor *mon, const QDict *qdict)
 {
     struct USBAutoFilter *f;
     struct USBHostDevice *s;
diff --git a/include/net/net.h b/include/net/net.h
index de42dd7..4a92b6c 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -112,7 +112,7 @@ ssize_t qemu_deliver_packet_iov(NetClientState *sender,
                             void *opaque);
 
 void print_net_client(Monitor *mon, NetClientState *nc);
-void do_info_network(Monitor *mon);
+void do_info_network(Monitor *mon, const QDict *qdict);
 
 /* NIC info */
 
diff --git a/include/net/slirp.h b/include/net/slirp.h
index 54b655c..0502389 100644
--- a/include/net/slirp.h
+++ b/include/net/slirp.h
@@ -40,7 +40,7 @@ int net_slirp_parse_legacy(QemuOptsList *opts_list, const char *optarg, int *ret
 
 int net_slirp_smb(const char *exported_dir);
 
-void do_info_usernet(Monitor *mon);
+void do_info_usernet(Monitor *mon, const QDict *qdict);
 
 #endif
 
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 28a783e..a7fd0be 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -68,7 +68,7 @@ void qemu_add_machine_init_done_notifier(Notifier *notify);
 void do_savevm(Monitor *mon, const QDict *qdict);
 int load_vmstate(const char *name);
 void do_delvm(Monitor *mon, const QDict *qdict);
-void do_info_snapshots(Monitor *mon);
+void do_info_snapshots(Monitor *mon, const QDict *qdict);
 
 void qemu_announce_self(void);
 
@@ -171,7 +171,7 @@ extern CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
 
 void do_usb_add(Monitor *mon, const QDict *qdict);
 void do_usb_del(Monitor *mon, const QDict *qdict);
-void usb_info(Monitor *mon);
+void usb_info(Monitor *mon, const QDict *qdict);
 
 void rtc_change_mon_event(struct tm *tm);
 
diff --git a/monitor.c b/monitor.c
index 9cf419b..c7b3014 100644
--- a/monitor.c
+++ b/monitor.c
@@ -123,7 +123,7 @@ typedef struct mon_cmd_t {
     const char *help;
     void (*user_print)(Monitor *mon, const QObject *data);
     union {
-        void (*info)(Monitor *mon);
+        void (*info)(Monitor *mon, const QDict *qdict);
         void (*cmd)(Monitor *mon, const QDict *qdict);
         int  (*cmd_new)(Monitor *mon, const QDict *params, QObject **ret_data);
         int  (*cmd_async)(Monitor *mon, const QDict *params,
@@ -824,7 +824,7 @@ static void do_info(Monitor *mon, const QDict *qdict)
         goto help;
     }
 
-    cmd->mhandler.info(mon);
+    cmd->mhandler.info(mon, NULL);
     return;
 
 help:
@@ -895,19 +895,19 @@ int monitor_get_cpu_index(void)
     return mon_get_cpu()->cpu_index;
 }
 
-static void do_info_registers(Monitor *mon)
+static void do_info_registers(Monitor *mon, const QDict *qdict)
 {
     CPUArchState *env;
     env = mon_get_cpu();
     cpu_dump_state(env, (FILE *)mon, monitor_fprintf, CPU_DUMP_FPU);
 }
 
-static void do_info_jit(Monitor *mon)
+static void do_info_jit(Monitor *mon, const QDict *qdict)
 {
     dump_exec_info((FILE *)mon, monitor_fprintf);
 }
 
-static void do_info_history(Monitor *mon)
+static void do_info_history(Monitor *mon, const QDict *qdict)
 {
     int i;
     const char *str;
@@ -926,7 +926,7 @@ static void do_info_history(Monitor *mon)
 
 #if defined(TARGET_PPC)
 /* XXX: not implemented in other targets */
-static void do_info_cpu_stats(Monitor *mon)
+static void do_info_cpu_stats(Monitor *mon, const QDict *qdict)
 {
     CPUArchState *env;
 
@@ -935,7 +935,7 @@ static void do_info_cpu_stats(Monitor *mon)
 }
 #endif
 
-static void do_trace_print_events(Monitor *mon)
+static void do_trace_print_events(Monitor *mon, const QDict *qdict)
 {
     trace_print_events((FILE *)mon, &monitor_fprintf);
 }
@@ -1487,7 +1487,7 @@ static void tlb_info_64(Monitor *mon, CPUArchState *env)
 }
 #endif
 
-static void tlb_info(Monitor *mon)
+static void tlb_info(Monitor *mon, const QDict *qdict)
 {
     CPUArchState *env;
 
@@ -1710,7 +1710,7 @@ static void mem_info_64(Monitor *mon, CPUArchState *env)
 }
 #endif
 
-static void mem_info(Monitor *mon)
+static void mem_info(Monitor *mon, const QDict *qdict)
 {
     CPUArchState *env;
 
@@ -1749,7 +1749,7 @@ static void print_tlb(Monitor *mon, int idx, tlb_t *tlb)
                    tlb->d, tlb->wt);
 }
 
-static void tlb_info(Monitor *mon)
+static void tlb_info(Monitor *mon, const QDict *qdict)
 {
     CPUArchState *env = mon_get_cpu();
     int i;
@@ -1765,7 +1765,7 @@ static void tlb_info(Monitor *mon)
 #endif
 
 #if defined(TARGET_SPARC) || defined(TARGET_PPC) || defined(TARGET_XTENSA)
-static void tlb_info(Monitor *mon)
+static void tlb_info(Monitor *mon, const QDict *qdict)
 {
     CPUArchState *env1 = mon_get_cpu();
 
@@ -1773,12 +1773,12 @@ static void tlb_info(Monitor *mon)
 }
 #endif
 
-static void do_info_mtree(Monitor *mon)
+static void do_info_mtree(Monitor *mon, const QDict *qdict)
 {
     mtree_info((fprintf_function)monitor_printf, mon);
 }
 
-static void do_info_numa(Monitor *mon)
+static void do_info_numa(Monitor *mon, const QDict *qdict)
 {
     int i;
     CPUArchState *env;
@@ -1802,7 +1802,7 @@ static void do_info_numa(Monitor *mon)
 int64_t qemu_time;
 int64_t dev_time;
 
-static void do_info_profile(Monitor *mon)
+static void do_info_profile(Monitor *mon, const QDict *qdict)
 {
     int64_t total;
     total = qemu_time;
@@ -1816,7 +1816,7 @@ static void do_info_profile(Monitor *mon)
     dev_time = 0;
 }
 #else
-static void do_info_profile(Monitor *mon)
+static void do_info_profile(Monitor *mon, const QDict *qdict)
 {
     monitor_printf(mon, "Internal profiler not compiled\n");
 }
@@ -1825,7 +1825,7 @@ static void do_info_profile(Monitor *mon)
 /* Capture support */
 static QLIST_HEAD (capture_list_head, CaptureState) capture_head;
 
-static void do_info_capture(Monitor *mon)
+static void do_info_capture(Monitor *mon, const QDict *qdict)
 {
     int i;
     CaptureState *s;
diff --git a/net/net.c b/net/net.c
index dbf3e1b..354d292 100644
--- a/net/net.c
+++ b/net/net.c
@@ -852,7 +852,7 @@ void print_net_client(Monitor *mon, NetClientState *nc)
                    NetClientOptionsKind_lookup[nc->info->type], nc->info_str);
 }
 
-void do_info_network(Monitor *mon)
+void do_info_network(Monitor *mon, const QDict *qdict)
 {
     NetClientState *nc, *peer;
     NetClientOptionsKind type;
diff --git a/net/slirp.c b/net/slirp.c
index c14259f..4df550f 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -670,7 +670,7 @@ static int slirp_guestfwd(SlirpState *s, const char *config_str,
     return -1;
 }
 
-void do_info_usernet(Monitor *mon)
+void do_info_usernet(Monitor *mon, const QDict *qdict)
 {
     SlirpState *s;
 
diff --git a/savevm.c b/savevm.c
index bcdb92e..720eea6 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2371,7 +2371,7 @@ void do_delvm(Monitor *mon, const QDict *qdict)
     }
 }
 
-void do_info_snapshots(Monitor *mon)
+void do_info_snapshots(Monitor *mon, const QDict *qdict)
 {
     BlockDriverState *bs, *bs1;
     QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = &s;
diff --git a/vl.c b/vl.c
index e6a8d89..1905266 100644
--- a/vl.c
+++ b/vl.c
@@ -1263,7 +1263,7 @@ void pcmcia_socket_unregister(PCMCIASocket *socket)
         }
 }
 
-void pcmcia_info(Monitor *mon)
+void pcmcia_info(Monitor *mon, const QDict *qdict)
 {
     struct pcmcia_socket_entry_s *iter;
 
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH V3 2/4] HMP: add infrastructure for sub command
  2012-12-27  9:40 [Qemu-devel] [PATCH V3 0/4] HMP: allow parsing for sub command Wenchao Xia
  2012-12-27  9:40 ` [Qemu-devel] [PATCH V3 1/4] HMP: add QDict to info callback handler Wenchao Xia
@ 2012-12-27  9:40 ` Wenchao Xia
  2013-01-09 12:57   ` Luiz Capitulino
  2012-12-27  9:40 ` [Qemu-devel] [PATCH V3 3/4] HMP: move define of mon_cmds Wenchao Xia
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Wenchao Xia @ 2012-12-27  9:40 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, chenwj, armbru, lcapitulino, pbonzini, Wenchao Xia

  This patch make parsing of hmp command aware of that it may
have sub command. Also discard simple encapsulation function
monitor_find_command() and qmp_find_cmd().

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 monitor.c |   44 +++++++++++++++++++++++++++-----------------
 1 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/monitor.c b/monitor.c
index c7b3014..e4d4c59 100644
--- a/monitor.c
+++ b/monitor.c
@@ -130,6 +130,7 @@ typedef struct mon_cmd_t {
                           MonitorCompletion *cb, void *opaque);
     } mhandler;
     int flags;
+    struct mon_cmd_t *sub_table;
 } mon_cmd_t;
 
 /* file descriptors passed via SCM_RIGHTS */
@@ -3534,21 +3535,17 @@ static const mon_cmd_t *search_dispatch_table(const mon_cmd_t *disp_table,
     return NULL;
 }
 
-static const mon_cmd_t *monitor_find_command(const char *cmdname)
-{
-    return search_dispatch_table(mon_cmds, cmdname);
-}
-
-static const mon_cmd_t *qmp_find_cmd(const char *cmdname)
-{
-    return search_dispatch_table(qmp_cmds, cmdname);
-}
-
-static const mon_cmd_t *monitor_parse_command(Monitor *mon,
-                                              const char *cmdline,
-                                              QDict *qdict)
+/*
+ * Try find the target mon_cmd_t instance, if it have sub_table and have string
+ * for it, this function will try search it with remain string, and if not
+ * found it return NULL.
+ */
+static const mon_cmd_t *hmp_parse_command(Monitor *mon,
+                                          const char *cmdline,
+                                          mon_cmd_t *table,
+                                          QDict *qdict)
 {
-    const char *p, *typestr;
+    const char *p, *p1, *typestr;
     int c;
     const mon_cmd_t *cmd;
     char cmdname[256];
@@ -3564,12 +3561,25 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
     if (!p)
         return NULL;
 
-    cmd = monitor_find_command(cmdname);
+    cmd = search_dispatch_table(table, cmdname);
     if (!cmd) {
         monitor_printf(mon, "unknown command: '%s'\n", cmdname);
         return NULL;
     }
 
+    /* search sub command */
+    if (cmd->sub_table != NULL) {
+        p1 = p;
+        /* check if user set additional command */
+        while (qemu_isspace(*p1)) {
+            p1++;
+        }
+        if (*p1 == '\0') {
+            return cmd;
+        }
+        return hmp_parse_command(mon, p, cmd->sub_table, qdict);
+    }
+
     /* parse the parameters */
     typestr = cmd->args_type;
     for(;;) {
@@ -3925,7 +3935,7 @@ static void handle_user_command(Monitor *mon, const char *cmdline)
 
     qdict = qdict_new();
 
-    cmd = monitor_parse_command(mon, cmdline, qdict);
+    cmd = hmp_parse_command(mon, cmdline, mon_cmds, qdict);
     if (!cmd)
         goto out;
 
@@ -4483,7 +4493,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
         goto err_out;
     }
 
-    cmd = qmp_find_cmd(cmd_name);
+    cmd = search_dispatch_table(qmp_cmds, cmd_name);
     if (!cmd) {
         qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
         goto err_out;
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH V3 3/4] HMP: move define of mon_cmds
  2012-12-27  9:40 [Qemu-devel] [PATCH V3 0/4] HMP: allow parsing for sub command Wenchao Xia
  2012-12-27  9:40 ` [Qemu-devel] [PATCH V3 1/4] HMP: add QDict to info callback handler Wenchao Xia
  2012-12-27  9:40 ` [Qemu-devel] [PATCH V3 2/4] HMP: add infrastructure for sub command Wenchao Xia
@ 2012-12-27  9:40 ` Wenchao Xia
  2012-12-27  9:40 ` [Qemu-devel] [PATCH V3 4/4] HMP: add sub command table to info Wenchao Xia
  2013-01-04  3:00 ` [Qemu-devel] [PATCH V3 0/4] HMP: allow parsing for sub command Wenchao Xia
  4 siblings, 0 replies; 10+ messages in thread
From: Wenchao Xia @ 2012-12-27  9:40 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, chenwj, armbru, lcapitulino, pbonzini, Wenchao Xia

  Because mon_cmds may use info_cmds, so adjust the declare sequence
of them.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 monitor.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/monitor.c b/monitor.c
index e4d4c59..a392df4 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2422,12 +2422,6 @@ int monitor_handle_fd_param(Monitor *mon, const char *fdname)
     return fd;
 }
 
-/* mon_cmds and info_cmds would be sorted at runtime */
-static mon_cmd_t mon_cmds[] = {
-#include "hmp-commands.h"
-    { NULL, NULL, },
-};
-
 /* Please update hmp-commands.hx when adding or changing commands */
 static mon_cmd_t info_cmds[] = {
     {
@@ -2741,6 +2735,12 @@ static mon_cmd_t info_cmds[] = {
     },
 };
 
+/* mon_cmds and info_cmds would be sorted at runtime */
+static mon_cmd_t mon_cmds[] = {
+#include "hmp-commands.h"
+    { NULL, NULL, },
+};
+
 static const mon_cmd_t qmp_cmds[] = {
 #include "qmp-commands-old.h"
     { /* NULL */ },
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH V3 4/4] HMP: add sub command table to info
  2012-12-27  9:40 [Qemu-devel] [PATCH V3 0/4] HMP: allow parsing for sub command Wenchao Xia
                   ` (2 preceding siblings ...)
  2012-12-27  9:40 ` [Qemu-devel] [PATCH V3 3/4] HMP: move define of mon_cmds Wenchao Xia
@ 2012-12-27  9:40 ` Wenchao Xia
  2013-01-04  3:00 ` [Qemu-devel] [PATCH V3 0/4] HMP: allow parsing for sub command Wenchao Xia
  4 siblings, 0 replies; 10+ messages in thread
From: Wenchao Xia @ 2012-12-27  9:40 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, chenwj, armbru, lcapitulino, pbonzini, Wenchao Xia

  Now info command takes a table of sub info commands,
and changed do_info() to do_info_help() to do help funtion
only.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 hmp-commands.hx |    3 ++-
 monitor.c       |   22 +---------------------
 2 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 010b8c9..87a411a 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1489,7 +1489,8 @@ ETEXI
         .args_type  = "item:s?",
         .params     = "[subcommand]",
         .help       = "show various information about the system state",
-        .mhandler.cmd = do_info,
+        .mhandler.cmd = do_info_help,
+        .sub_table = info_cmds,
     },
 
 STEXI
diff --git a/monitor.c b/monitor.c
index a392df4..e1bcaa2 100644
--- a/monitor.c
+++ b/monitor.c
@@ -807,28 +807,8 @@ static void user_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
     }
 }
 
-static void do_info(Monitor *mon, const QDict *qdict)
+static void do_info_help(Monitor *mon, const QDict *qdict)
 {
-    const mon_cmd_t *cmd;
-    const char *item = qdict_get_try_str(qdict, "item");
-
-    if (!item) {
-        goto help;
-    }
-
-    for (cmd = info_cmds; cmd->name != NULL; cmd++) {
-        if (compare_cmd(item, cmd->name))
-            break;
-    }
-
-    if (cmd->name == NULL) {
-        goto help;
-    }
-
-    cmd->mhandler.info(mon, NULL);
-    return;
-
-help:
     help_cmd(mon, "info");
 }
 
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH V3 0/4] HMP: allow parsing for sub command
  2012-12-27  9:40 [Qemu-devel] [PATCH V3 0/4] HMP: allow parsing for sub command Wenchao Xia
                   ` (3 preceding siblings ...)
  2012-12-27  9:40 ` [Qemu-devel] [PATCH V3 4/4] HMP: add sub command table to info Wenchao Xia
@ 2013-01-04  3:00 ` Wenchao Xia
  4 siblings, 0 replies; 10+ messages in thread
From: Wenchao Xia @ 2013-01-04  3:00 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: aliguori, chenwj, armbru, qemu-devel, lcapitulino, pbonzini

Hi, Markus
  Any comments for this? There are other serials of patch need this
infrastructure extension, so hope it can be reviewed faster....

>    These patches enhance HMP to allow it parse 2nd level of commands, such
> as info sub command list, which means now foldered command with parameter
> is possible.
> 
> V2:
>    Follow the way supposed by Markus, which make the infrastructure knows
> there is possible a 2nd level of command exist, instead of a hack. In this
> way extention of command folder level is easy.
>    Moved some code and better doc according to comments.
>    Removed the patch about info snapshots, which will goto another serial.
> V3:
>    Move changelog position.
>    Split out code moving patch.
> 
> Wenchao Xia (4):
>    HMP: add QDict to info callback handler
>    HMP: add infrastructure for sub command
>    HMP: move define of mon_cmds
>    HMP: add sub command table to info
> 
>   hmp-commands.hx         |    3 +-
>   hmp.c                   |   36 ++++++++--------
>   hmp.h                   |   36 ++++++++--------
>   hw/i8259.c              |    4 +-
>   hw/lm32_pic.c           |    4 +-
>   hw/lm32_pic.h           |    4 +-
>   hw/loader.c             |    2 +-
>   hw/loader.h             |    3 +-
>   hw/pc.h                 |    4 +-
>   hw/pcmcia.h             |    2 +-
>   hw/qdev-monitor.c       |    4 +-
>   hw/qdev-monitor.h       |    4 +-
>   hw/sun4m.c              |    4 +-
>   hw/sun4m.h              |    4 +-
>   hw/usb.h                |    2 +-
>   hw/usb/bus.c            |    2 +-
>   hw/usb/host-bsd.c       |    2 +-
>   hw/usb/host-linux.c     |    2 +-
>   include/net/net.h       |    2 +-
>   include/net/slirp.h     |    2 +-
>   include/sysemu/sysemu.h |    4 +-
>   monitor.c               |  108 +++++++++++++++++++++-------------------------
>   net/net.c               |    2 +-
>   net/slirp.c             |    2 +-
>   savevm.c                |    2 +-
>   vl.c                    |    2 +-
>   26 files changed, 119 insertions(+), 127 deletions(-)
> 
> 


-- 
Best Regards

Wenchao Xia

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH V3 2/4] HMP: add infrastructure for sub command
  2012-12-27  9:40 ` [Qemu-devel] [PATCH V3 2/4] HMP: add infrastructure for sub command Wenchao Xia
@ 2013-01-09 12:57   ` Luiz Capitulino
  2013-01-10  3:30     ` Wenchao Xia
  0 siblings, 1 reply; 10+ messages in thread
From: Luiz Capitulino @ 2013-01-09 12:57 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: aliguori, chenwj, qemu-devel, armbru, pbonzini

On Thu, 27 Dec 2012 17:40:25 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

>   This patch make parsing of hmp command aware of that it may
> have sub command. Also discard simple encapsulation function
> monitor_find_command() and qmp_find_cmd().
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  monitor.c |   44 +++++++++++++++++++++++++++-----------------
>  1 files changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index c7b3014..e4d4c59 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -130,6 +130,7 @@ typedef struct mon_cmd_t {
>                            MonitorCompletion *cb, void *opaque);
>      } mhandler;
>      int flags;
> +    struct mon_cmd_t *sub_table;
>  } mon_cmd_t;
>  
>  /* file descriptors passed via SCM_RIGHTS */
> @@ -3534,21 +3535,17 @@ static const mon_cmd_t *search_dispatch_table(const mon_cmd_t *disp_table,
>      return NULL;
>  }
>  
> -static const mon_cmd_t *monitor_find_command(const char *cmdname)
> -{
> -    return search_dispatch_table(mon_cmds, cmdname);
> -}
> -
> -static const mon_cmd_t *qmp_find_cmd(const char *cmdname)
> -{
> -    return search_dispatch_table(qmp_cmds, cmdname);
> -}

Touching qmp_find_cmd() is unrelated to this series.

> -
> -static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> -                                              const char *cmdline,
> -                                              QDict *qdict)
> +/*
> + * Try find the target mon_cmd_t instance, if it have sub_table and have string
> + * for it, this function will try search it with remain string, and if not
> + * found it return NULL.
> + */
> +static const mon_cmd_t *hmp_parse_command(Monitor *mon,
> +                                          const char *cmdline,
> +                                          mon_cmd_t *table,
> +                                          QDict *qdict)

Renaming is also not necessary, but if you do it please do in a different
patch.

>  {
> -    const char *p, *typestr;
> +    const char *p, *p1, *typestr;
>      int c;
>      const mon_cmd_t *cmd;
>      char cmdname[256];
> @@ -3564,12 +3561,25 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>      if (!p)
>          return NULL;
>  
> -    cmd = monitor_find_command(cmdname);
> +    cmd = search_dispatch_table(table, cmdname);
>      if (!cmd) {
>          monitor_printf(mon, "unknown command: '%s'\n", cmdname);
>          return NULL;
>      }
>  
> +    /* search sub command */
> +    if (cmd->sub_table != NULL) {
> +        p1 = p;
> +        /* check if user set additional command */
> +        while (qemu_isspace(*p1)) {
> +            p1++;
> +        }
> +        if (*p1 == '\0') {
> +            return cmd;
> +        }
> +        return hmp_parse_command(mon, p, cmd->sub_table, qdict);

This works, and seems less hack then the previous attempt.

Except for a small detail. This:

  (qemu) info bla

Currently returns a list of info commands, but with this series it returns
an error.

I'd like to get a reviewed-by from Markus before applying this as, besides
this being his idea, I honestly have a very hard time knowing how to move old
hmp hackery forward.

> +    }
> +
>      /* parse the parameters */
>      typestr = cmd->args_type;
>      for(;;) {
> @@ -3925,7 +3935,7 @@ static void handle_user_command(Monitor *mon, const char *cmdline)
>  
>      qdict = qdict_new();
>  
> -    cmd = monitor_parse_command(mon, cmdline, qdict);
> +    cmd = hmp_parse_command(mon, cmdline, mon_cmds, qdict);
>      if (!cmd)
>          goto out;
>  
> @@ -4483,7 +4493,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
>          goto err_out;
>      }
>  
> -    cmd = qmp_find_cmd(cmd_name);
> +    cmd = search_dispatch_table(qmp_cmds, cmd_name);
>      if (!cmd) {
>          qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
>          goto err_out;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH V3 2/4] HMP: add infrastructure for sub command
  2013-01-09 12:57   ` Luiz Capitulino
@ 2013-01-10  3:30     ` Wenchao Xia
  2013-01-10  7:07       ` wenchao xia
  0 siblings, 1 reply; 10+ messages in thread
From: Wenchao Xia @ 2013-01-10  3:30 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: pbonzini, aliguori, qemu-devel, chenwj, armbru

>> -static const mon_cmd_t *qmp_find_cmd(const char *cmdname)
>> -{
>> -    return search_dispatch_table(qmp_cmds, cmdname);
>> -}
>
> Touching qmp_find_cmd() is unrelated to this series.
>
   OK.

>> -
>> -static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>> -                                              const char *cmdline,
>> -                                              QDict *qdict)
>> +/*
>> + * Try find the target mon_cmd_t instance, if it have sub_table and have string
>> + * for it, this function will try search it with remain string, and if not
>> + * found it return NULL.
>> + */
>> +static const mon_cmd_t *hmp_parse_command(Monitor *mon,
>> +                                          const char *cmdline,
>> +                                          mon_cmd_t *table,
>> +                                          QDict *qdict)
>
> Renaming is also not necessary, but if you do it please do in a different
> patch.
>
   OK.

>>   {
>> +        }
>> +        return hmp_parse_command(mon, p, cmd->sub_table, qdict);
>
> This works, and seems less hack then the previous attempt.
>
> Except for a small detail. This:
>
>    (qemu) info bla
>
> Currently returns a list of info commands, but with this series it returns
> an error.
>
   Yep, I think command completion is another topic, but I am pretty
sure it can be solved in this framework, for that it is not hack and
what need to do is, let command completion function knows there may be
another sub layer too.

> I'd like to get a reviewed-by from Markus before applying this as, besides
> this being his idea, I honestly have a very hard time knowing how to move old
> hmp hackery forward.
>
>> +    }
>> +
>>       /* parse the parameters */
>>       typestr = cmd->args_type;
>>       for(;;) {
>> @@ -3925,7 +3935,7 @@ static void handle_user_command(Monitor *mon, const char *cmdline)
>>
>>       qdict = qdict_new();
>>
>> -    cmd = monitor_parse_command(mon, cmdline, qdict);
>> +    cmd = hmp_parse_command(mon, cmdline, mon_cmds, qdict);
>>       if (!cmd)
>>           goto out;
>>
>> @@ -4483,7 +4493,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
>>           goto err_out;
>>       }
>>
>> -    cmd = qmp_find_cmd(cmd_name);
>> +    cmd = search_dispatch_table(qmp_cmds, cmd_name);
>>       if (!cmd) {
>>           qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
>>           goto err_out;
>
>


-- 
Best Regards

Wenchao Xia

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH V3 2/4] HMP: add infrastructure for sub command
  2013-01-10  3:30     ` Wenchao Xia
@ 2013-01-10  7:07       ` wenchao xia
  2013-01-10  9:23         ` Markus Armbruster
  0 siblings, 1 reply; 10+ messages in thread
From: wenchao xia @ 2013-01-10  7:07 UTC (permalink / raw)
  To: Wenchao Xia
  Cc: aliguori, chenwj, armbru, qemu-devel, pbonzini, Luiz Capitulino

于 2013-1-10 11:30, Wenchao Xia 写道:
>>> -static const mon_cmd_t *qmp_find_cmd(const char *cmdname)
>>> -{
>>> -    return search_dispatch_table(qmp_cmds, cmdname);
>>> -}
>>
>> Touching qmp_find_cmd() is unrelated to this series.
>>
>    OK.
>
>>> -
>>> -static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>>> -                                              const char *cmdline,
>>> -                                              QDict *qdict)
>>> +/*
>>> + * Try find the target mon_cmd_t instance, if it have sub_table and
>>> have string
>>> + * for it, this function will try search it with remain string, and
>>> if not
>>> + * found it return NULL.
>>> + */
>>> +static const mon_cmd_t *hmp_parse_command(Monitor *mon,
>>> +                                          const char *cmdline,
>>> +                                          mon_cmd_t *table,
>>> +                                          QDict *qdict)
>>
>> Renaming is also not necessary, but if you do it please do in a different
>> patch.
>>
>    OK.
>
>>>   {
>>> +        }
>>> +        return hmp_parse_command(mon, p, cmd->sub_table, qdict);
>>
>> This works, and seems less hack then the previous attempt.
>>
>> Except for a small detail. This:
>>
>>    (qemu) info bla
>>
>> Currently returns a list of info commands, but with this series it
>> returns
>> an error.
>>
>    Yep, I think command completion is another topic, but I am pretty
> sure it can be solved in this framework, for that it is not hack and
> what need to do is, let command completion function knows there may be
> another sub layer too.
>
   Sorry I missed your point. Returning an error instead of info
commands, is made on purpose. IMHO always print the commands
make user confused, message tipping what is wrong is better.

>> I'd like to get a reviewed-by from Markus before applying this as,
>> besides
>> this being his idea, I honestly have a very hard time knowing how to
>> move old
>> hmp hackery forward.
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH V3 2/4] HMP: add infrastructure for sub command
  2013-01-10  7:07       ` wenchao xia
@ 2013-01-10  9:23         ` Markus Armbruster
  0 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2013-01-10  9:23 UTC (permalink / raw)
  To: wenchao xia
  Cc: aliguori, chenwj, qemu-devel, Luiz Capitulino, pbonzini,
	Wenchao Xia

wenchao xia <xiaxia347os@163.com> writes:

> 于 2013-1-10 11:30, Wenchao Xia 写道:
>>>> -static const mon_cmd_t *qmp_find_cmd(const char *cmdname)
>>>> -{
>>>> -    return search_dispatch_table(qmp_cmds, cmdname);
>>>> -}
>>>
>>> Touching qmp_find_cmd() is unrelated to this series.
>>>
>>    OK.
>>
>>>> -
>>>> -static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>>>> -                                              const char *cmdline,
>>>> -                                              QDict *qdict)
>>>> +/*
>>>> + * Try find the target mon_cmd_t instance, if it have sub_table and
>>>> have string
>>>> + * for it, this function will try search it with remain string, and
>>>> if not
>>>> + * found it return NULL.
>>>> + */
>>>> +static const mon_cmd_t *hmp_parse_command(Monitor *mon,
>>>> +                                          const char *cmdline,
>>>> +                                          mon_cmd_t *table,
>>>> +                                          QDict *qdict)
>>>
>>> Renaming is also not necessary, but if you do it please do in a different
>>> patch.
>>>
>>    OK.
>>
>>>>   {
>>>> +        }
>>>> +        return hmp_parse_command(mon, p, cmd->sub_table, qdict);
>>>
>>> This works, and seems less hack then the previous attempt.
>>>
>>> Except for a small detail. This:
>>>
>>>    (qemu) info bla
>>>
>>> Currently returns a list of info commands, but with this series it
>>> returns
>>> an error.
>>>
>>    Yep, I think command completion is another topic, but I am pretty
>> sure it can be solved in this framework, for that it is not hack and
>> what need to do is, let command completion function knows there may be
>> another sub layer too.
>>
>   Sorry I missed your point. Returning an error instead of info
> commands, is made on purpose. IMHO always print the commands
> make user confused, message tipping what is wrong is better.

I agree with the change, but it needs to be mentioned in the commit
message.  Separate patch would be even better.

>>> I'd like to get a reviewed-by from Markus before applying this as,
>>> besides
>>> this being his idea, I honestly have a very hard time knowing how to
>>> move old
>>> hmp hackery forward.

I'll review v4.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-01-10  9:24 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-27  9:40 [Qemu-devel] [PATCH V3 0/4] HMP: allow parsing for sub command Wenchao Xia
2012-12-27  9:40 ` [Qemu-devel] [PATCH V3 1/4] HMP: add QDict to info callback handler Wenchao Xia
2012-12-27  9:40 ` [Qemu-devel] [PATCH V3 2/4] HMP: add infrastructure for sub command Wenchao Xia
2013-01-09 12:57   ` Luiz Capitulino
2013-01-10  3:30     ` Wenchao Xia
2013-01-10  7:07       ` wenchao xia
2013-01-10  9:23         ` Markus Armbruster
2012-12-27  9:40 ` [Qemu-devel] [PATCH V3 3/4] HMP: move define of mon_cmds Wenchao Xia
2012-12-27  9:40 ` [Qemu-devel] [PATCH V3 4/4] HMP: add sub command table to info Wenchao Xia
2013-01-04  3:00 ` [Qemu-devel] [PATCH V3 0/4] HMP: allow parsing for sub command Wenchao Xia

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).