object: fix OBJ_PROP_LINK_UNREF_ON_RELEASE ambivalence

A link property can be set during creation, with
object_property_add_link() and later with object_property_set_link().

add_link() doesn't add a reference to the target object, while
set_link() does.

Furthemore, OBJ_PROP_LINK_UNREF_ON_RELEASE flags, set during add_link,
says whether a reference must be released when the property is destroyed.
This can lead to leaks if the property was later set_link(), as the
added reference is never released.

Instead, rename OBJ_PROP_LINK_UNREF_ON_RELEASE to OBJ_PROP_LINK_STRONG
and use that has an indication on how the link handle reference
management in set_link().

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-id: 20180531195119.22021-3-marcandre.lureau@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 989778ab7f8860746c63b8433661c1d80d5cc467..35072dec1ecf38a3b05d0bff70de397dc677854e 100644 (file)
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1308,7 +1308,7 @@ static void create_link_property(Object *obj, Property *prop, Error **errp)
     object_property_add_link(obj, prop->name, prop->link_type,
                              child,
                              qdev_prop_allow_set_link_before_realize,
-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             OBJ_PROP_LINK_STRONG,
                              errp);
 }
 

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index ffec46179135fd6b4ba5e5fb9559654300a27b14..cf0db4b6da44b7a7735de3a7f18b550c5d72c001 100644 (file)
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -409,7 +409,7 @@ void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
         object_property_add_link(OBJECT(dev), propname, TYPE_IRQ,
                                  (Object **)&pins[i],
                                  object_property_allow_set_link,
-                                 OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                                 OBJ_PROP_LINK_STRONG,
                                  &error_abort);
         g_free(propname);
     }

diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
index c32ab083f83a9a7ca8e41a9652baa9d62a59ffcd..51301220e8d28a2941cc2cf47d9e3b663febe528 100644 (file)
--- a/hw/display/xlnx_dp.c
+++ b/hw/display/xlnx_dp.c
@@ -1223,7 +1223,7 @@ static void xlnx_dp_init(Object *obj)
     object_property_add_link(obj, "dpdma", TYPE_XLNX_DPDMA,
                              (Object **) &s->dpdma,
                              xlnx_dp_set_dpdma,
-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             OBJ_PROP_LINK_STRONG,
                              &error_abort);
 
     /*

diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
index 9b4810357403c9a9a35d5d6c05998ba8642dec2b..401a328e275fb5c45cef00f9062c2d6c4c454738 100644 (file)
--- a/hw/dma/xilinx_axidma.c
+++ b/hw/dma/xilinx_axidma.c
@@ -525,12 +525,12 @@ static void xilinx_axidma_realize(DeviceState *dev, Error **errp)
     object_property_add_link(OBJECT(ds), "dma", TYPE_XILINX_AXI_DMA,
                              (Object **)&ds->dma,
                              object_property_allow_set_link,
-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             OBJ_PROP_LINK_STRONG,
                              &local_err);
     object_property_add_link(OBJECT(cs), "dma", TYPE_XILINX_AXI_DMA,
                              (Object **)&cs->dma,
                              object_property_allow_set_link,
-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             OBJ_PROP_LINK_STRONG,
                              &local_err);
     if (local_err) {
         goto xilinx_axidma_realize_fail;

diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c
index 8eea757aff039cfeeb18301e0a747fe5bb37c582..b6745f5bcfa3cdcdf4d95945c89c577cc69d4f44 100644 (file)
--- a/hw/dma/xlnx-zdma.c
+++ b/hw/dma/xlnx-zdma.c
@@ -787,7 +787,7 @@ static void zdma_init(Object *obj)
     object_property_add_link(obj, "dma", TYPE_MEMORY_REGION,
                              (Object **)&s->dma_mr,
                              qdev_prop_allow_set_link_before_realize,
-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             OBJ_PROP_LINK_STRONG,
                              &error_abort);
 }
 

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f3befe6721f0948d2477cce22f0702c3a56f6462..ea57a46f8196cb66cca0ced36f0cefa79a69892f 100644 (file)
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -483,7 +483,7 @@ void pc_cmos_init(PCMachineState *pcms,
                              TYPE_ISA_DEVICE,
                              (Object **)&pcms->rtc,
                              object_property_allow_set_link,
-                             OBJ_PROP_LINK_UNREF_ON_RELEASE, &error_abort);
+                             OBJ_PROP_LINK_STRONG, &error_abort);
     object_property_set_link(OBJECT(pcms), OBJECT(s),
                              "rtc_state", &error_abort);
 

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 3d811360659e4f03e106cdda6eaac0106d6eed7d..d2f0d60361e6fd0f9444954e391b20469b0218dc 100644 (file)
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -289,7 +289,7 @@ static void pc_init1(MachineState *machine,
                                  TYPE_HOTPLUG_HANDLER,
                                  (Object **)&pcms->acpi_dev,
                                  object_property_allow_set_link,
-                                 OBJ_PROP_LINK_UNREF_ON_RELEASE, &error_abort);
+                                 OBJ_PROP_LINK_STRONG, &error_abort);
         object_property_set_link(OBJECT(machine), OBJECT(piix4_pm),
                                  PC_MACHINE_ACPI_DEVICE_PROP, &error_abort);
     }

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index b60cbb9266475cfb184f0331b83bf54eddc4b304..5be6ef73bbcf29124c9523fb53e4d48e257296cd 100644 (file)
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -194,7 +194,7 @@ static void pc_q35_init(MachineState *machine)
                              TYPE_HOTPLUG_HANDLER,
                              (Object **)&pcms->acpi_dev,
                              object_property_allow_set_link,
-                             OBJ_PROP_LINK_UNREF_ON_RELEASE, &error_abort);
+                             OBJ_PROP_LINK_STRONG, &error_abort);
     object_property_set_link(OBJECT(machine), OBJECT(lpc),
                              PC_MACHINE_ACPI_DEVICE_PROP, &error_abort);
 

diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
index 9be281fd879e5de5dba6bac245c231d627cf2854..63c031703d4d4501de90afbbbd9104a000ff8665 100644 (file)
--- a/hw/ipmi/ipmi.c
+++ b/hw/ipmi/ipmi.c
@@ -104,7 +104,7 @@ void ipmi_bmc_find_and_link(Object *obj, Object **bmc)
 {
     object_property_add_link(obj, "bmc", TYPE_IPMI_BMC, bmc,
                              isa_ipmi_bmc_check,
-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             OBJ_PROP_LINK_STRONG,
                              &error_abort);
 }
 

diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index d4c2c89dc1547123d8a7ff0d98675038da88165f..cc880a3d08326c4900755a7fc769f763cd5f14fe 100644 (file)
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -951,12 +951,12 @@ static void xilinx_enet_realize(DeviceState *dev, Error **errp)
     object_property_add_link(OBJECT(ds), "enet", "xlnx.axi-ethernet",
                              (Object **) &ds->enet,
                              object_property_allow_set_link,
-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             OBJ_PROP_LINK_STRONG,
                              &local_err);
     object_property_add_link(OBJECT(cs), "enet", "xlnx.axi-ethernet",
                              (Object **) &cs->enet,
                              object_property_allow_set_link,
-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             OBJ_PROP_LINK_STRONG,
                              &local_err);
     if (local_err) {
         goto xilinx_enet_realize_fail;

diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
index 03f5faee4beefffd0309cb6dddc75d2166b56b47..f599025956de6b15ba1ea3e10c56808e23fee7e9 100644 (file)
--- a/hw/ssi/xilinx_spips.c
+++ b/hw/ssi/xilinx_spips.c
@@ -1346,7 +1346,7 @@ static void xlnx_zynqmp_qspips_init(Object *obj)
     object_property_add_link(obj, "stream-connected-dma", TYPE_STREAM_SLAVE,
                              (Object **)&rq->dma,
                              object_property_allow_set_link,
-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             OBJ_PROP_LINK_STRONG,
                              NULL);
 }
 

diff --git a/include/qom/object.h b/include/qom/object.h
index a0c78c76f7cd196200f2367927ac018a6fe11abd..f3d2308d5640aba96017e2954b83d963a3ffddae 100644 (file)
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1103,6 +1103,11 @@ char *object_property_get_str(Object *obj, const char *name,
  * @errp: returns an error if this function fails
  *
  * 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.
+ *
  */
 void object_property_set_link(Object *obj, Object *value,
                               const char *name, Error **errp);
