* Markus Armbruster (arm...@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilb...@redhat.com> writes:
>
> > * Markus Armbruster (arm...@redhat.com) wrote:
> >> "Dr. David Alan Gilbert" <dgilb...@redhat.com> writes:
> >>
> >> > * David Hildenbrand (da...@redhat.com) wrote:
> >> >> Print the memory device info just like for PCDIMM/NVDIMM.
> >> >>
> >> >> Signed-off-by: David Hildenbrand <da...@redhat.com>
> >> >> ---
> >> >> hmp.c | 27 +++++++++++++++------------
> >> >> 1 file changed, 15 insertions(+), 12 deletions(-)
> >> >>
> >> >> diff --git a/hmp.c b/hmp.c
> >> >> index 8da5fd8760..25c32e0810 100644
> >> >> --- a/hmp.c
> >> >> +++ b/hmp.c
> >> >> @@ -2553,6 +2553,7 @@ void hmp_info_memory_devices(Monitor *mon, const
> >> >> QDict *qdict)
> >> >> Error *err = NULL;
> >> >> MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err);
> >> >> MemoryDeviceInfoList *info;
> >> >> + VirtioPMEMDeviceInfo *vpi;
> >> >> MemoryDeviceInfo *value;
> >> >> PCDIMMDeviceInfo *di;
> >> >>
> >> >> @@ -2562,19 +2563,9 @@ void hmp_info_memory_devices(Monitor *mon, const
> >> >> QDict *qdict)
> >> >> 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) {
> >> >> + di = value->type == MEMORY_DEVICE_INFO_KIND_DIMM ?
> >> >> + value->u.dimm.data : value->u.nvdimm.data;
> >> >> monitor_printf(mon, "Memory device [%s]: \"%s\"\n",
> >> >> MemoryDeviceInfoKind_str(value->type),
> >> >> di->id ? di->id : "");
> >> >> @@ -2587,6 +2578,18 @@ 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);
> >> >> + break;
> >> >> + default:
> >> >> + g_assert_not_reached();
> >> >
> >> >
> >> > Although I'd prefer if that assert was replaced by a print
> >> > saying it was an unknown type.
> >>
> >> I would not. If we reach this, something must have scribbled over
> >> value->type and who knows what else. Continuing is unsafe. Looks like
> >> a textbook use of assertions to me.
> >
> > Or it could be that someone added a new type of memory device and forgot
> > to update the hmp code.
>
> Programming error -> assert. Nothing catches attention in testing like
> an assertion failure.
I don't want to get back through that; we've had this argument before;
but I'll still say in this case I'd prefer a clean error rather than
abort.
Dave
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
