Replace calls to object_child_foreach() with object_child_foreach_recursive()
when applicable: nvdimm_device_list, nmi_monitor_handle, find_sysbus_device,
pc_dimm_slot2bitmap, build_dimm_list.

Signed-off-by: Ernest Esene <eroken1@gmail.com>
---
 hw/acpi/nvdimm.c           | 5 +++--
 hw/core/sysbus.c           | 2 +-
 hw/mem/pc-dimm.c           | 5 +++--
 hw/virtio/virtio-balloon.c | 2 +-
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index e53b2cb..846f44b 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -41,7 +41,7 @@ static int nvdimm_device_list(Object *obj, void *opaque)
         *list = g_slist_append(*list, DEVICE(obj));
     }
 
-    object_child_foreach(obj, nvdimm_device_list, opaque);
+    object_child_foreach_recursive(obj, nvdimm_device_list, opaque);
     return 0;
 }
 
@@ -56,7 +56,8 @@ static GSList *nvdimm_get_device_list(void)
 {
     GSList *list = NULL;
 
-    object_child_foreach(qdev_get_machine(), nvdimm_device_list, &list);
+    object_child_foreach_recursive(qdev_get_machine(),
+                                   nvdimm_device_list, &list);
     return list;
 }
 
diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 9f9edbc..c16d57c 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -43,7 +43,7 @@ static int find_sysbus_device(Object *obj, void *opaque)
 
     if (!sbdev) {
         /* Container, traverse it for children */
-        return object_child_foreach(obj, find_sysbus_device, opaque);
+        return object_child_foreach_recursive(obj, find_sysbus_device, opaque);
     }
 
     find->func(sbdev, find->opaque);
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 152400b..844c8ac 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -84,7 +84,7 @@ static int pc_dimm_slot2bitmap(Object *obj, void *opaque)
         }
     }
 
-    object_child_foreach(obj, pc_dimm_slot2bitmap, opaque);
+    object_child_foreach_recursive(obj, pc_dimm_slot2bitmap, opaque);
     return 0;
 }
 
@@ -100,7 +100,8 @@ static int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp)
     }
 
     bitmap = bitmap_new(max_slots);
-    object_child_foreach(qdev_get_machine(), pc_dimm_slot2bitmap, bitmap);
+    object_child_foreach_recursive(qdev_get_machine(),
+                                   pc_dimm_slot2bitmap, bitmap);
 
     /* check if requested slot is not occupied */
     if (hint) {
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index e3a6594..ce4b8a5 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -591,7 +591,7 @@ static int build_dimm_list(Object *obj, void *opaque)
         }
     }
 
-    object_child_foreach(obj, build_dimm_list, opaque);
+    object_child_foreach_recursive(obj, build_dimm_list, opaque);
     return 0;
 }
 
-- 
2.14.2

On Sun, Mar 10, 2019 at 02:39:13AM +0100, Ernest Esene wrote:
> Replace calls to object_child_foreach() with object_child_foreach_recursive()
> when applicable: nvdimm_device_list, nmi_monitor_handle, find_sysbus_device,
> pc_dimm_slot2bitmap, build_dimm_list.

Ernest or Paolo: What is the rationale for this change?

> Signed-off-by: Ernest Esene <eroken1@gmail.com>
> ---
>  hw/acpi/nvdimm.c           | 5 +++--
>  hw/core/sysbus.c           | 2 +-
>  hw/mem/pc-dimm.c           | 5 +++--
>  hw/virtio/virtio-balloon.c | 2 +-
>  4 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index e53b2cb..846f44b 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -41,7 +41,7 @@ static int nvdimm_device_list(Object *obj, void *opaque)
>          *list = g_slist_append(*list, DEVICE(obj));
>      }
>  
> -    object_child_foreach(obj, nvdimm_device_list, opaque);
> +    object_child_foreach_recursive(obj, nvdimm_device_list, opaque);
>      return 0;
>  }
>  
> @@ -56,7 +56,8 @@ static GSList *nvdimm_get_device_list(void)
>  {
>      GSList *list = NULL;
>  
> -    object_child_foreach(qdev_get_machine(), nvdimm_device_list, &list);
> +    object_child_foreach_recursive(qdev_get_machine(),
> +                                   nvdimm_device_list, &list);
>      return list;
>  }
>  
> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> index 9f9edbc..c16d57c 100644
> --- a/hw/core/sysbus.c
> +++ b/hw/core/sysbus.c
> @@ -43,7 +43,7 @@ static int find_sysbus_device(Object *obj, void *opaque)
>  
>      if (!sbdev) {
>          /* Container, traverse it for children */
> -        return object_child_foreach(obj, find_sysbus_device, opaque);
> +        return object_child_foreach_recursive(obj, find_sysbus_device, opaque);
>      }
>  
>      find->func(sbdev, find->opaque);
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 152400b..844c8ac 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -84,7 +84,7 @@ static int pc_dimm_slot2bitmap(Object *obj, void *opaque)
>          }
>      }
>  
> -    object_child_foreach(obj, pc_dimm_slot2bitmap, opaque);
> +    object_child_foreach_recursive(obj, pc_dimm_slot2bitmap, opaque);
>      return 0;
>  }
>  
> @@ -100,7 +100,8 @@ static int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp)
>      }
>  
>      bitmap = bitmap_new(max_slots);
> -    object_child_foreach(qdev_get_machine(), pc_dimm_slot2bitmap, bitmap);
> +    object_child_foreach_recursive(qdev_get_machine(),
> +                                   pc_dimm_slot2bitmap, bitmap);
>  
>      /* check if requested slot is not occupied */
>      if (hint) {
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index e3a6594..ce4b8a5 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -591,7 +591,7 @@ static int build_dimm_list(Object *obj, void *opaque)
>          }
>      }
>  
> -    object_child_foreach(obj, build_dimm_list, opaque);
> +    object_child_foreach_recursive(obj, build_dimm_list, opaque);
>      return 0;
>  }
>  
> -- 
> 2.14.2
> 

