Re: [Qemu-devel] [PULL 05/23] pc-dimm: add pc_dimm_build_list()

[Michael S. Tsirkin]

On Thu, Feb 25, 2016 at 11:09:16AM +0100, Paolo Bonzini wrote:
> 
> 
> On 25/02/2016 10:54, Vladimir Sementsov-Ogievskiy wrote:
> > On 25.02.2016 12:11, Michael S. Tsirkin wrote:
> >> On Thu, Feb 25, 2016 at 11:55:56AM +0300, Vladimir Sementsov-Ogievskiy
> >> wrote:
> >>> On 25.02.2016 11:39, Michael S. Tsirkin wrote:
> >>>> On Thu, Feb 25, 2016 at 10:01:18AM +0300, Vladimir
> >>>> Sementsov-Ogievskiy wrote:
> >>>>> Stop!
> >>>> It builds fine here. Did you check out my tree?
> >>> anyway, this file should be here, as it is added into Makefile:
> >>>
> >>> --- a/stubs/Makefile.objs
> >>> +++ b/stubs/Makefile.objs
> >>> @@ -35,7 +35,7 @@ stub-obj-y += vmstate.o
> >>>   stub-obj-$(CONFIG_WIN32) += fd-register.o
> >>>   stub-obj-y += cpus.o
> >>>   stub-obj-y += kvm.o
> >>> -stub-obj-y += qmp_pc_dimm_device_list.o
> >>> +stub-obj-y += pc_dimm.o
> >>>   stub-obj-y += target-monitor-defs.o
> >>>   stub-obj-y += target-get-monitor-def.o
> >>>   stub-obj-y += vhost.o
> >>>
> >> Hmm that's right.
> >> Thanks!
> >> Looks like it was picking up an old object in the filesystem :(
> >> I'll respin - for now, does everything work for you if you drop
> >> this line from the makefile?
> > 
> > it will break compilation for targets without memory hotplug, it was
> > discussed on list. stubs are necessary.
> 
> mst,
> 
> indeed it's not clear to me why you're deleting qmp_pc_dimm_device_list.c...
> 
> On the other hand, pc_dimm_build_list doesn't need a stub.  Just put it
> in hw/virtio/virtio-balloon.c.
> 
> Paolo

Like this then?

diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index 0f587a4..f6cbc78 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -95,6 +95,4 @@ void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState 
*hpms,
 void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState *hpms,
                            MemoryRegion *mr);
 
-int pc_dimm_build_list(Object *obj, void *opaque);
-
 #endif
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 09e99ce..835179e 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -267,21 +267,6 @@ static int pc_dimm_build_list_sorted(Object *obj, void 
*opaque)
     return 0;
 }
 
-int pc_dimm_build_list(Object *obj, void *opaque)
-{
-    GSList **list = opaque;
-
-    if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
-        DeviceState *dev = DEVICE(obj);
-        if (dev->realized) { /* only realized DIMMs matter */
-            *list = g_slist_prepend(*list, dev);
-        }
-    }
-
-    object_child_foreach(obj, pc_dimm_build_list, opaque);
-    return 0;
-}
-
 uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
                                uint64_t address_space_size,
                                uint64_t *hint, uint64_t align, uint64_t size,
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 3fb6c52..24eab0a 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -305,12 +305,27 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, 
uint8_t *config_data)
     memcpy(config_data, &config, sizeof(struct virtio_balloon_config));
 }
 
+int build_dimm_list(Object *obj, void *opaque)
+{
+    GSList **list = opaque;
+
+    if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
+        DeviceState *dev = DEVICE(obj);
+        if (dev->realized) { /* only realized DIMMs matter */
+            *list = g_slist_prepend(*list, dev);
+        }
+    }
+
+    object_child_foreach(obj, build_dimm_list, opaque);
+    return 0;
+}
+
 static ram_addr_t get_current_ram_size(void)
 {
     GSList *list = NULL, *item;
     ram_addr_t size = ram_size;
 
-    pc_dimm_build_list(qdev_get_machine(), &list);
+    build_dimm_list(qdev_get_machine(), &list);
     for (item = list; item; item = g_slist_next(item)) {
         Object *obj = OBJECT(item->data);
         if (!strcmp(object_get_typename(obj), TYPE_PC_DIMM)) {