* [Qemu-devel] [PATCH v4] Add HMP command "info memory-devices" @ 2014-09-15 11:31 Zhu Guihua 2014-09-18 8:09 ` zhugh 0 siblings, 1 reply; 11+ messages in thread From: Zhu Guihua @ 2014-09-15 11:31 UTC (permalink / raw) To: qemu-devel, lcapitulino, imammedo, hani, stefanha, mst Cc: hutao, isimatu.yasuaki, Zhu Guihua, tangchen Provides HMP equivalent of QMP query-memory-devices command. Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> --- Changes since v3: - optimize the time to print memory devices' information. - change output format of di->addr and di->size. Changes since v2: - print address in hex. - change the loop control from while to for. - modify some variables' name. - optimize the time to print memory devices' kind. Changes since v1: - fix bug that accessing info->dimm when MemoryDeviceInfo is not PCDIMMDevice. - use enum to replace "dimm", and lookup typename in MemoryDeviceInfoKind_lookup[] instead of opencodding it. hmp-commands.hx | 2 ++ hmp.c | 38 ++++++++++++++++++++++++++++++++++++++ hmp.h | 1 + monitor.c | 7 +++++++ 4 files changed, 48 insertions(+) diff --git a/hmp-commands.hx b/hmp-commands.hx index f859f8d..0b1a4f7 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1778,6 +1778,8 @@ show qdev device model list show roms @item info tpm show the TPM device +@item info memory-devices +show the memory devices @end table ETEXI diff --git a/hmp.c b/hmp.c index 40a90da..feefeb4 100644 --- a/hmp.c +++ b/hmp.c @@ -1718,3 +1718,41 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict) qapi_free_MemdevList(memdev_list); } + +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) +{ + Error *err = NULL; + MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err); + MemoryDeviceInfoList *info; + MemoryDeviceInfo *value; + PCDIMMDeviceInfo *di; + + for (info = info_list; info; info = info->next) { + value = info->value; + + if (value) { + switch (value->kind) { + case MEMORY_DEVICE_INFO_KIND_DIMM: + di = value->dimm; + + monitor_printf(mon, "Memory device [%s]: %s\n", + MemoryDeviceInfoKind_lookup[value->kind], + di->id); + monitor_printf(mon, " addr: 0x%" PRIx64 "\n", di->addr); + monitor_printf(mon, " slot: %" PRId64 "\n", di->slot); + monitor_printf(mon, " node: %" PRId64 "\n", di->node); + monitor_printf(mon, " size: %" PRIu64 "\n", di->size); + monitor_printf(mon, " memdev: %s\n", di->memdev); + monitor_printf(mon, " hotplugged: %s\n", + di->hotplugged ? "true" : "false"); + monitor_printf(mon, " hotpluggable: %s\n", + di->hotpluggable ? "true" : "false"); + break; + default: + break; + } + } + } + + qapi_free_MemoryDeviceInfoList(info_list); +} diff --git a/hmp.h b/hmp.h index 4fd3c4a..4bb5dca 100644 --- a/hmp.h +++ b/hmp.h @@ -94,6 +94,7 @@ void hmp_cpu_add(Monitor *mon, const QDict *qdict); void hmp_object_add(Monitor *mon, const QDict *qdict); void hmp_object_del(Monitor *mon, const QDict *qdict); void hmp_info_memdev(Monitor *mon, const QDict *qdict); +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict); void object_add_completion(ReadLineState *rs, int nb_args, const char *str); void object_del_completion(ReadLineState *rs, int nb_args, const char *str); void device_add_completion(ReadLineState *rs, int nb_args, const char *str); diff --git a/monitor.c b/monitor.c index 34cee74..fe88e0d 100644 --- a/monitor.c +++ b/monitor.c @@ -2921,6 +2921,13 @@ static mon_cmd_t info_cmds[] = { .mhandler.cmd = hmp_info_memdev, }, { + .name = "memory-devices", + .args_type = "", + .params = "", + .help = "show memory devices", + .mhandler.cmd = hmp_info_memory_devices, + }, + { .name = NULL, }, }; -- 1.9.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add HMP command "info memory-devices" 2014-09-15 11:31 [Qemu-devel] [PATCH v4] Add HMP command "info memory-devices" Zhu Guihua @ 2014-09-18 8:09 ` zhugh 2014-09-19 13:30 ` Igor Mammedov 0 siblings, 1 reply; 11+ messages in thread From: zhugh @ 2014-09-18 8:09 UTC (permalink / raw) To: qemu-devel Cc: mst, hutao, lcapitulino, isimatu.yasuaki, hani, stefanha, imammedo, tangchen Hi, Could anyone help to review this patch? If there was no problem, could help to merge it? thanks! zhu On Mon, 2014-09-15 at 19:31 +0800, Zhu Guihua wrote: > Provides HMP equivalent of QMP query-memory-devices command. > > Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> > --- > > Changes since v3: > - optimize the time to print memory devices' information. > - change output format of di->addr and di->size. > > Changes since v2: > - print address in hex. > - change the loop control from while to for. > - modify some variables' name. > - optimize the time to print memory devices' kind. > > Changes since v1: > - fix bug that accessing info->dimm when MemoryDeviceInfo is not PCDIMMDevice. > - use enum to replace "dimm", and lookup typename in MemoryDeviceInfoKind_lookup[] instead of opencodding it. > > hmp-commands.hx | 2 ++ > hmp.c | 38 ++++++++++++++++++++++++++++++++++++++ > hmp.h | 1 + > monitor.c | 7 +++++++ > 4 files changed, 48 insertions(+) > > diff --git a/hmp-commands.hx b/hmp-commands.hx > index f859f8d..0b1a4f7 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -1778,6 +1778,8 @@ show qdev device model list > show roms > @item info tpm > show the TPM device > +@item info memory-devices > +show the memory devices > @end table > ETEXI > > diff --git a/hmp.c b/hmp.c > index 40a90da..feefeb4 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -1718,3 +1718,41 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict) > > qapi_free_MemdevList(memdev_list); > } > + > +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) > +{ > + Error *err = NULL; > + MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err); > + MemoryDeviceInfoList *info; > + MemoryDeviceInfo *value; > + PCDIMMDeviceInfo *di; > + > + for (info = info_list; info; info = info->next) { > + value = info->value; > + > + if (value) { > + switch (value->kind) { > + case MEMORY_DEVICE_INFO_KIND_DIMM: > + di = value->dimm; > + > + monitor_printf(mon, "Memory device [%s]: %s\n", > + MemoryDeviceInfoKind_lookup[value->kind], > + di->id); > + monitor_printf(mon, " addr: 0x%" PRIx64 "\n", di->addr); > + monitor_printf(mon, " slot: %" PRId64 "\n", di->slot); > + monitor_printf(mon, " node: %" PRId64 "\n", di->node); > + monitor_printf(mon, " size: %" PRIu64 "\n", di->size); > + monitor_printf(mon, " memdev: %s\n", di->memdev); > + monitor_printf(mon, " hotplugged: %s\n", > + di->hotplugged ? "true" : "false"); > + monitor_printf(mon, " hotpluggable: %s\n", > + di->hotpluggable ? "true" : "false"); > + break; > + default: > + break; > + } > + } > + } > + > + qapi_free_MemoryDeviceInfoList(info_list); > +} > diff --git a/hmp.h b/hmp.h > index 4fd3c4a..4bb5dca 100644 > --- a/hmp.h > +++ b/hmp.h > @@ -94,6 +94,7 @@ void hmp_cpu_add(Monitor *mon, const QDict *qdict); > void hmp_object_add(Monitor *mon, const QDict *qdict); > void hmp_object_del(Monitor *mon, const QDict *qdict); > void hmp_info_memdev(Monitor *mon, const QDict *qdict); > +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict); > void object_add_completion(ReadLineState *rs, int nb_args, const char *str); > void object_del_completion(ReadLineState *rs, int nb_args, const char *str); > void device_add_completion(ReadLineState *rs, int nb_args, const char *str); > diff --git a/monitor.c b/monitor.c > index 34cee74..fe88e0d 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -2921,6 +2921,13 @@ static mon_cmd_t info_cmds[] = { > .mhandler.cmd = hmp_info_memdev, > }, > { > + .name = "memory-devices", > + .args_type = "", > + .params = "", > + .help = "show memory devices", > + .mhandler.cmd = hmp_info_memory_devices, > + }, > + { > .name = NULL, > }, > }; ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add HMP command "info memory-devices" 2014-09-18 8:09 ` zhugh @ 2014-09-19 13:30 ` Igor Mammedov 2014-09-19 15:34 ` Luiz Capitulino 0 siblings, 1 reply; 11+ messages in thread From: Igor Mammedov @ 2014-09-19 13:30 UTC (permalink / raw) To: zhugh Cc: mst, hutao, qemu-devel, lcapitulino, isimatu.yasuaki, hani, stefanha, tangchen On Thu, 18 Sep 2014 16:09:32 +0800 zhugh <zhugh.fnst@cn.fujitsu.com> wrote: > Hi, > > Could anyone help to review this patch? > If there was no problem, could help to merge it? > > thanks! > zhu > > On Mon, 2014-09-15 at 19:31 +0800, Zhu Guihua wrote: > > Provides HMP equivalent of QMP query-memory-devices command. > > > > Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> > > --- > > > > Changes since v3: > > - optimize the time to print memory devices' information. > > - change output format of di->addr and di->size. > > > > Changes since v2: > > - print address in hex. > > - change the loop control from while to for. > > - modify some variables' name. > > - optimize the time to print memory devices' kind. > > > > Changes since v1: > > - fix bug that accessing info->dimm when MemoryDeviceInfo is not PCDIMMDevice. > > - use enum to replace "dimm", and lookup typename in MemoryDeviceInfoKind_lookup[] instead of opencodding it. > > > > hmp-commands.hx | 2 ++ > > hmp.c | 38 ++++++++++++++++++++++++++++++++++++++ > > hmp.h | 1 + > > monitor.c | 7 +++++++ > > 4 files changed, 48 insertions(+) > > > > diff --git a/hmp-commands.hx b/hmp-commands.hx > > index f859f8d..0b1a4f7 100644 > > --- a/hmp-commands.hx > > +++ b/hmp-commands.hx > > @@ -1778,6 +1778,8 @@ show qdev device model list > > show roms > > @item info tpm > > show the TPM device > > +@item info memory-devices > > +show the memory devices > > @end table > > ETEXI > > > > diff --git a/hmp.c b/hmp.c > > index 40a90da..feefeb4 100644 > > --- a/hmp.c > > +++ b/hmp.c > > @@ -1718,3 +1718,41 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict) > > > > qapi_free_MemdevList(memdev_list); > > } > > + > > +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) > > +{ > > + Error *err = NULL; > > + MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err); > > + MemoryDeviceInfoList *info; > > + MemoryDeviceInfo *value; > > + PCDIMMDeviceInfo *di; > > + > > + for (info = info_list; info; info = info->next) { > > + value = info->value; > > + > > + if (value) { > > + switch (value->kind) { > > + case MEMORY_DEVICE_INFO_KIND_DIMM: > > + di = value->dimm; > > + > > + monitor_printf(mon, "Memory device [%s]: %s\n", > > + MemoryDeviceInfoKind_lookup[value->kind], > > + di->id); 'id' might be null, here is what user will see: Memory device [dimm]: (null) I'd suggest to replace (null) with "" as it is done elsewhere. With that fixed Reviewed-By: Igor Mammedov <imammedo@redhat.com> > > + monitor_printf(mon, " addr: 0x%" PRIx64 "\n", di->addr); > > + monitor_printf(mon, " slot: %" PRId64 "\n", di->slot); > > + monitor_printf(mon, " node: %" PRId64 "\n", di->node); > > + monitor_printf(mon, " size: %" PRIu64 "\n", di->size); > > + monitor_printf(mon, " memdev: %s\n", di->memdev); > > + monitor_printf(mon, " hotplugged: %s\n", > > + di->hotplugged ? "true" : "false"); > > + monitor_printf(mon, " hotpluggable: %s\n", > > + di->hotpluggable ? "true" : "false"); > > + break; > > + default: > > + break; > > + } > > + } > > + } > > + > > + qapi_free_MemoryDeviceInfoList(info_list); > > +} > > diff --git a/hmp.h b/hmp.h > > index 4fd3c4a..4bb5dca 100644 > > --- a/hmp.h > > +++ b/hmp.h > > @@ -94,6 +94,7 @@ void hmp_cpu_add(Monitor *mon, const QDict *qdict); > > void hmp_object_add(Monitor *mon, const QDict *qdict); > > void hmp_object_del(Monitor *mon, const QDict *qdict); > > void hmp_info_memdev(Monitor *mon, const QDict *qdict); > > +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict); > > void object_add_completion(ReadLineState *rs, int nb_args, const char *str); > > void object_del_completion(ReadLineState *rs, int nb_args, const char *str); > > void device_add_completion(ReadLineState *rs, int nb_args, const char *str); > > diff --git a/monitor.c b/monitor.c > > index 34cee74..fe88e0d 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -2921,6 +2921,13 @@ static mon_cmd_t info_cmds[] = { > > .mhandler.cmd = hmp_info_memdev, > > }, > > { > > + .name = "memory-devices", > > + .args_type = "", > > + .params = "", > > + .help = "show memory devices", > > + .mhandler.cmd = hmp_info_memory_devices, > > + }, > > + { > > .name = NULL, > > }, > > }; > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add HMP command "info memory-devices" 2014-09-19 13:30 ` Igor Mammedov @ 2014-09-19 15:34 ` Luiz Capitulino 2014-09-20 4:51 ` zhugh 2014-09-22 7:08 ` zhugh 0 siblings, 2 replies; 11+ messages in thread From: Luiz Capitulino @ 2014-09-19 15:34 UTC (permalink / raw) To: Igor Mammedov Cc: zhugh, mst, hutao, qemu-devel, tangchen, isimatu.yasuaki, hani, stefanha On Fri, 19 Sep 2014 15:30:19 +0200 Igor Mammedov <imammedo@redhat.com> wrote: > On Thu, 18 Sep 2014 16:09:32 +0800 > zhugh <zhugh.fnst@cn.fujitsu.com> wrote: > > > Hi, > > > > Could anyone help to review this patch? > > If there was no problem, could help to merge it? > > > > thanks! > > zhu > > > > On Mon, 2014-09-15 at 19:31 +0800, Zhu Guihua wrote: > > > Provides HMP equivalent of QMP query-memory-devices command. > > > > > > Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> > > > --- > > > > > > Changes since v3: > > > - optimize the time to print memory devices' information. > > > - change output format of di->addr and di->size. > > > > > > Changes since v2: > > > - print address in hex. > > > - change the loop control from while to for. > > > - modify some variables' name. > > > - optimize the time to print memory devices' kind. > > > > > > Changes since v1: > > > - fix bug that accessing info->dimm when MemoryDeviceInfo is not PCDIMMDevice. > > > - use enum to replace "dimm", and lookup typename in MemoryDeviceInfoKind_lookup[] instead of opencodding it. > > > > > > hmp-commands.hx | 2 ++ > > > hmp.c | 38 ++++++++++++++++++++++++++++++++++++++ > > > hmp.h | 1 + > > > monitor.c | 7 +++++++ > > > 4 files changed, 48 insertions(+) > > > > > > diff --git a/hmp-commands.hx b/hmp-commands.hx > > > index f859f8d..0b1a4f7 100644 > > > --- a/hmp-commands.hx > > > +++ b/hmp-commands.hx > > > @@ -1778,6 +1778,8 @@ show qdev device model list > > > show roms > > > @item info tpm > > > show the TPM device > > > +@item info memory-devices > > > +show the memory devices > > > @end table > > > ETEXI > > > > > > diff --git a/hmp.c b/hmp.c > > > index 40a90da..feefeb4 100644 > > > --- a/hmp.c > > > +++ b/hmp.c > > > @@ -1718,3 +1718,41 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict) > > > > > > qapi_free_MemdevList(memdev_list); > > > } > > > + > > > +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) > > > +{ > > > + Error *err = NULL; > > > + MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err); > > > + MemoryDeviceInfoList *info; > > > + MemoryDeviceInfo *value; > > > + PCDIMMDeviceInfo *di; > > > + > > > + for (info = info_list; info; info = info->next) { > > > + value = info->value; > > > + > > > + if (value) { > > > + switch (value->kind) { > > > + case MEMORY_DEVICE_INFO_KIND_DIMM: > > > + di = value->dimm; > > > + > > > + monitor_printf(mon, "Memory device [%s]: %s\n", > > > + MemoryDeviceInfoKind_lookup[value->kind], > > > + di->id); > 'id' might be null, here is what user will see: > > Memory device [dimm]: (null) > > I'd suggest to replace (null) with "" as it is done elsewhere. Is the fix below what you're looking for? If it is I can apply it myself: diff --git a/hmp.c b/hmp.c index feefeb4..14cb9f8 100644 --- a/hmp.c +++ b/hmp.c @@ -1737,7 +1737,7 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) monitor_printf(mon, "Memory device [%s]: %s\n", MemoryDeviceInfoKind_lookup[value->kind], - di->id); + di->id ? di->id : ""); monitor_printf(mon, " addr: 0x%" PRIx64 "\n", di->addr); monitor_printf(mon, " slot: %" PRId64 "\n", di->slot); monitor_printf(mon, " node: %" PRId64 "\n", di->node); > > With that fixed > Reviewed-By: Igor Mammedov <imammedo@redhat.com> > > > > + monitor_printf(mon, " addr: 0x%" PRIx64 "\n", di->addr); > > > + monitor_printf(mon, " slot: %" PRId64 "\n", di->slot); > > > + monitor_printf(mon, " node: %" PRId64 "\n", di->node); > > > + monitor_printf(mon, " size: %" PRIu64 "\n", di->size); > > > + monitor_printf(mon, " memdev: %s\n", di->memdev); > > > + monitor_printf(mon, " hotplugged: %s\n", > > > + di->hotplugged ? "true" : "false"); > > > + monitor_printf(mon, " hotpluggable: %s\n", > > > + di->hotpluggable ? "true" : "false"); > > > + break; > > > + default: > > > + break; > > > + } > > > + } > > > + } > > > + > > > + qapi_free_MemoryDeviceInfoList(info_list); > > > +} > > > diff --git a/hmp.h b/hmp.h > > > index 4fd3c4a..4bb5dca 100644 > > > --- a/hmp.h > > > +++ b/hmp.h > > > @@ -94,6 +94,7 @@ void hmp_cpu_add(Monitor *mon, const QDict *qdict); > > > void hmp_object_add(Monitor *mon, const QDict *qdict); > > > void hmp_object_del(Monitor *mon, const QDict *qdict); > > > void hmp_info_memdev(Monitor *mon, const QDict *qdict); > > > +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict); > > > void object_add_completion(ReadLineState *rs, int nb_args, const char *str); > > > void object_del_completion(ReadLineState *rs, int nb_args, const char *str); > > > void device_add_completion(ReadLineState *rs, int nb_args, const char *str); > > > diff --git a/monitor.c b/monitor.c > > > index 34cee74..fe88e0d 100644 > > > --- a/monitor.c > > > +++ b/monitor.c > > > @@ -2921,6 +2921,13 @@ static mon_cmd_t info_cmds[] = { > > > .mhandler.cmd = hmp_info_memdev, > > > }, > > > { > > > + .name = "memory-devices", > > > + .args_type = "", > > > + .params = "", > > > + .help = "show memory devices", > > > + .mhandler.cmd = hmp_info_memory_devices, > > > + }, > > > + { > > > .name = NULL, > > > }, > > > }; > > > > > ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add HMP command "info memory-devices" 2014-09-19 15:34 ` Luiz Capitulino @ 2014-09-20 4:51 ` zhugh 2014-09-22 7:08 ` zhugh 1 sibling, 0 replies; 11+ messages in thread From: zhugh @ 2014-09-20 4:51 UTC (permalink / raw) To: Luiz Capitulino Cc: mst, hutao, qemu-devel, tangchen, isimatu.yasuaki, hani, stefanha, Igor Mammedov On Fri, 2014-09-19 at 11:34 -0400, Luiz Capitulino wrote: > On Fri, 19 Sep 2014 15:30:19 +0200 > Igor Mammedov <imammedo@redhat.com> wrote: > > > On Thu, 18 Sep 2014 16:09:32 +0800 > > zhugh <zhugh.fnst@cn.fujitsu.com> wrote: > > > > > Hi, > > > > > > Could anyone help to review this patch? > > > If there was no problem, could help to merge it? > > > > > > thanks! > > > zhu > > > > > > On Mon, 2014-09-15 at 19:31 +0800, Zhu Guihua wrote: > > > > Provides HMP equivalent of QMP query-memory-devices command. > > > > > > > > Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> > > > > --- > > > > > > > > Changes since v3: > > > > - optimize the time to print memory devices' information. > > > > - change output format of di->addr and di->size. > > > > > > > > Changes since v2: > > > > - print address in hex. > > > > - change the loop control from while to for. > > > > - modify some variables' name. > > > > - optimize the time to print memory devices' kind. > > > > > > > > Changes since v1: > > > > - fix bug that accessing info->dimm when MemoryDeviceInfo is not PCDIMMDevice. > > > > - use enum to replace "dimm", and lookup typename in MemoryDeviceInfoKind_lookup[] instead of opencodding it. > > > > > > > > hmp-commands.hx | 2 ++ > > > > hmp.c | 38 ++++++++++++++++++++++++++++++++++++++ > > > > hmp.h | 1 + > > > > monitor.c | 7 +++++++ > > > > 4 files changed, 48 insertions(+) > > > > > > > > diff --git a/hmp-commands.hx b/hmp-commands.hx > > > > index f859f8d..0b1a4f7 100644 > > > > --- a/hmp-commands.hx > > > > +++ b/hmp-commands.hx > > > > @@ -1778,6 +1778,8 @@ show qdev device model list > > > > show roms > > > > @item info tpm > > > > show the TPM device > > > > +@item info memory-devices > > > > +show the memory devices > > > > @end table > > > > ETEXI > > > > > > > > diff --git a/hmp.c b/hmp.c > > > > index 40a90da..feefeb4 100644 > > > > --- a/hmp.c > > > > +++ b/hmp.c > > > > @@ -1718,3 +1718,41 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict) > > > > > > > > qapi_free_MemdevList(memdev_list); > > > > } > > > > + > > > > +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) > > > > +{ > > > > + Error *err = NULL; > > > > + MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err); > > > > + MemoryDeviceInfoList *info; > > > > + MemoryDeviceInfo *value; > > > > + PCDIMMDeviceInfo *di; > > > > + > > > > + for (info = info_list; info; info = info->next) { > > > > + value = info->value; > > > > + > > > > + if (value) { > > > > + switch (value->kind) { > > > > + case MEMORY_DEVICE_INFO_KIND_DIMM: > > > > + di = value->dimm; > > > > + > > > > + monitor_printf(mon, "Memory device [%s]: %s\n", > > > > + MemoryDeviceInfoKind_lookup[value->kind], > > > > + di->id); > > 'id' might be null, here is what user will see: > > > > Memory device [dimm]: (null) > > > > I'd suggest to replace (null) with "" as it is done elsewhere. > > Is the fix below what you're looking for? If it is I can apply it myself: Yes, I think the fix is what Igor expects. So could you apply the fix? If not, I can send patch v5 to fix the problem. > > diff --git a/hmp.c b/hmp.c > index feefeb4..14cb9f8 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -1737,7 +1737,7 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) > > monitor_printf(mon, "Memory device [%s]: %s\n", > MemoryDeviceInfoKind_lookup[value->kind], > - di->id); > + di->id ? di->id : ""); > monitor_printf(mon, " addr: 0x%" PRIx64 "\n", di->addr); > monitor_printf(mon, " slot: %" PRId64 "\n", di->slot); > monitor_printf(mon, " node: %" PRId64 "\n", di->node); > > > > With that fixed > > Reviewed-By: Igor Mammedov <imammedo@redhat.com> > > > > > > + monitor_printf(mon, " addr: 0x%" PRIx64 "\n", di->addr); > > > > + monitor_printf(mon, " slot: %" PRId64 "\n", di->slot); > > > > + monitor_printf(mon, " node: %" PRId64 "\n", di->node); > > > > + monitor_printf(mon, " size: %" PRIu64 "\n", di->size); > > > > + monitor_printf(mon, " memdev: %s\n", di->memdev); > > > > + monitor_printf(mon, " hotplugged: %s\n", > > > > + di->hotplugged ? "true" : "false"); > > > > + monitor_printf(mon, " hotpluggable: %s\n", > > > > + di->hotpluggable ? "true" : "false"); > > > > + break; > > > > + default: > > > > + break; > > > > + } > > > > + } > > > > + } > > > > + > > > > + qapi_free_MemoryDeviceInfoList(info_list); > > > > +} > > > > diff --git a/hmp.h b/hmp.h > > > > index 4fd3c4a..4bb5dca 100644 > > > > --- a/hmp.h > > > > +++ b/hmp.h > > > > @@ -94,6 +94,7 @@ void hmp_cpu_add(Monitor *mon, const QDict *qdict); > > > > void hmp_object_add(Monitor *mon, const QDict *qdict); > > > > void hmp_object_del(Monitor *mon, const QDict *qdict); > > > > void hmp_info_memdev(Monitor *mon, const QDict *qdict); > > > > +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict); > > > > void object_add_completion(ReadLineState *rs, int nb_args, const char *str); > > > > void object_del_completion(ReadLineState *rs, int nb_args, const char *str); > > > > void device_add_completion(ReadLineState *rs, int nb_args, const char *str); > > > > diff --git a/monitor.c b/monitor.c > > > > index 34cee74..fe88e0d 100644 > > > > --- a/monitor.c > > > > +++ b/monitor.c > > > > @@ -2921,6 +2921,13 @@ static mon_cmd_t info_cmds[] = { > > > > .mhandler.cmd = hmp_info_memdev, > > > > }, > > > > { > > > > + .name = "memory-devices", > > > > + .args_type = "", > > > > + .params = "", > > > > + .help = "show memory devices", > > > > + .mhandler.cmd = hmp_info_memory_devices, > > > > + }, > > > > + { > > > > .name = NULL, > > > > }, > > > > }; > > > > > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add HMP command "info memory-devices" 2014-09-19 15:34 ` Luiz Capitulino 2014-09-20 4:51 ` zhugh @ 2014-09-22 7:08 ` zhugh 2014-09-22 7:59 ` Markus Armbruster 1 sibling, 1 reply; 11+ messages in thread From: zhugh @ 2014-09-22 7:08 UTC (permalink / raw) To: Luiz Capitulino Cc: mst, hutao, qemu-devel, tangchen, isimatu.yasuaki, hani, stefanha, Igor Mammedov On Fri, 2014-09-19 at 11:34 -0400, Luiz Capitulino wrote: > On Fri, 19 Sep 2014 15:30:19 +0200 > Igor Mammedov <imammedo@redhat.com> wrote: > > > On Thu, 18 Sep 2014 16:09:32 +0800 > > zhugh <zhugh.fnst@cn.fujitsu.com> wrote: > > > > > Hi, > > > > > > Could anyone help to review this patch? > > > If there was no problem, could help to merge it? > > > > > > thanks! > > > zhu > > > > > > On Mon, 2014-09-15 at 19:31 +0800, Zhu Guihua wrote: > > > > Provides HMP equivalent of QMP query-memory-devices command. > > > > > > > > Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> > > > > --- > > > > > > > > Changes since v3: > > > > - optimize the time to print memory devices' information. > > > > - change output format of di->addr and di->size. > > > > > > > > Changes since v2: > > > > - print address in hex. > > > > - change the loop control from while to for. > > > > - modify some variables' name. > > > > - optimize the time to print memory devices' kind. > > > > > > > > Changes since v1: > > > > - fix bug that accessing info->dimm when MemoryDeviceInfo is not PCDIMMDevice. > > > > - use enum to replace "dimm", and lookup typename in MemoryDeviceInfoKind_lookup[] instead of opencodding it. > > > > > > > > hmp-commands.hx | 2 ++ > > > > hmp.c | 38 ++++++++++++++++++++++++++++++++++++++ > > > > hmp.h | 1 + > > > > monitor.c | 7 +++++++ > > > > 4 files changed, 48 insertions(+) > > > > > > > > diff --git a/hmp-commands.hx b/hmp-commands.hx > > > > index f859f8d..0b1a4f7 100644 > > > > --- a/hmp-commands.hx > > > > +++ b/hmp-commands.hx > > > > @@ -1778,6 +1778,8 @@ show qdev device model list > > > > show roms > > > > @item info tpm > > > > show the TPM device > > > > +@item info memory-devices > > > > +show the memory devices > > > > @end table > > > > ETEXI > > > > > > > > diff --git a/hmp.c b/hmp.c > > > > index 40a90da..feefeb4 100644 > > > > --- a/hmp.c > > > > +++ b/hmp.c > > > > @@ -1718,3 +1718,41 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict) > > > > > > > > qapi_free_MemdevList(memdev_list); > > > > } > > > > + > > > > +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) > > > > +{ > > > > + Error *err = NULL; > > > > + MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err); > > > > + MemoryDeviceInfoList *info; > > > > + MemoryDeviceInfo *value; > > > > + PCDIMMDeviceInfo *di; > > > > + > > > > + for (info = info_list; info; info = info->next) { > > > > + value = info->value; > > > > + > > > > + if (value) { > > > > + switch (value->kind) { > > > > + case MEMORY_DEVICE_INFO_KIND_DIMM: > > > > + di = value->dimm; > > > > + > > > > + monitor_printf(mon, "Memory device [%s]: %s\n", > > > > + MemoryDeviceInfoKind_lookup[value->kind], > > > > + di->id); > > 'id' might be null, here is what user will see: > > > > Memory device [dimm]: (null) > > > > I'd suggest to replace (null) with "" as it is done elsewhere. > > Is the fix below what you're looking for? If it is I can apply it myself: I am sorry, I did not test this fix in my code last time. But your fix would print nothing when the id was null. So I think the fix would like the below, could you apply it? Thanks! diff --git a/hmp.c b/hmp.c index feefeb4..39a6490 100644 --- a/hmp.c +++ b/hmp.c @@ -1737,7 +1737,7 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) monitor_printf(mon, "Memory device [%s]: %s\n", MemoryDeviceInfoKind_lookup[value->kind], - di->id); + di->id ? di->id : "\"\""); monitor_printf(mon, " addr: 0x%" PRIx64 "\n",di->addr); monitor_printf(mon, " slot: %" PRId64 "\n", di->slot); monitor_printf(mon, " node: %" PRId64 "\n", di->node); > > diff --git a/hmp.c b/hmp.c > index feefeb4..14cb9f8 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -1737,7 +1737,7 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) > > monitor_printf(mon, "Memory device [%s]: %s\n", > MemoryDeviceInfoKind_lookup[value->kind], > - di->id); > + di->id ? di->id : ""); > monitor_printf(mon, " addr: 0x%" PRIx64 "\n", di->addr); > monitor_printf(mon, " slot: %" PRId64 "\n", di->slot); > monitor_printf(mon, " node: %" PRId64 "\n", di->node); > > > > With that fixed > > Reviewed-By: Igor Mammedov <imammedo@redhat.com> > > > > > > + monitor_printf(mon, " addr: 0x%" PRIx64 "\n", di->addr); > > > > + monitor_printf(mon, " slot: %" PRId64 "\n", di->slot); > > > > + monitor_printf(mon, " node: %" PRId64 "\n", di->node); > > > > + monitor_printf(mon, " size: %" PRIu64 "\n", di->size); > > > > + monitor_printf(mon, " memdev: %s\n", di->memdev); > > > > + monitor_printf(mon, " hotplugged: %s\n", > > > > + di->hotplugged ? "true" : "false"); > > > > + monitor_printf(mon, " hotpluggable: %s\n", > > > > + di->hotpluggable ? "true" : "false"); > > > > + break; > > > > + default: > > > > + break; > > > > + } > > > > + } > > > > + } > > > > + > > > > + qapi_free_MemoryDeviceInfoList(info_list); > > > > +} > > > > diff --git a/hmp.h b/hmp.h > > > > index 4fd3c4a..4bb5dca 100644 > > > > --- a/hmp.h > > > > +++ b/hmp.h > > > > @@ -94,6 +94,7 @@ void hmp_cpu_add(Monitor *mon, const QDict *qdict); > > > > void hmp_object_add(Monitor *mon, const QDict *qdict); > > > > void hmp_object_del(Monitor *mon, const QDict *qdict); > > > > void hmp_info_memdev(Monitor *mon, const QDict *qdict); > > > > +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict); > > > > void object_add_completion(ReadLineState *rs, int nb_args, const char *str); > > > > void object_del_completion(ReadLineState *rs, int nb_args, const char *str); > > > > void device_add_completion(ReadLineState *rs, int nb_args, const char *str); > > > > diff --git a/monitor.c b/monitor.c > > > > index 34cee74..fe88e0d 100644 > > > > --- a/monitor.c > > > > +++ b/monitor.c > > > > @@ -2921,6 +2921,13 @@ static mon_cmd_t info_cmds[] = { > > > > .mhandler.cmd = hmp_info_memdev, > > > > }, > > > > { > > > > + .name = "memory-devices", > > > > + .args_type = "", > > > > + .params = "", > > > > + .help = "show memory devices", > > > > + .mhandler.cmd = hmp_info_memory_devices, > > > > + }, > > > > + { > > > > .name = NULL, > > > > }, > > > > }; > > > > > > > > > ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add HMP command "info memory-devices" 2014-09-22 7:08 ` zhugh @ 2014-09-22 7:59 ` Markus Armbruster 2014-09-22 8:05 ` Igor Mammedov 0 siblings, 1 reply; 11+ messages in thread From: Markus Armbruster @ 2014-09-22 7:59 UTC (permalink / raw) To: zhugh Cc: mst, hutao, qemu-devel, tangchen, isimatu.yasuaki, hani, stefanha, Igor Mammedov, Luiz Capitulino zhugh <zhugh.fnst@cn.fujitsu.com> writes: > On Fri, 2014-09-19 at 11:34 -0400, Luiz Capitulino wrote: >> On Fri, 19 Sep 2014 15:30:19 +0200 >> Igor Mammedov <imammedo@redhat.com> wrote: >> >> > On Thu, 18 Sep 2014 16:09:32 +0800 >> > zhugh <zhugh.fnst@cn.fujitsu.com> wrote: >> > >> > > Hi, >> > > >> > > Could anyone help to review this patch? >> > > If there was no problem, could help to merge it? >> > > >> > > thanks! >> > > zhu >> > > >> > > On Mon, 2014-09-15 at 19:31 +0800, Zhu Guihua wrote: >> > > > Provides HMP equivalent of QMP query-memory-devices command. >> > > > >> > > > Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> [...] >> > > > diff --git a/hmp.c b/hmp.c >> > > > index 40a90da..feefeb4 100644 >> > > > --- a/hmp.c >> > > > +++ b/hmp.c >> > > > @@ -1718,3 +1718,41 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict) >> > > > >> > > > qapi_free_MemdevList(memdev_list); >> > > > } >> > > > + >> > > > +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) >> > > > +{ >> > > > + Error *err = NULL; >> > > > + MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err); >> > > > + MemoryDeviceInfoList *info; >> > > > + MemoryDeviceInfo *value; >> > > > + PCDIMMDeviceInfo *di; >> > > > + >> > > > + for (info = info_list; info; info = info->next) { >> > > > + value = info->value; >> > > > + >> > > > + if (value) { >> > > > + switch (value->kind) { >> > > > + case MEMORY_DEVICE_INFO_KIND_DIMM: >> > > > + di = value->dimm; >> > > > + >> > > > + monitor_printf(mon, "Memory device [%s]: %s\n", >> > > > + MemoryDeviceInfoKind_lookup[value->kind], >> > > > + di->id); >> > 'id' might be null, here is what user will see: >> > >> > Memory device [dimm]: (null) >> > >> > I'd suggest to replace (null) with "" as it is done elsewhere. >> >> Is the fix below what you're looking for? If it is I can apply it myself: > > I am sorry, I did not test this fix in my code last time. But your fix > would print nothing when the id was null. Printing nothing when there's no ID has a certain logic to it :) > So I think the fix would like the below, could you apply it? > Thanks! > > diff --git a/hmp.c b/hmp.c > index feefeb4..39a6490 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -1737,7 +1737,7 @@ void hmp_info_memory_devices(Monitor *mon, const > QDict *qdict) > > monitor_printf(mon, "Memory device [%s]: %s\n", > MemoryDeviceInfoKind_lookup[value->kind], > - di->id); > + di->id ? di->id : "\"\""); > monitor_printf(mon, " addr: 0x%" PRIx64 "\n",di->addr); > monitor_printf(mon, " slot: %" PRId64 "\n", di->slot); > monitor_printf(mon, " node: %" PRId64 "\n", di->node); I'd stick to printing nothing. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add HMP command "info memory-devices" 2014-09-22 7:59 ` Markus Armbruster @ 2014-09-22 8:05 ` Igor Mammedov 2014-09-22 9:29 ` Markus Armbruster 0 siblings, 1 reply; 11+ messages in thread From: Igor Mammedov @ 2014-09-22 8:05 UTC (permalink / raw) To: Markus Armbruster Cc: zhugh, mst, hutao, qemu-devel, tangchen, isimatu.yasuaki, hani, stefanha, Luiz Capitulino On Mon, 22 Sep 2014 09:59:06 +0200 Markus Armbruster <armbru@redhat.com> wrote: > zhugh <zhugh.fnst@cn.fujitsu.com> writes: > > > On Fri, 2014-09-19 at 11:34 -0400, Luiz Capitulino wrote: > >> On Fri, 19 Sep 2014 15:30:19 +0200 > >> Igor Mammedov <imammedo@redhat.com> wrote: > >> > >> > On Thu, 18 Sep 2014 16:09:32 +0800 > >> > zhugh <zhugh.fnst@cn.fujitsu.com> wrote: > >> > > >> > > Hi, > >> > > > >> > > Could anyone help to review this patch? > >> > > If there was no problem, could help to merge it? > >> > > > >> > > thanks! > >> > > zhu > >> > > > >> > > On Mon, 2014-09-15 at 19:31 +0800, Zhu Guihua wrote: > >> > > > Provides HMP equivalent of QMP query-memory-devices command. > >> > > > > >> > > > Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> > [...] > >> > > > diff --git a/hmp.c b/hmp.c > >> > > > index 40a90da..feefeb4 100644 > >> > > > --- a/hmp.c > >> > > > +++ b/hmp.c > >> > > > @@ -1718,3 +1718,41 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict) > >> > > > > >> > > > qapi_free_MemdevList(memdev_list); > >> > > > } > >> > > > + > >> > > > +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) > >> > > > +{ > >> > > > + Error *err = NULL; > >> > > > + MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err); > >> > > > + MemoryDeviceInfoList *info; > >> > > > + MemoryDeviceInfo *value; > >> > > > + PCDIMMDeviceInfo *di; > >> > > > + > >> > > > + for (info = info_list; info; info = info->next) { > >> > > > + value = info->value; > >> > > > + > >> > > > + if (value) { > >> > > > + switch (value->kind) { > >> > > > + case MEMORY_DEVICE_INFO_KIND_DIMM: > >> > > > + di = value->dimm; > >> > > > + > >> > > > + monitor_printf(mon, "Memory device [%s]: %s\n", > >> > > > + MemoryDeviceInfoKind_lookup[value->kind], > >> > > > + di->id); > >> > 'id' might be null, here is what user will see: > >> > > >> > Memory device [dimm]: (null) > >> > > >> > I'd suggest to replace (null) with "" as it is done elsewhere. > >> > >> Is the fix below what you're looking for? If it is I can apply it myself: > > > > I am sorry, I did not test this fix in my code last time. But your fix > > would print nothing when the id was null. > > Printing nothing when there's no ID has a certain logic to it :) When I do 'info qtree' it displays empty IDs with as empty "" I think we should be consistent and apply above correction. > > > So I think the fix would like the below, could you apply it? > > Thanks! > > > > diff --git a/hmp.c b/hmp.c > > index feefeb4..39a6490 100644 > > --- a/hmp.c > > +++ b/hmp.c > > @@ -1737,7 +1737,7 @@ void hmp_info_memory_devices(Monitor *mon, const > > QDict *qdict) > > > > monitor_printf(mon, "Memory device [%s]: %s\n", > > MemoryDeviceInfoKind_lookup[value->kind], > > - di->id); > > + di->id ? di->id : "\"\""); > > monitor_printf(mon, " addr: 0x%" PRIx64 "\n",di->addr); > > monitor_printf(mon, " slot: %" PRId64 "\n", di->slot); > > monitor_printf(mon, " node: %" PRId64 "\n", di->node); > > I'd stick to printing nothing. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add HMP command "info memory-devices" 2014-09-22 8:05 ` Igor Mammedov @ 2014-09-22 9:29 ` Markus Armbruster 2014-09-22 13:03 ` Luiz Capitulino 0 siblings, 1 reply; 11+ messages in thread From: Markus Armbruster @ 2014-09-22 9:29 UTC (permalink / raw) To: Igor Mammedov Cc: zhugh, mst, hutao, qemu-devel, tangchen, isimatu.yasuaki, hani, stefanha, Luiz Capitulino Igor Mammedov <imammedo@redhat.com> writes: > On Mon, 22 Sep 2014 09:59:06 +0200 > Markus Armbruster <armbru@redhat.com> wrote: > >> zhugh <zhugh.fnst@cn.fujitsu.com> writes: >> >> > On Fri, 2014-09-19 at 11:34 -0400, Luiz Capitulino wrote: >> >> On Fri, 19 Sep 2014 15:30:19 +0200 >> >> Igor Mammedov <imammedo@redhat.com> wrote: >> >> >> >> > On Thu, 18 Sep 2014 16:09:32 +0800 >> >> > zhugh <zhugh.fnst@cn.fujitsu.com> wrote: >> >> > >> >> > > Hi, >> >> > > >> >> > > Could anyone help to review this patch? >> >> > > If there was no problem, could help to merge it? >> >> > > >> >> > > thanks! >> >> > > zhu >> >> > > >> >> > > On Mon, 2014-09-15 at 19:31 +0800, Zhu Guihua wrote: >> >> > > > Provides HMP equivalent of QMP query-memory-devices command. >> >> > > > >> >> > > > Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> >> [...] >> >> > > > diff --git a/hmp.c b/hmp.c >> >> > > > index 40a90da..feefeb4 100644 >> >> > > > --- a/hmp.c >> >> > > > +++ b/hmp.c >> >> > > > @@ -1718,3 +1718,41 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict) >> >> > > > >> >> > > > qapi_free_MemdevList(memdev_list); >> >> > > > } >> >> > > > + >> >> > > > +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) >> >> > > > +{ >> >> > > > + Error *err = NULL; >> >> > > > + MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err); >> >> > > > + MemoryDeviceInfoList *info; >> >> > > > + MemoryDeviceInfo *value; >> >> > > > + PCDIMMDeviceInfo *di; >> >> > > > + >> >> > > > + for (info = info_list; info; info = info->next) { >> >> > > > + value = info->value; >> >> > > > + >> >> > > > + if (value) { >> >> > > > + switch (value->kind) { >> >> > > > + case MEMORY_DEVICE_INFO_KIND_DIMM: >> >> > > > + di = value->dimm; >> >> > > > + >> >> > > > + monitor_printf(mon, "Memory device [%s]: %s\n", >> >> > > > + MemoryDeviceInfoKind_lookup[value->kind], >> >> > > > + di->id); >> >> > 'id' might be null, here is what user will see: >> >> > >> >> > Memory device [dimm]: (null) >> >> > >> >> > I'd suggest to replace (null) with "" as it is done elsewhere. >> >> >> >> Is the fix below what you're looking for? If it is I can apply it myself: >> > >> > I am sorry, I did not test this fix in my code last time. But your fix >> > would print nothing when the id was null. >> >> Printing nothing when there's no ID has a certain logic to it :) > > When I do 'info qtree' it displays empty IDs with as empty "" > I think we should be consistent and apply above correction. If you want consistency with "info qtree", you should enclose non-null ID in double quotes. [...] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add HMP command "info memory-devices" 2014-09-22 9:29 ` Markus Armbruster @ 2014-09-22 13:03 ` Luiz Capitulino 2014-09-23 1:08 ` Zhu Guihua 0 siblings, 1 reply; 11+ messages in thread From: Luiz Capitulino @ 2014-09-22 13:03 UTC (permalink / raw) To: Markus Armbruster Cc: zhugh, mst, hutao, qemu-devel, tangchen, isimatu.yasuaki, hani, stefanha, Igor Mammedov On Mon, 22 Sep 2014 11:29:00 +0200 Markus Armbruster <armbru@redhat.com> wrote: > Igor Mammedov <imammedo@redhat.com> writes: > > > On Mon, 22 Sep 2014 09:59:06 +0200 > > Markus Armbruster <armbru@redhat.com> wrote: > > > >> zhugh <zhugh.fnst@cn.fujitsu.com> writes: > >> > >> > On Fri, 2014-09-19 at 11:34 -0400, Luiz Capitulino wrote: > >> >> On Fri, 19 Sep 2014 15:30:19 +0200 > >> >> Igor Mammedov <imammedo@redhat.com> wrote: > >> >> > >> >> > On Thu, 18 Sep 2014 16:09:32 +0800 > >> >> > zhugh <zhugh.fnst@cn.fujitsu.com> wrote: > >> >> > > >> >> > > Hi, > >> >> > > > >> >> > > Could anyone help to review this patch? > >> >> > > If there was no problem, could help to merge it? > >> >> > > > >> >> > > thanks! > >> >> > > zhu > >> >> > > > >> >> > > On Mon, 2014-09-15 at 19:31 +0800, Zhu Guihua wrote: > >> >> > > > Provides HMP equivalent of QMP query-memory-devices command. > >> >> > > > > >> >> > > > Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> > >> [...] > >> >> > > > diff --git a/hmp.c b/hmp.c > >> >> > > > index 40a90da..feefeb4 100644 > >> >> > > > --- a/hmp.c > >> >> > > > +++ b/hmp.c > >> >> > > > @@ -1718,3 +1718,41 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict) > >> >> > > > > >> >> > > > qapi_free_MemdevList(memdev_list); > >> >> > > > } > >> >> > > > + > >> >> > > > +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) > >> >> > > > +{ > >> >> > > > + Error *err = NULL; > >> >> > > > + MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err); > >> >> > > > + MemoryDeviceInfoList *info; > >> >> > > > + MemoryDeviceInfo *value; > >> >> > > > + PCDIMMDeviceInfo *di; > >> >> > > > + > >> >> > > > + for (info = info_list; info; info = info->next) { > >> >> > > > + value = info->value; > >> >> > > > + > >> >> > > > + if (value) { > >> >> > > > + switch (value->kind) { > >> >> > > > + case MEMORY_DEVICE_INFO_KIND_DIMM: > >> >> > > > + di = value->dimm; > >> >> > > > + > >> >> > > > + monitor_printf(mon, "Memory device [%s]: %s\n", > >> >> > > > + MemoryDeviceInfoKind_lookup[value->kind], > >> >> > > > + di->id); > >> >> > 'id' might be null, here is what user will see: > >> >> > > >> >> > Memory device [dimm]: (null) > >> >> > > >> >> > I'd suggest to replace (null) with "" as it is done elsewhere. > >> >> > >> >> Is the fix below what you're looking for? If it is I can apply it myself: > >> > > >> > I am sorry, I did not test this fix in my code last time. But your fix > >> > would print nothing when the id was null. > >> > >> Printing nothing when there's no ID has a certain logic to it :) > > > > When I do 'info qtree' it displays empty IDs with as empty "" > > I think we should be consistent and apply above correction. > > If you want consistency with "info qtree", you should enclose non-null > ID in double quotes. zhugh, at this point is better to respin the patch. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add HMP command "info memory-devices" 2014-09-22 13:03 ` Luiz Capitulino @ 2014-09-23 1:08 ` Zhu Guihua 0 siblings, 0 replies; 11+ messages in thread From: Zhu Guihua @ 2014-09-23 1:08 UTC (permalink / raw) To: Luiz Capitulino Cc: mst, hutao, qemu-devel, tangchen, isimatu.yasuaki, hani, stefanha, Igor Mammedov, Markus Armbruster On Mon, 2014-09-22 at 09:03 -0400, Luiz Capitulino wrote: > On Mon, 22 Sep 2014 11:29:00 +0200 > Markus Armbruster <armbru@redhat.com> wrote: > > > Igor Mammedov <imammedo@redhat.com> writes: > > > > > On Mon, 22 Sep 2014 09:59:06 +0200 > > > Markus Armbruster <armbru@redhat.com> wrote: > > > > > >> zhugh <zhugh.fnst@cn.fujitsu.com> writes: > > >> > > >> > On Fri, 2014-09-19 at 11:34 -0400, Luiz Capitulino wrote: > > >> >> On Fri, 19 Sep 2014 15:30:19 +0200 > > >> >> Igor Mammedov <imammedo@redhat.com> wrote: > > >> >> > > >> >> > On Thu, 18 Sep 2014 16:09:32 +0800 > > >> >> > zhugh <zhugh.fnst@cn.fujitsu.com> wrote: > > >> >> > > > >> >> > > Hi, > > >> >> > > > > >> >> > > Could anyone help to review this patch? > > >> >> > > If there was no problem, could help to merge it? > > >> >> > > > > >> >> > > thanks! > > >> >> > > zhu > > >> >> > > > > >> >> > > On Mon, 2014-09-15 at 19:31 +0800, Zhu Guihua wrote: > > >> >> > > > Provides HMP equivalent of QMP query-memory-devices command. > > >> >> > > > > > >> >> > > > Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> > > >> [...] > > >> >> > > > diff --git a/hmp.c b/hmp.c > > >> >> > > > index 40a90da..feefeb4 100644 > > >> >> > > > --- a/hmp.c > > >> >> > > > +++ b/hmp.c > > >> >> > > > @@ -1718,3 +1718,41 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict) > > >> >> > > > > > >> >> > > > qapi_free_MemdevList(memdev_list); > > >> >> > > > } > > >> >> > > > + > > >> >> > > > +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) > > >> >> > > > +{ > > >> >> > > > + Error *err = NULL; > > >> >> > > > + MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err); > > >> >> > > > + MemoryDeviceInfoList *info; > > >> >> > > > + MemoryDeviceInfo *value; > > >> >> > > > + PCDIMMDeviceInfo *di; > > >> >> > > > + > > >> >> > > > + for (info = info_list; info; info = info->next) { > > >> >> > > > + value = info->value; > > >> >> > > > + > > >> >> > > > + if (value) { > > >> >> > > > + switch (value->kind) { > > >> >> > > > + case MEMORY_DEVICE_INFO_KIND_DIMM: > > >> >> > > > + di = value->dimm; > > >> >> > > > + > > >> >> > > > + monitor_printf(mon, "Memory device [%s]: %s\n", > > >> >> > > > + MemoryDeviceInfoKind_lookup[value->kind], > > >> >> > > > + di->id); > > >> >> > 'id' might be null, here is what user will see: > > >> >> > > > >> >> > Memory device [dimm]: (null) > > >> >> > > > >> >> > I'd suggest to replace (null) with "" as it is done elsewhere. > > >> >> > > >> >> Is the fix below what you're looking for? If it is I can apply it myself: > > >> > > > >> > I am sorry, I did not test this fix in my code last time. But your fix > > >> > would print nothing when the id was null. > > >> > > >> Printing nothing when there's no ID has a certain logic to it :) > > > > > > When I do 'info qtree' it displays empty IDs with as empty "" > > > I think we should be consistent and apply above correction. > > > > If you want consistency with "info qtree", you should enclose non-null > > ID in double quotes. > > zhugh, at this point is better to respin the patch. Ok, I will send v5 to fix this. Thanks! Zhu ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-09-23 1:08 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-09-15 11:31 [Qemu-devel] [PATCH v4] Add HMP command "info memory-devices" Zhu Guihua 2014-09-18 8:09 ` zhugh 2014-09-19 13:30 ` Igor Mammedov 2014-09-19 15:34 ` Luiz Capitulino 2014-09-20 4:51 ` zhugh 2014-09-22 7:08 ` zhugh 2014-09-22 7:59 ` Markus Armbruster 2014-09-22 8:05 ` Igor Mammedov 2014-09-22 9:29 ` Markus Armbruster 2014-09-22 13:03 ` Luiz Capitulino 2014-09-23 1:08 ` Zhu Guihua
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).