[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 2/4] qom: make QOM link property unref optional
|
From: |
Stefan Hajnoczi |
|
Subject: |
[Qemu-devel] [PATCH 2/4] qom: make QOM link property unref optional |
|
Date: |
Tue, 4 Mar 2014 22:45:10 +0100 |
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 the bool unref_on_release argument to
object_property_add_link() so the caller can specify which refcount
behavior they require.
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: "Andreas Färber" <address@hidden>
Cc: Peter Crosthwaite <address@hidden>
Cc: Paolo Bonzini <address@hidden>
Cc: Alexander Graf <address@hidden>
Cc: Anthony Liguori <address@hidden>
Cc: "Michael S. Tsirkin" <address@hidden>
Signed-off-by: Stefan Hajnoczi <address@hidden>
---
hw/core/qdev.c | 7 +++++--
hw/dma/xilinx_axidma.c | 12 ++++++++----
hw/net/xilinx_axienet.c | 12 ++++++++----
hw/pcmcia/pxa2xx.c | 4 +++-
hw/s390x/s390-virtio-bus.c | 3 ++-
hw/s390x/virtio-ccw.c | 3 ++-
hw/virtio/virtio-pci.c | 3 ++-
hw/virtio/virtio-rng.c | 3 ++-
include/qom/object.h | 7 ++++++-
qom/object.c | 39 ++++++++++++++++++++++++++++++++++-----
ui/console.c | 3 ++-
11 files changed, 74 insertions(+), 22 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index c0b857f..273c737 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -98,6 +98,7 @@ static void bus_add_child(BusState *bus, DeviceState *child)
object_property_add_link(OBJECT(bus), name,
object_get_typename(OBJECT(child)),
(Object **)&kid->child,
+ false, /* return ownership on prop deletion */
NULL);
}
@@ -761,7 +762,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, false,
+ &error_abort);
}
static void device_post_init(Object *obj)
@@ -881,7 +883,8 @@ 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,
+ true, NULL);
}
static char *default_bus_get_fw_dev_path(DeviceState *dev)
diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
index 19f07b3..cc5826d 100644
--- a/hw/dma/xilinx_axidma.c
+++ b/hw/dma/xilinx_axidma.c
@@ -537,9 +537,11 @@ 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,
+ true, &local_errp);
object_property_add_link(OBJECT(cs), "dma", TYPE_XILINX_AXI_DMA,
- (Object **)&cs->dma, &local_errp);
+ (Object **)&cs->dma,
+ true, &local_errp);
if (local_errp) {
goto xilinx_axidma_realize_fail;
}
@@ -571,10 +573,12 @@ 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,
+ true, &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,
+ true, &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 0bd5eda..385e059 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -945,9 +945,11 @@ 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,
+ true, &local_errp);
object_property_add_link(OBJECT(cs), "enet", "xlnx.axi-ethernet",
- (Object **) &cs->enet, &local_errp);
+ (Object **) &cs->enet,
+ true, &local_errp);
if (local_errp) {
goto xilinx_enet_realize_fail;
}
@@ -982,10 +984,12 @@ 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,
+ true, &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,
+ true, &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 8f17596..f55a806 100644
--- a/hw/pcmcia/pxa2xx.c
+++ b/hw/pcmcia/pxa2xx.c
@@ -198,7 +198,9 @@ 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,
+ false, /* don't unref on prop deletion */
+ 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 e4fc353..0c9ff92 100644
--- 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,
+ true, 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 f6e0e3e..2fe782b 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1184,7 +1184,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,
+ true, NULL);
}
static Property virtio_ccw_rng_properties[] = {
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 7b91841..e91de26 100644
--- 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,
+ true, NULL);
}
diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index a16e3bc..54ae848 100644
--- 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,
+ true, NULL);
}
static const TypeInfo virtio_rng_info = {
diff --git a/include/qom/object.h b/include/qom/object.h
index 9c7c361..276d02e 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1065,6 +1065,8 @@ void object_property_add_child(Object *obj, const char
*name,
* @name: the name of the property
* @type: the qobj type of the link
* @child: a pointer to where the link object reference is stored
+ * @unref_on_release: whether to release the link object reference when
+ * the property is deleted
* @errp: if an error occurs, a pointer to an area to store the area
*
* Links establish relationships between objects. Links are unidirectional
@@ -1076,10 +1078,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>address@hidden</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
+ * <code>@unref_on_release</code> is true, 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,
+ bool unref_on_release,
Error **errp);
/**
diff --git a/qom/object.c b/qom/object.c
index c11ea1e..89c5358 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1023,10 +1023,16 @@ out:
g_free(type);
}
+typedef struct {
+ Object **child;
+ bool unref_on_release;
+} 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) {
@@ -1079,8 +1085,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;
- Object *old_target = *child;
+ LinkProperty *prop = opaque;
+ Object *old_target = *prop->child;
Object *new_target = NULL;
char *path = NULL;
@@ -1097,24 +1103,47 @@ static void object_set_link_property(Object *obj,
Visitor *v, void *opaque,
if (new_target) {
object_ref(new_target);
}
- *child = new_target;
+ *prop->child = new_target;
if (old_target != NULL) {
object_unref(old_target);
}
}
+static void object_release_link_property(Object *obj, const char *name,
+ void *opaque)
+{
+ LinkProperty *prop = opaque;
+
+ if (prop->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,
+ bool unref_on_release,
Error **errp)
{
+ Error *local_err = NULL;
+ LinkProperty *prop = g_malloc(sizeof(*prop));
gchar *full_type;
+ prop->child = child;
+ prop->unref_on_release = unref_on_release;
+
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 502e160..af2d617 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1178,7 +1178,8 @@ 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,
+ true, &local_err);
if (!active_console || ((active_console->console_type != GRAPHIC_CONSOLE)
&&
(console_type == GRAPHIC_CONSOLE))) {
--
1.8.5.3