| Message ID | 20190310013913.GA16519@erokenlabserver (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Replace calls to object_child_foreach() with object_child_foreach_recursive() | expand |
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
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; }
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(-)