@@ -1394,7 +1399,7 @@ void object_property_add_child(Object *obj, const char *name,
 
 typedef enum {
     /* Unref the link pointer when the property is deleted */
-    OBJ_PROP_LINK_UNREF_ON_RELEASE = 0x1,
+    OBJ_PROP_LINK_STRONG = 0x1,
 } ObjectPropertyLinkFlags;
 
 /**
@@ -1432,8 +1437,9 @@ void object_property_allow_set_link(const Object *, const char *,
  * 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_UNREF_ON_RELEASE</code> bit is set,
- * the reference count is decremented when the property is deleted.
+ * <code>@flags</code> <code>OBJ_PROP_LINK_STRONG</code> bit is set,
+ * the reference count is decremented when the property is deleted or
+ * modified.
  */
 void object_property_add_link(Object *obj, const char *name,
                               const char *type, Object **child,

diff --git a/net/can/can_host.c b/net/can/can_host.c
index c3d26521cd9d536aac8c1a52252e568ca858a007..c79347ababf6f78a7af5798f32cf15874ca45538 100644 (file)
--- a/net/can/can_host.c
+++ b/net/can/can_host.c
@@ -77,7 +77,7 @@ static void can_host_instance_init(Object *obj)
     object_property_add_link(obj, "canbus", TYPE_CAN_BUS,
                              (Object **)&ch->bus,
                              object_property_allow_set_link,
-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             OBJ_PROP_LINK_STRONG,
                              &error_abort);
 }
 

diff --git a/net/colo-compare.c b/net/colo-compare.c
index c3a2be4c90853aaa0abfd7a35b1c12037079440a..dd745a491ba37262466c9e4586bf5624ca5ffd94 100644 (file)
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -980,7 +980,7 @@ static void colo_compare_init(Object *obj)
     object_property_add_link(obj, "iothread", TYPE_IOTHREAD,
                             (Object **)&s->iothread,
                             object_property_allow_set_link,
-                            OBJ_PROP_LINK_UNREF_ON_RELEASE, NULL);
+                            OBJ_PROP_LINK_STRONG, NULL);
 
     s->vnet_hdr = false;
     object_property_add_bool(obj, "vnet_hdr_support", compare_get_vnet_hdr,

diff --git a/qom/object.c b/qom/object.c
index cb7a8cd589b3dde1e2b9950105e05f3925956da4..e6462f289cca1ea114d40b4876da51d48b4bb138 100644 (file)
--- a/qom/object.c
+++ b/qom/object.c
@@ -1564,9 +1564,11 @@ static void object_set_link_property(Object *obj, Visitor *v,
         return;
     }
 
-    object_ref(new_target);
     *child = new_target;
-    object_unref(old_target);
+    if (prop->flags == OBJ_PROP_LINK_STRONG) {
+        object_ref(new_target);
+        object_unref(old_target);
+    }
 }
 
 static Object *object_resolve_link_property(Object *parent, void *opaque, const gchar *part)
@@ -1581,7 +1583,7 @@ static void object_release_link_property(Object *obj, const char *name,
 {
     LinkProperty *prop = opaque;
 
-    if ((prop->flags & OBJ_PROP_LINK_UNREF_ON_RELEASE) && *prop->child) {
+    if ((prop->flags & OBJ_PROP_LINK_STRONG) && *prop->child) {
         object_unref(*prop->child);
     }
     g_free(prop);

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 5d60893a076890132e0d34b43af2b4c8c2e95053..ab047b94021559c37928d122f5bcdb2d368b85c7 100644 (file)
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -690,7 +690,7 @@ static void arm_cpu_post_init(Object *obj)
                                  TYPE_MEMORY_REGION,
                                  (Object **)&cpu->secure_memory,
                                  qdev_prop_allow_set_link_before_realize,
-                                 OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                                 OBJ_PROP_LINK_STRONG,
                                  &error_abort);
 #endif
     }
@@ -718,7 +718,7 @@ static void arm_cpu_post_init(Object *obj)
     if (arm_feature(&cpu->env, ARM_FEATURE_M_SECURITY)) {
         object_property_add_link(obj, "idau", TYPE_IDAU_INTERFACE, &cpu->idau,
                                  qdev_prop_allow_set_link_before_realize,
-                                 OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                                 OBJ_PROP_LINK_STRONG,
                                  &error_abort);
         qdev_property_add_static(DEVICE(obj), &arm_cpu_initsvtor_property,
                                  &error_abort);

diff --git a/ui/console.c b/ui/console.c
index ef1247f872b86d06001a903a40647d78b3e3699c..bc58458ee84decf4fc72866403a35190048823ac 100644 (file)
--- a/ui/console.c
+++ b/ui/console.c
@@ -1287,7 +1287,7 @@ static QemuConsole *new_console(DisplayState *ds, console_type_t console_type,
     object_property_add_link(obj, "device", TYPE_DEVICE,
                              (Object **)&s->device,
                              object_property_allow_set_link,
-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             OBJ_PROP_LINK_STRONG,
                              &error_abort);
     object_property_add_uint32_ptr(obj, "head",
                                    &s->head, &error_abort);