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

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

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 function declaration and better doc according to comments.
  Removed the patch about info snapshots, which will goto another serial.
V3:
  Split out code moving patch.
V4:
  Removed change of qmp_find_cmd().
  Removed name change of monitor_parse_command().

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               |   95 ++++++++++++++++++++++------------------------
 net/net.c               |    2 +-
 net/slirp.c             |    2 +-
 savevm.c                |    2 +-
 vl.c                    |    2 +-
 26 files changed, 115 insertions(+), 118 deletions(-)

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

* [Qemu-devel] [PATCH V4 1/4] HMP: add QDict to info callback handler
  2013-01-10  8:32 [Qemu-devel] [PATCH V4 0/4] HMP: allow parsing for sub command Wenchao Xia
@ 2013-01-10  8:32 ` Wenchao Xia
  2013-01-10 13:10   ` Luiz Capitulino
  2013-01-10  8:32 ` [Qemu-devel] [PATCH V4 2/4] HMP: add infrastructure for sub command Wenchao Xia
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Wenchao Xia @ 2013-01-10  8:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, chenwj, armbru, lcapitulino, 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>
---
 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 4134aa9..fbcf43d 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 50c297f..bc42639 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -435,7 +435,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 180d1d7..01b7ac0 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -542,7 +542,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 529d60e..021420f 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2333,7 +2333,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 79e5122..085e950 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] 15+ messages in thread

* [Qemu-devel] [PATCH V4 2/4] HMP: add infrastructure for sub command
  2013-01-10  8:32 [Qemu-devel] [PATCH V4 0/4] HMP: allow parsing for sub command Wenchao Xia
  2013-01-10  8:32 ` [Qemu-devel] [PATCH V4 1/4] HMP: add QDict to info callback handler Wenchao Xia
@ 2013-01-10  8:32 ` Wenchao Xia
  2013-01-10 10:18   ` Markus Armbruster
                     ` (2 more replies)
  2013-01-10  8:32 ` [Qemu-devel] [PATCH V4 3/4] HMP: move define of mon_cmds Wenchao Xia
  2013-01-10  8:32 ` [Qemu-devel] [PATCH V4 4/4] HMP: add sub command table to info Wenchao Xia
  3 siblings, 3 replies; 15+ messages in thread
From: Wenchao Xia @ 2013-01-10  8:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, chenwj, armbru, lcapitulino, 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 |   31 +++++++++++++++++++++++--------
 1 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/monitor.c b/monitor.c
index c7b3014..d49a362 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,22 @@ 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);
 }
 
+/*
+ * 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 remaining string, and if not
+ * found it return NULL.
+ */
 static const mon_cmd_t *monitor_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 +3566,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 monitor_parse_command(mon, p, cmd->sub_table, qdict);
+    }
+
     /* parse the parameters */
     typestr = cmd->args_type;
     for(;;) {
@@ -3925,7 +3940,7 @@ static void handle_user_command(Monitor *mon, const char *cmdline)
 
     qdict = qdict_new();
 
-    cmd = monitor_parse_command(mon, cmdline, qdict);
+    cmd = monitor_parse_command(mon, cmdline, mon_cmds, qdict);
     if (!cmd)
         goto out;
 
-- 
1.7.1

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

* [Qemu-devel] [PATCH V4 3/4] HMP: move define of mon_cmds
  2013-01-10  8:32 [Qemu-devel] [PATCH V4 0/4] HMP: allow parsing for sub command Wenchao Xia
  2013-01-10  8:32 ` [Qemu-devel] [PATCH V4 1/4] HMP: add QDict to info callback handler Wenchao Xia
  2013-01-10  8:32 ` [Qemu-devel] [PATCH V4 2/4] HMP: add infrastructure for sub command Wenchao Xia
@ 2013-01-10  8:32 ` Wenchao Xia
  2013-01-10  8:32 ` [Qemu-devel] [PATCH V4 4/4] HMP: add sub command table to info Wenchao Xia
  3 siblings, 0 replies; 15+ messages in thread
From: Wenchao Xia @ 2013-01-10  8:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, chenwj, armbru, lcapitulino, 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 d49a362..3657224 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] 15+ messages in thread

* [Qemu-devel] [PATCH V4 4/4] HMP: add sub command table to info
  2013-01-10  8:32 [Qemu-devel] [PATCH V4 0/4] HMP: allow parsing for sub command Wenchao Xia
                   ` (2 preceding siblings ...)
  2013-01-10  8:32 ` [Qemu-devel] [PATCH V4 3/4] HMP: move define of mon_cmds Wenchao Xia
@ 2013-01-10  8:32 ` Wenchao Xia
  2013-01-10 10:24   ` Markus Armbruster
  3 siblings, 1 reply; 15+ messages in thread
From: Wenchao Xia @ 2013-01-10  8:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, chenwj, armbru, lcapitulino, 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 3657224..9daaa63 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] 15+ messages in thread

* Re: [Qemu-devel] [PATCH V4 2/4] HMP: add infrastructure for sub command
  2013-01-10  8:32 ` [Qemu-devel] [PATCH V4 2/4] HMP: add infrastructure for sub command Wenchao Xia
