qom: Make QOM link property unref optional

Some object_property_add_link() callers expect property deletion to
unref the link property object.  Other callers expect to manage the
refcount themselves.  The former are currently broken and therefore leak
the link property object.

This patch adds a flags argument to object_property_add_link() so the
caller can specify which refcount behavior they require.  The new
OBJ_PROP_LINK_UNREF_ON_RELEASE flag causes the link pointer to be
unreferenced when the property is deleted.

This fixes refcount leaks in qdev.c, xilinx_axidma.c, xilinx_axienet.c,
s390-virtio-bus.c, virtio-pci.c, virtio-rng.c, and ui/console.c.

Rationale for refcount behavior:

 * hw/core/qdev.c
   - bus children are explicitly unreferenced, don't interfere
   - parent_bus is essentially a read-only property that doesn't hold a
     refcount, don't unref
   - hotplug_handler is leaked, do unref

 * hw/dma/xilinx_axidma.c
   - rx stream "dma" links are set using set_link, therefore they
     need unref
   - tx streams are set using set_link, therefore they need unref

 * hw/net/xilinx_axienet.c
   - same reasoning as hw/dma/xilinx_axidma.c

 * hw/pcmcia/pxa2xx.c
   - pxa2xx bypasses set_link and therefore does not use refcounts

 * hw/s390x/s390-virtio-bus.c
 * hw/virtio/virtio-pci.c
 * hw/virtio/virtio-rng.c
 * ui/console.c
   - set_link is used and there is no explicit unref, do unref

Cc: Peter Crosthwaite <peter.crosthwaite@petalogix.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: Anthony Liguori <aliguori@amazon.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 9f0a522ee8749e3495064ca83e052e04b4b791b6..a182917222fcffd4a3ea2dcca702560fe94a37e3 100644 (file)
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -97,8 +97,7 @@ static void bus_add_child(BusState *bus, DeviceState *child)
     snprintf(name, sizeof(name), "child[%d]", kid->index);
     object_property_add_link(OBJECT(bus), name,
                              object_get_typename(OBJECT(child)),
-                             (Object **)&kid->child,
-                             NULL);
+                             (Object **)&kid->child, 0, NULL);
 }
 
 void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
@@ -824,7 +823,8 @@ static void device_initfn(Object *obj)
     } while (class != object_class_by_name(TYPE_DEVICE));
 
     object_property_add_link(OBJECT(dev), "parent_bus", TYPE_BUS,
-                             (Object **)&dev->parent_bus, &error_abort);
+                             (Object **)&dev->parent_bus, 0,
+                             &error_abort);
 }
 
 static void device_post_init(Object *obj)
@@ -944,7 +944,9 @@ static void qbus_initfn(Object *obj)
     QTAILQ_INIT(&bus->children);
     object_property_add_link(obj, QDEV_HOTPLUG_HANDLER_PROPERTY,
                              TYPE_HOTPLUG_HANDLER,
-                             (Object **)&bus->hotplug_handler, NULL);
+                             (Object **)&bus->hotplug_handler,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             NULL);
     object_property_add_bool(obj, "realized",
                              bus_get_realized, bus_set_realized, NULL);
 }

diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
index 19f07b3b25fd5a22137199fe4592b49c5662288d..c8fda391228e6e7241a6b59a4d88ebb72bbb4438 100644 (file)
--- a/hw/dma/xilinx_axidma.c
+++ b/hw/dma/xilinx_axidma.c
@@ -537,9 +537,13 @@ static void xilinx_axidma_realize(DeviceState *dev, Error **errp)
     Error *local_errp = NULL;
 
     object_property_add_link(OBJECT(ds), "dma", TYPE_XILINX_AXI_DMA,
-                             (Object **)&ds->dma, &local_errp);
+                             (Object **)&ds->dma,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             &local_errp);
     object_property_add_link(OBJECT(cs), "dma", TYPE_XILINX_AXI_DMA,
-                             (Object **)&cs->dma, &local_errp);
+                             (Object **)&cs->dma,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             &local_errp);
     if (local_errp) {
         goto xilinx_axidma_realize_fail;
     }
@@ -571,10 +575,14 @@ static void xilinx_axidma_init(Object *obj)
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 
     object_property_add_link(obj, "axistream-connected", TYPE_STREAM_SLAVE,
-                             (Object **)&s->tx_data_dev, &error_abort);
+                             (Object **)&s->tx_data_dev,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             &error_abort);
     object_property_add_link(obj, "axistream-control-connected",
                              TYPE_STREAM_SLAVE,
-                             (Object **)&s->tx_control_dev, &error_abort);
+                             (Object **)&s->tx_control_dev,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             &error_abort);
 
     object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev),
                       TYPE_XILINX_AXI_DMA_DATA_STREAM);

diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index 0bd5eda199d7e4281a69cce422b146fc8a55f213..7ecf9251a76bcb550c877324d60455e3a3b7b797 100644 (file)
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -945,9 +945,13 @@ static void xilinx_enet_realize(DeviceState *dev, Error **errp)
     Error *local_errp = NULL;
 
     object_property_add_link(OBJECT(ds), "enet", "xlnx.axi-ethernet",
-                             (Object **) &ds->enet, &local_errp);
+                             (Object **) &ds->enet,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             &local_errp);
     object_property_add_link(OBJECT(cs), "enet", "xlnx.axi-ethernet",
-                             (Object **) &cs->enet, &local_errp);
+                             (Object **) &cs->enet,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             &local_errp);
     if (local_errp) {
         goto xilinx_enet_realize_fail;
     }
@@ -982,10 +986,14 @@ static void xilinx_enet_init(Object *obj)
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 
     object_property_add_link(obj, "axistream-connected", TYPE_STREAM_SLAVE,
-                             (Object **) &s->tx_data_dev, &error_abort);
+                             (Object **) &s->tx_data_dev,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             &error_abort);
     object_property_add_link(obj, "axistream-control-connected",
                              TYPE_STREAM_SLAVE,
-                             (Object **) &s->tx_control_dev, &error_abort);
+                             (Object **) &s->tx_control_dev,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             &error_abort);
 
     object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev),
                       TYPE_XILINX_AXI_ENET_DATA_STREAM);

diff --git a/hw/pcmcia/pxa2xx.c b/hw/pcmcia/pxa2xx.c
index 8f17596cc3684ac3312edd70372fa517aceaa9ba..6949214df35dac0e1a86722c7ceb0d05062778b9 100644 (file)
--- a/hw/pcmcia/pxa2xx.c
+++ b/hw/pcmcia/pxa2xx.c
@@ -198,7 +198,7 @@ static void pxa2xx_pcmcia_initfn(Object *obj)
     s->slot.irq = qemu_allocate_irqs(pxa2xx_pcmcia_set_irq, s, 1)[0];
 
     object_property_add_link(obj, "card", TYPE_PCMCIA_CARD,
-                             (Object **)&s->card, NULL);
+                             (Object **)&s->card, 0, NULL);
 }
 
 /* Insert a new card into a slot */

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index e4fc35366b2ee5f87e9a111c4b5207308a116cd4..930b4f7cd011093d6545a832868b6bc158fa66f8 100644 (file)
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -313,7 +313,8 @@ static void s390_virtio_rng_instance_init(Object *obj)
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_RNG);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
     object_property_add_link(obj, "rng", TYPE_RNG_BACKEND,
-                             (Object **)&dev->vdev.conf.rng, NULL);
+                             (Object **)&dev->vdev.conf.rng,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE, NULL);
 }
 
 static uint64_t s390_virtio_device_vq_token(VirtIOS390Device *dev, int vq)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index a01801e342ba7a2cf6c83c52968e512007dbfdde..aebb2dec6dda0ab8bacf84169b7736d9f85941f0 100644 (file)
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1272,7 +1272,8 @@ static void virtio_ccw_rng_instance_init(Object *obj)
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_RNG);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
     object_property_add_link(obj, "rng", TYPE_RNG_BACKEND,
-                             (Object **)&dev->vdev.conf.rng, NULL);
+                             (Object **)&dev->vdev.conf.rng,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE, NULL);
 }
 
 static Property virtio_ccw_rng_properties[] = {

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 7b91841a1df5edb32a347503d41fa509ca9a7141..eebb819d9876332ecf71e59193fa56a4690d66ca 100644 (file)
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1517,7 +1517,8 @@ static void virtio_rng_initfn(Object *obj)
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_RNG);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
     object_property_add_link(obj, "rng", TYPE_RNG_BACKEND,
-                             (Object **)&dev->vdev.conf.rng, NULL);
+                             (Object **)&dev->vdev.conf.rng,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE, NULL);
 
 }
 

diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index a16e3bc52e6140a29dabd6e43d671c3a5e52596f..2efda8b4437aa80fb9bca96fd85585ee4a5dfd16 100644 (file)
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -223,7 +223,8 @@ static void virtio_rng_initfn(Object *obj)
     VirtIORNG *vrng = VIRTIO_RNG(obj);
 
     object_property_add_link(obj, "rng", TYPE_RNG_BACKEND,
-                             (Object **)&vrng->conf.rng, NULL);
+                             (Object **)&vrng->conf.rng,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE, NULL);
 }
 
 static const TypeInfo virtio_rng_info = {

diff --git a/include/qom/object.h b/include/qom/object.h
index 4cd77049e4ee5d314f53a022e65410a07c2dd9e0..9feb4419862cd36c1f8f5b61487d5b3bf0bae23a 100644 (file)
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1067,12 +1067,18 @@ Object *object_resolve_path_component(Object *parent, const gchar *part);
 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 */
+    OBJ_PROP_LINK_UNREF_ON_RELEASE = 0x1,
+} ObjectPropertyLinkFlags;
+
 /**
  * object_property_add_link:
  * @obj: the object to add a property to
  * @name: the name of the property
  * @type: the qobj type of the link
  * @child: a pointer to where the link object reference is stored
+ * @flags: additional options for the link
  * @errp: if an error occurs, a pointer to an area to store the area
  *
  * Links establish relationships between objects.  Links are unidirectional
@@ -1084,10 +1090,13 @@ void object_property_add_child(Object *obj, const char *name,
  * 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().
+ * property is deleted with object_property_del().  If the
+ * <code>@flags</code> <code>OBJ_PROP_LINK_UNREF_ON_RELEASE</code> bit is set,
+ * the reference count is decremented when the property is deleted.
  */
 void object_property_add_link(Object *obj, const char *name,
                               const char *type, Object **child,
+                              ObjectPropertyLinkFlags flags,
                               Error **errp);
 
 /**

diff --git a/qom/object.c b/qom/object.c
index cc946d9dacff2dd8cc3f4901bd83fad3e7683d03..9e22f11e21d7ab4635b360563e923ca962db323e 100644 (file)
--- a/qom/object.c
+++ b/qom/object.c
@@ -1023,10 +1023,16 @@ out:
     g_free(type);
 }
 
+typedef struct {
+    Object **child;
+    ObjectPropertyLinkFlags flags;
+} LinkProperty;
+
 static void object_get_link_property(Object *obj, Visitor *v, void *opaque,
                                      const char *name, Error **errp)
 {
-    Object **child = opaque;
+    LinkProperty *lprop = opaque;
+    Object **child = lprop->child;
     gchar *path;
 
     if (*child) {
@@ -1081,7 +1087,8 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque,
                                      const char *name, Error **errp)
 {
     Error *local_err = NULL;
-    Object **child = opaque;
+    LinkProperty *prop = opaque;
+    Object **child = prop->child;
     Object *old_target = *child;
     Object *new_target = NULL;
     char *path = NULL;
@@ -1107,18 +1114,41 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque,
     }
 }
 
+static void object_release_link_property(Object *obj, const char *name,
+                                         void *opaque)
+{
+    LinkProperty *prop = opaque;
+
+    if ((prop->flags & OBJ_PROP_LINK_UNREF_ON_RELEASE) && *prop->child) {
+        object_unref(*prop->child);
+    }
+    g_free(prop);
+}
+
 void object_property_add_link(Object *obj, const char *name,
                               const char *type, Object **child,
+                              ObjectPropertyLinkFlags flags,
                               Error **errp)
 {
+    Error *local_err = NULL;
+    LinkProperty *prop = g_malloc(sizeof(*prop));
     gchar *full_type;
 
+    prop->child = child;
+    prop->flags = flags;
+
     full_type = g_strdup_printf("link<%s>", type);
 
     object_property_add(obj, name, full_type,
                         object_get_link_property,
                         object_set_link_property,
-                        NULL, child, errp);
+                        object_release_link_property,
+                        prop,
+                        &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        g_free(prop);
+    }
 
     g_free(full_type);
 }

diff --git a/ui/console.c b/ui/console.c
index 4df251d57973d3e9ece7f6ce0faf0744a19f387c..9974212409b73eb9f4e23d0e12c56f5e43f5bec1 100644 (file)
--- a/ui/console.c
+++ b/ui/console.c
@@ -1180,7 +1180,9 @@ static QemuConsole *new_console(DisplayState *ds, console_type_t console_type)
     obj = object_new(TYPE_QEMU_CONSOLE);
     s = QEMU_CONSOLE(obj);
     object_property_add_link(obj, "device", TYPE_DEVICE,
-                             (Object **)&s->device, &local_err);
+                             (Object **)&s->device,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             &local_err);
     object_property_add_uint32_ptr(obj, "head",
                                    &s->head, &local_err);