Re: [Qemu-devel] [PATCH v3 19/22] hmp: handle virtio-pmem when printing memory device infos

[David Hildenbrand]

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


-- 

Thanks,

David / dhildenb