Re: [Qemu-devel] [PATCH 4/5] virtio-balloon: rewrite get_current_ram_size()

[Igor Mammedov]

On Fri, 5 Feb 2016 17:12:55 +0300
Vladimir Sementsov-Ogievskiy <address@hidden> wrote:

> On 05.02.2016 15:57, Igor Mammedov wrote:
> > On Fri,  5 Feb 2016 11:19:00 +0300
> > Vladimir Sementsov-Ogievskiy <address@hidden> 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 <address@hidden>
> >> ---
> >>   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;
> >>   }  
> 
>