Re: [Qemu-devel] [PATCH] Replace calls to object_child_foreach() with object_child_foreach_recursive()

[Paolo Bonzini]

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)