| Message ID | 20180920103243.28474-20-david@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | memory-device: complete refactoring + virtio-pmem | expand |
* David Hildenbrand (david@redhat.com) wrote: > Print the memory device info just like for PCDIMM/NVDIMM. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > hmp.c | 29 +++++++++++++++++------------ > 1 file changed, 17 insertions(+), 12 deletions(-) > > diff --git a/hmp.c b/hmp.c > index 4975fa56b0..a592a4af9d 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -2529,27 +2529,21 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) > MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err); > MemoryDeviceInfoList *info; > MemoryDeviceInfo *value; > - PCDIMMDeviceInfo *di; > + VirtioPMemDeviceInfo *vpi; > > for (info = info_list; info; info = info->next) { > + PCDIMMDeviceInfo *di = NULL; > + > value = info->value; > > if (value) { > switch (value->type) { > case MEMORY_DEVICE_INFO_KIND_DIMM: > di = value->u.dimm.data; > - break; > - > case MEMORY_DEVICE_INFO_KIND_NVDIMM: > - di = value->u.nvdimm.data; > - break; > - > - default: > - di = NULL; > - break; > - } > - > - if (di) { > + if (!di) { > + di = value->u.nvdimm.data; > + } > monitor_printf(mon, "Memory device [%s]: \"%s\"\n", > MemoryDeviceInfoKind_str(value->type), > di->id ? di->id : ""); That all works out as something like: > + PCDIMMDeviceInfo *di = NULL; > + > value = info->value; > > if (value) { > switch (value->type) { > case MEMORY_DEVICE_INFO_KIND_DIMM: > di = value->u.dimm.data; > case MEMORY_DEVICE_INFO_KIND_NVDIMM: > + if (!di) { > + di = value->u.nvdimm.data; > + } > monitor_printf(mon, "Memory device [%s]: \"%s\"\n", > MemoryDeviceInfoKind_str(value->type), > di->id ? di->id : ""); I'd prefer to avoid the drop through case statement; something like: PCDIMMDeviceInfo *di = NULL; value = info->value; if (value) { switch (value->type) { case MEMORY_DEVICE_INFO_KIND_DIMM: case MEMORY_DEVICE_INFO_KIND_NVDIMM: di = (value->type == MEMORY_DEVICE_INFO_KIND_DIMM) ? value->u.dimm.data : value->u.nvdimm.data; monitor_printf(mon, "Memory device [%s]: \"%s\"\n", > @@ -2562,6 +2556,17 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) > di->hotplugged ? "true" : "false"); > monitor_printf(mon, " hotpluggable: %s\n", > di->hotpluggable ? "true" : "false"); > + break; > + case MEMORY_DEVICE_INFO_KIND_VIRTIO_PMEM: > + vpi = value->u.virtio_pmem.data; > + monitor_printf(mon, "Memory device [%s]: \"%s\"\n", > + MemoryDeviceInfoKind_str(value->type), > + vpi->id ? vpi->id : ""); > + monitor_printf(mon, " memaddr: 0x%" PRIx64 "\n", vpi->memaddr); > + monitor_printf(mon, " size: %" PRIu64 "\n", vpi->size); > + monitor_printf(mon, " memdev: %s\n", vpi->memdev); Do you not get any equivalent of the 'hotplugged' flag or node? Dave > + default: > + break; > } > } > } > -- > 2.17.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> That all works out as something like: >> + PCDIMMDeviceInfo *di = NULL; >> + >> value = info->value; >> >> if (value) { >> switch (value->type) { >> case MEMORY_DEVICE_INFO_KIND_DIMM: >> di = value->u.dimm.data; >> case MEMORY_DEVICE_INFO_KIND_NVDIMM: >> + if (!di) { >> + di = value->u.nvdimm.data; >> + } >> monitor_printf(mon, "Memory device [%s]: \"%s\"\n", >> MemoryDeviceInfoKind_str(value->type), >> di->id ? di->id : ""); > > I'd prefer to avoid the drop through case statement; something like: Sure, I can do that! Thanks! > PCDIMMDeviceInfo *di = NULL; > > value = info->value; > > if (value) { > switch (value->type) { > case MEMORY_DEVICE_INFO_KIND_DIMM: > case MEMORY_DEVICE_INFO_KIND_NVDIMM: > di = (value->type == MEMORY_DEVICE_INFO_KIND_DIMM) ? > value->u.dimm.data : value->u.nvdimm.data; > monitor_printf(mon, "Memory device [%s]: \"%s\"\n", > >> @@ -2562,6 +2556,17 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) >> di->hotplugged ? "true" : "false"); >> monitor_printf(mon, " hotpluggable: %s\n", >> di->hotpluggable ? "true" : "false"); >> + break; >> + case MEMORY_DEVICE_INFO_KIND_VIRTIO_PMEM: >> + vpi = value->u.virtio_pmem.data; >> + monitor_printf(mon, "Memory device [%s]: \"%s\"\n", >> + MemoryDeviceInfoKind_str(value->type), >> + vpi->id ? vpi->id : ""); >> + monitor_printf(mon, " memaddr: 0x%" PRIx64 "\n", vpi->memaddr); >> + monitor_printf(mon, " size: %" PRIu64 "\n", vpi->size); >> + monitor_printf(mon, " memdev: %s\n", vpi->memdev); > > Do you not get any equivalent of the 'hotplugged' flag or node? You discovered the node remark in the following patch :) "hotplugged" doesn't seem to apply in the context of virtio-pmem. We can consider all as "hotplugged", although unplugging is most likely not supported (I assume just like nvdimm, we can't rip these out of a running operating system).
diff --git a/hmp.c b/hmp.c index 4975fa56b0..a592a4af9d 100644 --- a/hmp.c +++ b/hmp.c @@ -2529,27 +2529,21 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err); MemoryDeviceInfoList *info; MemoryDeviceInfo *value; - PCDIMMDeviceInfo *di; + VirtioPMemDeviceInfo *vpi; for (info = info_list; info; info = info->next) { + PCDIMMDeviceInfo *di = NULL; + value = info->value; if (value) { switch (value->type) { case MEMORY_DEVICE_INFO_KIND_DIMM: di = value->u.dimm.data; - break; - case MEMORY_DEVICE_INFO_KIND_NVDIMM: - di = value->u.nvdimm.data; - break; - - default: - di = NULL; - break; - } - - if (di) { + if (!di) { + di = value->u.nvdimm.data; + } monitor_printf(mon, "Memory device [%s]: \"%s\"\n", MemoryDeviceInfoKind_str(value->type), di->id ? di->id : ""); @@ -2562,6 +2556,17 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) di->hotplugged ? "true" : "false"); monitor_printf(mon, " hotpluggable: %s\n", di->hotpluggable ? "true" : "false"); + break; + case MEMORY_DEVICE_INFO_KIND_VIRTIO_PMEM: + vpi = value->u.virtio_pmem.data; + monitor_printf(mon, "Memory device [%s]: \"%s\"\n", + MemoryDeviceInfoKind_str(value->type), + vpi->id ? vpi->id : ""); + monitor_printf(mon, " memaddr: 0x%" PRIx64 "\n", vpi->memaddr); + monitor_printf(mon, " size: %" PRIu64 "\n", vpi->size); + monitor_printf(mon, " memdev: %s\n", vpi->memdev); + default: + break; } } }
Print the memory device info just like for PCDIMM/NVDIMM. Signed-off-by: David Hildenbrand <david@redhat.com> --- hmp.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-)