diff mbox

[4/5] virtio-balloon: rewrite get_current_ram_size()

Message ID 1454660341-45244-5-git-send-email-vsementsov@virtuozzo.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vladimir Sementsov-Ogievskiy Feb. 5, 2016, 8:19 a.m. UTC
Use pc_dimm_built_list() instead of qmp_pc_dimm_device_list()

Actually, Qapi is not related to this internal helper.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 hw/virtio/virtio-balloon.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

Comments

Igor Mammedov Feb. 5, 2016, 12:57 p.m. UTC | #1
On Fri,  5 Feb 2016 11:19:00 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:

> Use pc_dimm_built_list() instead of qmp_pc_dimm_device_list()
> 
> Actually, Qapi is not related to this internal helper.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  hw/virtio/virtio-balloon.c | 23 ++++++-----------------
>  1 file changed, 6 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 6a4c4d2..b9c1964 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -296,26 +296,15 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
>  
>  static ram_addr_t get_current_ram_size(void)
>  {
> -    MemoryDeviceInfoList *info_list = NULL;
> -    MemoryDeviceInfoList **prev = &info_list;
> -    MemoryDeviceInfoList *info;
> +    GSList *list = NULL, *item;
>      ram_addr_t size = ram_size;
>  
> -    qmp_pc_dimm_device_list(qdev_get_machine(), &prev);
> -    for (info = info_list; info; info = info->next) {
> -        MemoryDeviceInfo *value = info->value;
> -
> -        if (value) {
> -            switch (value->type) {
> -            case MEMORY_DEVICE_INFO_KIND_DIMM:
> -                size += value->u.dimm->size;
> -                break;
> -            default:
> -                break;
> -            }
> -        }
> +    pc_dimm_build_list(qdev_get_machine(), &list);
> +    for (item = list; item; item = g_slist_next(item)) {
> +        PCDIMMDevice *dimm = item->data;
dimm is accessed via properties so you can use Object* instead of PCDIMMDevice

Object *obj = OBJECT(item->data)

> +        size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);
maybe
s/NULL/error_abort/

>      }
> -    qapi_free_MemoryDeviceInfoList(info_list);
> +    g_slist_free(list);
>  
>      return size;
>  }
Vladimir Sementsov-Ogievskiy Feb. 5, 2016, 2:12 p.m. UTC | #2
On 05.02.2016 15:57, Igor Mammedov wrote:
> On Fri,  5 Feb 2016 11:19:00 +0300
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:
>
>> Use pc_dimm_built_list() instead of qmp_pc_dimm_device_list()
>>
>> Actually, Qapi is not related to this internal helper.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   hw/virtio/virtio-balloon.c | 23 ++++++-----------------
>>   1 file changed, 6 insertions(+), 17 deletions(-)
>>
>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>> index 6a4c4d2..b9c1964 100644
>> --- a/hw/virtio/virtio-balloon.c
>> +++ b/hw/virtio/virtio-balloon.c
>> @@ -296,26 +296,15 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
>>   
>>   static ram_addr_t get_current_ram_size(void)
>>   {
>> -    MemoryDeviceInfoList *info_list = NULL;
>> -    MemoryDeviceInfoList **prev = &info_list;
>> -    MemoryDeviceInfoList *info;
>> +    GSList *list = NULL, *item;
>>       ram_addr_t size = ram_size;
>>   
>> -    qmp_pc_dimm_device_list(qdev_get_machine(), &prev);
>> -    for (info = info_list; info; info = info->next) {
>> -        MemoryDeviceInfo *value = info->value;
>> -
>> -        if (value) {
>> -            switch (value->type) {
>> -            case MEMORY_DEVICE_INFO_KIND_DIMM:
>> -                size += value->u.dimm->size;
>> -                break;
>> -            default:
>> -                break;
>> -            }
>> -        }
>> +    pc_dimm_build_list(qdev_get_machine(), &list);
>> +    for (item = list; item; item = g_slist_next(item)) {
>> +        PCDIMMDevice *dimm = item->data;
> dimm is accessed via properties so you can use Object* instead of PCDIMMDevice
>
> Object *obj = OBJECT(item->data)

ok

>
>> +        size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);
> maybe
> s/NULL/error_abort/

Not sure, but agree, absence of size for memory device is very strange.

>
>>       }
>> -    qapi_free_MemoryDeviceInfoList(info_list);
>> +    g_slist_free(list);
>>   
>>       return size;
>>   }
Igor Mammedov Feb. 5, 2016, 2:53 p.m. UTC | #3
On Fri, 5 Feb 2016 17:12:55 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:

> On 05.02.2016 15:57, Igor Mammedov wrote:
> > On Fri,  5 Feb 2016 11:19:00 +0300
> > Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:
> >  
> >> Use pc_dimm_built_list() instead of qmp_pc_dimm_device_list()
> >>
> >> Actually, Qapi is not related to this internal helper.
> >>
> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> >> ---
> >>   hw/virtio/virtio-balloon.c | 23 ++++++-----------------
> >>   1 file changed, 6 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> >> index 6a4c4d2..b9c1964 100644
> >> --- a/hw/virtio/virtio-balloon.c
> >> +++ b/hw/virtio/virtio-balloon.c
> >> @@ -296,26 +296,15 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
> >>   
> >>   static ram_addr_t get_current_ram_size(void)
> >>   {
> >> -    MemoryDeviceInfoList *info_list = NULL;
> >> -    MemoryDeviceInfoList **prev = &info_list;
> >> -    MemoryDeviceInfoList *info;
> >> +    GSList *list = NULL, *item;
> >>       ram_addr_t size = ram_size;
> >>   
> >> -    qmp_pc_dimm_device_list(qdev_get_machine(), &prev);
> >> -    for (info = info_list; info; info = info->next) {
> >> -        MemoryDeviceInfo *value = info->value;
> >> -
> >> -        if (value) {
> >> -            switch (value->type) {
> >> -            case MEMORY_DEVICE_INFO_KIND_DIMM:
> >> -                size += value->u.dimm->size;
> >> -                break;
> >> -            default:
> >> -                break;
> >> -            }
> >> -        }
> >> +    pc_dimm_build_list(qdev_get_machine(), &list);
> >> +    for (item = list; item; item = g_slist_next(item)) {
> >> +        PCDIMMDevice *dimm = item->data;  
> > dimm is accessed via properties so you can use Object* instead of PCDIMMDevice
> >
> > Object *obj = OBJECT(item->data)  
> 
> ok
> 
> >  
> >> +        size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);  
> > maybe
> > s/NULL/error_abort/  
> 
> Not sure, but agree, absence of size for memory device is very strange.
it shouldn't happen ever, if error happens it's programming error
and we should fix it then but not hide it.

> 
> >  
> >>       }
> >> -    qapi_free_MemoryDeviceInfoList(info_list);
> >> +    g_slist_free(list);
> >>   
> >>       return size;
> >>   }  
> 
>
diff mbox

Patch

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 6a4c4d2..b9c1964 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -296,26 +296,15 @@  static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
 
 static ram_addr_t get_current_ram_size(void)
 {
-    MemoryDeviceInfoList *info_list = NULL;
-    MemoryDeviceInfoList **prev = &info_list;
-    MemoryDeviceInfoList *info;
+    GSList *list = NULL, *item;
     ram_addr_t size = ram_size;
 
-    qmp_pc_dimm_device_list(qdev_get_machine(), &prev);
-    for (info = info_list; info; info = info->next) {
-        MemoryDeviceInfo *value = info->value;
-
-        if (value) {
-            switch (value->type) {
-            case MEMORY_DEVICE_INFO_KIND_DIMM:
-                size += value->u.dimm->size;
-                break;
-            default:
-                break;
-            }
-        }
+    pc_dimm_build_list(qdev_get_machine(), &list);
+    for (item = list; item; item = g_slist_next(item)) {
+        PCDIMMDevice *dimm = item->data;
+        size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);
     }
-    qapi_free_MemoryDeviceInfoList(info_list);
+    g_slist_free(list);
 
     return size;
 }