On 26/03/19 08:43, Stefan Hajnoczi wrote:
> On Sun, Mar 10, 2019 at 02:39:13AM +0100, Ernest Esene wrote:
>> Replace calls to object_child_foreach() with object_child_foreach_recursive()
>> when applicable: nvdimm_device_list, nmi_monitor_handle, find_sysbus_device,
>> pc_dimm_slot2bitmap, build_dimm_list.
> 
> Ernest or Paolo: What is the rationale for this change?

The change could have been described better probably...  The idea is
that for example nvdimm_device_list could just stop invoking
object_child_foreach, because recursion is handled by
nvdimm_get_device_list:

>> @@ -41,7 +41,7 @@ static int nvdimm_device_list(Object *obj, void *opaque)
>>          *list = g_slist_append(*list, DEVICE(obj));
>>      }
>>  
>> -    object_child_foreach(obj, nvdimm_device_list, opaque);
>> +    object_child_foreach_recursive(obj, nvdimm_device_list, opaque);
>>      return 0;
>>  }
>>  
>> @@ -56,7 +56,8 @@ static GSList *nvdimm_get_device_list(void)
>>  {
>>      GSList *list = NULL;
>>  
>> -    object_child_foreach(qdev_get_machine(), nvdimm_device_list, &list);
>> +    object_child_foreach_recursive(qdev_get_machine(),
>> +                                   nvdimm_device_list, &list);
>>      return list;
>>  }

Likewise, the call to object_child_foreach could disappear here
(replaced by just a "return"):

>> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
>> index 9f9edbc..c16d57c 100644
>> --- a/hw/core/sysbus.c
>> +++ b/hw/core/sysbus.c
>> @@ -43,7 +43,7 @@ static int find_sysbus_device(Object *obj, void *opaque)
>>  
>>      if (!sbdev) {
>>          /* Container, traverse it for children */
>> -        return object_child_foreach(obj, find_sysbus_device, opaque);
>> +        return object_child_foreach_recursive(obj, find_sysbus_device, opaque);
>>      }
>>  
>>      find->func(sbdev, find->opaque);

and instead foreach_dynamic_sysbus_device would call
object_child_foreach_recursive.

You get the idea of what the change would be here:

>> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
>> index 152400b..844c8ac 100644
>> --- a/hw/mem/pc-dimm.c
>> +++ b/hw/mem/pc-dimm.c
>> @@ -84,7 +84,7 @@ static int pc_dimm_slot2bitmap(Object *obj, void *opaque)
>>          }
>>      }
>>  
>> -    object_child_foreach(obj, pc_dimm_slot2bitmap, opaque);
>> +    object_child_foreach_recursive(obj, pc_dimm_slot2bitmap, opaque);
>>      return 0;
>>  }
>>  
>> @@ -100,7 +100,8 @@ static int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp)
>>      }
>>  
>>      bitmap = bitmap_new(max_slots);
>> -    object_child_foreach(qdev_get_machine(), pc_dimm_slot2bitmap, bitmap);
>> +    object_child_foreach_recursive(qdev_get_machine(),
>> +                                   pc_dimm_slot2bitmap, bitmap);
>>  
>>      /* check if requested slot is not occupied */
>>      if (hint) {

(similar to nvdimm_device_list) and here:

>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>> index e3a6594..ce4b8a5 100644
>> --- a/hw/virtio/virtio-balloon.c
>> +++ b/hw/virtio/virtio-balloon.c
>> @@ -591,7 +591,7 @@ static int build_dimm_list(Object *obj, void *opaque)
>>          }
>>      }
>>  
>> -    object_child_foreach(obj, build_dimm_list, opaque);
>> +    object_child_foreach_recursive(obj, build_dimm_list, opaque);
>>      return 0;
>>  }

