[Qemu-devel] [PATCH] qom: Make object_child_foreach safe for objects removal

[Qemu-devel] [PATCH] qom: Make object_child_foreach safe for objects removal

[Alexey Kardashevskiy]

Current object_child_foreach() uses QTAILQ_FOREACH() to walk
through children and that makes children removal from the callback
impossible.

This makes object_child_foreach() use QTAILQ_FOREACH_SAFE().

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---

The problem I am trying to solve is:
there is a PHB with multiple DMA windows a.k.a. sPAPRTCETable's which are
QOM children of PHB. One of RTAS functions is "reset" which is supposed to
remove all windows (now just one) except the default one.

I could call QTAILQ_FOREACH_SAFE in sPAPR PHB code but object_property_is_child()
is static and we probably do not want to make it public.


---
 qom/object.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index 0e8267b..4a814dc 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -678,10 +678,10 @@ void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
 int object_child_foreach(Object *obj, int (*fn)(Object *child, void *opaque),
                          void *opaque)
 {
-    ObjectProperty *prop;
+    ObjectProperty *prop, *next;
     int ret = 0;
 
-    QTAILQ_FOREACH(prop, &obj->properties, node) {
+    QTAILQ_FOREACH_SAFE(prop, &obj->properties, node, next) {
         if (object_property_is_child(prop)) {
             ret = fn(prop->opaque, opaque);
             if (ret != 0) {
-- 
2.0.0

Re: [Qemu-devel] [PATCH] qom: Make object_child_foreach safe for objects removal

[Hu Tao]

On Mon, Jul 14, 2014 at 12:41:08AM +1000, Alexey Kardashevskiy wrote:
> Current object_child_foreach() uses QTAILQ_FOREACH() to walk
> through children and that makes children removal from the callback
> impossible.
> 
> This makes object_child_foreach() use QTAILQ_FOREACH_SAFE().
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> 
> The problem I am trying to solve is:
> there is a PHB with multiple DMA windows a.k.a. sPAPRTCETable's which are
> QOM children of PHB. One of RTAS functions is "reset" which is supposed to
> remove all windows (now just one) except the default one.
> 
> I could call QTAILQ_FOREACH_SAFE in sPAPR PHB code but object_property_is_child()
> is static and we probably do not want to make it public.

Reviewed-by: Hu Tao <hutao@cn.fujitsu.com>

But people tend to accept changes like this one together with the
patch(es) that need it.

Regards,
Hu

> 
> 
> ---
>  qom/object.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/qom/object.c b/qom/object.c
> index 0e8267b..4a814dc 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -678,10 +678,10 @@ void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
>  int object_child_foreach(Object *obj, int (*fn)(Object *child, void *opaque),
>                           void *opaque)
>  {
> -    ObjectProperty *prop;
> +    ObjectProperty *prop, *next;
>      int ret = 0;
>  
> -    QTAILQ_FOREACH(prop, &obj->properties, node) {
> +    QTAILQ_FOREACH_SAFE(prop, &obj->properties, node, next) {
>          if (object_property_is_child(prop)) {
>              ret = fn(prop->opaque, opaque);
>              if (ret != 0) {
> -- 
> 2.0.0
> 

Re: [Qemu-devel] [PATCH] qom: Make object_child_foreach safe for objects removal

[Paolo Bonzini]

Il 13/07/2014 16:41, Alexey Kardashevskiy ha scritto:
> Current object_child_foreach() uses QTAILQ_FOREACH() to walk
> through children and that makes children removal from the callback
> impossible.
>
> This makes object_child_foreach() use QTAILQ_FOREACH_SAFE().
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>
> The problem I am trying to solve is:
> there is a PHB with multiple DMA windows a.k.a. sPAPRTCETable's which are
> QOM children of PHB. One of RTAS functions is "reset" which is supposed to
> remove all windows (now just one) except the default one.
>
> I could call QTAILQ_FOREACH_SAFE in sPAPR PHB code but object_property_is_child()
> is static and we probably do not want to make it public.
>
>
> ---
>  qom/object.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/qom/object.c b/qom/object.c
> index 0e8267b..4a814dc 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -678,10 +678,10 @@ void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
>  int object_child_foreach(Object *obj, int (*fn)(Object *child, void *opaque),
>                           void *opaque)
>  {
> -    ObjectProperty *prop;
> +    ObjectProperty *prop, *next;
>      int ret = 0;
>
> -    QTAILQ_FOREACH(prop, &obj->properties, node) {
> +    QTAILQ_FOREACH_SAFE(prop, &obj->properties, node, next) {
>          if (object_property_is_child(prop)) {
>              ret = fn(prop->opaque, opaque);
>              if (ret != 0) {
>

The patch is certainly okay, do you need it in 2.1?

Paolo

Re: [Qemu-devel] [PATCH] qom: Make object_child_foreach safe for objects removal

[Alexey Kardashevskiy]

On 07/14/2014 06:14 PM, Paolo Bonzini wrote:
> Il 13/07/2014 16:41, Alexey Kardashevskiy ha scritto:
>> Current object_child_foreach() uses QTAILQ_FOREACH() to walk
>> through children and that makes children removal from the callback
>> impossible.
>>
>> This makes object_child_foreach() use QTAILQ_FOREACH_SAFE().
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>
>> The problem I am trying to solve is:
>> there is a PHB with multiple DMA windows a.k.a. sPAPRTCETable's which are
>> QOM children of PHB. One of RTAS functions is "reset" which is supposed to
>> remove all windows (now just one) except the default one.
>>
>> I could call QTAILQ_FOREACH_SAFE in sPAPR PHB code but
>> object_property_is_child()
>> is static and we probably do not want to make it public.
>>
>>
>> ---
>>  qom/object.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/qom/object.c b/qom/object.c
>> index 0e8267b..4a814dc 100644
>> --- a/qom/object.c
>> +++ b/qom/object.c
>> @@ -678,10 +678,10 @@ void object_class_foreach(void (*fn)(ObjectClass
>> *klass, void *opaque),
>>  int object_child_foreach(Object *obj, int (*fn)(Object *child, void
>> *opaque),
>>                           void *opaque)
>>  {
>> -    ObjectProperty *prop;
>> +    ObjectProperty *prop, *next;
>>      int ret = 0;
>>
>> -    QTAILQ_FOREACH(prop, &obj->properties, node) {
>> +    QTAILQ_FOREACH_SAFE(prop, &obj->properties, node, next) {
>>          if (object_property_is_child(prop)) {
>>              ret = fn(prop->opaque, opaque);
>>              if (ret != 0) {
>>
> 
> The patch is certainly okay, do you need it in 2.1?


Nope, I will need it for dynamic DMA windows (VFIO, PPC) which are not for
2.1 for sure.



-- 
Alexey

Re: [Qemu-devel] [PATCH] qom: Make object_child_foreach safe for objects removal

[Andreas Färber]

Am 14.07.2014 15:12, schrieb Alexey Kardashevskiy:
> On 07/14/2014 06:14 PM, Paolo Bonzini wrote:
>> Il 13/07/2014 16:41, Alexey Kardashevskiy ha scritto:
>>> Current object_child_foreach() uses QTAILQ_FOREACH() to walk
>>> through children and that makes children removal from the callback
>>> impossible.
>>>
>>> This makes object_child_foreach() use QTAILQ_FOREACH_SAFE().
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>>
>>> The problem I am trying to solve is:
>>> there is a PHB with multiple DMA windows a.k.a. sPAPRTCETable's which are
>>> QOM children of PHB. One of RTAS functions is "reset" which is supposed to
>>> remove all windows (now just one) except the default one.
>>>
>>> I could call QTAILQ_FOREACH_SAFE in sPAPR PHB code but
>>> object_property_is_child()
>>> is static and we probably do not want to make it public.
>>>
>>>
>>> ---
>>>  qom/object.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/qom/object.c b/qom/object.c
>>> index 0e8267b..4a814dc 100644
>>> --- a/qom/object.c
>>> +++ b/qom/object.c
>>> @@ -678,10 +678,10 @@ void object_class_foreach(void (*fn)(ObjectClass
>>> *klass, void *opaque),
>>>  int object_child_foreach(Object *obj, int (*fn)(Object *child, void
>>> *opaque),
>>>                           void *opaque)
>>>  {
>>> -    ObjectProperty *prop;
>>> +    ObjectProperty *prop, *next;
>>>      int ret = 0;
>>>
>>> -    QTAILQ_FOREACH(prop, &obj->properties, node) {
>>> +    QTAILQ_FOREACH_SAFE(prop, &obj->properties, node, next) {
>>>          if (object_property_is_child(prop)) {
>>>              ret = fn(prop->opaque, opaque);
>>>              if (ret != 0) {
>>>
>>
>> The patch is certainly okay, do you need it in 2.1?
> 
> 
> Nope, I will need it for dynamic DMA windows (VFIO, PPC) which are not for
> 2.1 for sure.

Thanks, applied to qom-next so that it doesn't get forgotten.
https://github.com/afaerber/qemu-cpu/commits/qom-next

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg