[Qemu-devel] [RFC 0/3] qom/qdev: Try to clarify ownership rules

[Qemu-devel] [RFC 0/3] qom/qdev: Try to clarify ownership rules

[Eduardo Habkost]

The ownership rules of some QOM and qdev functions are not very
clear.  Some of the rules are not trivial because they depend
on other arguments or flags specified when a property was
created.

This is an attempt to clarify the existing ownership rules of
those functions to avoid confusion in the future.

Eduardo Habkost (3):
  qom: Document reference count ownership rules
  qdev: Document ownership rules of qbus_create*()
  pci: Document ownership rules of pci_root_bus_new*()

 include/hw/pci/pci.h   | 15 +++++++++
 include/hw/qdev-core.h | 24 ++++++++++++++
 include/qom/object.h   | 73 ++++++++++++++++++++++++------------------
 hw/core/bus.c          |  5 +++
 4 files changed, 86 insertions(+), 31 deletions(-)

-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [RFC 1/3] qom: Document reference count ownership rules

[Eduardo Habkost]

The documentation for QOM is not clear about who owns references
to objects (i.e. who is responsible for calling object_unref()
later).

This is important considering there are a few inconsistencies in
the API (e.g. callers of object_new() need to call object_unref()
later, but callers of object_new_with_props() must not do it).

Update the documentation so that every mention of object
references also mention who exactly owns the reference.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/qom/object.h | 73 +++++++++++++++++++++++++-------------------
 1 file changed, 42 insertions(+), 31 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index f3d2308d56..08a1bbba7d 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -376,7 +376,7 @@ typedef void (ObjectUnparent)(Object *obj);
  * ObjectFree:
  * @obj: the object being freed
  *
- * Called when an object's last reference is removed.
+ * Called when an object's last reference is dropped using object_unref().
  */
 typedef void (ObjectFree)(void *obj);
 
@@ -601,8 +601,8 @@ struct InterfaceClass
  * @typename: The name of the type of the object to instantiate.
  *
  * This function will initialize a new object using heap allocated memory.
- * The returned object has a reference count of 1, and will be freed when
- * the last reference is dropped.
+ * The returned object has a reference count of 1, and the reference will be
+ * owned by the caller.
  *
  * Returns: The newly allocated and instantiated object.
  */
@@ -617,8 +617,8 @@ Object *object_new(const char *typename);
  * @...: list of property names and values
  *
  * This function will initialize a new object using heap allocated memory.
- * The returned object has a reference count of 1, and will be freed when
- * the last reference is dropped.
+ * The returned object has a reference count of 1, and the reference will
+ * be owned by the caller.
  *
  * The @id parameter will be used when registering the object as a
  * child of @parent in the composition tree.
@@ -652,8 +652,8 @@ Object *object_new(const char *typename);
  *   </programlisting>
  * </example>
  *
- * The returned object will have one stable reference maintained
- * for as long as it is present in the object hierarchy.
+ * The returned object will have one reference, <emphasis>owned by the
+ * parent object</emphasis> (not by the caller).
  *
  * Returns: The newly allocated, instantiated & initialized object.
  */
@@ -713,9 +713,6 @@ Object *object_new_with_propv(const char *typename,
  *   </programlisting>
  * </example>
  *
- * The returned object will have one stable reference maintained
- * for as long as it is present in the object hierarchy.
- *
  * Returns: -1 on error, 0 on success
  */
 int object_set_props(Object *obj,
@@ -744,7 +741,7 @@ int object_set_propv(Object *obj,
  *
  * This function will initialize an object.  The memory for the object should
  * have already been allocated.  The returned object has a reference count of 1,
- * and will be finalized when the last reference is dropped.
+ * and the reference will be owned by the caller.
  */
 void object_initialize(void *obj, size_t size, const char *typename);
 
@@ -928,8 +925,12 @@ GSList *object_class_get_list_sorted(const char *implements_type,
  * object_ref:
  * @obj: the object
  *
- * Increase the reference count of a object.  A object cannot be freed as long
- * as its reference count is greater than zero.
+ * Increase the reference count of a object.  A object won't be freed as
+ * long as its reference count is greater than zero.
+ *
+ * The new reference will be owned by the caller, meaning the caller is
+ * responsible for ensuring object_unref() will be called once the
+ * reference is not needed anymore.
  */
 void object_ref(Object *obj);
 
@@ -937,8 +938,10 @@ void object_ref(Object *obj);
  * object_unref:
  * @obj: the object
  *
- * Decrease the reference count of a object.  A object cannot be freed as long
- * as its reference count is greater than zero.
+ * Drop a reference to a object, decreasing its reference count.  This
+ * function should be called only if the caller owns a reference taken using
+ * object_ref() or returned by functions like object_initialize() and
+ * object_new().
  */
 void object_unref(Object *obj);
 
@@ -1104,10 +1107,13 @@ char *object_property_get_str(Object *obj, const char *name,
  *
  * Writes an object's canonical path to a property.
  *
- * If the link property was created with
- * <code>OBJ_PROP_LINK_STRONG</code> bit, the old target object is
- * unreferenced, and a reference is added to the new target object.
+ * If the link property was created with <code>OBJ_PROP_LINK_STRONG</code>
+ * bit, the property will take a reference to the object, and drop
+ * a reference to the old object.
  *
+ * If <code>OBJ_PROP_LINK_STRONG</code> was not set for the property, the
+ * caller is responsible for ensuring the object will stay alive until the
+ * property is modified or deleted.
  */
 void object_property_set_link(Object *obj, Object *value,
                               const char *name, Error **errp);
@@ -1398,7 +1404,10 @@ void object_property_add_child(Object *obj, const char *name,
                                Object *child, Error **errp);
 
 typedef enum {
-    /* Unref the link pointer when the property is deleted */
+    /*
+     * Property owns a reference to the target object.  Unref the link
+     * pointer when the property is modified or deleted.
+     */
     OBJ_PROP_LINK_STRONG = 0x1,
 } ObjectPropertyLinkFlags;
 
@@ -1433,13 +1442,14 @@ void object_property_allow_set_link(const Object *, const char *,
  * link being set.  If <code>@check</code> is NULL, the property is read-only
  * and cannot be set.
  *
- * Ownership of the pointer that @child points to is transferred to the
- * link property.  The reference count for <code>*@child</code> is
- * managed by the property from after the function returns till the
- * property is deleted with object_property_del().  If the
- * <code>@flags</code> <code>OBJ_PROP_LINK_STRONG</code> bit is set,
- * the reference count is decremented when the property is deleted or
- * modified.
+ * If the <code>OBJ_PROP_LINK_STRONG</code> bit is set in @flags,
+ * ownership of the pointer that @child points to is transferred to the
+ * link property, and the reference will be dropped when the property is
+ * modified or deleted with object_property_del().
+ *
+ * If <code>OBJ_PROP_LINK_STRONG</code> is not set in @flags, the caller
+ * is responsible for ensuring the object pointed by @child will stay alive
+ * until the property is modified or deleted.
  */
 void object_property_add_link(Object *obj, const char *name,
                               const char *type, Object **child,
@@ -1607,7 +1617,8 @@ void object_class_property_add_uint64_ptr(ObjectClass *klass, const char *name,
  * Add an alias for a property on an object.  This function will add a property
  * of the same type as the forwarded property.
  *
- * The caller must ensure that <code>@target_obj</code> stays alive as long as
+ * The property won't take a reference to <code>@target_obj</code>, so the
+ * caller must ensure that <code>@target_obj</code> stays alive as long as
  * this property exists.  In the case of a child object or an alias on the same
  * object this will be the case.  For aliases to other objects the caller is
  * responsible for taking a reference.
@@ -1626,10 +1637,10 @@ void object_property_add_alias(Object *obj, const char *name,
  * Add an unmodifiable link for a property on an object.  This function will
  * add a property of type link<TYPE> where TYPE is the type of @target.
  *
- * The caller must ensure that @target stays alive as long as
- * this property exists.  In the case @target is a child of @obj,
- * this will be the case.  Otherwise, the caller is responsible for
- * taking a reference.
+ * The property won't take a reference to <code>@target_obj</code>, so the
+ * caller must ensure that @target stays alive as long as this property
+ * exists.  In the case @target is a child of @obj, this will be the case.
+ * Otherwise, the caller is responsible for taking a reference.
  */
 void object_property_add_const_link(Object *obj, const char *name,
                                     Object *target, Error **errp);
-- 
2.18.0.rc1.1.g3f1ff2140

Re: [Qemu-devel] [RFC 1/3] qom: Document reference count ownership rules

[Thomas Huth]

On 12.07.2018 21:45, Eduardo Habkost wrote:
> The documentation for QOM is not clear about who owns references
> to objects (i.e. who is responsible for calling object_unref()
> later).
> 
> This is important considering there are a few inconsistencies in
> the API (e.g. callers of object_new() need to call object_unref()
> later, but callers of object_new_with_props() must not do it).
> 
> Update the documentation so that every mention of object
> references also mention who exactly owns the reference.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  include/qom/object.h | 73 +++++++++++++++++++++++++-------------------
>  1 file changed, 42 insertions(+), 31 deletions(-)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index f3d2308d56..08a1bbba7d 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -376,7 +376,7 @@ typedef void (ObjectUnparent)(Object *obj);
>   * ObjectFree:
>   * @obj: the object being freed
>   *
> - * Called when an object's last reference is removed.
> + * Called when an object's last reference is dropped using object_unref().
>   */
>  typedef void (ObjectFree)(void *obj);
>  
> @@ -601,8 +601,8 @@ struct InterfaceClass
>   * @typename: The name of the type of the object to instantiate.
>   *
>   * This function will initialize a new object using heap allocated memory.
> - * The returned object has a reference count of 1, and will be freed when
> - * the last reference is dropped.
> + * The returned object has a reference count of 1, and the reference will be
> + * owned by the caller.
>   *
>   * Returns: The newly allocated and instantiated object.
>   */
> @@ -617,8 +617,8 @@ Object *object_new(const char *typename);
>   * @...: list of property names and values
>   *
>   * This function will initialize a new object using heap allocated memory.
> - * The returned object has a reference count of 1, and will be freed when
> - * the last reference is dropped.
> + * The returned object has a reference count of 1, and the reference will
> + * be owned by the caller.

That's the description of object_new_with_props here already, isn't it?
So the reference will be owned by the parent object, not by the caller.

>   * The @id parameter will be used when registering the object as a
>   * child of @parent in the composition tree.
> @@ -652,8 +652,8 @@ Object *object_new(const char *typename);
>   *   </programlisting>
>   * </example>
>   *
> - * The returned object will have one stable reference maintained
> - * for as long as it is present in the object hierarchy.
> + * The returned object will have one reference, <emphasis>owned by the
> + * parent object</emphasis> (not by the caller).

... and then this information here is somewhat redundant. I suggest to
remove one of the two spots.

 Thomas

Re: [Qemu-devel] [RFC 1/3] qom: Document reference count ownership rules

[Eduardo Habkost]

On Fri, Jul 13, 2018 at 11:07:15AM +0200, Thomas Huth wrote:
> On 12.07.2018 21:45, Eduardo Habkost wrote:
> > The documentation for QOM is not clear about who owns references
> > to objects (i.e. who is responsible for calling object_unref()
> > later).
> > 
> > This is important considering there are a few inconsistencies in
> > the API (e.g. callers of object_new() need to call object_unref()
> > later, but callers of object_new_with_props() must not do it).
> > 
> > Update the documentation so that every mention of object
> > references also mention who exactly owns the reference.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  include/qom/object.h | 73 +++++++++++++++++++++++++-------------------
> >  1 file changed, 42 insertions(+), 31 deletions(-)
> > 
> > diff --git a/include/qom/object.h b/include/qom/object.h
> > index f3d2308d56..08a1bbba7d 100644
> > --- a/include/qom/object.h
> > +++ b/include/qom/object.h
> > @@ -376,7 +376,7 @@ typedef void (ObjectUnparent)(Object *obj);
> >   * ObjectFree:
> >   * @obj: the object being freed
> >   *
> > - * Called when an object's last reference is removed.
> > + * Called when an object's last reference is dropped using object_unref().
> >   */
> >  typedef void (ObjectFree)(void *obj);
> >  
> > @@ -601,8 +601,8 @@ struct InterfaceClass
> >   * @typename: The name of the type of the object to instantiate.
> >   *
> >   * This function will initialize a new object using heap allocated memory.
> > - * The returned object has a reference count of 1, and will be freed when
> > - * the last reference is dropped.
> > + * The returned object has a reference count of 1, and the reference will be
> > + * owned by the caller.
> >   *
> >   * Returns: The newly allocated and instantiated object.
> >   */
> > @@ -617,8 +617,8 @@ Object *object_new(const char *typename);
> >   * @...: list of property names and values
> >   *
> >   * This function will initialize a new object using heap allocated memory.
> > - * The returned object has a reference count of 1, and will be freed when
> > - * the last reference is dropped.
> > + * The returned object has a reference count of 1, and the reference will
> > + * be owned by the caller.
> 
> That's the description of object_new_with_props here already, isn't it?
> So the reference will be owned by the parent object, not by the caller.

Oops, you're right.  Thanks!

> 
> >   * The @id parameter will be used when registering the object as a
> >   * child of @parent in the composition tree.
> > @@ -652,8 +652,8 @@ Object *object_new(const char *typename);
> >   *   </programlisting>
> >   * </example>
> >   *
> > - * The returned object will have one stable reference maintained
> > - * for as long as it is present in the object hierarchy.
> > + * The returned object will have one reference, <emphasis>owned by the
> > + * parent object</emphasis> (not by the caller).
> 
> ... and then this information here is somewhat redundant. I suggest to
> remove one of the two spots.

Will change this to:

 /**
  * object_new_with_props:
  * @typename:  The name of the type of the object to instantiate.
  * @parent: the parent object
  * @id: The unique ID of the object
  * @errp: pointer to error object
  * @...: list of property names and values
  *
  * This function will initialize a new object using heap allocated memory.
- * The returned object has a reference count of 1, and will be freed when
- * the last reference is dropped.
+ * The returned object will have one reference, <emphasis>owned by the
+ * parent object</emphasis> (not by the caller).
  *
  * The @id parameter will be used when registering the object as a
  * child of @parent in the composition tree.
  *
  * The variadic parameters are a list of pairs of (propname, propvalue)
  * strings. The propname of %NULL indicates the end of the property
  * list. If the object implements the user creatable interface, the
  * object will be marked complete once all the properties have been
  * processed.
  *
  * <example>
  *   <title>Creating an object with properties</title>
  *   <programlisting>
  *   Error *err = NULL;
  *   Object *obj;
  *
  *   obj = object_new_with_props(TYPE_MEMORY_BACKEND_FILE,
  *                               object_get_objects_root(),
  *                               "hostmem0",
  *                               &err,
  *                               "share", "yes",
  *                               "mem-path", "/dev/shm/somefile",
  *                               "prealloc", "yes",
  *                               "size", "1048576",
  *                               NULL);
  *
  *   if (!obj) {
  *     g_printerr("Cannot create memory backend: %s\n",
  *                error_get_pretty(err));
  *   }
  *   </programlisting>
  * </example>
  *
- * The returned object will have one stable reference maintained
- * for as long as it is present in the object hierarchy.
- *
  * Returns: The newly allocated, instantiated & initialized object.
  */
 Object *object_new_with_props(const char *typename,
                               Object *parent,
                               const char *id,
                               Error **errp,
                               ...) QEMU_SENTINEL;

-- 
Eduardo

[Qemu-devel] [RFC 2/3] qdev: Document ownership rules of qbus_create*()

[Eduardo Habkost]

The ownership rules of those functions aren't trivial: the caller
owns the new object if parent is NULL, otherwise ownership is
transferred to the parent.  Clarify that on comments.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/hw/qdev-core.h | 24 ++++++++++++++++++++++++
 hw/core/bus.c          |  5 +++++
 2 files changed, 29 insertions(+)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index f1fd0f8736..27d1ac3781 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -343,8 +343,32 @@ DeviceState *qdev_find_recursive(BusState *bus, const char *id);
 typedef int (qbus_walkerfn)(BusState *bus, void *opaque);
 typedef int (qdev_walkerfn)(DeviceState *dev, void *opaque);
 
+/**
+ * qbus_create_inplace:
+ * @bus: A pointer to the memory to be used for the bus object.
+ * @size: The maximum size available at @bus for the bus object.
+ * @typename: The name of the type of bus to instantiate.
+ * @parent: parent device
+ * @name: name for the new bus
+ *
+ * Creates bus in place.
+ *
+ * If @parent is not NULL the bus will be owned by @parent, otherwise
+ * the bus will be owned by the caller.
+ */
 void qbus_create_inplace(void *bus, size_t size, const char *typename,
                          DeviceState *parent, const char *name);
+/**
+ * qbus_create:
+ * @typename: The name of the type of bus to instantiate.
+ * @parent: parent device
+ * @name: name for the new bus
+ *
+ * Creates bus object.
+ *
+ * If @parent is not NULL the returned object will be owned by @parent,
+ * otherwise it will be owned by the caller.
+ */
 BusState *qbus_create(const char *typename, DeviceState *parent, const char *name);
 /* Returns > 0 if either devfn or busfn skip walk somewhere in cursion,
  *         < 0 if either devfn or busfn terminate walk somewhere in cursion,
diff --git a/hw/core/bus.c b/hw/core/bus.c
index 4651f24486..68a0d5b085 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -74,6 +74,7 @@ int qbus_walk_children(BusState *bus,
     return 0;
 }
 
+/* If @parent is not NULL, ownership of @bus is transferred to @parent */
 static void qbus_realize(BusState *bus, DeviceState *parent, const char *name)
 {
     const char *typename = object_get_typename(OBJECT(bus));
@@ -102,6 +103,10 @@ static void qbus_realize(BusState *bus, DeviceState *parent, const char *name)
         QLIST_INSERT_HEAD(&bus->parent->child_bus, bus, sibling);
         bus->parent->num_child_bus++;
         object_property_add_child(OBJECT(bus->parent), bus->name, OBJECT(bus), NULL);
+        /*
+         * object_property_add_child() takes a new reference, drop the
+         * reference that was transferred to us.
+         */
         object_unref(OBJECT(bus));
     } else if (bus != sysbus_get_default()) {
         /* TODO: once all bus devices are qdevified,
-- 
2.18.0.rc1.1.g3f1ff2140

Re: [Qemu-devel] [RFC 2/3] qdev: Document ownership rules of qbus_create*()

[Marcel Apfelbaum]

On 07/12/2018 10:45 PM, Eduardo Habkost wrote:
> The ownership rules of those functions aren't trivial: the caller
> owns the new object if parent is NULL, otherwise ownership is
> transferred to the parent.  Clarify that on comments.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>   include/hw/qdev-core.h | 24 ++++++++++++++++++++++++
>   hw/core/bus.c          |  5 +++++
>   2 files changed, 29 insertions(+)
>
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index f1fd0f8736..27d1ac3781 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -343,8 +343,32 @@ DeviceState *qdev_find_recursive(BusState *bus, const char *id);
>   typedef int (qbus_walkerfn)(BusState *bus, void *opaque);
>   typedef int (qdev_walkerfn)(DeviceState *dev, void *opaque);
>   
> +/**
> + * qbus_create_inplace:
> + * @bus: A pointer to the memory to be used for the bus object.
> + * @size: The maximum size available at @bus for the bus object.
> + * @typename: The name of the type of bus to instantiate.
> + * @parent: parent device
> + * @name: name for the new bus
> + *
> + * Creates bus in place.
> + *
> + * If @parent is not NULL the bus will be owned by @parent, otherwise
> + * the bus will be owned by the caller.
> + */
>   void qbus_create_inplace(void *bus, size_t size, const char *typename,
>                            DeviceState *parent, const char *name);
> +/**
> + * qbus_create:
> + * @typename: The name of the type of bus to instantiate.
> + * @parent: parent device
> + * @name: name for the new bus
> + *
> + * Creates bus object.
> + *
> + * If @parent is not NULL the returned object will be owned by @parent,
> + * otherwise it will be owned by the caller.
> + */
>   BusState *qbus_create(const char *typename, DeviceState *parent, const char *name);
>   /* Returns > 0 if either devfn or busfn skip walk somewhere in cursion,
>    *         < 0 if either devfn or busfn terminate walk somewhere in cursion,
> diff --git a/hw/core/bus.c b/hw/core/bus.c
> index 4651f24486..68a0d5b085 100644
> --- a/hw/core/bus.c
> +++ b/hw/core/bus.c
> @@ -74,6 +74,7 @@ int qbus_walk_children(BusState *bus,
>       return 0;
>   }
>   
> +/* If @parent is not NULL, ownership of @bus is transferred to @parent */
>   static void qbus_realize(BusState *bus, DeviceState *parent, const char *name)
>   {
>       const char *typename = object_get_typename(OBJECT(bus));
> @@ -102,6 +103,10 @@ static void qbus_realize(BusState *bus, DeviceState *parent, const char *name)
>           QLIST_INSERT_HEAD(&bus->parent->child_bus, bus, sibling);
>           bus->parent->num_child_bus++;
>           object_property_add_child(OBJECT(bus->parent), bus->name, OBJECT(bus), NULL);
> +        /*
> +         * object_property_add_child() takes a new reference, drop the
> +         * reference that was transferred to us.
> +         */
>           object_unref(OBJECT(bus));
>       } else if (bus != sysbus_get_default()) {
>           /* TODO: once all bus devices are qdevified,

Reviewed-by: Marcel Apfelbaum<marcel.apfelbaum@gmail.com>

Thanks,
Marcel

[Qemu-devel] [RFC 3/3] pci: Document ownership rules of pci_root_bus_new*()

[Eduardo Habkost]

The ownership rules of pci_root_bus_new*() aren't trivial: the
caller owns the new object if parent is NULL, otherwise ownership
is transferred to the parent.  Clarify that on comments.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/hw/pci/pci.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 990d6fcbde..5d445c431c 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -396,15 +396,30 @@ typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin);
 
 bool pci_bus_is_express(PCIBus *bus);
 bool pci_bus_is_root(PCIBus *bus);
+
+/**
+ * pci_root_bus_new_inplace:
+ *
+* If @parent is not NULL the returned object will be owned by @parent,
+* otherwise it will be owned by the caller.
+*/
 void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
                               const char *name,
                               MemoryRegion *address_space_mem,
                               MemoryRegion *address_space_io,
                               uint8_t devfn_min, const char *typename);
+
+/**
+ * pci_root_bus_new:
+ *
+* If @parent is not NULL the returned object will be owned by @parent,
+* otherwise it will be owned by the caller.
+*/
 PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
                          MemoryRegion *address_space_mem,
                          MemoryRegion *address_space_io,
                          uint8_t devfn_min, const char *typename);
+
 void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
                   void *irq_opaque, int nirq);
 int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
-- 
2.18.0.rc1.1.g3f1ff2140

Re: [Qemu-devel] [RFC 3/3] pci: Document ownership rules of pci_root_bus_new*()

[Marcel Apfelbaum]

On 07/12/2018 10:45 PM, Eduardo Habkost wrote:
> The ownership rules of pci_root_bus_new*() aren't trivial: the
> caller owns the new object if parent is NULL, otherwise ownership
> is transferred to the parent.  Clarify that on comments.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>   include/hw/pci/pci.h | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 990d6fcbde..5d445c431c 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -396,15 +396,30 @@ typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin);
>   
>   bool pci_bus_is_express(PCIBus *bus);
>   bool pci_bus_is_root(PCIBus *bus);
> +
> +/**
> + * pci_root_bus_new_inplace:
> + *
> +* If @parent is not NULL the returned object will be owned by @parent,
> +* otherwise it will be owned by the caller.
> +*/
>   void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
>                                 const char *name,
>                                 MemoryRegion *address_space_mem,
>                                 MemoryRegion *address_space_io,
>                                 uint8_t devfn_min, const char *typename);
> +
> +/**
> + * pci_root_bus_new:
> + *
> +* If @parent is not NULL the returned object will be owned by @parent,
> +* otherwise it will be owned by the caller.
> +*/
>   PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
>                            MemoryRegion *address_space_mem,
>                            MemoryRegion *address_space_io,
>                            uint8_t devfn_min, const char *typename);
> +
>   void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>                     void *irq_opaque, int nirq);
>   int pci_bus_get_irq_level(PCIBus *bus, int irq_num);

Reviewed-by: Marcel Apfelbaum<marcel.apfelbaum@gmail.com>

Thanks,
Marcel