(similar to find_sysbus_device)

On Tue, Mar 26, 2019 at 10:12:56AM +0100, Paolo Bonzini wrote:
> On 26/03/19 08:43, Stefan Hajnoczi wrote:
> > On Sun, Mar 10, 2019 at 02:39:13AM +0100, Ernest Esene wrote:
> >> Replace calls to object_child_foreach() with object_child_foreach_recursive()
> >> when applicable: nvdimm_device_list, nmi_monitor_handle, find_sysbus_device,
> >> pc_dimm_slot2bitmap, build_dimm_list.
> > 
> > Ernest or Paolo: What is the rationale for this change?
> 
> The change could have been described better probably...  The idea is
> that for example nvdimm_device_list could just stop invoking
> object_child_foreach, because recursion is handled by
> nvdimm_get_device_list:
> 
> >> @@ -41,7 +41,7 @@ static int nvdimm_device_list(Object *obj, void *opaque)
> >>          *list = g_slist_append(*list, DEVICE(obj));
> >>      }
> >>  
> >> -    object_child_foreach(obj, nvdimm_device_list, opaque);
> >> +    object_child_foreach_recursive(obj, nvdimm_device_list, opaque);
> >>      return 0;
> >>  }
> >>  
> >> @@ -56,7 +56,8 @@ static GSList *nvdimm_get_device_list(void)
> >>  {
> >>      GSList *list = NULL;
> >>  
> >> -    object_child_foreach(qdev_get_machine(), nvdimm_device_list, &list);
> >> +    object_child_foreach_recursive(qdev_get_machine(),
> >> +                                   nvdimm_device_list, &list);
> >>      return list;
> >>  }

Thanks, that makes sense!

Ernest: If you want to continue with this task, please make the changes
Paolo has suggested and update the commit description to say that
manually calling object_child_foreach() is unnecessary for children
since a single object_child_foreach_recursive() call can be used
instead.

Thanks,
Stefan

On Fri, Mar 29, 2019 at 10:47:22AM +0000, Stefan Hajnoczi wrote:
> On Tue, Mar 26, 2019 at 10:12:56AM +0100, Paolo Bonzini wrote:
> > On 26/03/19 08:43, Stefan Hajnoczi wrote:
> > > On Sun, Mar 10, 2019 at 02:39:13AM +0100, Ernest Esene wrote:
> > >> Replace calls to object_child_foreach() with object_child_foreach_recursive()
> > >> when applicable: nvdimm_device_list, nmi_monitor_handle, find_sysbus_device,
> > >> pc_dimm_slot2bitmap, build_dimm_list.
> > > 
> > > Ernest or Paolo: What is the rationale for this change?
> > 
> > The change could have been described better probably...  The idea is
> > that for example nvdimm_device_list could just stop invoking
> > object_child_foreach, because recursion is handled by
> > nvdimm_get_device_list:
> > 
> > >> @@ -41,7 +41,7 @@ static int nvdimm_device_list(Object *obj, void *opaque)
> > >>          *list = g_slist_append(*list, DEVICE(obj));
> > >>      }
> > >>  
> > >> -    object_child_foreach(obj, nvdimm_device_list, opaque);
> > >> +    object_child_foreach_recursive(obj, nvdimm_device_list, opaque);
> > >>      return 0;
> > >>  }
> > >>  
> > >> @@ -56,7 +56,8 @@ static GSList *nvdimm_get_device_list(void)
> > >>  {
> > >>      GSList *list = NULL;
> > >>  
> > >> -    object_child_foreach(qdev_get_machine(), nvdimm_device_list, &list);
> > >> +    object_child_foreach_recursive(qdev_get_machine(),
> > >> +                                   nvdimm_device_list, &list);
> > >>      return list;
> > >>  }
> 
> Thanks, that makes sense!
> 
> Ernest: If you want to continue with this task, please make the changes
> Paolo has suggested and update the commit description to say that
> manually calling object_child_foreach() is unnecessary for children
> since a single object_child_foreach_recursive() call can be used
> instead.
> 
> Thanks,
> Stefan

Thank you Stefan and Paolo, I shall do it.

-Ernest