* [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).