1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v3 00/22] memory-device: complete refactoring + virtio-pmem
  4. View patch

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

David Hildenbrand posted 22 patches 7 years, 4 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v3 19/22] hmp: handle virtio-pmem when printing memory device infos
Posted by David Hildenbrand 7 years, 4 months ago 
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 : "");
@@ -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;
             }
         }
     }
-- 
2.17.1
Re: [Qemu-devel] [PATCH v3 19/22] hmp: handle virtio-pmem when printing memory device infos
Posted by Dr. David Alan Gilbert 7 years, 4 months ago 
* 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
Re: [Qemu-devel] [PATCH v3 19/22] hmp: handle virtio-pmem when printing memory device infos
Posted by David Hildenbrand 7 years, 4 months ago 
> 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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.