Patchwork [8/9] qdev: make qdev_set_parent_bus() just set a link property

login
register
mail settings
Submitter Anthony Liguori
Date Aug. 26, 2012, 3:51 p.m.
Message ID <1345996298-4892-9-git-send-email-aliguori@us.ibm.com>
Download mbox | patch
Permalink /patch/180106/
State New
Headers show

Comments

Anthony Liguori - Aug. 26, 2012, 3:51 p.m.
Also make setting the link to NULL break the bus link

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/qdev.c |   48 ++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 42 insertions(+), 6 deletions(-)
pingfan liu - Aug. 27, 2012, 7:22 a.m.
On Sun, Aug 26, 2012 at 11:51 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Also make setting the link to NULL break the bus link
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  hw/qdev.c |   48 ++++++++++++++++++++++++++++++++++++++++++------
>  1 files changed, 42 insertions(+), 6 deletions(-)
>
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 86e1337..525a0cb 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -100,8 +100,7 @@ static void bus_add_child(BusState *bus, DeviceState *child)
>
>  void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
>  {
> -    dev->parent_bus = bus;
> -    bus_add_child(bus, dev);
> +    object_property_set_link(OBJECT(dev), OBJECT(bus), "parent_bus", NULL);
>  }
>
>  /* Create a new device.  This only initializes the device state structure
> @@ -241,8 +240,8 @@ void qbus_reset_all_fn(void *opaque)
>  /* can be used as ->unplug() callback for the simple cases */
>  int qdev_simple_unplug_cb(DeviceState *dev)
>  {
> -    /* just zap it */
> -    qdev_free(dev);
> +    /* Unplug from parent bus via a forced eject */
> +    qdev_set_parent_bus(dev, NULL);

I think it is more reliable to remove the reference property(child,
link) before object_finialize().  So when uplug-finish, we delete all
the refers:  bus->child, bus<-child by _del_property not using
_set_property.

>      return 0;
>  }
>
> @@ -646,6 +645,40 @@ void qdev_property_add_static(DeviceState *dev, Property *prop,
>      assert_no_error(local_err);
>  }
>
> +static void qdev_set_link_property(Object *obj, Visitor *v, void *opaque,
> +                                   const char *name, Error **errp)
> +{
> +    DeviceState *dev = DEVICE(obj);
> +    BusState *parent_bus = dev->parent_bus;
> +
> +    object_set_link_property(obj, v, opaque, name, errp);
> +
> +    if (parent_bus) {
> +        bus_remove_child(parent_bus, dev);
> +    }
> +
> +    if (dev->parent_bus) {
> +        bus_add_child(dev->parent_bus, dev);
> +    }
> +
> +    if (!dev->parent_bus) {
> +        notifier_list_notify(&dev->eject_notifier, dev);
> +    }
> +}
> +
> +static void qdev_release_link_property(Object *obj, const char *name,
> +                                       void *opaque)
> +{
> +    DeviceState *dev = DEVICE(obj);
> +
> +    if (dev->parent_bus) {
> +        bus_remove_child(dev->parent_bus, dev);
> +        object_unref(OBJECT(dev->parent_bus));
> +    }
> +
> +    dev->parent_bus = NULL;
> +}
> +
>  static void device_initfn(Object *obj)
>  {
>      DeviceState *dev = DEVICE(obj);
> @@ -670,8 +703,11 @@ static void device_initfn(Object *obj)
>      } while (class != object_class_by_name(TYPE_DEVICE));
>      qdev_prop_set_globals(dev);
>
> -    object_property_add_link(OBJECT(dev), "parent_bus", TYPE_BUS,
> -                             (Object **)&dev->parent_bus, NULL);
> +    object_property_add(OBJECT(dev), "parent_bus", "link<" TYPE_BUS ">",
> +                        object_get_link_property,
> +                        qdev_set_link_property,
> +                        qdev_release_link_property,
> +                        &dev->parent_bus, NULL);
>
>      notifier_list_init(&dev->eject_notifier);
>  }
> --
> 1.7.5.4
>
>
Anthony Liguori - Aug. 27, 2012, 1:12 p.m.
liu ping fan <qemulist@gmail.com> writes:

> On Sun, Aug 26, 2012 at 11:51 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
>> Also make setting the link to NULL break the bus link
>>
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> ---
>>  hw/qdev.c |   48 ++++++++++++++++++++++++++++++++++++++++++------
>>  1 files changed, 42 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/qdev.c b/hw/qdev.c
>> index 86e1337..525a0cb 100644
>> --- a/hw/qdev.c
>> +++ b/hw/qdev.c
>> @@ -100,8 +100,7 @@ static void bus_add_child(BusState *bus, DeviceState *child)
>>
>>  void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
>>  {
>> -    dev->parent_bus = bus;
>> -    bus_add_child(bus, dev);
>> +    object_property_set_link(OBJECT(dev), OBJECT(bus), "parent_bus", NULL);
>>  }
>>
>>  /* Create a new device.  This only initializes the device state structure
>> @@ -241,8 +240,8 @@ void qbus_reset_all_fn(void *opaque)
>>  /* can be used as ->unplug() callback for the simple cases */
>>  int qdev_simple_unplug_cb(DeviceState *dev)
>>  {
>> -    /* just zap it */
>> -    qdev_free(dev);
>> +    /* Unplug from parent bus via a forced eject */
>> +    qdev_set_parent_bus(dev, NULL);
>
> I think it is more reliable to remove the reference property(child,
> link) before object_finialize().  So when uplug-finish, we delete all
> the refers:  bus->child, bus<-child by _del_property not using
> _set_property.

object_finalize is called when ref=0.  You cannot remove refs in
finalize because by definition, ref=0.

Regards,

Anthony Liguori

Patch

diff --git a/hw/qdev.c b/hw/qdev.c
index 86e1337..525a0cb 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -100,8 +100,7 @@  static void bus_add_child(BusState *bus, DeviceState *child)
 
 void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
 {
-    dev->parent_bus = bus;
-    bus_add_child(bus, dev);
+    object_property_set_link(OBJECT(dev), OBJECT(bus), "parent_bus", NULL);
 }
 
 /* Create a new device.  This only initializes the device state structure
@@ -241,8 +240,8 @@  void qbus_reset_all_fn(void *opaque)
 /* can be used as ->unplug() callback for the simple cases */
 int qdev_simple_unplug_cb(DeviceState *dev)
 {
-    /* just zap it */
-    qdev_free(dev);
+    /* Unplug from parent bus via a forced eject */
+    qdev_set_parent_bus(dev, NULL);
     return 0;
 }
 
@@ -646,6 +645,40 @@  void qdev_property_add_static(DeviceState *dev, Property *prop,
     assert_no_error(local_err);
 }
 
+static void qdev_set_link_property(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    BusState *parent_bus = dev->parent_bus;
+
+    object_set_link_property(obj, v, opaque, name, errp);
+
+    if (parent_bus) {
+        bus_remove_child(parent_bus, dev);
+    }
+
+    if (dev->parent_bus) {
+        bus_add_child(dev->parent_bus, dev);
+    }
+
+    if (!dev->parent_bus) {
+        notifier_list_notify(&dev->eject_notifier, dev);
+    }
+}
+
+static void qdev_release_link_property(Object *obj, const char *name,
+                                       void *opaque)
+{
+    DeviceState *dev = DEVICE(obj);
+
+    if (dev->parent_bus) {
+        bus_remove_child(dev->parent_bus, dev);
+        object_unref(OBJECT(dev->parent_bus));
+    }
+
+    dev->parent_bus = NULL;
+}
+
 static void device_initfn(Object *obj)
 {
     DeviceState *dev = DEVICE(obj);
@@ -670,8 +703,11 @@  static void device_initfn(Object *obj)
     } while (class != object_class_by_name(TYPE_DEVICE));
     qdev_prop_set_globals(dev);
 
-    object_property_add_link(OBJECT(dev), "parent_bus", TYPE_BUS,
-                             (Object **)&dev->parent_bus, NULL);
+    object_property_add(OBJECT(dev), "parent_bus", "link<" TYPE_BUS ">",
+                        object_get_link_property,
+                        qdev_set_link_property,
+                        qdev_release_link_property,
+                        &dev->parent_bus, NULL);
 
     notifier_list_init(&dev->eject_notifier);
 }