@ 2013-01-10 10:18   ` Markus Armbruster
  2013-01-11  3:11     ` Wenchao Xia
  2013-01-10 10:23   ` Markus Armbruster
  2013-01-10 13:12   ` Luiz Capitulino
  2 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2013-01-10 10:18 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: aliguori, qemu-devel, chenwj, lcapitulino

Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:

>   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 |   31 +++++++++++++++++++++++--------
>  1 files changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index c7b3014..d49a362 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,22 @@ 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);
>  }
>  
> +/*
> + * 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 remaining string, and if not
> + * found it return NULL.
> + */

Thanks for going the extra mile and document the function.  What about:

   /*
    * Parse @cmdline according to command table @table.
    * If @cmdline is blank, return NULL.
    * If it can't be parsed, report to @mon, and return NULL.
    * Else, insert command arguments into @qdict, and return the command.
    * Do not assume the returned command points into @table!  It doesn't
    * when the command is a sub-command.
    */

>  static const mon_cmd_t *monitor_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 +3566,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 monitor_parse_command(mon, p, cmd->sub_table, qdict);
> +    }
> +
>      /* parse the parameters */
>      typestr = cmd->args_type;
>      for(;;) {

The check whether non-space characters follow is awkward.  We need it
only because we want to handle "@cmdline is blank" differently than "it
can't be parsed", but monitor_parse_command() returns NULL for both
cases.  If we care, we can try to do better in a follow-up patch.

We'll get sub-optimal error messages when the infrastructure is put to
use in patch 4/4, e.g.:

    (qemu) info apple pie
    unknown command: 'apple'

That's because cmdname is just "apple" at the place where the error is
diagnosed.

Several other messages also print cmdname, and thus are similarly
affected.  Many of them could probably just omit printing it, but that's
a separate issue.

A possible way to improve this: additional parameter int start, parse
starts at cmdline + start, and instead of printing cmdname, print
cmdline up to end of cmdname.  Something like

        monitor_printf(mon, "unknown command: '%.*s'\n",
                       (int)(p - cmdline), cmdline);

> @@ -3925,7 +3940,7 @@ static void handle_user_command(Monitor *mon, const char *cmdline)
>  
>      qdict = qdict_new();
>  
> -    cmd = monitor_parse_command(mon, cmdline, qdict);
> +    cmd = monitor_parse_command(mon, cmdline, mon_cmds, qdict);
>      if (!cmd)
>          goto out;

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

* Re: [Qemu-devel] [PATCH V4 2/4] HMP: add infrastructure for sub command
  2013-01-10  8:32 ` [Qemu-devel] [PATCH V4 2/4] HMP: add infrastructure for sub command Wenchao Xia
  2013-01-10 10:18   ` Markus Armbruster
@ 2013-01-10 10:23   ` Markus Armbruster
  2013-01-11  3:11     ` Wenchao Xia
  2013-01-10 13:12   ` Luiz Capitulino
  2 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2013-01-10 10:23 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: aliguori, qemu-devel, chenwj, lcapitulino

Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:

>   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 |   31 +++++++++++++++++++++++--------
>  1 files changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index c7b3014..d49a362 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 */

Forgot to mention: please explain in a comment how mhandler and
sub_table interact.  Namely, if there are arguments, we use sub_table,
else mhandler.

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

* Re: [Qemu-devel] [PATCH V4 4/4] HMP: add sub command table to info
  2013-01-10  8:32 ` [Qemu-devel] [PATCH V4 4/4] HMP: add sub command table to info Wenchao Xia
@ 2013-01-10 10:24   ` Markus Armbruster
  2013-01-11  3:13     ` Wenchao Xia
  0 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2013-01-10 10:24 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: aliguori, qemu-devel, chenwj, lcapitulino

Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:

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

Commit message should document that info <unknown-topic> no longer lists
the topics.

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

* Re: [Qemu-devel] [PATCH V4 1/4] HMP: add QDict to info callback handler
  2013-01-10  8:32 ` [Qemu-devel] [PATCH V4 1/4] HMP: add QDict to info callback handler Wenchao Xia
@ 2013-01-10 13:10   ` Luiz Capitulino
  2013-01-11  3:16     ` Wenchao Xia
  0 siblings, 1 reply; 15+ messages in thread
From: Luiz Capitulino @ 2013-01-10 13:10 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: aliguori, qemu-devel, chenwj, armbru

On Thu, 10 Jan 2013 16:32:31 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

> 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);

Is it possible to eliminate 'info' and use 'cmd' instead?

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

* Re: [Qemu-devel] [PATCH V4 2/4] HMP: add infrastructure for sub command
  2013-01-10  8:32 ` [Qemu-devel] [PATCH V4 2/4] HMP: add infrastructure for sub command Wenchao Xia
  2013-01-10 10:18   ` Markus Armbruster
  2013-01-10 10:23   ` Markus Armbruster
@ 2013-01-10 13:12   ` Luiz Capitulino
  2013-01-11  3:17     ` Wenchao Xia
  2 siblings, 1 reply; 15+ messages in thread
From: Luiz Capitulino @ 2013-01-10 13:12 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: aliguori, qemu-devel, chenwj, armbru

On Thu, 10 Jan 2013 16:32:32 +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().

You don't qmp_find_cmd() anymore..

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

* Re: [Qemu-devel] [PATCH V4 2/4] HMP: add infrastructure for sub command
  2013-01-10 10:18   ` Markus Armbruster
@ 2013-01-11  3:11     ` Wenchao Xia
  0 siblings, 0 replies; 15+ messages in thread
From: Wenchao Xia @ 2013-01-11  3:11 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: aliguori, qemu-devel, chenwj, lcapitulino

> Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:
> 
>>    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 |   31 +++++++++++++++++++++++--------
>>   1 files changed, 23 insertions(+), 8 deletions(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index c7b3014..d49a362 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,22 @@ 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);
>>   }
>>   
>> +/*
>> + * 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 remaining string, and if not
>> + * found it return NULL.
>> + */
> 
> Thanks for going the extra mile and document the function.  What about:
> 
>     /*
>      * Parse @cmdline according to command table @table.
>      * If @cmdline is blank, return NULL.
>      * If it can't be parsed, report to @mon, and return NULL.
>      * Else, insert command arguments into @qdict, and return the command.
>      * Do not assume the returned command points into @table!  It doesn't
>      * when the command is a sub-command.
>      */
> 
  OK, I'll use it in V5.

>>   static const mon_cmd_t *monitor_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 +3566,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 monitor_parse_command(mon, p, cmd->sub_table, qdict);
>> +    }
>> +
>>       /* parse the parameters */
>>       typestr = cmd->args_type;
>>       for(;;) {
> 
> The check whether non-space characters follow is awkward.  We need it
> only because we want to handle "@cmdline is blank" differently than "it
> can't be parsed", but monitor_parse_command() returns NULL for both
> cases.  If we care, we can try to do better in a follow-up patch.
> 
  Yes it is checked to avoid getting NULL for case "info ". I'll split
up a patch for the check code.

> We'll get sub-optimal error messages when the infrastructure is put to
> use in patch 4/4, e.g.:
> 
>      (qemu) info apple pie
>      unknown command: 'apple'
> 
> That's because cmdname is just "apple" at the place where the error is
> diagnosed.
> 
> Several other messages also print cmdname, and thus are similarly
> affected.  Many of them could probably just omit printing it, but that's
> a separate issue.
> 
> A possible way to improve this: additional parameter int start, parse
> starts at cmdline + start, and instead of printing cmdname, print
> cmdline up to end of cmdname.  Something like
> 
>          monitor_printf(mon, "unknown command: '%.*s'\n",
>                         (int)(p - cmdline), cmdline);
> 
  Thanks, I think this could work.


>> @@ -3925,7 +3940,7 @@ static void handle_user_command(Monitor *mon, const char *cmdline)
>>   
>>       qdict = qdict_new();
>>   
>> -    cmd = monitor_parse_command(mon, cmdline, qdict);
>> +    cmd = monitor_parse_command(mon, cmdline, mon_cmds, qdict);
>>       if (!cmd)
>>           goto out;
> 


-- 
Best Regards

Wenchao Xia

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

* Re: [Qemu-devel] [PATCH V4 2/4] HMP: add infrastructure for sub command
  2013-01-10 10:23   ` Markus Armbruster
@ 2013-01-11  3:11     ` Wenchao Xia
  0 siblings, 0 replies; 15+ messages in thread
From: Wenchao Xia @ 2013-01-11  3:11 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: aliguori, qemu-devel, chenwj, lcapitulino

于 2013-1-10 18:23, Markus Armbruster 写道:
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:
> 
>>    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 |   31 +++++++++++++++++++++++--------
>>   1 files changed, 23 insertions(+), 8 deletions(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index c7b3014..d49a362 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 */
> 
> Forgot to mention: please explain in a comment how mhandler and
> sub_table interact.  Namely, if there are arguments, we use sub_table,
> else mhandler.
> 
OK.

-- 
Best Regards

Wenchao Xia

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

* Re: [Qemu-devel] [PATCH V4 4/4] HMP: add sub command table to info
  2013-01-10 10:24   ` Markus Armbruster
@ 2013-01-11  3:13     ` Wenchao Xia
  0 siblings, 0 replies; 15+ messages in thread
From: Wenchao Xia @ 2013-01-11  3:13 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: aliguori, qemu-devel, chenwj, lcapitulino

于 2013-1-10 18:24, Markus Armbruster 写道:
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:
> 
>>    Now info command takes a table of sub info commands,
>> and changed do_info() to do_info_help() to do help funtion
>> only.
> 
> Commit message should document that info <unknown-topic> no longer lists
> the topics.
> 
  OK.


-- 
Best Regards

Wenchao Xia

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

* Re: [Qemu-devel] [PATCH V4 1/4] HMP: add QDict to info callback handler
  2013-01-10 13:10   ` Luiz Capitulino
@ 2013-01-11  3:16     ` Wenchao Xia
  0 siblings, 0 replies; 15+ messages in thread
From: Wenchao Xia @ 2013-01-11  3:16 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: aliguori, qemu-devel, chenwj, armbru

于 2013-1-10 21:10, Luiz Capitulino 写道:
> On Thu, 10 Jan 2013 16:32:31 +0800
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>
>> 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);
>
> Is it possible to eliminate 'info' and use 'cmd' instead?
>
   yes, now info is not a special case but a sub command.
If you agree I'll delete info function.

-- 
Best Regards

Wenchao Xia

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

* Re: [Qemu-devel] [PATCH V4 2/4] HMP: add infrastructure for sub command
  2013-01-10 13:12   ` Luiz Capitulino
@ 2013-01-11  3:17     ` Wenchao Xia
  0 siblings, 0 replies; 15+ messages in thread
From: Wenchao Xia @ 2013-01-11  3:17 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: aliguori, qemu-devel, chenwj, armbru

于 2013-1-10 21:12, Luiz Capitulino 写道:
> On Thu, 10 Jan 2013 16:32:32 +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().
>
> You don't qmp_find_cmd() anymore..
>
   Sorry I have forgot change that, will mention it in next version.

-- 
Best Regards

Wenchao Xia

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

end of thread, other threads:[~2013-01-11  3:17 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-10  8:32 [Qemu-devel] [PATCH V4 0/4] HMP: allow parsing for sub command Wenchao Xia
2013-01-10  8:32 ` [Qemu-devel] [PATCH V4 1/4] HMP: add QDict to info callback handler Wenchao Xia
2013-01-10 13:10   ` Luiz Capitulino
2013-01-11  3:16     ` Wenchao Xia
2013-01-10  8:32 ` [Qemu-devel] [PATCH V4 2/4] HMP: add infrastructure for sub command Wenchao Xia
2013-01-10 10:18   ` Markus Armbruster
2013-01-11  3:11     ` Wenchao Xia
2013-01-10 10:23   ` Markus Armbruster
2013-01-11  3:11     ` Wenchao Xia
2013-01-10 13:12   ` Luiz Capitulino
2013-01-11  3:17     ` Wenchao Xia
2013-01-10  8:32 ` [Qemu-devel] [PATCH V4 3/4] HMP: move define of mon_cmds Wenchao Xia
2013-01-10  8:32 ` [Qemu-devel] [PATCH V4 4/4] HMP: add sub command table to info Wenchao Xia
2013-01-10 10:24   ` Markus Armbruster
2013-01-11  3:13     ` 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).