[Qemu-devel] [PATCH] qom/object.h: remove some child/parent doc

[Qemu-devel] [PATCH] qom/object.h: remove some child/parent doc

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

It looks like this documentation is obsolete: a child object may lookup
its parent stored in the Object struct.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qom/object.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 2f95ab3..653699f 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1261,9 +1261,6 @@ Object *object_resolve_path_component(Object *parent, const gchar *part);
  * Child properties form the composition tree.  All objects need to be a child
  * of another object.  Objects can only be a child of one object.
  *
- * There is no way for a child to determine what its parent is.  It is not
- * a bidirectional relationship.  This is by design.
- *
  * The value of a child property as a C string will be the child object's
  * canonical path. It can be retrieved using object_property_get_str().
  * The child object itself can be retrieved using object_property_get_link().
-- 
2.4.3

Re: [Qemu-devel] [PATCH] qom/object.h: remove some child/parent doc

[Peter Crosthwaite]

On Fri, Sep 4, 2015 at 2:01 PM,  <marcandre.lureau@redhat.com> wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> It looks like this documentation is obsolete: a child object may lookup
> its parent stored in the Object struct.
>

Using a function wrapper or direct access to Object structure field?

The latter would indicate they are workarounds. Which legit uses of
parent identification are you referring to?

Regards,
Peter

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qom/object.h | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 2f95ab3..653699f 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -1261,9 +1261,6 @@ Object *object_resolve_path_component(Object *parent, const gchar *part);
>   * Child properties form the composition tree.  All objects need to be a child
>   * of another object.  Objects can only be a child of one object.
>   *
> - * There is no way for a child to determine what its parent is.  It is not
> - * a bidirectional relationship.  This is by design.
> - *
>   * The value of a child property as a C string will be the child object's
>   * canonical path. It can be retrieved using object_property_get_str().
>   * The child object itself can be retrieved using object_property_get_link().
> --
> 2.4.3
>
>

Re: [Qemu-devel] [PATCH] qom/object.h: remove some child/parent doc

[Marc-André Lureau]

Hi

On Fri, Sep 4, 2015 at 11:25 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Fri, Sep 4, 2015 at 2:01 PM,  <marcandre.lureau@redhat.com> wrote:
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> It looks like this documentation is obsolete: a child object may lookup
>> its parent stored in the Object struct.
>>
>
> Using a function wrapper or direct access to Object structure field?

By direct access to the struct field.

>
> The latter would indicate they are workarounds. Which legit uses of
> parent identification are you referring to?

I am not saying this is legit or not to use this field, it's just
wrong documentation to me: the relationship is bidirectionnal. That's
one of the few differences with the link<> properties.

regards

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH] qom/object.h: remove some child/parent doc

[Peter Crosthwaite]

On Fri, Sep 4, 2015 at 2:42 PM, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
> Hi
>
> On Fri, Sep 4, 2015 at 11:25 PM, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Fri, Sep 4, 2015 at 2:01 PM,  <marcandre.lureau@redhat.com> wrote:
>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>
>>> It looks like this documentation is obsolete: a child object may lookup
>>> its parent stored in the Object struct.
>>>
>>
>> Using a function wrapper or direct access to Object structure field?
>
> By direct access to the struct field.
>
>>
>> The latter would indicate they are workarounds. Which legit uses of
>> parent identification are you referring to?
>
> I am not saying this is legit or not to use this field, it's just
> wrong documentation to me: the relationship is bidirectionnal. That's
> one of the few differences with the link<> properties.
>

It is biderctional for the implementation but not for the QOM API. I
think the comment is correct as long as there is no exposed API to do
this.

Regards,
Peter

> regards
>
> --
> Marc-André Lureau

Re: [Qemu-devel] [PATCH] qom/object.h: remove some child/parent doc

[Andreas Färber]

Am 04.09.2015 um 23:01 schrieb marcandre.lureau@redhat.com:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> It looks like this documentation is obsolete: a child object may lookup
> its parent stored in the Object struct.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qom/object.h | 3 ---
>  1 file changed, 3 deletions(-)

Either once again you are trying to do stuff behind my back, or your
setup is really broken: I double-checked that include/qom/ is listed in
MAINTAINERS, so I should've been CC'ed rather than just -trivial.

It's been a valid rule not to mess with these internal fields, therefore
this is not trivial at all, and that's one reason why my x86 CPU series
using it was an RFC. We should either come up with a proper wrapper
function object_get_parent(), or with a wrapper function adding a link<>
property (where we would need to be careful with ref counts) - long time
only the composition tree needed to mess with an object's parent.

If you have a concrete use case of parent access, please point to it.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH] qom/object.h: remove some child/parent doc

[Michael Tokarev]

07.09.2015 18:46, Andreas Färber wrote:
> Am 04.09.2015 um 23:01 schrieb marcandre.lureau@redhat.com:
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> It looks like this documentation is obsolete: a child object may lookup
>> its parent stored in the Object struct.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  include/qom/object.h | 3 ---
>>  1 file changed, 3 deletions(-)
> 
> Either once again you are trying to do stuff behind my back, or your
> setup is really broken: I double-checked that include/qom/ is listed in
> MAINTAINERS, so I should've been CC'ed rather than just -trivial.
> 
> It's been a valid rule not to mess with these internal fields, therefore
> this is not trivial at all, and that's one reason why my x86 CPU series
> using it was an RFC. We should either come up with a proper wrapper
> function object_get_parent(), or with a wrapper function adding a link<>
> property (where we would need to be careful with ref counts) - long time
> only the composition tree needed to mess with an object's parent.
> 
> If you have a concrete use case of parent access, please point to it.

So, should the current documentation remain, or should the patch be
applied?  If the latter, Andreas, please take care of it.

Thanks,

/mjt

Re: [Qemu-devel] [PATCH] qom/object.h: remove some child/parent doc

[Andreas Färber]

Am 11.09.2015 um 12:13 schrieb Michael Tokarev:
> 07.09.2015 18:46, Andreas Färber wrote:
>> Am 04.09.2015 um 23:01 schrieb marcandre.lureau@redhat.com:
>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>
>>> It looks like this documentation is obsolete: a child object may lookup
>>> its parent stored in the Object struct.
>>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>>  include/qom/object.h | 3 ---
>>>  1 file changed, 3 deletions(-)
>>
>> Either once again you are trying to do stuff behind my back, or your
>> setup is really broken: I double-checked that include/qom/ is listed in
>> MAINTAINERS, so I should've been CC'ed rather than just -trivial.
>>
>> It's been a valid rule not to mess with these internal fields, therefore
>> this is not trivial at all, and that's one reason why my x86 CPU series
>> using it was an RFC. We should either come up with a proper wrapper
>> function object_get_parent(), or with a wrapper function adding a link<>
>> property (where we would need to be careful with ref counts) - long time
>> only the composition tree needed to mess with an object's parent.
>>
>> If you have a concrete use case of parent access, please point to it.
> 
> So, should the current documentation remain, or should the patch be
> applied?  If the latter, Andreas, please take care of it.

This patch should not be applied, NACK.

I outlined ideas for a v2 series above though.

Thanks,
Andreas

> 
> Thanks,
> 
> /mjt
> 


-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)