* [Qemu-devel] [PATCH 0/3] HMP: enable info sub command taking parameter
@ 2012-12-19 10:17 Wenchao Xia
2012-12-19 10:17 ` [Qemu-devel] [PATCH 1/3] HMP: add QDict to info callback handler Wenchao Xia
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Wenchao Xia @ 2012-12-19 10:17 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, aliguori, stefanha, armbru, lcapitulino, pbonzini, Wenchao Xia
This serial of patch will enhance HMP's info command, make all command
possible to take additional parameter. Please review to see if it have
problem.
Wenchao Xia (3):
HMP: add QDict to info callback handler
HMP: pass in parameter for info sub command
HMP: show internal snapshots on a single device
hmp-commands.hx | 2 +-
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 +-
hw/usb/host-stub.c | 2 +-
monitor.c | 115 +++++++++++++++++++++++++++++++++------------------
net.c | 2 +-
net.h | 2 +-
net/slirp.c | 2 +-
net/slirp.h | 2 +-
savevm.c | 55 ++++++++++++++++++++++++-
sysemu.h | 4 +-
vl.c | 2 +-
27 files changed, 197 insertions(+), 110 deletions(-)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 1/3] HMP: add QDict to info callback handler
2012-12-19 10:17 [Qemu-devel] [PATCH 0/3] HMP: enable info sub command taking parameter Wenchao Xia
@ 2012-12-19 10:17 ` Wenchao Xia
2012-12-21 14:01 ` Markus Armbruster
2012-12-19 10:17 ` [Qemu-devel] [PATCH 2/3] HMP: pass in parameter for info sub command Wenchao Xia
2012-12-19 10:17 ` [Qemu-devel] [PATCH 3/3] HMP: show internal snapshots on a single device Wenchao Xia
2 siblings, 1 reply; 14+ messages in thread
From: Wenchao Xia @ 2012-12-19 10:17 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, aliguori, stefanha, armbru, lcapitulino, pbonzini, Wenchao Xia
This patch change all info call back function to take
additional QDict * parameter, which allow those command
take parameter.
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 +-
hw/usb/host-stub.c | 2 +-
monitor.c | 32 ++++++++++++++++----------------
net.c | 2 +-
net.h | 2 +-
net/slirp.c | 2 +-
net/slirp.h | 2 +-
savevm.c | 2 +-
sysemu.h | 4 ++--
vl.c | 2 +-
26 files changed, 85 insertions(+), 84 deletions(-)
diff --git a/hmp.c b/hmp.c
index 180ba2b..0aab1d8 100644
--- a/hmp.c
+++ b/hmp.c
@@ -30,7 +30,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;
@@ -41,7 +41,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;
@@ -54,7 +54,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;
@@ -69,7 +69,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;
@@ -88,7 +88,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;
@@ -97,7 +97,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;
@@ -110,7 +110,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;
@@ -130,7 +130,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;
@@ -208,7 +208,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;
@@ -227,13 +227,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;
@@ -271,7 +271,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;
@@ -325,7 +325,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;
@@ -359,7 +359,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;
@@ -405,7 +405,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;
@@ -452,7 +452,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;
@@ -569,7 +569,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;
@@ -592,7 +592,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 0ab03be..c3f3a3f 100644
--- a/hmp.h
+++ b/hmp.h
@@ -18,24 +18,24 @@
#include "qapi-types.h"
#include "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 af0ba4d..a143039 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 32f65db..bec1fee 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 ba01ca6..d1e7bf2 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..d2b63f6 100644
--- a/hw/loader.h
+++ b/hw/loader.h
@@ -1,5 +1,6 @@
#ifndef LOADER_H
#define LOADER_H
+#include "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 2237e86..e284892 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 50648c9..3db98be 100644
--- a/hw/pcmcia.h
+++ b/hw/pcmcia.h
@@ -11,7 +11,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 a1b4d6a..9a473a2 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -563,13 +563,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 220ceba..ada9e15 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 52cf82b..de52b5e 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 efae65d..5116ee9 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 8264c24..13b813b 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 dae0009..791c905 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 bdafb6b..62de0d6 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/hw/usb/host-stub.c b/hw/usb/host-stub.c
index b4e10c1..7907841 100644
--- a/hw/usb/host-stub.c
+++ b/hw/usb/host-stub.c
@@ -35,7 +35,7 @@
#include "hw/usb.h"
#include "monitor.h"
-void usb_host_info(Monitor *mon)
+void usb_host_info(Monitor *mon, const QDict *qdict)
{
monitor_printf(mon, "USB host devices not supported\n");
}
diff --git a/monitor.c b/monitor.c
index c0e32d6..797680f 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.c b/net.c
index e8ae13e..49735d1 100644
--- a/net.c
+++ b/net.c
@@ -851,7 +851,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.h b/net.h
index 04fda1d..83f38ef 100644
--- a/net.h
+++ b/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/net/slirp.c b/net/slirp.c
index afb52c3..8e3ec77 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -669,7 +669,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/net/slirp.h b/net/slirp.h
index 2ca09b6..051b7c7 100644
--- a/net/slirp.h
+++ b/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/savevm.c b/savevm.c
index 5d04d59..fa32171 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2358,7 +2358,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/sysemu.h b/sysemu.h
index 1b6add2..93147e1 100644
--- a/sysemu.h
+++ b/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);
@@ -170,7 +170,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/vl.c b/vl.c
index 3ebf01f..7263a02 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] 14+ messages in thread
* [Qemu-devel] [PATCH 2/3] HMP: pass in parameter for info sub command
2012-12-19 10:17 [Qemu-devel] [PATCH 0/3] HMP: enable info sub command taking parameter Wenchao Xia
2012-12-19 10:17 ` [Qemu-devel] [PATCH 1/3] HMP: add QDict to info callback handler Wenchao Xia
@ 2012-12-19 10:17 ` Wenchao Xia
2012-12-19 18:07 ` Luiz Capitulino
2012-12-19 10:17 ` [Qemu-devel] [PATCH 3/3] HMP: show internal snapshots on a single device Wenchao Xia
2 siblings, 1 reply; 14+ messages in thread
From: Wenchao Xia @ 2012-12-19 10:17 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, aliguori, stefanha, armbru, lcapitulino, pbonzini, Wenchao Xia
This patch enable sub info command handler getting meaningful
parameter.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
hmp-commands.hx | 2 +-
monitor.c | 79 +++++++++++++++++++++++++++++++++++++++----------------
2 files changed, 57 insertions(+), 24 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 010b8c9..667fab8 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1486,7 +1486,7 @@ ETEXI
{
.name = "info",
- .args_type = "item:s?",
+ .args_type = "item:S?",
.params = "[subcommand]",
.help = "show various information about the system state",
.mhandler.cmd = do_info,
diff --git a/monitor.c b/monitor.c
index 797680f..ce0e74d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -464,6 +464,11 @@ QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) != QEVENT_MAX)
MonitorEventState monitor_event_state[QEVENT_MAX];
QemuMutex monitor_event_state_lock;
+static const mon_cmd_t *monitor_parse_command(Monitor *mon,
+ const char *cmdline,
+ const mon_cmd_t *table,
+ QDict *qdict);
+
/*
* Emits the event to every monitor instance
*/
@@ -809,26 +814,29 @@ static void user_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
static void do_info(Monitor *mon, const QDict *qdict)
{
const mon_cmd_t *cmd;
+ QDict *qdict_info;
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;
- }
+ qdict_info = qdict_new();
- if (cmd->name == NULL) {
- goto help;
+ cmd = monitor_parse_command(mon, item, info_cmds, qdict_info);
+ if (!cmd) {
+ QDECREF(qdict_info);
+ /* don't help here, to avoid error message got ignored */
+ return;
}
- cmd->mhandler.info(mon, NULL);
+ cmd->mhandler.info(mon, qdict_info);
+ QDECREF(qdict_info);
return;
help:
help_cmd(mon, "info");
+ return;
}
CommandInfoList *qmp_query_commands(Error **errp)
@@ -3534,18 +3542,15 @@ 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)
+static const mon_cmd_t *monitor_find_command(const char *cmdname,
+ const mon_cmd_t *table)
{
- 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);
+ return search_dispatch_table(table, cmdname);
}
static const mon_cmd_t *monitor_parse_command(Monitor *mon,
const char *cmdline,
+ const mon_cmd_t *table,
QDict *qdict)
{
const char *p, *typestr;
@@ -3564,7 +3569,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
if (!p)
return NULL;
- cmd = monitor_find_command(cmdname);
+ cmd = monitor_find_command(cmdname, table);
if (!cmd) {
monitor_printf(mon, "unknown command: '%s'\n", cmdname);
return NULL;
@@ -3872,6 +3877,31 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
}
}
break;
+ case 'S':
+ {
+ /* package all remaining string */
+ int len;
+
+ while (qemu_isspace(*p)) {
+ p++;
+ }
+ if (*typestr == '?') {
+ typestr++;
+ if (*p == '\0') {
+ /* no remaining string: NULL argument */
+ break;
+ }
+ }
+ len = strlen(p);
+ if (len <= 0) {
+ monitor_printf(mon, "%s: string expected\n",
+ cmdname);
+ break;
+ }
+ qdict_put(qdict, key, qstring_from_str(p));
+ p += len;
+ }
+ break;
default:
bad_type:
monitor_printf(mon, "%s: unknown type '%c'\n", cmdname, c);
@@ -3925,7 +3955,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;
@@ -4144,12 +4174,7 @@ static void monitor_find_completion(const char *cmdline)
break;
case 's':
/* XXX: more generic ? */
- if (!strcmp(cmd->name, "info")) {
- readline_set_completion_index(cur_mon->rs, strlen(str));
- for(cmd = info_cmds; cmd->name != NULL; cmd++) {
- cmd_completion(str, cmd->name);
- }
- } else if (!strcmp(cmd->name, "sendkey")) {
+ if (!strcmp(cmd->name, "sendkey")) {
char *sep = strrchr(str, '-');
if (sep)
str = sep + 1;
@@ -4163,6 +4188,14 @@ static void monitor_find_completion(const char *cmdline)
cmd_completion(str, cmd->name);
}
}
+ case 'S':
+ /* generic string packaged */
+ if (!strcmp(cmd->name, "info")) {
+ readline_set_completion_index(cur_mon->rs, strlen(str));
+ for (cmd = info_cmds; cmd->name != NULL; cmd++) {
+ cmd_completion(str, cmd->name);
+ }
+ }
break;
default:
break;
@@ -4483,7 +4516,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
goto err_out;
}
- cmd = qmp_find_cmd(cmd_name);
+ cmd = monitor_find_command(cmd_name, qmp_cmds);
if (!cmd) {
qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
goto err_out;
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 3/3] HMP: show internal snapshots on a single device
2012-12-19 10:17 [Qemu-devel] [PATCH 0/3] HMP: enable info sub command taking parameter Wenchao Xia
2012-12-19 10:17 ` [Qemu-devel] [PATCH 1/3] HMP: add QDict to info callback handler Wenchao Xia
2012-12-19 10:17 ` [Qemu-devel] [PATCH 2/3] HMP: pass in parameter for info sub command Wenchao Xia
@ 2012-12-19 10:17 ` Wenchao Xia
2012-12-21 14:07 ` Markus Armbruster
2 siblings, 1 reply; 14+ messages in thread
From: Wenchao Xia @ 2012-12-19 10:17 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, aliguori, stefanha, armbru, lcapitulino, pbonzini, Wenchao Xia
This patch add an option to show snapshots on a single block
device, so some snapshot do not exist on other block device
could be shown.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
monitor.c | 6 +++---
savevm.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 57 insertions(+), 4 deletions(-)
diff --git a/monitor.c b/monitor.c
index ce0e74d..b019618 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2613,9 +2613,9 @@ static mon_cmd_t info_cmds[] = {
},
{
.name = "snapshots",
- .args_type = "",
- .params = "",
- .help = "show the currently saved VM snapshots",
+ .args_type = "device:B?",
+ .params = "[device]",
+ .help = "show snapshots of whole vm or a single device",
.mhandler.info = do_info_snapshots,
},
{
diff --git a/savevm.c b/savevm.c
index fa32171..438eb24 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2358,7 +2358,7 @@ void do_delvm(Monitor *mon, const QDict *qdict)
}
}
-void do_info_snapshots(Monitor *mon, const QDict *qdict)
+static void do_info_snapshots_vm(Monitor *mon)
{
BlockDriverState *bs, *bs1;
QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = &s;
@@ -2422,6 +2422,59 @@ void do_info_snapshots(Monitor *mon, const QDict *qdict)
}
+static void do_info_snapshots_blk(Monitor *mon, const char *device)
+{
+ BlockDriverState *bs;
+ QEMUSnapshotInfo *sn_tab, *sn;
+ int nb_sns, i;
+ char buf[256];
+
+ /* find the target bs */
+ bs = bdrv_find(device);
+ if (!bs) {
+ monitor_printf(mon, "Device '%s' not found.\n", device);
+ return ;
+ }
+
+ if (!bdrv_can_snapshot(bs)) {
+ monitor_printf(mon, "Device '%s' can't have snapshot.\n", device);
+ return ;
+ }
+
+ nb_sns = bdrv_snapshot_list(bs, &sn_tab);
+ if (nb_sns < 0) {
+ monitor_printf(mon, "Device %s bdrv_snapshot_list: error %d\n",
+ device, nb_sns);
+ return;
+ }
+
+ if (nb_sns == 0) {
+ monitor_printf(mon, "There is no snapshot available.\n");
+ return;
+ }
+
+ monitor_printf(mon, "Device %s:\n", device);
+ monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+ for (i = 0; i < nb_sns; i++) {
+ sn = &sn_tab[i];
+ monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
+ }
+ g_free(sn_tab);
+ return;
+}
+
+void do_info_snapshots(Monitor *mon, const QDict *qdict)
+{
+ /* Todo, there should be a layer rebuild qdict before enter this func. */
+ const char *device = qdict_get_try_str(qdict, "device");
+ if (!device) {
+ do_info_snapshots_vm(mon);
+ } else {
+ do_info_snapshots_blk(mon, device);
+ }
+ return;
+}
+
void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
{
qemu_ram_set_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK,
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] HMP: pass in parameter for info sub command
2012-12-19 10:17 ` [Qemu-devel] [PATCH 2/3] HMP: pass in parameter for info sub command Wenchao Xia
@ 2012-12-19 18:07 ` Luiz Capitulino
2012-12-20 3:02 ` Wenchao Xia
2012-12-21 14:49 ` Markus Armbruster
0 siblings, 2 replies; 14+ messages in thread
From: Luiz Capitulino @ 2012-12-19 18:07 UTC (permalink / raw)
To: Wenchao Xia; +Cc: kwolf, aliguori, stefanha, qemu-devel, armbru, pbonzini
On Wed, 19 Dec 2012 18:17:09 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
> This patch enable sub info command handler getting meaningful
> parameter.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
> hmp-commands.hx | 2 +-
> monitor.c | 79 +++++++++++++++++++++++++++++++++++++++----------------
> 2 files changed, 57 insertions(+), 24 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 010b8c9..667fab8 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1486,7 +1486,7 @@ ETEXI
>
> {
> .name = "info",
> - .args_type = "item:s?",
> + .args_type = "item:S?",
> .params = "[subcommand]",
> .help = "show various information about the system state",
> .mhandler.cmd = do_info,
> diff --git a/monitor.c b/monitor.c
> index 797680f..ce0e74d 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -464,6 +464,11 @@ QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) != QEVENT_MAX)
> MonitorEventState monitor_event_state[QEVENT_MAX];
> QemuMutex monitor_event_state_lock;
>
> +static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> + const char *cmdline,
> + const mon_cmd_t *table,
> + QDict *qdict);
Please, move the function instead.
> +
> /*
> * Emits the event to every monitor instance
> */
> @@ -809,26 +814,29 @@ static void user_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
> static void do_info(Monitor *mon, const QDict *qdict)
> {
> const mon_cmd_t *cmd;
> + QDict *qdict_info;
> 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;
> - }
> + qdict_info = qdict_new();
>
> - if (cmd->name == NULL) {
> - goto help;
> + cmd = monitor_parse_command(mon, item, info_cmds, qdict_info);
> + if (!cmd) {
> + QDECREF(qdict_info);
> + /* don't help here, to avoid error message got ignored */
> + return;
> }
>
> - cmd->mhandler.info(mon, NULL);
> + cmd->mhandler.info(mon, qdict_info);
> + QDECREF(qdict_info);
> return;
>
> help:
> help_cmd(mon, "info");
> + return;
> }
>
> CommandInfoList *qmp_query_commands(Error **errp)
> @@ -3534,18 +3542,15 @@ 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)
> +static const mon_cmd_t *monitor_find_command(const char *cmdname,
> + const mon_cmd_t *table)
> {
> - 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);
> + return search_dispatch_table(table, cmdname);
Please, keep only search_dispatch_table().
Actually, maybe you could change handle_qmp_command() to just loop through
command names and compare them with memcpy() (in a different patch, please)
then you keep monitor_find_command() for handle_hmp_command().
> }
>
> static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> const char *cmdline,
> + const mon_cmd_t *table,
> QDict *qdict)
> {
> const char *p, *typestr;
> @@ -3564,7 +3569,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> if (!p)
> return NULL;
>
> - cmd = monitor_find_command(cmdname);
> + cmd = monitor_find_command(cmdname, table);
> if (!cmd) {
> monitor_printf(mon, "unknown command: '%s'\n", cmdname);
> return NULL;
> @@ -3872,6 +3877,31 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> }
> }
> break;
> + case 'S':
> + {
> + /* package all remaining string */
> + int len;
> +
> + while (qemu_isspace(*p)) {
> + p++;
> + }
> + if (*typestr == '?') {
> + typestr++;
> + if (*p == '\0') {
> + /* no remaining string: NULL argument */
> + break;
> + }
> + }
> + len = strlen(p);
> + if (len <= 0) {
> + monitor_printf(mon, "%s: string expected\n",
> + cmdname);
> + break;
> + }
> + qdict_put(qdict, key, qstring_from_str(p));
> + p += len;
> + }
This is a true HMP-style hack :)
I don't know how to make this better though (at least not without asking you
to re-write the whole thing). Maybe Markus has a better idea?
> + break;
> default:
> bad_type:
> monitor_printf(mon, "%s: unknown type '%c'\n", cmdname, c);
> @@ -3925,7 +3955,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;
>
> @@ -4144,12 +4174,7 @@ static void monitor_find_completion(const char *cmdline)
> break;
> case 's':
> /* XXX: more generic ? */
> - if (!strcmp(cmd->name, "info")) {
> - readline_set_completion_index(cur_mon->rs, strlen(str));
> - for(cmd = info_cmds; cmd->name != NULL; cmd++) {
> - cmd_completion(str, cmd->name);
> - }
> - } else if (!strcmp(cmd->name, "sendkey")) {
> + if (!strcmp(cmd->name, "sendkey")) {
> char *sep = strrchr(str, '-');
> if (sep)
> str = sep + 1;
> @@ -4163,6 +4188,14 @@ static void monitor_find_completion(const char *cmdline)
> cmd_completion(str, cmd->name);
> }
> }
> + case 'S':
> + /* generic string packaged */
> + if (!strcmp(cmd->name, "info")) {
> + readline_set_completion_index(cur_mon->rs, strlen(str));
> + for (cmd = info_cmds; cmd->name != NULL; cmd++) {
> + cmd_completion(str, cmd->name);
> + }
> + }
> break;
> default:
> break;
> @@ -4483,7 +4516,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
> goto err_out;
> }
>
> - cmd = qmp_find_cmd(cmd_name);
> + cmd = monitor_find_command(cmd_name, qmp_cmds);
> if (!cmd) {
> qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
> goto err_out;
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] HMP: pass in parameter for info sub command
2012-12-19 18:07 ` Luiz Capitulino
@ 2012-12-20 3:02 ` Wenchao Xia
2012-12-22 21:36 ` Luiz Capitulino
2012-12-21 14:49 ` Markus Armbruster
1 sibling, 1 reply; 14+ messages in thread
From: Wenchao Xia @ 2012-12-20 3:02 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: kwolf, aliguori, stefanha, qemu-devel, armbru, pbonzini
> On Wed, 19 Dec 2012 18:17:09 +0800
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>
>> This patch enable sub info command handler getting meaningful
>> parameter.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>> hmp-commands.hx | 2 +-
>> monitor.c | 79 +++++++++++++++++++++++++++++++++++++++----------------
>> 2 files changed, 57 insertions(+), 24 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index 010b8c9..667fab8 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -1486,7 +1486,7 @@ ETEXI
>>
>> {
>> .name = "info",
>> - .args_type = "item:s?",
>> + .args_type = "item:S?",
>> .params = "[subcommand]",
>> .help = "show various information about the system state",
>> .mhandler.cmd = do_info,
>> diff --git a/monitor.c b/monitor.c
>> index 797680f..ce0e74d 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -464,6 +464,11 @@ QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) != QEVENT_MAX)
>> MonitorEventState monitor_event_state[QEVENT_MAX];
>> QemuMutex monitor_event_state_lock;
>>
>> +static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>> + const char *cmdline,
>> + const mon_cmd_t *table,
>> + QDict *qdict);
>
> Please, move the function instead.
>
There will be a bit big of code moving then, hope you would be OK with
it.
>> +
>> /*
>> * Emits the event to every monitor instance
>> */
>> @@ -809,26 +814,29 @@ static void user_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
>> static void do_info(Monitor *mon, const QDict *qdict)
>> {
>> const mon_cmd_t *cmd;
>> + QDict *qdict_info;
>> 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;
>> - }
>> + qdict_info = qdict_new();
>>
>> - if (cmd->name == NULL) {
>> - goto help;
>> + cmd = monitor_parse_command(mon, item, info_cmds, qdict_info);
>> + if (!cmd) {
>> + QDECREF(qdict_info);
>> + /* don't help here, to avoid error message got ignored */
>> + return;
>> }
>>
>> - cmd->mhandler.info(mon, NULL);
>> + cmd->mhandler.info(mon, qdict_info);
>> + QDECREF(qdict_info);
>> return;
>>
>> help:
>> help_cmd(mon, "info");
>> + return;
>> }
>>
>> CommandInfoList *qmp_query_commands(Error **errp)
>> @@ -3534,18 +3542,15 @@ 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)
>> +static const mon_cmd_t *monitor_find_command(const char *cmdname,
>> + const mon_cmd_t *table)
>> {
>> - 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);
>> + return search_dispatch_table(table, cmdname);
>
> Please, keep only search_dispatch_table().
>
OK.
> Actually, maybe you could change handle_qmp_command() to just loop through
> command names and compare them with memcpy() (in a different patch, please)
> then you keep monitor_find_command() for handle_hmp_command().
>
ah, then monitor_find_command() serve HMP only. How about rename it to
hmp_find_command(table, name)? This can have a more clear meaning. Or
just remove this funtion but use search_dispatch_table() every where?
>> }
>>
>> static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>> const char *cmdline,
>> + const mon_cmd_t *table,
>> QDict *qdict)
>> {
>> const char *p, *typestr;
>> @@ -3564,7 +3569,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>> if (!p)
>> return NULL;
>>
>> - cmd = monitor_find_command(cmdname);
>> + cmd = monitor_find_command(cmdname, table);
>> if (!cmd) {
>> monitor_printf(mon, "unknown command: '%s'\n", cmdname);
>> return NULL;
>> @@ -3872,6 +3877,31 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>> }
>> }
>> break;
>> + case 'S':
>> + {
>> + /* package all remaining string */
>> + int len;
>> +
>> + while (qemu_isspace(*p)) {
>> + p++;
>> + }
>> + if (*typestr == '?') {
>> + typestr++;
>> + if (*p == '\0') {
>> + /* no remaining string: NULL argument */
>> + break;
>> + }
>> + }
>> + len = strlen(p);
>> + if (len <= 0) {
>> + monitor_printf(mon, "%s: string expected\n",
>> + cmdname);
>> + break;
>> + }
>> + qdict_put(qdict, key, qstring_from_str(p));
>> + p += len;
>> + }
>
> This is a true HMP-style hack :)
>
> I don't know how to make this better though (at least not without asking you
> to re-write the whole thing). Maybe Markus has a better idea?
>
My idea was adding a new tag meaning "package all remaining string
into the key value", what was I can found best before. Hope you have
a better solution.
>> + break;
>> default:
>> bad_type:
>> monitor_printf(mon, "%s: unknown type '%c'\n", cmdname, c);
>> @@ -3925,7 +3955,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;
>>
>> @@ -4144,12 +4174,7 @@ static void monitor_find_completion(const char *cmdline)
>> break;
>> case 's':
>> /* XXX: more generic ? */
>> - if (!strcmp(cmd->name, "info")) {
>> - readline_set_completion_index(cur_mon->rs, strlen(str));
>> - for(cmd = info_cmds; cmd->name != NULL; cmd++) {
>> - cmd_completion(str, cmd->name);
>> - }
>> - } else if (!strcmp(cmd->name, "sendkey")) {
>> + if (!strcmp(cmd->name, "sendkey")) {
>> char *sep = strrchr(str, '-');
>> if (sep)
>> str = sep + 1;
>> @@ -4163,6 +4188,14 @@ static void monitor_find_completion(const char *cmdline)
>> cmd_completion(str, cmd->name);
>> }
>> }
>> + case 'S':
>> + /* generic string packaged */
>> + if (!strcmp(cmd->name, "info")) {
>> + readline_set_completion_index(cur_mon->rs, strlen(str));
>> + for (cmd = info_cmds; cmd->name != NULL; cmd++) {
>> + cmd_completion(str, cmd->name);
>> + }
>> + }
>> break;
>> default:
>> break;
>> @@ -4483,7 +4516,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
>> goto err_out;
>> }
>>
>> - cmd = qmp_find_cmd(cmd_name);
>> + cmd = monitor_find_command(cmd_name, qmp_cmds);
>> if (!cmd) {
>> qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
>> goto err_out;
>
>
--
Best Regards
Wenchao Xia
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] HMP: add QDict to info callback handler
2012-12-19 10:17 ` [Qemu-devel] [PATCH 1/3] HMP: add QDict to info callback handler Wenchao Xia
@ 2012-12-21 14:01 ` Markus Armbruster
2012-12-25 2:52 ` Wenchao Xia
0 siblings, 1 reply; 14+ messages in thread
From: Markus Armbruster @ 2012-12-21 14:01 UTC (permalink / raw)
To: Wenchao Xia; +Cc: kwolf, aliguori, stefanha, qemu-devel, lcapitulino, pbonzini
Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:
> This patch change all info call back function to take
> additional QDict * parameter, which allow those command
> take parameter.
[...]
> diff --git a/monitor.c b/monitor.c
> index c0e32d6..797680f 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;
>
For now, the new argument is NULL, not an empty dictionary. Could be
mentioned in the commit message. Not worth a respin.
[...]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] HMP: show internal snapshots on a single device
2012-12-19 10:17 ` [Qemu-devel] [PATCH 3/3] HMP: show internal snapshots on a single device Wenchao Xia
@ 2012-12-21 14:07 ` Markus Armbruster
2012-12-25 3:11 ` Wenchao Xia
0 siblings, 1 reply; 14+ messages in thread
From: Markus Armbruster @ 2012-12-21 14:07 UTC (permalink / raw)
To: Wenchao Xia; +Cc: kwolf, aliguori, stefanha, qemu-devel, lcapitulino, pbonzini
Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:
> This patch add an option to show snapshots on a single block
> device, so some snapshot do not exist on other block device
> could be shown.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
> monitor.c | 6 +++---
> savevm.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 57 insertions(+), 4 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index ce0e74d..b019618 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2613,9 +2613,9 @@ static mon_cmd_t info_cmds[] = {
> },
> {
> .name = "snapshots",
> - .args_type = "",
> - .params = "",
> - .help = "show the currently saved VM snapshots",
> + .args_type = "device:B?",
> + .params = "[device]",
> + .help = "show snapshots of whole vm or a single device",
> .mhandler.info = do_info_snapshots,
> },
> {
> diff --git a/savevm.c b/savevm.c
> index fa32171..438eb24 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -2358,7 +2358,7 @@ void do_delvm(Monitor *mon, const QDict *qdict)
> }
> }
>
> -void do_info_snapshots(Monitor *mon, const QDict *qdict)
> +static void do_info_snapshots_vm(Monitor *mon)
> {
> BlockDriverState *bs, *bs1;
> QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = &s;
> @@ -2422,6 +2422,59 @@ void do_info_snapshots(Monitor *mon, const QDict *qdict)
>
> }
>
> +static void do_info_snapshots_blk(Monitor *mon, const char *device)
> +{
> + BlockDriverState *bs;
> + QEMUSnapshotInfo *sn_tab, *sn;
> + int nb_sns, i;
> + char buf[256];
> +
> + /* find the target bs */
> + bs = bdrv_find(device);
> + if (!bs) {
> + monitor_printf(mon, "Device '%s' not found.\n", device);
> + return ;
> + }
> +
> + if (!bdrv_can_snapshot(bs)) {
> + monitor_printf(mon, "Device '%s' can't have snapshot.\n", device);
> + return ;
> + }
Rest of the function is is essentially code copied from
do_info_snapshots_vm(). Could it be factored out?
> +
> + nb_sns = bdrv_snapshot_list(bs, &sn_tab);
> + if (nb_sns < 0) {
> + monitor_printf(mon, "Device %s bdrv_snapshot_list: error %d\n",
> + device, nb_sns);
> + return;
> + }
> +
> + if (nb_sns == 0) {
> + monitor_printf(mon, "There is no snapshot available.\n");
> + return;
> + }
> +
> + monitor_printf(mon, "Device %s:\n", device);
> + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
> + for (i = 0; i < nb_sns; i++) {
> + sn = &sn_tab[i];
> + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
> + }
> + g_free(sn_tab);
> + return;
> +}
> +
> +void do_info_snapshots(Monitor *mon, const QDict *qdict)
> +{
> + /* Todo, there should be a layer rebuild qdict before enter this func. */
I'm not sure I get this comment.
We usually capitalize TODO for easy grepping.
> + const char *device = qdict_get_try_str(qdict, "device");
> + if (!device) {
> + do_info_snapshots_vm(mon);
> + } else {
> + do_info_snapshots_blk(mon, device);
> + }
> + return;
> +}
> +
> void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
> {
> qemu_ram_set_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK,
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] HMP: pass in parameter for info sub command
2012-12-19 18:07 ` Luiz Capitulino
2012-12-20 3:02 ` Wenchao Xia
@ 2012-12-21 14:49 ` Markus Armbruster
2012-12-21 18:17 ` Eric Blake
2012-12-25 4:29 ` Wenchao Xia
1 sibling, 2 replies; 14+ messages in thread
From: Markus Armbruster @ 2012-12-21 14:49 UTC (permalink / raw)
To: Luiz Capitulino
Cc: kwolf, aliguori, stefanha, qemu-devel, pbonzini, Wenchao Xia
Luiz Capitulino <lcapitulino@redhat.com> writes:
> On Wed, 19 Dec 2012 18:17:09 +0800
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>
>> This patch enable sub info command handler getting meaningful
>> parameter.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>> hmp-commands.hx | 2 +-
>> monitor.c | 79 +++++++++++++++++++++++++++++++++++++++----------------
>> 2 files changed, 57 insertions(+), 24 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index 010b8c9..667fab8 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -1486,7 +1486,7 @@ ETEXI
>>
>> {
>> .name = "info",
>> - .args_type = "item:s?",
>> + .args_type = "item:S?",
>> .params = "[subcommand]",
>> .help = "show various information about the system state",
>> .mhandler.cmd = do_info,
>> diff --git a/monitor.c b/monitor.c
>> index 797680f..ce0e74d 100644
>> --- a/monitor.c
>> +++ b/monitor.c
Missing: documentation for the new arg type S in the comment that starts
with "Supported types".
>> @@ -464,6 +464,11 @@ QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) != QEVENT_MAX)
>> MonitorEventState monitor_event_state[QEVENT_MAX];
>> QemuMutex monitor_event_state_lock;
>>
>> +static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>> + const char *cmdline,
>> + const mon_cmd_t *table,
>> + QDict *qdict);
>
> Please, move the function instead.
Move will make review harder. I don't mind forward declarations.
>> +
>> /*
>> * Emits the event to every monitor instance
>> */
>> @@ -809,26 +814,29 @@ static void user_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
>> static void do_info(Monitor *mon, const QDict *qdict)
>> {
>> const mon_cmd_t *cmd;
>> + QDict *qdict_info;
>> 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;
>> - }
>> + qdict_info = qdict_new();
>>
>> - if (cmd->name == NULL) {
>> - goto help;
>> + cmd = monitor_parse_command(mon, item, info_cmds, qdict_info);
>> + if (!cmd) {
>> + QDECREF(qdict_info);
>> + /* don't help here, to avoid error message got ignored */
I'm afraid I don't understand your comment.
Oh, you mean you don't want to call help_cmd() here!
>> + return;
>> }
>>
>> - cmd->mhandler.info(mon, NULL);
>> + cmd->mhandler.info(mon, qdict_info);
>> + QDECREF(qdict_info);
>> return;
>>
>> help:
>> help_cmd(mon, "info");
>> + return;
>> }
The function now looks like this:
static void do_info(Monitor *mon, const QDict *qdict)
{
const mon_cmd_t *cmd;
QDict *qdict_info;
const char *item = qdict_get_try_str(qdict, "item");
if (!item) {
goto help;
}
qdict_info = qdict_new();
cmd = monitor_parse_command(mon, item, info_cmds, qdict_info);
if (!cmd) {
QDECREF(qdict_info);
/* don't help here, to avoid error message got ignored */
return;
}
cmd->mhandler.info(mon, qdict_info);
QDECREF(qdict_info);
return;
help:
help_cmd(mon, "info");
return;
}
Control flow isn't nice. What about:
static void do_info(Monitor *mon, const QDict *qdict)
{
const mon_cmd_t *cmd;
QDict *qdict_info;
const char *item = qdict_get_try_str(qdict, "item");
if (!item) {
help_cmd(mon, "info");
return;
}
qdict_info = qdict_new();
cmd = monitor_parse_command(mon, item, info_cmds, qdict_info);
if (!cmd) {
goto out;
}
cmd->mhandler.info(mon, qdict_info);
out:
QDECREF(qdict_info);
return;
}
>>
>> CommandInfoList *qmp_query_commands(Error **errp)
>> @@ -3534,18 +3542,15 @@ 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)
>> +static const mon_cmd_t *monitor_find_command(const char *cmdname,
>> + const mon_cmd_t *table)
>> {
>> - 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);
>> + return search_dispatch_table(table, cmdname);
>
> Please, keep only search_dispatch_table().
Yes, because the resulting function is silly :)
static const mon_cmd_t *monitor_find_command(const char *cmdname,
const mon_cmd_t *table)
{
return search_dispatch_table(table, cmdname);
}
> Actually, maybe you could change handle_qmp_command() to just loop through
> command names and compare them with memcpy() (in a different patch, please)
> then you keep monitor_find_command() for handle_hmp_command().
>
>> }
>>
>> static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>> const char *cmdline,
>> + const mon_cmd_t *table,
>> QDict *qdict)
>> {
>> const char *p, *typestr;
>> @@ -3564,7 +3569,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>> if (!p)
>> return NULL;
>>
>> - cmd = monitor_find_command(cmdname);
>> + cmd = monitor_find_command(cmdname, table);
>> if (!cmd) {
>> monitor_printf(mon, "unknown command: '%s'\n", cmdname);
>> return NULL;
>> @@ -3872,6 +3877,31 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>> }
>> }
>> break;
>> + case 'S':
>> + {
>> + /* package all remaining string */
>> + int len;
>> +
>> + while (qemu_isspace(*p)) {
>> + p++;
>> + }
>> + if (*typestr == '?') {
>> + typestr++;
>> + if (*p == '\0') {
>> + /* no remaining string: NULL argument */
>> + break;
>> + }
>> + }
>> + len = strlen(p);
>> + if (len <= 0) {
>> + monitor_printf(mon, "%s: string expected\n",
>> + cmdname);
A "string" in this context is an argument for arg type 's', i.e. a
sequence of characters delimited by unescaped '"', or a sequence of
non-whitespace characters not starting with '"'. That's not what we
expect here. We expect arbitrary arguments.
Suggest "arguments expected".
>> + break;
>> + }
>> + qdict_put(qdict, key, qstring_from_str(p));
>> + p += len;
>> + }
>
> This is a true HMP-style hack :)
>
> I don't know how to make this better though (at least not without asking you
> to re-write the whole thing). Maybe Markus has a better idea?
Let me try :)
The new arg type 'S' consumes the rest of the line unparsed, and puts it
into the argument qdict. Has to come last, obviously.
do_info() extracts it, then parses it like a full command. The info
command effectively adds like a prefix that switches command parsing to
an alternate table of commands. Works, quite flexible, but pretty
arcane.
Try #1:
Implement the command prefix concept the direct way. Mark the command
as prefix. Instead of a handler, it gets a pointer to the alternate
table of commands. When monitor_parse_command() recognizes a prefix
command, it drops the first word, switches to the alternate table, and
starts over.
Try #2:
We already have commands that take key=value,... arguments (arg type
'O'): device_add and netdev_add. Perhaps info could use args_type
"item:s?,opts:O". First argument is unchanged. The new second argument
accepts key=value,... options. 'O' argument is always optional. One
key can be declared optional, so that value (no '=') is shorthand for
that key=value.
>> + break;
>> default:
>> bad_type:
>> monitor_printf(mon, "%s: unknown type '%c'\n", cmdname, c);
>> @@ -3925,7 +3955,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;
>>
>> @@ -4144,12 +4174,7 @@ static void monitor_find_completion(const char *cmdline)
>> break;
>> case 's':
>> /* XXX: more generic ? */
>> - if (!strcmp(cmd->name, "info")) {
>> - readline_set_completion_index(cur_mon->rs, strlen(str));
>> - for(cmd = info_cmds; cmd->name != NULL; cmd++) {
>> - cmd_completion(str, cmd->name);
>> - }
>> - } else if (!strcmp(cmd->name, "sendkey")) {
>> + if (!strcmp(cmd->name, "sendkey")) {
>> char *sep = strrchr(str, '-');
>> if (sep)
>> str = sep + 1;
You move the special case hack for info argument completion to case 'S'
(next hunk), but leave behind the /* XXX: more generic ? */ that marks
it as hack! Please move it along with the hack.
>> @@ -4163,6 +4188,14 @@ static void monitor_find_completion(const char *cmdline)
>> cmd_completion(str, cmd->name);
>> }
>> }
>> + case 'S':
>> + /* generic string packaged */
>> + if (!strcmp(cmd->name, "info")) {
>> + readline_set_completion_index(cur_mon->rs, strlen(str));
>> + for (cmd = info_cmds; cmd->name != NULL; cmd++) {
>> + cmd_completion(str, cmd->name);
>> + }
>> + }
>> break;
>> default:
>> break;
>> @@ -4483,7 +4516,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
>> goto err_out;
>> }
>>
>> - cmd = qmp_find_cmd(cmd_name);
>> + cmd = monitor_find_command(cmd_name, qmp_cmds);
>> if (!cmd) {
>> qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
>> goto err_out;
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] HMP: pass in parameter for info sub command
2012-12-21 14:49 ` Markus Armbruster
@ 2012-12-21 18:17 ` Eric Blake
2012-12-25 4:29 ` Wenchao Xia
1 sibling, 0 replies; 14+ messages in thread
From: Eric Blake @ 2012-12-21 18:17 UTC (permalink / raw)
To: Markus Armbruster
Cc: kwolf, aliguori, stefanha, qemu-devel, Luiz Capitulino, pbonzini,
Wenchao Xia
[-- Attachment #1: Type: text/plain, Size: 720 bytes --]
On 12/21/2012 07:49 AM, Markus Armbruster wrote:
>>> +static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>>> + const char *cmdline,
>>> + const mon_cmd_t *table,
>>> + QDict *qdict);
>>
>> Please, move the function instead.
>
> Move will make review harder. I don't mind forward declarations.
If you do move the function, then split this into two patches - one for
JUST the code motion, and another that adds the 'table' argument to the
function.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] HMP: pass in parameter for info sub command
2012-12-20 3:02 ` Wenchao Xia
@ 2012-12-22 21:36 ` Luiz Capitulino
0 siblings, 0 replies; 14+ messages in thread
From: Luiz Capitulino @ 2012-12-22 21:36 UTC (permalink / raw)
To: Wenchao Xia; +Cc: kwolf, aliguori, stefanha, qemu-devel, armbru, pbonzini
On Thu, 20 Dec 2012 11:02:16 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
> > On Wed, 19 Dec 2012 18:17:09 +0800
> > Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
> >
> >> This patch enable sub info command handler getting meaningful
> >> parameter.
> >>
> >> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> >> ---
> >> hmp-commands.hx | 2 +-
> >> monitor.c | 79 +++++++++++++++++++++++++++++++++++++++----------------
> >> 2 files changed, 57 insertions(+), 24 deletions(-)
> >>
> >> diff --git a/hmp-commands.hx b/hmp-commands.hx
> >> index 010b8c9..667fab8 100644
> >> --- a/hmp-commands.hx
> >> +++ b/hmp-commands.hx
> >> @@ -1486,7 +1486,7 @@ ETEXI
> >>
> >> {
> >> .name = "info",
> >> - .args_type = "item:s?",
> >> + .args_type = "item:S?",
> >> .params = "[subcommand]",
> >> .help = "show various information about the system state",
> >> .mhandler.cmd = do_info,
> >> diff --git a/monitor.c b/monitor.c
> >> index 797680f..ce0e74d 100644
> >> --- a/monitor.c
> >> +++ b/monitor.c
> >> @@ -464,6 +464,11 @@ QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) != QEVENT_MAX)
> >> MonitorEventState monitor_event_state[QEVENT_MAX];
> >> QemuMutex monitor_event_state_lock;
> >>
> >> +static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> >> + const char *cmdline,
> >> + const mon_cmd_t *table,
> >> + QDict *qdict);
> >
> > Please, move the function instead.
> >
> There will be a bit big of code moving then, hope you would be OK with
> it.
That's fine, but please do it in a different patch(es).
> >> +
> >> /*
> >> * Emits the event to every monitor instance
> >> */
> >> @@ -809,26 +814,29 @@ static void user_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
> >> static void do_info(Monitor *mon, const QDict *qdict)
> >> {
> >> const mon_cmd_t *cmd;
> >> + QDict *qdict_info;
> >> 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;
> >> - }
> >> + qdict_info = qdict_new();
> >>
> >> - if (cmd->name == NULL) {
> >> - goto help;
> >> + cmd = monitor_parse_command(mon, item, info_cmds, qdict_info);
> >> + if (!cmd) {
> >> + QDECREF(qdict_info);
> >> + /* don't help here, to avoid error message got ignored */
> >> + return;
> >> }
> >>
> >> - cmd->mhandler.info(mon, NULL);
> >> + cmd->mhandler.info(mon, qdict_info);
> >> + QDECREF(qdict_info);
> >> return;
> >>
> >> help:
> >> help_cmd(mon, "info");
> >> + return;
> >> }
> >>
> >> CommandInfoList *qmp_query_commands(Error **errp)
> >> @@ -3534,18 +3542,15 @@ 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)
> >> +static const mon_cmd_t *monitor_find_command(const char *cmdname,
> >> + const mon_cmd_t *table)
> >> {
> >> - 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);
> >> + return search_dispatch_table(table, cmdname);
> >
> > Please, keep only search_dispatch_table().
> >
> OK.
>
> > Actually, maybe you could change handle_qmp_command() to just loop through
> > command names and compare them with memcpy() (in a different patch, please)
> > then you keep monitor_find_command() for handle_hmp_command().
> >
> ah, then monitor_find_command() serve HMP only. How about rename it to
> hmp_find_command(table, name)? This can have a more clear meaning. Or
That would be fine.
> just remove this funtion but use search_dispatch_table() every where?
>
> >> }
> >>
> >> static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> >> const char *cmdline,
> >> + const mon_cmd_t *table,
> >> QDict *qdict)
> >> {
> >> const char *p, *typestr;
> >> @@ -3564,7 +3569,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> >> if (!p)
> >> return NULL;
> >>
> >> - cmd = monitor_find_command(cmdname);
> >> + cmd = monitor_find_command(cmdname, table);
> >> if (!cmd) {
> >> monitor_printf(mon, "unknown command: '%s'\n", cmdname);
> >> return NULL;
> >> @@ -3872,6 +3877,31 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> >> }
> >> }
> >> break;
> >> + case 'S':
> >> + {
> >> + /* package all remaining string */
> >> + int len;
> >> +
> >> + while (qemu_isspace(*p)) {
> >> + p++;
> >> + }
> >> + if (*typestr == '?') {
> >> + typestr++;
> >> + if (*p == '\0') {
> >> + /* no remaining string: NULL argument */
> >> + break;
> >> + }
> >> + }
> >> + len = strlen(p);
> >> + if (len <= 0) {
> >> + monitor_printf(mon, "%s: string expected\n",
> >> + cmdname);
> >> + break;
> >> + }
> >> + qdict_put(qdict, key, qstring_from_str(p));
> >> + p += len;
> >> + }
> >
> > This is a true HMP-style hack :)
> >
> > I don't know how to make this better though (at least not without asking you
> > to re-write the whole thing). Maybe Markus has a better idea?
> >
> My idea was adding a new tag meaning "package all remaining string
> into the key value", what was I can found best before. Hope you have
> a better solution.
>
> >> + break;
> >> default:
> >> bad_type:
> >> monitor_printf(mon, "%s: unknown type '%c'\n", cmdname, c);
> >> @@ -3925,7 +3955,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;
> >>
> >> @@ -4144,12 +4174,7 @@ static void monitor_find_completion(const char *cmdline)
> >> break;
> >> case 's':
> >> /* XXX: more generic ? */
> >> - if (!strcmp(cmd->name, "info")) {
> >> - readline_set_completion_index(cur_mon->rs, strlen(str));
> >> - for(cmd = info_cmds; cmd->name != NULL; cmd++) {
> >> - cmd_completion(str, cmd->name);
> >> - }
> >> - } else if (!strcmp(cmd->name, "sendkey")) {
> >> + if (!strcmp(cmd->name, "sendkey")) {
> >> char *sep = strrchr(str, '-');
> >> if (sep)
> >> str = sep + 1;
> >> @@ -4163,6 +4188,14 @@ static void monitor_find_completion(const char *cmdline)
> >> cmd_completion(str, cmd->name);
> >> }
> >> }
> >> + case 'S':
> >> + /* generic string packaged */
> >> + if (!strcmp(cmd->name, "info")) {
> >> + readline_set_completion_index(cur_mon->rs, strlen(str));
> >> + for (cmd = info_cmds; cmd->name != NULL; cmd++) {
> >> + cmd_completion(str, cmd->name);
> >> + }
> >> + }
> >> break;
> >> default:
> >> break;
> >> @@ -4483,7 +4516,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
> >> goto err_out;
> >> }
> >>
> >> - cmd = qmp_find_cmd(cmd_name);
> >> + cmd = monitor_find_command(cmd_name, qmp_cmds);
> >> if (!cmd) {
> >> qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
> >> goto err_out;
> >
> >
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] HMP: add QDict to info callback handler
2012-12-21 14:01 ` Markus Armbruster
@ 2012-12-25 2:52 ` Wenchao Xia
0 siblings, 0 replies; 14+ messages in thread
From: Wenchao Xia @ 2012-12-25 2:52 UTC (permalink / raw)
To: Markus Armbruster
Cc: kwolf, aliguori, stefanha, qemu-devel, lcapitulino, pbonzini
OK, will add that in commit message.
>
> For now, the new argument is NULL, not an empty dictionary. Could be
> mentioned in the commit message. Not worth a respin.
>
> [...]
>
--
Best Regards
Wenchao Xia
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] HMP: show internal snapshots on a single device
2012-12-21 14:07 ` Markus Armbruster
@ 2012-12-25 3:11 ` Wenchao Xia
0 siblings, 0 replies; 14+ messages in thread
From: Wenchao Xia @ 2012-12-25 3:11 UTC (permalink / raw)
To: Markus Armbruster
Cc: kwolf, aliguori, stefanha, qemu-devel, lcapitulino, pbonzini
>>
>> +static void do_info_snapshots_blk(Monitor *mon, const char *device)
>> +{
>> + BlockDriverState *bs;
>> + QEMUSnapshotInfo *sn_tab, *sn;
>> + int nb_sns, i;
>> + char buf[256];
>> +
>> + /* find the target bs */
>> + bs = bdrv_find(device);
>> + if (!bs) {
>> + monitor_printf(mon, "Device '%s' not found.\n", device);
>> + return ;
>> + }
>> +
>> + if (!bdrv_can_snapshot(bs)) {
>> + monitor_printf(mon, "Device '%s' can't have snapshot.\n", device);
>> + return ;
>> + }
>
> Rest of the function is is essentially code copied from
> do_info_snapshots_vm(). Could it be factored out?
>
Sure, I forgot that before. I guess a better way is adding a QMP
interface first and using common code from it.
>> +
>> + nb_sns = bdrv_snapshot_list(bs, &sn_tab);
>> + if (nb_sns < 0) {
>> + monitor_printf(mon, "Device %s bdrv_snapshot_list: error %d\n",
>> + device, nb_sns);
>> + return;
>> + }
>> +
>> + if (nb_sns == 0) {
>> + monitor_printf(mon, "There is no snapshot available.\n");
>> + return;
>> + }
>> +
>> + monitor_printf(mon, "Device %s:\n", device);
>> + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
>> + for (i = 0; i < nb_sns; i++) {
>> + sn = &sn_tab[i];
>> + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
>> + }
>> + g_free(sn_tab);
>> + return;
>> +}
>> +
>> +void do_info_snapshots(Monitor *mon, const QDict *qdict)
>> +{
>> + /* Todo, there should be a layer rebuild qdict before enter this func. */
>
> I'm not sure I get this comment.
>
> We usually capitalize TODO for easy grepping.
>
My mistake, this comments should have been removed, it is used before
where qdict was not rebuilt as a new one.
>> + const char *device = qdict_get_try_str(qdict, "device");
>> + if (!device) {
>> + do_info_snapshots_vm(mon);
>> + } else {
>> + do_info_snapshots_blk(mon, device);
>> + }
>> + return;
>> +}
>> +
>> void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
>> {
>> qemu_ram_set_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK,
>
--
Best Regards
Wenchao Xia
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] HMP: pass in parameter for info sub command
2012-12-21 14:49 ` Markus Armbruster
2012-12-21 18:17 ` Eric Blake
@ 2012-12-25 4:29 ` Wenchao Xia
1 sibling, 0 replies; 14+ messages in thread
From: Wenchao Xia @ 2012-12-25 4:29 UTC (permalink / raw)
To: Markus Armbruster
Cc: kwolf, aliguori, stefanha, qemu-devel, Luiz Capitulino, pbonzini
于 2012-12-21 22:49, Markus Armbruster 写道:
> Luiz Capitulino <lcapitulino@redhat.com> writes:
>
>> On Wed, 19 Dec 2012 18:17:09 +0800
>> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>
>>> This patch enable sub info command handler getting meaningful
>>> parameter.
>>>
>>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>>> ---
>>> hmp-commands.hx | 2 +-
>>> monitor.c | 79 +++++++++++++++++++++++++++++++++++++++----------------
>>> 2 files changed, 57 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>>> index 010b8c9..667fab8 100644
>>> --- a/hmp-commands.hx
>>> +++ b/hmp-commands.hx
>>> @@ -1486,7 +1486,7 @@ ETEXI
>>>
>>> {
>>> .name = "info",
>>> - .args_type = "item:s?",
>>> + .args_type = "item:S?",
>>> .params = "[subcommand]",
>>> .help = "show various information about the system state",
>>> .mhandler.cmd = do_info,
>>> diff --git a/monitor.c b/monitor.c
>>> index 797680f..ce0e74d 100644
>>> --- a/monitor.c
>>> +++ b/monitor.c
>
> Missing: documentation for the new arg type S in the comment that starts
> with "Supported types".
>
OK, will add it.
>>> @@ -464,6 +464,11 @@ QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) != QEVENT_MAX)
>>> MonitorEventState monitor_event_state[QEVENT_MAX];
>>> QemuMutex monitor_event_state_lock;
>>>
>>> +static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>>> + const char *cmdline,
>>> + const mon_cmd_t *table,
>>> + QDict *qdict);
>>
>> Please, move the function instead.
>
> Move will make review harder. I don't mind forward declarations.
>
I will move the function in a separate patch.
>>> +
>>> /*
>>> * Emits the event to every monitor instance
>>> */
>>> @@ -809,26 +814,29 @@ static void user_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
>>> static void do_info(Monitor *mon, const QDict *qdict)
>>> {
>>> const mon_cmd_t *cmd;
>>> + QDict *qdict_info;
>>> 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;
>>> - }
>>> + qdict_info = qdict_new();
>>>
>>> - if (cmd->name == NULL) {
>>> - goto help;
>>> + cmd = monitor_parse_command(mon, item, info_cmds, qdict_info);
>>> + if (!cmd) {
>>> + QDECREF(qdict_info);
>>> + /* don't help here, to avoid error message got ignored */
>
> I'm afraid I don't understand your comment.
>
> Oh, you mean you don't want to call help_cmd() here!
>
Yes, not to show the help contents, same as not call help_cmd().
>>> + return;
>>> }
>>>
>>> - cmd->mhandler.info(mon, NULL);
>>> + cmd->mhandler.info(mon, qdict_info);
>>> + QDECREF(qdict_info);
>>> return;
>>>
>>> help:
>>> help_cmd(mon, "info");
>>> + return;
>>> }
>
> The function now looks like this:
>
> static void do_info(Monitor *mon, const QDict *qdict)
> {
> const mon_cmd_t *cmd;
> QDict *qdict_info;
> const char *item = qdict_get_try_str(qdict, "item");
>
> if (!item) {
> goto help;
> }
>
> qdict_info = qdict_new();
>
> cmd = monitor_parse_command(mon, item, info_cmds, qdict_info);
> if (!cmd) {
> QDECREF(qdict_info);
> /* don't help here, to avoid error message got ignored */
> return;
> }
>
> cmd->mhandler.info(mon, qdict_info);
> QDECREF(qdict_info);
> return;
>
> help:
> help_cmd(mon, "info");
> return;
> }
>
> Control flow isn't nice. What about:
>
> static void do_info(Monitor *mon, const QDict *qdict)
> {
> const mon_cmd_t *cmd;
> QDict *qdict_info;
> const char *item = qdict_get_try_str(qdict, "item");
>
> if (!item) {
> help_cmd(mon, "info");
> return;
> }
>
> qdict_info = qdict_new();
>
> cmd = monitor_parse_command(mon, item, info_cmds, qdict_info);
> if (!cmd) {
> goto out;
> }
>
> cmd->mhandler.info(mon, qdict_info);
> out:
> QDECREF(qdict_info);
> return;
> }
>
OK, I'll following this way.
>>>
>>> CommandInfoList *qmp_query_commands(Error **errp)
>>> @@ -3534,18 +3542,15 @@ 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)
>>> +static const mon_cmd_t *monitor_find_command(const char *cmdname,
>>> + const mon_cmd_t *table)
>>> {
>>> - 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);
>>> + return search_dispatch_table(table, cmdname);
>>
>> Please, keep only search_dispatch_table().
>
> Yes, because the resulting function is silly :)
>
> static const mon_cmd_t *monitor_find_command(const char *cmdname,
> const mon_cmd_t *table)
> {
> return search_dispatch_table(table, cmdname);
> }
>
I'll remove simple encapsulate function monitor_find_command(),
which's naming make me confused.
>> Actually, maybe you could change handle_qmp_command() to just loop through
>> command names and compare them with memcpy() (in a different patch, please)
>> then you keep monitor_find_command() for handle_hmp_command().
>>
>>> }
>>>
>>> static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>>> const char *cmdline,
>>> + const mon_cmd_t *table,
>>> QDict *qdict)
>>> {
>>> const char *p, *typestr;
>>> @@ -3564,7 +3569,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>>> if (!p)
>>> return NULL;
>>>
>>> - cmd = monitor_find_command(cmdname);
>>> + cmd = monitor_find_command(cmdname, table);
>>> if (!cmd) {
>>> monitor_printf(mon, "unknown command: '%s'\n", cmdname);
>>> return NULL;
>>> @@ -3872,6 +3877,31 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>>> }
>>> }
>>> break;
>>> + case 'S':
>>> + {
>>> + /* package all remaining string */
>>> + int len;
>>> +
>>> + while (qemu_isspace(*p)) {
>>> + p++;
>>> + }
>>> + if (*typestr == '?') {
>>> + typestr++;
>>> + if (*p == '\0') {
>>> + /* no remaining string: NULL argument */
>>> + break;
>>> + }
>>> + }
>>> + len = strlen(p);
>>> + if (len <= 0) {
>>> + monitor_printf(mon, "%s: string expected\n",
>>> + cmdname);
>
> A "string" in this context is an argument for arg type 's', i.e. a
> sequence of characters delimited by unescaped '"', or a sequence of
> non-whitespace characters not starting with '"'. That's not what we
> expect here. We expect arbitrary arguments.
>
> Suggest "arguments expected".
>
OK.
>>> + break;
>>> + }
>>> + qdict_put(qdict, key, qstring_from_str(p));
>>> + p += len;
>>> + }
>>
>> This is a true HMP-style hack :)
>>
>> I don't know how to make this better though (at least not without asking you
>> to re-write the whole thing). Maybe Markus has a better idea?
>
> Let me try :)
>
> The new arg type 'S' consumes the rest of the line unparsed, and puts it
> into the argument qdict. Has to come last, obviously.
>
> do_info() extracts it, then parses it like a full command. The info
> command effectively adds like a prefix that switches command parsing to
> an alternate table of commands. Works, quite flexible, but pretty
> arcane.
>
> Try #1:
>
> Implement the command prefix concept the direct way. Mark the command
> as prefix. Instead of a handler, it gets a pointer to the alternate
> table of commands. When monitor_parse_command() recognizes a prefix
> command, it drops the first word, switches to the alternate table, and
> starts over.
>
It seems "info" command need to be marked and searched in
monitor_parse_command(),
> Try #2:
>
> We already have commands that take key=value,... arguments (arg type
> 'O'): device_add and netdev_add. Perhaps info could use args_type
> "item:s?,opts:O". First argument is unchanged. The new second argument
> accepts key=value,... options. 'O' argument is always optional. One
> key can be declared optional, so that value (no '=') is shorthand for
> that key=value.
Personally I feel approach #1 is better, which let the command parse
layer knows there would be another sub command layer, and enable embbed
sub commands with deeper layers transparently. #2 and my previous patch
is a hack but each layer don't know sub command exist.
For #1 I guess additional tag about "sub command' need to be added,
I guess that would be
typedef struct mon_cmd_t {
const char *name;
const char *args_type;
const char *params;
const char *help;
void (*user_print)(Monitor *mon, const QObject *data);
union {
void (*info)(Monitor *mon);
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,
MonitorCompletion *cb, void *opaque);
} mhandler;
int flags;
struct mon_cmd_t *sub_table;
} mon_cmd_t;
*sub_table flag if it have a 2nd level of command table.
>
>>> + break;
>>> default:
>>> bad_type:
>>> monitor_printf(mon, "%s: unknown type '%c'\n", cmdname, c);
>>> @@ -3925,7 +3955,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;
>>>
>>> @@ -4144,12 +4174,7 @@ static void monitor_find_completion(const char *cmdline)
>>> break;
>>> case 's':
>>> /* XXX: more generic ? */
>>> - if (!strcmp(cmd->name, "info")) {
>>> - readline_set_completion_index(cur_mon->rs, strlen(str));
>>> - for(cmd = info_cmds; cmd->name != NULL; cmd++) {
>>> - cmd_completion(str, cmd->name);
>>> - }
>>> - } else if (!strcmp(cmd->name, "sendkey")) {
>>> + if (!strcmp(cmd->name, "sendkey")) {
>>> char *sep = strrchr(str, '-');
>>> if (sep)
>>> str = sep + 1;
>
> You move the special case hack for info argument completion to case 'S'
> (next hunk), but leave behind the /* XXX: more generic ? */ that marks
> it as hack! Please move it along with the hack.
>
OK.
>>> @@ -4163,6 +4188,14 @@ static void monitor_find_completion(const char *cmdline)
>>> cmd_completion(str, cmd->name);
>>> }
>>> }
>>> + case 'S':
>>> + /* generic string packaged */
>>> + if (!strcmp(cmd->name, "info")) {
>>> + readline_set_completion_index(cur_mon->rs, strlen(str));
>>> + for (cmd = info_cmds; cmd->name != NULL; cmd++) {
>>> + cmd_completion(str, cmd->name);
>>> + }
>>> + }
>>> break;
>>> default:
>>> break;
>>> @@ -4483,7 +4516,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
>>> goto err_out;
>>> }
>>>
>>> - cmd = qmp_find_cmd(cmd_name);
>>> + cmd = monitor_find_command(cmd_name, qmp_cmds);
>>> if (!cmd) {
>>> qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
>>> goto err_out;
>
--
Best Regards
Wenchao Xia
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2012-12-25 4:31 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-19 10:17 [Qemu-devel] [PATCH 0/3] HMP: enable info sub command taking parameter Wenchao Xia
2012-12-19 10:17 ` [Qemu-devel] [PATCH 1/3] HMP: add QDict to info callback handler Wenchao Xia
2012-12-21 14:01 ` Markus Armbruster
2012-12-25 2:52 ` Wenchao Xia
2012-12-19 10:17 ` [Qemu-devel] [PATCH 2/3] HMP: pass in parameter for info sub command Wenchao Xia
2012-12-19 18:07 ` Luiz Capitulino
2012-12-20 3:02 ` Wenchao Xia
2012-12-22 21:36 ` Luiz Capitulino
2012-12-21 14:49 ` Markus Armbruster
2012-12-21 18:17 ` Eric Blake
2012-12-25 4:29 ` Wenchao Xia
2012-12-19 10:17 ` [Qemu-devel] [PATCH 3/3] HMP: show internal snapshots on a single device Wenchao Xia
2012-12-21 14:07 ` Markus Armbruster
2012-12-25 3:11 ` Wenchao Xia
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.