qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports
@ 2014-09-30  6:10 arei.gonglei
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 01/12] virtio-net: use aliases instead of duplicate qdev properties arei.gonglei
                   ` (13 more replies)
  0 siblings, 14 replies; 34+ messages in thread
From: arei.gonglei @ 2014-09-30  6:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: weidong.huang, mst, armbru, luonengjun, peter.huangpeng, agraf,
	borntraeger, Gonglei, stefanha, cornelia.huck, pbonzini, rth

From: Gonglei <arei.gonglei@huawei.com>

virtio-$device-{pci, s390, ccw} all duplicate the
qdev properties of their virtio child. This approach does
not work well with string or pointer properties since we
must be careful about leaking or double-freeing them.

Use the QOM alias property to forward property accesses to the
VirtIORNG child.  This way no duplication is necessary.

For their child, object_initialize() leaves the object with a refcount of 1.
object_property_add_child() adds its own reference which is dropped
again when the property is deleted.

The upshot of this is that we always have a refcount >= 1.  Upon hot
unplug the virtio-$device child is not finalized!

Drop our reference after the child property has been added to the
parent.

Changs since v1:
 - add the same handling for virtio-9p-pci device in PATCH 10 and PATCH 11.
 - add a wrapper function for better code sharing 
   in PATCH 12 (Cornelia/Michael/Paolo)


Attention please:
 As Michael's comments, the patches will introduce regresion about
 "-device FOO,help", which had been fixed by my other patch series:
  [PATCH v4 0/5] add description field in ObjectProperty and PropertyInfo struct
 http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg05740.html

Acknowledgements:
 I copied Stefan's commit c5d49db message about virtio-blk which
 summarized reasons very well, I cannot agree more with him.
 Holp Stefan do not mind, thanks!

Thanks for your review!
-Gonglei


Gonglei (12):
  virtio-net: use aliases instead of duplicate qdev properties
  virtio-net: fix virtio-net child refcount in transports
  virtio/vhost-scsi: use aliases instead of duplicate qdev properties
  virtio/vhost-scsi: fix virtio-scsi/vhost-scsi child refcount in
    transports
  virtio-serial: use aliases instead of duplicate qdev properties
  virtio-serial: fix virtio-serial child refcount in transports
  virtio-rng: use aliases instead of duplicate qdev properties
  virtio-rng: fix virtio-rng child refcount in transports
  virtio-balloon: fix virtio-balloon child refcount in transports
  virtio-9p: use aliases instead of duplicate qdev properties
  virtio-9p: fix virtio-9p child refcount in transports
  virtio: add a wrapper for virtio-backend initialization

 hw/s390x/s390-virtio-bus.c | 38 ++++++++++++++++++--------------------
 hw/s390x/virtio-ccw.c      | 40 +++++++++++++++++++---------------------
 hw/virtio/virtio-pci.c     | 46 ++++++++++++++++++++++------------------------
 hw/virtio/virtio.c         | 11 +++++++++++
 include/hw/virtio/virtio.h |  3 +++
 5 files changed, 73 insertions(+), 65 deletions(-)

-- 
1.7.12.4

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCH v2 01/12] virtio-net: use aliases instead of duplicate qdev properties
  2014-09-30  6:10 [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports arei.gonglei
@ 2014-09-30  6:10 ` arei.gonglei
  2014-09-30  8:18   ` Cornelia Huck
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 02/12] virtio-net: fix virtio-net child refcount in transports arei.gonglei
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: arei.gonglei @ 2014-09-30  6:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: weidong.huang, mst, armbru, luonengjun, peter.huangpeng, agraf,
	borntraeger, Gonglei, stefanha, cornelia.huck, pbonzini, rth

From: Gonglei <arei.gonglei@huawei.com>

virtio-net-pci, virtio-net-s390, and virtio-net-ccw all duplicate the
qdev properties of their VirtIONet child. This approach does not work
well with string or pointer properties since we must be careful about
leaking or double-freeing them.

Use the QOM alias property to forward property accesses to the
VirtIONet child.  This way no duplication is necessary.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/s390x/s390-virtio-bus.c | 3 +--
 hw/s390x/virtio-ccw.c      | 3 +--
 hw/virtio/virtio-pci.c     | 3 +--
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 6b6fb61..5b5d595 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -161,6 +161,7 @@ static void s390_virtio_net_instance_init(Object *obj)
     VirtIONetS390 *dev = VIRTIO_NET_S390(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_NET);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 
 static int s390_virtio_blk_init(VirtIOS390Device *s390_dev)
@@ -493,10 +494,8 @@ static unsigned virtio_s390_get_features(DeviceState *d)
 /**************** S390 Virtio Bus Device Descriptions *******************/
 
 static Property s390_virtio_net_properties[] = {
-    DEFINE_NIC_PROPERTIES(VirtIONetS390, vdev.nic_conf),
     DEFINE_VIRTIO_COMMON_FEATURES(VirtIOS390Device, host_features),
     DEFINE_VIRTIO_NET_FEATURES(VirtIOS390Device, host_features),
-    DEFINE_VIRTIO_NET_PROPERTIES(VirtIONetS390, vdev.net_conf),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 33a1d86..7d67577 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -794,6 +794,7 @@ static void virtio_ccw_net_instance_init(Object *obj)
     VirtIONetCcw *dev = VIRTIO_NET_CCW(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_NET);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 
 static int virtio_ccw_blk_init(VirtioCcwDevice *ccw_dev)
@@ -1374,8 +1375,6 @@ static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f)
 static Property virtio_ccw_net_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
     DEFINE_VIRTIO_NET_FEATURES(VirtioCcwDevice, host_features[0]),
-    DEFINE_VIRTIO_NET_PROPERTIES(VirtIONetCcw, vdev.net_conf),
-    DEFINE_NIC_PROPERTIES(VirtIONetCcw, vdev.nic_conf),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index f560814..155fac9 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1425,8 +1425,6 @@ static Property virtio_net_properties[] = {
                     VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, false),
     DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3),
     DEFINE_VIRTIO_NET_FEATURES(VirtIOPCIProxy, host_features),
-    DEFINE_NIC_PROPERTIES(VirtIONetPCI, vdev.nic_conf),
-    DEFINE_VIRTIO_NET_PROPERTIES(VirtIONetPCI, vdev.net_conf),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1467,6 +1465,7 @@ static void virtio_net_pci_instance_init(Object *obj)
     VirtIONetPCI *dev = VIRTIO_NET_PCI(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_NET);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 
 static const TypeInfo virtio_net_pci_info = {
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCH v2 02/12] virtio-net: fix virtio-net child refcount in transports
  2014-09-30  6:10 [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports arei.gonglei
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 01/12] virtio-net: use aliases instead of duplicate qdev properties arei.gonglei
@ 2014-09-30  6:10 ` arei.gonglei
  2014-09-30  8:19   ` Cornelia Huck
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 03/12] virtio/vhost-scsi: use aliases instead of duplicate qdev properties arei.gonglei
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: arei.gonglei @ 2014-09-30  6:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: weidong.huang, mst, armbru, luonengjun, peter.huangpeng, agraf,
	borntraeger, Gonglei, stefanha, cornelia.huck, pbonzini, rth

From: Gonglei <arei.gonglei@huawei.com>

object_initialize() leaves the object with a refcount of 1.
object_property_add_child() adds its own reference which is dropped
again when the property is deleted.

The upshot of this is that we always have a refcount >= 1.  Upon hot
unplug the virtio-net child is not finalized!

Drop our reference after the child property has been added to the
parent.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/s390x/s390-virtio-bus.c | 1 +
 hw/s390x/virtio-ccw.c      | 1 +
 hw/virtio/virtio-pci.c     | 1 +
 3 files changed, 3 insertions(+)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 5b5d595..297eac2 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -161,6 +161,7 @@ static void s390_virtio_net_instance_init(Object *obj)
     VirtIONetS390 *dev = VIRTIO_NET_S390(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_NET);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    object_unref(OBJECT(&dev->vdev));
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 7d67577..bb699f2 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -794,6 +794,7 @@ static void virtio_ccw_net_instance_init(Object *obj)
     VirtIONetCcw *dev = VIRTIO_NET_CCW(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_NET);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    object_unref(OBJECT(&dev->vdev));
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 155fac9..b82b738 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1465,6 +1465,7 @@ static void virtio_net_pci_instance_init(Object *obj)
     VirtIONetPCI *dev = VIRTIO_NET_PCI(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_NET);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    object_unref(OBJECT(&dev->vdev));
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCH v2 03/12] virtio/vhost-scsi: use aliases instead of duplicate qdev properties
  2014-09-30  6:10 [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports arei.gonglei
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 01/12] virtio-net: use aliases instead of duplicate qdev properties arei.gonglei
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 02/12] virtio-net: fix virtio-net child refcount in transports arei.gonglei
@ 2014-09-30  6:10 ` arei.gonglei
  2014-09-30  8:43   ` Cornelia Huck
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 04/12] virtio/vhost-scsi: fix virtio-scsi/vhost-scsi child refcount in transports arei.gonglei
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: arei.gonglei @ 2014-09-30  6:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: weidong.huang, mst, armbru, luonengjun, peter.huangpeng, agraf,
	borntraeger, Gonglei, stefanha, cornelia.huck, pbonzini, rth

From: Gonglei <arei.gonglei@huawei.com>

{virtio, vhost}-scsi-{pci, s390, ccw} all duplicate the
qdev properties of their VirtIOSCSI/VHostSCSI child.
This approach does not work well with string or pointer
properties since we must be careful about leaking or
double-freeing them.

Use the QOM alias property to forward property accesses to the
VirtIOSCSI/VHostSCSI child. This way no duplication is necessary.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/s390x/s390-virtio-bus.c | 4 ++--
 hw/s390x/virtio-ccw.c      | 4 ++--
 hw/virtio/virtio-pci.c     | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 297eac2..eaaa275 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -258,6 +258,7 @@ static void s390_virtio_scsi_instance_init(Object *obj)
     VirtIOSCSIS390 *dev = VIRTIO_SCSI_S390(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SCSI);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 
 #ifdef CONFIG_VHOST_SCSI
@@ -279,6 +280,7 @@ static void s390_vhost_scsi_instance_init(Object *obj)
     VHostSCSIS390 *dev = VHOST_SCSI_S390(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VHOST_SCSI);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 #endif
 
@@ -614,7 +616,6 @@ static const TypeInfo virtio_s390_device_info = {
 };
 
 static Property s390_virtio_scsi_properties[] = {
-    DEFINE_VIRTIO_SCSI_PROPERTIES(VirtIOSCSIS390, vdev.parent_obj.conf),
     DEFINE_VIRTIO_COMMON_FEATURES(VirtIOS390Device, host_features),
     DEFINE_VIRTIO_SCSI_FEATURES(VirtIOS390Device, host_features),
     DEFINE_PROP_END_OF_LIST(),
@@ -640,7 +641,6 @@ static const TypeInfo s390_virtio_scsi = {
 #ifdef CONFIG_VHOST_SCSI
 static Property s390_vhost_scsi_properties[] = {
     DEFINE_VIRTIO_COMMON_FEATURES(VirtIOS390Device, host_features),
-    DEFINE_VHOST_SCSI_PROPERTIES(VHostSCSIS390, vdev.parent_obj.conf),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index bb699f2..458aabc 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -938,6 +938,7 @@ static void virtio_ccw_scsi_instance_init(Object *obj)
     VirtIOSCSICcw *dev = VIRTIO_SCSI_CCW(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SCSI);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 
 #ifdef CONFIG_VHOST_SCSI
@@ -959,6 +960,7 @@ static void vhost_ccw_scsi_instance_init(Object *obj)
     VHostSCSICcw *dev = VHOST_SCSI_CCW(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VHOST_SCSI);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 #endif
 
@@ -1481,7 +1483,6 @@ static const TypeInfo virtio_ccw_balloon = {
 
 static Property virtio_ccw_scsi_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
-    DEFINE_VIRTIO_SCSI_PROPERTIES(VirtIOSCSICcw, vdev.parent_obj.conf),
     DEFINE_VIRTIO_SCSI_FEATURES(VirtioCcwDevice, host_features[0]),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
@@ -1510,7 +1511,6 @@ static const TypeInfo virtio_ccw_scsi = {
 #ifdef CONFIG_VHOST_SCSI
 static Property vhost_ccw_scsi_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
-    DEFINE_VHOST_SCSI_PROPERTIES(VirtIOSCSICcw, vdev.parent_obj.conf),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index b82b738..ef48983 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1135,7 +1135,6 @@ static Property virtio_scsi_pci_properties[] = {
     DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
                        DEV_NVECTORS_UNSPECIFIED),
     DEFINE_VIRTIO_SCSI_FEATURES(VirtIOPCIProxy, host_features),
-    DEFINE_VIRTIO_SCSI_PROPERTIES(VirtIOSCSIPCI, vdev.parent_obj.conf),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1187,6 +1186,7 @@ static void virtio_scsi_pci_instance_init(Object *obj)
     VirtIOSCSIPCI *dev = VIRTIO_SCSI_PCI(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SCSI);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 
 static const TypeInfo virtio_scsi_pci_info = {
@@ -1203,7 +1203,6 @@ static const TypeInfo virtio_scsi_pci_info = {
 static Property vhost_scsi_pci_properties[] = {
     DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
                        DEV_NVECTORS_UNSPECIFIED),
-    DEFINE_VHOST_SCSI_PROPERTIES(VHostSCSIPCI, vdev.parent_obj.conf),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1243,6 +1242,7 @@ static void vhost_scsi_pci_instance_init(Object *obj)
     VHostSCSIPCI *dev = VHOST_SCSI_PCI(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VHOST_SCSI);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 
 static const TypeInfo vhost_scsi_pci_info = {
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCH v2 04/12] virtio/vhost-scsi: fix virtio-scsi/vhost-scsi child refcount in transports
  2014-09-30  6:10 [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports arei.gonglei
                   ` (2 preceding siblings ...)
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 03/12] virtio/vhost-scsi: use aliases instead of duplicate qdev properties arei.gonglei
@ 2014-09-30  6:10 ` arei.gonglei
  2014-09-30  8:44   ` Cornelia Huck
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 05/12] virtio-serial: use aliases instead of duplicate qdev properties arei.gonglei
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: arei.gonglei @ 2014-09-30  6:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: weidong.huang, mst, armbru, luonengjun, peter.huangpeng, agraf,
	borntraeger, Gonglei, stefanha, cornelia.huck, pbonzini, rth

From: Gonglei <arei.gonglei@huawei.com>

object_initialize() leaves the object with a refcount of 1.
object_property_add_child() adds its own reference which is dropped
again when the property is deleted.

The upshot of this is that we always have a refcount >= 1.  Upon hot
unplug the virtio-scsi/vhost-scsi child is not finalized!

Drop our reference after the child property has been added to the
parent.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/s390x/s390-virtio-bus.c | 2 ++
 hw/s390x/virtio-ccw.c      | 2 ++
 hw/virtio/virtio-pci.c     | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index eaaa275..4276034 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -258,6 +258,7 @@ static void s390_virtio_scsi_instance_init(Object *obj)
     VirtIOSCSIS390 *dev = VIRTIO_SCSI_S390(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SCSI);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    object_unref(OBJECT(&dev->vdev));
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 
@@ -280,6 +281,7 @@ static void s390_vhost_scsi_instance_init(Object *obj)
     VHostSCSIS390 *dev = VHOST_SCSI_S390(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VHOST_SCSI);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    object_unref(OBJECT(&dev->vdev));
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 #endif
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 458aabc..a466674 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -938,6 +938,7 @@ static void virtio_ccw_scsi_instance_init(Object *obj)
     VirtIOSCSICcw *dev = VIRTIO_SCSI_CCW(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SCSI);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    object_unref(OBJECT(&dev->vdev));
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 
@@ -960,6 +961,7 @@ static void vhost_ccw_scsi_instance_init(Object *obj)
     VHostSCSICcw *dev = VHOST_SCSI_CCW(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VHOST_SCSI);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    object_unref(OBJECT(&dev->vdev));
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 #endif
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index ef48983..09f2093 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1186,6 +1186,7 @@ static void virtio_scsi_pci_instance_init(Object *obj)
     VirtIOSCSIPCI *dev = VIRTIO_SCSI_PCI(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SCSI);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    object_unref(OBJECT(&dev->vdev));
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 
@@ -1242,6 +1243,7 @@ static void vhost_scsi_pci_instance_init(Object *obj)
     VHostSCSIPCI *dev = VHOST_SCSI_PCI(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VHOST_SCSI);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    object_unref(OBJECT(&dev->vdev));
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCH v2 05/12] virtio-serial: use aliases instead of duplicate qdev properties
  2014-09-30  6:10 [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports arei.gonglei
                   ` (3 preceding siblings ...)
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 04/12] virtio/vhost-scsi: fix virtio-scsi/vhost-scsi child refcount in transports arei.gonglei
@ 2014-09-30  6:10 ` arei.gonglei
  2014-09-30  8:46   ` Cornelia Huck
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 06/12] virtio-serial: fix virtio-serial child refcount in transports arei.gonglei
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: arei.gonglei @ 2014-09-30  6:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: weidong.huang, mst, armbru, luonengjun, peter.huangpeng, agraf,
	borntraeger, Gonglei, stefanha, cornelia.huck, pbonzini, rth

From: Gonglei <arei.gonglei@huawei.com>

virtio-serial-{pci, s390, ccw} all duplicate the
qdev properties of their VirtIOSerial child.
This approach does not work well with string or pointer
properties since we must be careful about leaking or
double-freeing them.

Use the QOM alias property to forward property accesses to the
VirtIOSerial child.  This way no duplication is necessary.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/s390x/s390-virtio-bus.c | 2 +-
 hw/s390x/virtio-ccw.c      | 2 +-
 hw/virtio/virtio-pci.c     | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 4276034..31f5286 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -226,6 +226,7 @@ static void s390_virtio_serial_instance_init(Object *obj)
     VirtIOSerialS390 *dev = VIRTIO_SERIAL_S390(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SERIAL);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 
 static int s390_virtio_scsi_init(VirtIOS390Device *s390_dev)
@@ -537,7 +538,6 @@ static const TypeInfo s390_virtio_blk = {
 };
 
 static Property s390_virtio_serial_properties[] = {
-    DEFINE_VIRTIO_SERIAL_PROPERTIES(VirtIOSerialS390, vdev.serial),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index a466674..271104d 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -852,6 +852,7 @@ static void virtio_ccw_serial_instance_init(Object *obj)
     VirtioSerialCcw *dev = VIRTIO_SERIAL_CCW(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SERIAL);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 
 static int virtio_ccw_balloon_init(VirtioCcwDevice *ccw_dev)
@@ -1432,7 +1433,6 @@ static const TypeInfo virtio_ccw_blk = {
 
 static Property virtio_ccw_serial_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
-    DEFINE_VIRTIO_SERIAL_PROPERTIES(VirtioSerialCcw, vdev.serial),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 09f2093..3c1f37b 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1387,7 +1387,6 @@ static Property virtio_serial_pci_properties[] = {
                     VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
     DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
-    DEFINE_VIRTIO_SERIAL_PROPERTIES(VirtIOSerialPCI, vdev.serial),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1410,6 +1409,7 @@ static void virtio_serial_pci_instance_init(Object *obj)
     VirtIOSerialPCI *dev = VIRTIO_SERIAL_PCI(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SERIAL);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 
 static const TypeInfo virtio_serial_pci_info = {
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCH v2 06/12] virtio-serial: fix virtio-serial child refcount in transports
  2014-09-30  6:10 [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports arei.gonglei
                   ` (4 preceding siblings ...)
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 05/12] virtio-serial: use aliases instead of duplicate qdev properties arei.gonglei
@ 2014-09-30  6:10 ` arei.gonglei
  2014-09-30  8:47   ` Cornelia Huck
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 07/12] virtio-rng: use aliases instead of duplicate qdev properties arei.gonglei
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: arei.gonglei @ 2014-09-30  6:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: weidong.huang, mst, armbru, luonengjun, peter.huangpeng, agraf,
	borntraeger, Gonglei, stefanha, cornelia.huck, pbonzini, rth

From: Gonglei <arei.gonglei@huawei.com>

object_initialize() leaves the object with a refcount of 1.
object_property_add_child() adds its own reference which is dropped
again when the property is deleted.

The upshot of this is that we always have a refcount >= 1.  Upon hot
unplug the virtio-serial child is not finalized!

Drop our reference after the child property has been added to the
parent.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/s390x/s390-virtio-bus.c | 1 +
 hw/s390x/virtio-ccw.c      | 1 +
 hw/virtio/virtio-pci.c     | 1 +
 3 files changed, 3 insertions(+)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 31f5286..422402e 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -227,6 +227,7 @@ static void s390_virtio_serial_instance_init(Object *obj)
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SERIAL);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+    object_unref(OBJECT(&dev->vdev));
 }
 
 static int s390_virtio_scsi_init(VirtIOS390Device *s390_dev)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 271104d..5d7f3a6 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -853,6 +853,7 @@ static void virtio_ccw_serial_instance_init(Object *obj)
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SERIAL);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+    object_unref(OBJECT(&dev->vdev));
 }
 
 static int virtio_ccw_balloon_init(VirtioCcwDevice *ccw_dev)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 3c1f37b..4446d79 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1410,6 +1410,7 @@ static void virtio_serial_pci_instance_init(Object *obj)
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SERIAL);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+    object_unref(OBJECT(&dev->vdev));
 }
 
 static const TypeInfo virtio_serial_pci_info = {
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCH v2 07/12] virtio-rng: use aliases instead of duplicate qdev properties
  2014-09-30  6:10 [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports arei.gonglei
                   ` (5 preceding siblings ...)
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 06/12] virtio-serial: fix virtio-serial child refcount in transports arei.gonglei
@ 2014-09-30  6:10 ` arei.gonglei
  2014-09-30  8:50   ` Cornelia Huck
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 08/12] virtio-rng: fix virtio-rng child refcount in transports arei.gonglei
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: arei.gonglei @ 2014-09-30  6:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: weidong.huang, mst, armbru, luonengjun, peter.huangpeng, agraf,
	borntraeger, Gonglei, stefanha, cornelia.huck, pbonzini, rth

From: Gonglei <arei.gonglei@huawei.com>

virtio-rng-{pci, s390, ccw} all duplicate the
qdev properties of their VirtIORNG child.
This approach does not work well with string or pointer
properties since we must be careful about leaking or
double-freeing them.

Use the QOM alias property to forward property accesses to the
VirtIORNG child.  This way no duplication is necessary.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/s390x/s390-virtio-bus.c | 2 +-
 hw/s390x/virtio-ccw.c      | 2 +-
 hw/virtio/virtio-pci.c     | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 422402e..6d0a7f3 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -311,6 +311,7 @@ static void s390_virtio_rng_instance_init(Object *obj)
     VirtIORNGS390 *dev = VIRTIO_RNG_S390(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_RNG);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
     object_property_add_link(obj, "rng", TYPE_RNG_BACKEND,
                              (Object **)&dev->vdev.conf.rng,
                              qdev_prop_allow_set_link_before_realize,
@@ -561,7 +562,6 @@ static const TypeInfo s390_virtio_serial = {
 
 static Property s390_virtio_rng_properties[] = {
     DEFINE_VIRTIO_COMMON_FEATURES(VirtIOS390Device, host_features),
-    DEFINE_VIRTIO_RNG_PROPERTIES(VirtIORNGS390, vdev.conf),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 5d7f3a6..da2e427 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1542,6 +1542,7 @@ static void virtio_ccw_rng_instance_init(Object *obj)
     VirtIORNGCcw *dev = VIRTIO_RNG_CCW(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_RNG);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
     object_property_add_link(obj, "rng", TYPE_RNG_BACKEND,
                              (Object **)&dev->vdev.conf.rng,
                              qdev_prop_allow_set_link_before_realize,
@@ -1550,7 +1551,6 @@ static void virtio_ccw_rng_instance_init(Object *obj)
 
 static Property virtio_ccw_rng_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
-    DEFINE_VIRTIO_RNG_PROPERTIES(VirtIORNGCcw, vdev.conf),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 4446d79..2b3a941 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1483,7 +1483,6 @@ static const TypeInfo virtio_net_pci_info = {
 /* virtio-rng-pci */
 
 static Property virtio_rng_pci_properties[] = {
-    DEFINE_VIRTIO_RNG_PROPERTIES(VirtIORngPCI, vdev.conf),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1525,6 +1524,7 @@ static void virtio_rng_initfn(Object *obj)
     VirtIORngPCI *dev = VIRTIO_RNG_PCI(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_RNG);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
     object_property_add_link(obj, "rng", TYPE_RNG_BACKEND,
                              (Object **)&dev->vdev.conf.rng,
                              qdev_prop_allow_set_link_before_realize,
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCH v2 08/12] virtio-rng: fix virtio-rng child refcount in transports
  2014-09-30  6:10 [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports arei.gonglei
                   ` (6 preceding siblings ...)
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 07/12] virtio-rng: use aliases instead of duplicate qdev properties arei.gonglei
@ 2014-09-30  6:10 ` arei.gonglei
  2014-09-30  8:51   ` Cornelia Huck
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 09/12] virtio-balloon: fix virtio-balloon " arei.gonglei
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: arei.gonglei @ 2014-09-30  6:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: weidong.huang, mst, armbru, luonengjun, peter.huangpeng, agraf,
	borntraeger, Gonglei, stefanha, cornelia.huck, pbonzini, rth

From: Gonglei <arei.gonglei@huawei.com>

object_initialize() leaves the object with a refcount of 1.
object_property_add_child() adds its own reference which is dropped
again when the property is deleted.

The upshot of this is that we always have a refcount >= 1.  Upon hot
unplug the virtio-rng child is not finalized!

Drop our reference after the child property has been added to the
parent.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/s390x/s390-virtio-bus.c | 1 +
 hw/s390x/virtio-ccw.c      | 1 +
 hw/virtio/virtio-pci.c     | 1 +
 3 files changed, 3 insertions(+)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 6d0a7f3..ca682bb 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -312,6 +312,7 @@ 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);
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+    object_unref(OBJECT(&dev->vdev));
     object_property_add_link(obj, "rng", TYPE_RNG_BACKEND,
                              (Object **)&dev->vdev.conf.rng,
                              qdev_prop_allow_set_link_before_realize,
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index da2e427..de0764d 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1543,6 +1543,7 @@ 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);
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+    object_unref(OBJECT(&dev->vdev));
     object_property_add_link(obj, "rng", TYPE_RNG_BACKEND,
                              (Object **)&dev->vdev.conf.rng,
                              qdev_prop_allow_set_link_before_realize,
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 2b3a941..40652a7 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1525,6 +1525,7 @@ 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);
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+    object_unref(OBJECT(&dev->vdev));
     object_property_add_link(obj, "rng", TYPE_RNG_BACKEND,
                              (Object **)&dev->vdev.conf.rng,
                              qdev_prop_allow_set_link_before_realize,
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCH v2 09/12] virtio-balloon: fix virtio-balloon child refcount in transports
  2014-09-30  6:10 [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports arei.gonglei
                   ` (7 preceding siblings ...)
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 08/12] virtio-rng: fix virtio-rng child refcount in transports arei.gonglei
@ 2014-09-30  6:10 ` arei.gonglei
  2014-09-30  8:52   ` Cornelia Huck
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 10/12] virtio-9p: use aliases instead of duplicate qdev properties arei.gonglei
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: arei.gonglei @ 2014-09-30  6:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: weidong.huang, mst, armbru, luonengjun, peter.huangpeng, agraf,
	borntraeger, Gonglei, stefanha, cornelia.huck, pbonzini, rth

From: Gonglei <arei.gonglei@huawei.com>

object_initialize() leaves the object with a refcount of 1.
object_property_add_child() adds its own reference which is dropped
again when the property is deleted.

The upshot of this is that we always have a refcount >= 1.  Upon hot
unplug the virtio-balloon child is not finalized!

Drop our reference after the child property has been added to the
parent.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/s390x/virtio-ccw.c  | 2 +-
 hw/virtio/virtio-pci.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index de0764d..c074f64 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -900,7 +900,7 @@ static void virtio_ccw_balloon_instance_init(Object *obj)
     VirtIOBalloonCcw *dev = VIRTIO_BALLOON_CCW(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BALLOON);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-
+    object_unref(OBJECT(&dev->vdev));
     object_property_add(obj, "guest-stats", "guest statistics",
                         balloon_ccw_stats_get_all, NULL, NULL, dev, NULL);
 
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 40652a7..62f84c4 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1325,7 +1325,7 @@ static void virtio_balloon_pci_instance_init(Object *obj)
     VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BALLOON);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-
+    object_unref(OBJECT(&dev->vdev));
     object_property_add(obj, "guest-stats", "guest statistics",
                         balloon_pci_stats_get_all, NULL, NULL, dev,
                         NULL);
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCH v2 10/12] virtio-9p: use aliases instead of duplicate qdev properties
  2014-09-30  6:10 [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports arei.gonglei
                   ` (8 preceding siblings ...)
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 09/12] virtio-balloon: fix virtio-balloon " arei.gonglei
@ 2014-09-30  6:10 ` arei.gonglei
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 11/12] virtio-9p: fix virtio-9p child refcount in transports arei.gonglei
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 34+ messages in thread
From: arei.gonglei @ 2014-09-30  6:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: weidong.huang, mst, armbru, luonengjun, peter.huangpeng, agraf,
	borntraeger, Gonglei, stefanha, cornelia.huck, pbonzini, rth

From: Gonglei <arei.gonglei@huawei.com>

virtio-9p-pci all duplicate the qdev properties of their
V9fsState child. This approach does not work well with
string or pointer properties since we must be careful
about leaking or double-freeing them.

Use the QOM alias property to forward property accesses to the
V9fsState child.  This way no duplication is necessary.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/virtio/virtio-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 62f84c4..714286d 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -926,7 +926,6 @@ static Property virtio_9p_pci_properties[] = {
     DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
                     VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
-    DEFINE_VIRTIO_9P_PROPERTIES(V9fsPCIState, vdev.fsconf),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -950,6 +949,7 @@ static void virtio_9p_pci_instance_init(Object *obj)
     V9fsPCIState *dev = VIRTIO_9P_PCI(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_9P);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
 }
 
 static const TypeInfo virtio_9p_pci_info = {
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCH v2 11/12] virtio-9p: fix virtio-9p child refcount in transports
  2014-09-30  6:10 [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports arei.gonglei
                   ` (9 preceding siblings ...)
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 10/12] virtio-9p: use aliases instead of duplicate qdev properties arei.gonglei
@ 2014-09-30  6:10 ` arei.gonglei
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 12/12] virtio: add a wrapper for virtio-backend initialization arei.gonglei
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 34+ messages in thread
From: arei.gonglei @ 2014-09-30  6:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: weidong.huang, mst, armbru, luonengjun, peter.huangpeng, agraf,
	borntraeger, Gonglei, stefanha, cornelia.huck, pbonzini, rth

From: Gonglei <arei.gonglei@huawei.com>

object_initialize() leaves the object with a refcount of 1.
object_property_add_child() adds its own reference which is
dropped again when the property is deleted.

The upshot of this is that we always have a refcount >= 1. Upon
unplug the virtio-9p child is not finalized!

Drop our reference after the child property has been added to the
parent.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/virtio/virtio-pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 714286d..8f3b79b 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -950,6 +950,7 @@ static void virtio_9p_pci_instance_init(Object *obj)
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_9P);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+    object_unref(OBJECT(&dev->vdev));
 }
 
 static const TypeInfo virtio_9p_pci_info = {
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCH v2 12/12] virtio: add a wrapper for virtio-backend initialization
  2014-09-30  6:10 [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports arei.gonglei
                   ` (10 preceding siblings ...)
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 11/12] virtio-9p: fix virtio-9p child refcount in transports arei.gonglei
@ 2014-09-30  6:10 ` arei.gonglei
  2014-09-30  9:04   ` Cornelia Huck
  2014-09-30  8:02 ` [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports Cornelia Huck
  2014-09-30  8:41 ` Paolo Bonzini
  13 siblings, 1 reply; 34+ messages in thread
From: arei.gonglei @ 2014-09-30  6:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: weidong.huang, mst, armbru, luonengjun, peter.huangpeng, agraf,
	borntraeger, Gonglei, stefanha, cornelia.huck, pbonzini, rth

From: Gonglei <arei.gonglei@huawei.com>

For better code sharing, add a wrapper help funciton
for various virtio devices.

Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/s390x/s390-virtio-bus.c | 42 +++++++++++++++++----------------------
 hw/s390x/virtio-ccw.c      | 42 +++++++++++++++++----------------------
 hw/virtio/virtio-pci.c     | 49 ++++++++++++++++++++--------------------------
 hw/virtio/virtio.c         | 11 +++++++++++
 include/hw/virtio/virtio.h |  3 +++
 5 files changed, 71 insertions(+), 76 deletions(-)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index ca682bb..f451ca1 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -159,10 +159,9 @@ static int s390_virtio_net_init(VirtIOS390Device *s390_dev)
 static void s390_virtio_net_instance_init(Object *obj)
 {
     VirtIONetS390 *dev = VIRTIO_NET_S390(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_NET);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    object_unref(OBJECT(&dev->vdev));
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_NET);
 }
 
 static int s390_virtio_blk_init(VirtIOS390Device *s390_dev)
@@ -179,10 +178,9 @@ static int s390_virtio_blk_init(VirtIOS390Device *s390_dev)
 static void s390_virtio_blk_instance_init(Object *obj)
 {
     VirtIOBlkS390 *dev = VIRTIO_BLK_S390(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BLK);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    object_unref(OBJECT(&dev->vdev));
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_BLK);
     object_property_add_alias(obj, "iothread", OBJECT(&dev->vdev),"iothread",
                               &error_abort);
 }
@@ -224,10 +222,9 @@ static int s390_virtio_serial_init(VirtIOS390Device *s390_dev)
 static void s390_virtio_serial_instance_init(Object *obj)
 {
     VirtIOSerialS390 *dev = VIRTIO_SERIAL_S390(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SERIAL);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
-    object_unref(OBJECT(&dev->vdev));
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_SERIAL);
 }
 
 static int s390_virtio_scsi_init(VirtIOS390Device *s390_dev)
@@ -258,10 +255,9 @@ static int s390_virtio_scsi_init(VirtIOS390Device *s390_dev)
 static void s390_virtio_scsi_instance_init(Object *obj)
 {
     VirtIOSCSIS390 *dev = VIRTIO_SCSI_S390(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SCSI);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    object_unref(OBJECT(&dev->vdev));
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_SCSI);
 }
 
 #ifdef CONFIG_VHOST_SCSI
@@ -281,10 +277,9 @@ static int s390_vhost_scsi_init(VirtIOS390Device *s390_dev)
 static void s390_vhost_scsi_instance_init(Object *obj)
 {
     VHostSCSIS390 *dev = VHOST_SCSI_S390(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VHOST_SCSI);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    object_unref(OBJECT(&dev->vdev));
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VHOST_SCSI);
 }
 #endif
 
@@ -309,10 +304,9 @@ static int s390_virtio_rng_init(VirtIOS390Device *s390_dev)
 static void s390_virtio_rng_instance_init(Object *obj)
 {
     VirtIORNGS390 *dev = VIRTIO_RNG_S390(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_RNG);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
-    object_unref(OBJECT(&dev->vdev));
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_RNG);
     object_property_add_link(obj, "rng", TYPE_RNG_BACKEND,
                              (Object **)&dev->vdev.conf.rng,
                              qdev_prop_allow_set_link_before_realize,
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index c074f64..5175d57 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -792,10 +792,9 @@ static int virtio_ccw_net_init(VirtioCcwDevice *ccw_dev)
 static void virtio_ccw_net_instance_init(Object *obj)
 {
     VirtIONetCcw *dev = VIRTIO_NET_CCW(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_NET);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    object_unref(OBJECT(&dev->vdev));
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_NET);
 }
 
 static int virtio_ccw_blk_init(VirtioCcwDevice *ccw_dev)
@@ -813,10 +812,9 @@ static int virtio_ccw_blk_init(VirtioCcwDevice *ccw_dev)
 static void virtio_ccw_blk_instance_init(Object *obj)
 {
     VirtIOBlkCcw *dev = VIRTIO_BLK_CCW(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BLK);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    object_unref(OBJECT(&dev->vdev));
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_BLK);
     object_property_add_alias(obj, "iothread", OBJECT(&dev->vdev),"iothread",
                               &error_abort);
 }
@@ -850,10 +848,9 @@ static int virtio_ccw_serial_init(VirtioCcwDevice *ccw_dev)
 static void virtio_ccw_serial_instance_init(Object *obj)
 {
     VirtioSerialCcw *dev = VIRTIO_SERIAL_CCW(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SERIAL);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
-    object_unref(OBJECT(&dev->vdev));
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_SERIAL);
 }
 
 static int virtio_ccw_balloon_init(VirtioCcwDevice *ccw_dev)
@@ -938,10 +935,9 @@ static int virtio_ccw_scsi_init(VirtioCcwDevice *ccw_dev)
 static void virtio_ccw_scsi_instance_init(Object *obj)
 {
     VirtIOSCSICcw *dev = VIRTIO_SCSI_CCW(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SCSI);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    object_unref(OBJECT(&dev->vdev));
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_SCSI);
 }
 
 #ifdef CONFIG_VHOST_SCSI
@@ -961,10 +957,9 @@ static int vhost_ccw_scsi_init(VirtioCcwDevice *ccw_dev)
 static void vhost_ccw_scsi_instance_init(Object *obj)
 {
     VHostSCSICcw *dev = VHOST_SCSI_CCW(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VHOST_SCSI);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    object_unref(OBJECT(&dev->vdev));
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VHOST_SCSI);
 }
 #endif
 
@@ -1540,10 +1535,9 @@ static const TypeInfo vhost_ccw_scsi = {
 static void virtio_ccw_rng_instance_init(Object *obj)
 {
     VirtIORNGCcw *dev = VIRTIO_RNG_CCW(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_RNG);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
-    object_unref(OBJECT(&dev->vdev));
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_RNG);
     object_property_add_link(obj, "rng", TYPE_RNG_BACKEND,
                              (Object **)&dev->vdev.conf.rng,
                              qdev_prop_allow_set_link_before_realize,
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 8f3b79b..83a699f 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -947,10 +947,9 @@ static void virtio_9p_pci_class_init(ObjectClass *klass, void *data)
 static void virtio_9p_pci_instance_init(Object *obj)
 {
     V9fsPCIState *dev = VIRTIO_9P_PCI(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_9P);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
-    object_unref(OBJECT(&dev->vdev));
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_9P);
 }
 
 static const TypeInfo virtio_9p_pci_info = {
@@ -1112,10 +1111,9 @@ static void virtio_blk_pci_class_init(ObjectClass *klass, void *data)
 static void virtio_blk_pci_instance_init(Object *obj)
 {
     VirtIOBlkPCI *dev = VIRTIO_BLK_PCI(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BLK);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    object_unref(OBJECT(&dev->vdev));
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_BLK);
     object_property_add_alias(obj, "iothread", OBJECT(&dev->vdev),"iothread",
                               &error_abort);
 }
@@ -1185,10 +1183,9 @@ static void virtio_scsi_pci_class_init(ObjectClass *klass, void *data)
 static void virtio_scsi_pci_instance_init(Object *obj)
 {
     VirtIOSCSIPCI *dev = VIRTIO_SCSI_PCI(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SCSI);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    object_unref(OBJECT(&dev->vdev));
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_SCSI);
 }
 
 static const TypeInfo virtio_scsi_pci_info = {
@@ -1242,10 +1239,9 @@ static void vhost_scsi_pci_class_init(ObjectClass *klass, void *data)
 static void vhost_scsi_pci_instance_init(Object *obj)
 {
     VHostSCSIPCI *dev = VHOST_SCSI_PCI(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VHOST_SCSI);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    object_unref(OBJECT(&dev->vdev));
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VHOST_SCSI);
 }
 
 static const TypeInfo vhost_scsi_pci_info = {
@@ -1408,10 +1404,9 @@ static void virtio_serial_pci_class_init(ObjectClass *klass, void *data)
 static void virtio_serial_pci_instance_init(Object *obj)
 {
     VirtIOSerialPCI *dev = VIRTIO_SERIAL_PCI(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SERIAL);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
-    object_unref(OBJECT(&dev->vdev));
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_SERIAL);
 }
 
 static const TypeInfo virtio_serial_pci_info = {
@@ -1467,10 +1462,9 @@ static void virtio_net_pci_class_init(ObjectClass *klass, void *data)
 static void virtio_net_pci_instance_init(Object *obj)
 {
     VirtIONetPCI *dev = VIRTIO_NET_PCI(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_NET);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    object_unref(OBJECT(&dev->vdev));
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_NET);
 }
 
 static const TypeInfo virtio_net_pci_info = {
@@ -1523,10 +1517,9 @@ static void virtio_rng_pci_class_init(ObjectClass *klass, void *data)
 static void virtio_rng_initfn(Object *obj)
 {
     VirtIORngPCI *dev = VIRTIO_RNG_PCI(obj);
-    object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_RNG);
-    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-    qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
-    object_unref(OBJECT(&dev->vdev));
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_RNG);
     object_property_add_link(obj, "rng", TYPE_RNG_BACKEND,
                              (Object **)&dev->vdev.conf.rng,
                              qdev_prop_allow_set_link_before_realize,
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 5c98180..2c236bf 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1123,6 +1123,17 @@ static void virtio_vmstate_change(void *opaque, int running, RunState state)
     }
 }
 
+void virtio_instance_init_common(Object *proxy_obj, void *data,
+                                 size_t vdev_size, const char *vdev_name)
+{
+    DeviceState *vdev = data;
+
+    object_initialize(vdev, vdev_size, vdev_name);
+    object_property_add_child(proxy_obj, "virtio-backend", OBJECT(vdev), NULL);
+    object_unref(OBJECT(vdev));
+    qdev_alias_all_properties(vdev, proxy_obj);
+}
+
 void virtio_init(VirtIODevice *vdev, const char *name,
                  uint16_t device_id, size_t config_size)
 {
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index a60104c..0726d76 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -161,6 +161,9 @@ typedef struct VirtioDeviceClass {
     int (*load)(VirtIODevice *vdev, QEMUFile *f, int version_id);
 } VirtioDeviceClass;
 
+void virtio_instance_init_common(Object *proxy_obj, void *data,
+                                 size_t vdev_size, const char *vdev_name);
+
 void virtio_init(VirtIODevice *vdev, const char *name,
                          uint16_t device_id, size_t config_size);
 void virtio_cleanup(VirtIODevice *vdev);
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports
  2014-09-30  6:10 [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports arei.gonglei
                   ` (11 preceding siblings ...)
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 12/12] virtio: add a wrapper for virtio-backend initialization arei.gonglei
@ 2014-09-30  8:02 ` Cornelia Huck
  2014-09-30  8:06   ` Gonglei (Arei)
  2014-09-30  8:41 ` Paolo Bonzini
  13 siblings, 1 reply; 34+ messages in thread
From: Cornelia Huck @ 2014-09-30  8:02 UTC (permalink / raw)
  To: arei.gonglei
  Cc: weidong.huang, mst, armbru, luonengjun, agraf, qemu-devel,
	borntraeger, stefanha, pbonzini, peter.huangpeng, rth

On Tue, 30 Sep 2014 14:10:26 +0800
<arei.gonglei@huawei.com> wrote:

> From: Gonglei <arei.gonglei@huawei.com>
> 
> virtio-$device-{pci, s390, ccw} all duplicate the
> qdev properties of their virtio child. This approach does
> not work well with string or pointer properties since we
> must be careful about leaking or double-freeing them.
> 
> Use the QOM alias property to forward property accesses to the
> VirtIORNG child.  This way no duplication is necessary.

VirtIO$devtype?

> 
> For their child, object_initialize() leaves the object with a refcount of 1.
> object_property_add_child() adds its own reference which is dropped
> again when the property is deleted.
> 
> The upshot of this is that we always have a refcount >= 1.  Upon hot
> unplug the virtio-$device child is not finalized!
> 
> Drop our reference after the child property has been added to the
> parent.
> 
> Changs since v1:
>  - add the same handling for virtio-9p-pci device in PATCH 10 and PATCH 11.
>  - add a wrapper function for better code sharing 
>    in PATCH 12 (Cornelia/Michael/Paolo)

This patchset, applied on current master, survives some light testing
for s390-virtio and virtio-ccw (booting Linux, device_add/device_del
for the various device types).

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports
  2014-09-30  8:02 ` [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports Cornelia Huck
@ 2014-09-30  8:06   ` Gonglei (Arei)
  0 siblings, 0 replies; 34+ messages in thread
From: Gonglei (Arei) @ 2014-09-30  8:06 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: Huangweidong (C), mst@redhat.com, armbru@redhat.com, Luonengjun,
	agraf@suse.de, qemu-devel@nongnu.org, borntraeger@de.ibm.com,
	stefanha@redhat.com, pbonzini@redhat.com, Huangpeng (Peter),
	rth@twiddle.net

> Subject: Re: [PATCH v2 00/12] virtio: fix virtio child recount in transports
> 
> On Tue, 30 Sep 2014 14:10:26 +0800
> <arei.gonglei@huawei.com> wrote:
> 
> > From: Gonglei <arei.gonglei@huawei.com>
> >
> > virtio-$device-{pci, s390, ccw} all duplicate the
> > qdev properties of their virtio child. This approach does
> > not work well with string or pointer properties since we
> > must be careful about leaking or double-freeing them.
> >
> > Use the QOM alias property to forward property accesses to the
> > VirtIORNG child.  This way no duplication is necessary.
> 
> VirtIO$devtype?
> 
Yes, sorry for my wrong coping.
 
> >
> > For their child, object_initialize() leaves the object with a refcount of 1.
> > object_property_add_child() adds its own reference which is dropped
> > again when the property is deleted.
> >
> > The upshot of this is that we always have a refcount >= 1.  Upon hot
> > unplug the virtio-$device child is not finalized!
> >
> > Drop our reference after the child property has been added to the
> > parent.
> >
> > Changs since v1:
> >  - add the same handling for virtio-9p-pci device in PATCH 10 and PATCH 11.
> >  - add a wrapper function for better code sharing
> >    in PATCH 12 (Cornelia/Michael/Paolo)
> 
> This patchset, applied on current master, survives some light testing
> for s390-virtio and virtio-ccw (booting Linux, device_add/device_del
> for the various device types).

Thanks a lot! Cornelia. Please let us know the results :)

Best regards,
-Gonglei

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 01/12] virtio-net: use aliases instead of duplicate qdev properties
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 01/12] virtio-net: use aliases instead of duplicate qdev properties arei.gonglei
@ 2014-09-30  8:18   ` Cornelia Huck
  0 siblings, 0 replies; 34+ messages in thread
From: Cornelia Huck @ 2014-09-30  8:18 UTC (permalink / raw)
  To: arei.gonglei
  Cc: weidong.huang, mst, armbru, luonengjun, agraf, qemu-devel,
	borntraeger, stefanha, pbonzini, peter.huangpeng, rth

On Tue, 30 Sep 2014 14:10:27 +0800
<arei.gonglei@huawei.com> wrote:

> From: Gonglei <arei.gonglei@huawei.com>
> 
> virtio-net-pci, virtio-net-s390, and virtio-net-ccw all duplicate the
> qdev properties of their VirtIONet child. This approach does not work
> well with string or pointer properties since we must be careful about
> leaking or double-freeing them.
> 
> Use the QOM alias property to forward property accesses to the
> VirtIONet child.  This way no duplication is necessary.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  hw/s390x/s390-virtio-bus.c | 3 +--
>  hw/s390x/virtio-ccw.c      | 3 +--
>  hw/virtio/virtio-pci.c     | 3 +--
>  3 files changed, 3 insertions(+), 6 deletions(-)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 02/12] virtio-net: fix virtio-net child refcount in transports
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 02/12] virtio-net: fix virtio-net child refcount in transports arei.gonglei
@ 2014-09-30  8:19   ` Cornelia Huck
  0 siblings, 0 replies; 34+ messages in thread
From: Cornelia Huck @ 2014-09-30  8:19 UTC (permalink / raw)
  To: arei.gonglei
  Cc: weidong.huang, mst, armbru, luonengjun, agraf, qemu-devel,
	borntraeger, stefanha, pbonzini, peter.huangpeng, rth

On Tue, 30 Sep 2014 14:10:28 +0800
<arei.gonglei@huawei.com> wrote:

> From: Gonglei <arei.gonglei@huawei.com>
> 
> object_initialize() leaves the object with a refcount of 1.
> object_property_add_child() adds its own reference which is dropped
> again when the property is deleted.
> 
> The upshot of this is that we always have a refcount >= 1.  Upon hot
> unplug the virtio-net child is not finalized!
> 
> Drop our reference after the child property has been added to the
> parent.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  hw/s390x/s390-virtio-bus.c | 1 +
>  hw/s390x/virtio-ccw.c      | 1 +
>  hw/virtio/virtio-pci.c     | 1 +
>  3 files changed, 3 insertions(+)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports
  2014-09-30  6:10 [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports arei.gonglei
                   ` (12 preceding siblings ...)
  2014-09-30  8:02 ` [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports Cornelia Huck
@ 2014-09-30  8:41 ` Paolo Bonzini
  2014-09-30 11:08   ` Michael S. Tsirkin
  13 siblings, 1 reply; 34+ messages in thread
From: Paolo Bonzini @ 2014-09-30  8:41 UTC (permalink / raw)
  To: arei.gonglei, qemu-devel
  Cc: agraf, weidong.huang, mst, luonengjun, peter.huangpeng, armbru,
	borntraeger, stefanha, cornelia.huck, rth

Il 30/09/2014 08:10, arei.gonglei@huawei.com ha scritto:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> virtio-$device-{pci, s390, ccw} all duplicate the
> qdev properties of their virtio child. This approach does
> not work well with string or pointer properties since we
> must be careful about leaking or double-freeing them.
> 
> Use the QOM alias property to forward property accesses to the
> VirtIORNG child.  This way no duplication is necessary.
> 
> For their child, object_initialize() leaves the object with a refcount of 1.
> object_property_add_child() adds its own reference which is dropped
> again when the property is deleted.
> 
> The upshot of this is that we always have a refcount >= 1.  Upon hot
> unplug the virtio-$device child is not finalized!
> 
> Drop our reference after the child property has been added to the
> parent.
> 
> Changs since v1:
>  - add the same handling for virtio-9p-pci device in PATCH 10 and PATCH 11.
>  - add a wrapper function for better code sharing 
>    in PATCH 12 (Cornelia/Michael/Paolo)

I would like to take these patches through the SCSI tree, because of a
small conflict with virtio-scsi dataplane.  Any objections?

Paolo

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 03/12] virtio/vhost-scsi: use aliases instead of duplicate qdev properties
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 03/12] virtio/vhost-scsi: use aliases instead of duplicate qdev properties arei.gonglei
@ 2014-09-30  8:43   ` Cornelia Huck
  2014-09-30 14:00     ` Cornelia Huck
  0 siblings, 1 reply; 34+ messages in thread
From: Cornelia Huck @ 2014-09-30  8:43 UTC (permalink / raw)
  To: arei.gonglei
  Cc: weidong.huang, mst, armbru, luonengjun, agraf, qemu-devel,
	borntraeger, stefanha, pbonzini, peter.huangpeng, rth

On Tue, 30 Sep 2014 14:10:29 +0800
<arei.gonglei@huawei.com> wrote:

> From: Gonglei <arei.gonglei@huawei.com>
> 
> {virtio, vhost}-scsi-{pci, s390, ccw} all duplicate the
> qdev properties of their VirtIOSCSI/VHostSCSI child.
> This approach does not work well with string or pointer
> properties since we must be careful about leaking or
> double-freeing them.
> 
> Use the QOM alias property to forward property accesses to the
> VirtIOSCSI/VHostSCSI child. This way no duplication is necessary.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  hw/s390x/s390-virtio-bus.c | 4 ++--
>  hw/s390x/virtio-ccw.c      | 4 ++--
>  hw/virtio/virtio-pci.c     | 4 ++--
>  3 files changed, 6 insertions(+), 6 deletions(-)

I just noticed that qemu-system-s390x -device vhost-scsi-{s390,ccw},?
segfaults on me - but it also does on master, so that is an orthogonal
problem I'll look into later. This patch looks fine to me and actually
also fixes a bug for ccw where vhost-scsi referenced VirtIOSCSICcw
instead of VHostSCSICcw, so:

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 04/12] virtio/vhost-scsi: fix virtio-scsi/vhost-scsi child refcount in transports
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 04/12] virtio/vhost-scsi: fix virtio-scsi/vhost-scsi child refcount in transports arei.gonglei
@ 2014-09-30  8:44   ` Cornelia Huck
  0 siblings, 0 replies; 34+ messages in thread
From: Cornelia Huck @ 2014-09-30  8:44 UTC (permalink / raw)
  To: arei.gonglei
  Cc: weidong.huang, mst, armbru, luonengjun, agraf, qemu-devel,
	borntraeger, stefanha, pbonzini, peter.huangpeng, rth

On Tue, 30 Sep 2014 14:10:30 +0800
<arei.gonglei@huawei.com> wrote:

> From: Gonglei <arei.gonglei@huawei.com>
> 
> object_initialize() leaves the object with a refcount of 1.
> object_property_add_child() adds its own reference which is dropped
> again when the property is deleted.
> 
> The upshot of this is that we always have a refcount >= 1.  Upon hot
> unplug the virtio-scsi/vhost-scsi child is not finalized!
> 
> Drop our reference after the child property has been added to the
> parent.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  hw/s390x/s390-virtio-bus.c | 2 ++
>  hw/s390x/virtio-ccw.c      | 2 ++
>  hw/virtio/virtio-pci.c     | 2 ++
>  3 files changed, 6 insertions(+)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 05/12] virtio-serial: use aliases instead of duplicate qdev properties
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 05/12] virtio-serial: use aliases instead of duplicate qdev properties arei.gonglei
@ 2014-09-30  8:46   ` Cornelia Huck
  0 siblings, 0 replies; 34+ messages in thread
From: Cornelia Huck @ 2014-09-30  8:46 UTC (permalink / raw)
  To: arei.gonglei
  Cc: weidong.huang, mst, armbru, luonengjun, agraf, qemu-devel,
	borntraeger, stefanha, pbonzini, peter.huangpeng, rth

On Tue, 30 Sep 2014 14:10:31 +0800
<arei.gonglei@huawei.com> wrote:

> From: Gonglei <arei.gonglei@huawei.com>
> 
> virtio-serial-{pci, s390, ccw} all duplicate the
> qdev properties of their VirtIOSerial child.
> This approach does not work well with string or pointer
> properties since we must be careful about leaking or
> double-freeing them.
> 
> Use the QOM alias property to forward property accesses to the
> VirtIOSerial child.  This way no duplication is necessary.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  hw/s390x/s390-virtio-bus.c | 2 +-
>  hw/s390x/virtio-ccw.c      | 2 +-
>  hw/virtio/virtio-pci.c     | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 06/12] virtio-serial: fix virtio-serial child refcount in transports
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 06/12] virtio-serial: fix virtio-serial child refcount in transports arei.gonglei
@ 2014-09-30  8:47   ` Cornelia Huck
  0 siblings, 0 replies; 34+ messages in thread
From: Cornelia Huck @ 2014-09-30  8:47 UTC (permalink / raw)
  To: arei.gonglei
  Cc: weidong.huang, mst, armbru, luonengjun, agraf, qemu-devel,
	borntraeger, stefanha, pbonzini, peter.huangpeng, rth

On Tue, 30 Sep 2014 14:10:32 +0800
<arei.gonglei@huawei.com> wrote:

> From: Gonglei <arei.gonglei@huawei.com>
> 
> object_initialize() leaves the object with a refcount of 1.
> object_property_add_child() adds its own reference which is dropped
> again when the property is deleted.
> 
> The upshot of this is that we always have a refcount >= 1.  Upon hot
> unplug the virtio-serial child is not finalized!
> 
> Drop our reference after the child property has been added to the
> parent.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  hw/s390x/s390-virtio-bus.c | 1 +
>  hw/s390x/virtio-ccw.c      | 1 +
>  hw/virtio/virtio-pci.c     | 1 +
>  3 files changed, 3 insertions(+)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 07/12] virtio-rng: use aliases instead of duplicate qdev properties
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 07/12] virtio-rng: use aliases instead of duplicate qdev properties arei.gonglei
@ 2014-09-30  8:50   ` Cornelia Huck
  0 siblings, 0 replies; 34+ messages in thread
From: Cornelia Huck @ 2014-09-30  8:50 UTC (permalink / raw)
  To: arei.gonglei
  Cc: weidong.huang, mst, armbru, luonengjun, agraf, qemu-devel,
	borntraeger, stefanha, pbonzini, peter.huangpeng, rth

On Tue, 30 Sep 2014 14:10:33 +0800
<arei.gonglei@huawei.com> wrote:

> From: Gonglei <arei.gonglei@huawei.com>
> 
> virtio-rng-{pci, s390, ccw} all duplicate the
> qdev properties of their VirtIORNG child.
> This approach does not work well with string or pointer
> properties since we must be careful about leaking or
> double-freeing them.
> 
> Use the QOM alias property to forward property accesses to the
> VirtIORNG child.  This way no duplication is necessary.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  hw/s390x/s390-virtio-bus.c | 2 +-
>  hw/s390x/virtio-ccw.c      | 2 +-
>  hw/virtio/virtio-pci.c     | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 08/12] virtio-rng: fix virtio-rng child refcount in transports
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 08/12] virtio-rng: fix virtio-rng child refcount in transports arei.gonglei
@ 2014-09-30  8:51   ` Cornelia Huck
  0 siblings, 0 replies; 34+ messages in thread
From: Cornelia Huck @ 2014-09-30  8:51 UTC (permalink / raw)
  To: arei.gonglei
  Cc: weidong.huang, mst, armbru, luonengjun, agraf, qemu-devel,
	borntraeger, stefanha, pbonzini, peter.huangpeng, rth

On Tue, 30 Sep 2014 14:10:34 +0800
<arei.gonglei@huawei.com> wrote:

> From: Gonglei <arei.gonglei@huawei.com>
> 
> object_initialize() leaves the object with a refcount of 1.
> object_property_add_child() adds its own reference which is dropped
> again when the property is deleted.
> 
> The upshot of this is that we always have a refcount >= 1.  Upon hot
> unplug the virtio-rng child is not finalized!
> 
> Drop our reference after the child property has been added to the
> parent.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  hw/s390x/s390-virtio-bus.c | 1 +
>  hw/s390x/virtio-ccw.c      | 1 +
>  hw/virtio/virtio-pci.c     | 1 +
>  3 files changed, 3 insertions(+)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 09/12] virtio-balloon: fix virtio-balloon child refcount in transports
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 09/12] virtio-balloon: fix virtio-balloon " arei.gonglei
@ 2014-09-30  8:52   ` Cornelia Huck
  0 siblings, 0 replies; 34+ messages in thread
From: Cornelia Huck @ 2014-09-30  8:52 UTC (permalink / raw)
  To: arei.gonglei
  Cc: weidong.huang, mst, armbru, luonengjun, agraf, qemu-devel,
	borntraeger, stefanha, pbonzini, peter.huangpeng, rth

On Tue, 30 Sep 2014 14:10:35 +0800
<arei.gonglei@huawei.com> wrote:

> From: Gonglei <arei.gonglei@huawei.com>
> 
> object_initialize() leaves the object with a refcount of 1.
> object_property_add_child() adds its own reference which is dropped
> again when the property is deleted.
> 
> The upshot of this is that we always have a refcount >= 1.  Upon hot
> unplug the virtio-balloon child is not finalized!
> 
> Drop our reference after the child property has been added to the
> parent.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  hw/s390x/virtio-ccw.c  | 2 +-
>  hw/virtio/virtio-pci.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 12/12] virtio: add a wrapper for virtio-backend initialization
  2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 12/12] virtio: add a wrapper for virtio-backend initialization arei.gonglei
@ 2014-09-30  9:04   ` Cornelia Huck
  2014-09-30  9:10     ` Paolo Bonzini
                       ` (2 more replies)
  0 siblings, 3 replies; 34+ messages in thread
From: Cornelia Huck @ 2014-09-30  9:04 UTC (permalink / raw)
  To: arei.gonglei
  Cc: weidong.huang, mst, armbru, luonengjun, agraf, qemu-devel,
	borntraeger, stefanha, pbonzini, peter.huangpeng, rth

On Tue, 30 Sep 2014 14:10:38 +0800
<arei.gonglei@huawei.com> wrote:

> From: Gonglei <arei.gonglei@huawei.com>
> 
> For better code sharing, add a wrapper help funciton

typo: s/funciton/function/

> for various virtio devices.

s/for various virtio devices/that handles referencing the virtio
backend for virtio proxy devices/ ?

> 
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  hw/s390x/s390-virtio-bus.c | 42 +++++++++++++++++----------------------
>  hw/s390x/virtio-ccw.c      | 42 +++++++++++++++++----------------------
>  hw/virtio/virtio-pci.c     | 49 ++++++++++++++++++++--------------------------
>  hw/virtio/virtio.c         | 11 +++++++++++
>  include/hw/virtio/virtio.h |  3 +++
>  5 files changed, 71 insertions(+), 76 deletions(-)

I'm wondering whether we should call the wrapper for virtio-balloon as
well. Even if it does not have any properties to alias, calling
qdev_alias_all_properties() probably doesn't hurt, does it?

Looks good to me otherwise.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 12/12] virtio: add a wrapper for virtio-backend initialization
  2014-09-30  9:04   ` Cornelia Huck
@ 2014-09-30  9:10     ` Paolo Bonzini
  2014-09-30  9:45       ` Gonglei (Arei)
  2014-09-30  9:17     ` Paolo Bonzini
  2014-09-30 10:51     ` Markus Armbruster
  2 siblings, 1 reply; 34+ messages in thread
From: Paolo Bonzini @ 2014-09-30  9:10 UTC (permalink / raw)
  To: Cornelia Huck, arei.gonglei
  Cc: weidong.huang, mst, armbru, luonengjun, qemu-devel, agraf,
	borntraeger, stefanha, peter.huangpeng, rth

Il 30/09/2014 11:04, Cornelia Huck ha scritto:
>> > For better code sharing, add a wrapper help funciton
> typo: s/funciton/function/
> 
>> > for various virtio devices.
> s/for various virtio devices/that handles referencing the virtio
> backend for virtio proxy devices/ ?
> 

"For better code sharing, add a helper function that handles
reference counting of the virtio backend for virtio proxy devices."

?

Paolo

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 12/12] virtio: add a wrapper for virtio-backend initialization
  2014-09-30  9:04   ` Cornelia Huck
  2014-09-30  9:10     ` Paolo Bonzini
@ 2014-09-30  9:17     ` Paolo Bonzini
  2014-09-30  9:35       ` Gonglei (Arei)
  2014-09-30 10:58       ` Cornelia Huck
  2014-09-30 10:51     ` Markus Armbruster
  2 siblings, 2 replies; 34+ messages in thread
From: Paolo Bonzini @ 2014-09-30  9:17 UTC (permalink / raw)
  To: Cornelia Huck, arei.gonglei
  Cc: weidong.huang, mst, armbru, luonengjun, qemu-devel, agraf,
	borntraeger, stefanha, peter.huangpeng, rth

Il 30/09/2014 11:04, Cornelia Huck ha scritto:
> On Tue, 30 Sep 2014 14:10:38 +0800
> <arei.gonglei@huawei.com> wrote:
> 
>> From: Gonglei <arei.gonglei@huawei.com>
>>
>> For better code sharing, add a wrapper help funciton
> 
> typo: s/funciton/function/
> 
>> for various virtio devices.
> 
> s/for various virtio devices/that handles referencing the virtio
> backend for virtio proxy devices/ ?
> 
>>
>> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> ---
>>  hw/s390x/s390-virtio-bus.c | 42 +++++++++++++++++----------------------
>>  hw/s390x/virtio-ccw.c      | 42 +++++++++++++++++----------------------
>>  hw/virtio/virtio-pci.c     | 49 ++++++++++++++++++++--------------------------
>>  hw/virtio/virtio.c         | 11 +++++++++++
>>  include/hw/virtio/virtio.h |  3 +++
>>  5 files changed, 71 insertions(+), 76 deletions(-)
> 
> I'm wondering whether we should call the wrapper for virtio-balloon as
> well. Even if it does not have any properties to alias, calling
> qdev_alias_all_properties() probably doesn't hurt, does it?

Actually it has properties to alias: guest-stats and
guest-stats-polling-interval are effectively aliases,
so we could remove 30-odd lines of code from virtio-pci
and virtio-ccw.  It's okay IMO to put it in a separate patch,
since it is a bit different from the trivial replacement
done here.

Paolo

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 12/12] virtio: add a wrapper for virtio-backend initialization
  2014-09-30  9:17     ` Paolo Bonzini
@ 2014-09-30  9:35       ` Gonglei (Arei)
  2014-09-30 10:58       ` Cornelia Huck
  1 sibling, 0 replies; 34+ messages in thread
From: Gonglei (Arei) @ 2014-09-30  9:35 UTC (permalink / raw)
  To: Paolo Bonzini, Cornelia Huck
  Cc: Huangweidong (C), mst@redhat.com, armbru@redhat.com, Luonengjun,
	qemu-devel@nongnu.org, agraf@suse.de, borntraeger@de.ibm.com,
	stefanha@redhat.com, Huangpeng (Peter), rth@twiddle.net

> Subject: Re: [PATCH v2 12/12] virtio: add a wrapper for virtio-backend
> initialization
> 
> Il 30/09/2014 11:04, Cornelia Huck ha scritto:
> > On Tue, 30 Sep 2014 14:10:38 +0800
> > <arei.gonglei@huawei.com> wrote:
> >
> >> From: Gonglei <arei.gonglei@huawei.com>
> >>
> >> For better code sharing, add a wrapper help funciton
> >
> > typo: s/funciton/function/
> >
> >> for various virtio devices.
> >
> > s/for various virtio devices/that handles referencing the virtio
> > backend for virtio proxy devices/ ?
> >
> >>
> >> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> >> Cc: Michael S. Tsirkin <mst@redhat.com>
> >> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> >> ---
> >>  hw/s390x/s390-virtio-bus.c | 42 +++++++++++++++++----------------------
> >>  hw/s390x/virtio-ccw.c      | 42 +++++++++++++++++----------------------
> >>  hw/virtio/virtio-pci.c     | 49 ++++++++++++++++++++--------------------------
> >>  hw/virtio/virtio.c         | 11 +++++++++++
> >>  include/hw/virtio/virtio.h |  3 +++
> >>  5 files changed, 71 insertions(+), 76 deletions(-)
> >
> > I'm wondering whether we should call the wrapper for virtio-balloon as
> > well. Even if it does not have any properties to alias, calling
> > qdev_alias_all_properties() probably doesn't hurt, does it?
> 
> Actually it has properties to alias: guest-stats and
> guest-stats-polling-interval are effectively aliases,
> so we could remove 30-odd lines of code from virtio-pci
> and virtio-ccw.  It's okay IMO to put it in a separate patch,
> since it is a bit different from the trivial replacement
> done here.
> 
> Paolo

Agreed. I can post a separate patch or patchset for this. :)

Best regards,
-Gonglei

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 12/12] virtio: add a wrapper for virtio-backend initialization
  2014-09-30  9:10     ` Paolo Bonzini
@ 2014-09-30  9:45       ` Gonglei (Arei)
  0 siblings, 0 replies; 34+ messages in thread
From: Gonglei (Arei) @ 2014-09-30  9:45 UTC (permalink / raw)
  To: Paolo Bonzini, Cornelia Huck
  Cc: Huangweidong (C), mst@redhat.com, armbru@redhat.com, Luonengjun,
	qemu-devel@nongnu.org, agraf@suse.de, borntraeger@de.ibm.com,
	stefanha@redhat.com, Huangpeng (Peter), rth@twiddle.net

> Subject: Re: [PATCH v2 12/12] virtio: add a wrapper for virtio-backend
> initialization
> 
> Il 30/09/2014 11:04, Cornelia Huck ha scritto:
> >> > For better code sharing, add a wrapper help funciton
> > typo: s/funciton/function/
> >
> >> > for various virtio devices.
> > s/for various virtio devices/that handles referencing the virtio
> > backend for virtio proxy devices/ ?
> >
> 
> "For better code sharing, add a helper function that handles
> reference counting of the virtio backend for virtio proxy devices."
> 

Looks good to me. Thanks you both. :)

> ?
> 
> Paolo

Best regards,
-Gonglei

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 12/12] virtio: add a wrapper for virtio-backend initialization
  2014-09-30  9:04   ` Cornelia Huck
  2014-09-30  9:10     ` Paolo Bonzini
  2014-09-30  9:17     ` Paolo Bonzini
@ 2014-09-30 10:51     ` Markus Armbruster
  2 siblings, 0 replies; 34+ messages in thread
From: Markus Armbruster @ 2014-09-30 10:51 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: weidong.huang, mst, luonengjun, qemu-devel, agraf, borntraeger,
	arei.gonglei, stefanha, pbonzini, peter.huangpeng, rth

Cornelia Huck <cornelia.huck@de.ibm.com> writes:

> On Tue, 30 Sep 2014 14:10:38 +0800
> <arei.gonglei@huawei.com> wrote:
>
>> From: Gonglei <arei.gonglei@huawei.com>
>> 
>> For better code sharing, add a wrapper help funciton
>
> typo: s/funciton/function/
>
>> for various virtio devices.
>
> s/for various virtio devices/that handles referencing the virtio
> backend for virtio proxy devices/ ?
>
>> 
>> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> ---
>>  hw/s390x/s390-virtio-bus.c | 42 +++++++++++++++++----------------------
>>  hw/s390x/virtio-ccw.c      | 42 +++++++++++++++++----------------------
>>  hw/virtio/virtio-pci.c | 49
>> ++++++++++++++++++++--------------------------
>>  hw/virtio/virtio.c         | 11 +++++++++++
>>  include/hw/virtio/virtio.h |  3 +++
>>  5 files changed, 71 insertions(+), 76 deletions(-)
>
> I'm wondering whether we should call the wrapper for virtio-balloon as
> well. Even if it does not have any properties to alias, calling
> qdev_alias_all_properties() probably doesn't hurt, does it?
>
> Looks good to me otherwise.

I didn't follow this, pardon me if I babble nonsense: could the hackery
that creates usb-storage's implied scsi-disk profit from this mechanism?
Start reading at usb_msd_realize_storage().

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 12/12] virtio: add a wrapper for virtio-backend initialization
  2014-09-30  9:17     ` Paolo Bonzini
  2014-09-30  9:35       ` Gonglei (Arei)
@ 2014-09-30 10:58       ` Cornelia Huck
  1 sibling, 0 replies; 34+ messages in thread
From: Cornelia Huck @ 2014-09-30 10:58 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: weidong.huang, mst, qemu-devel, luonengjun, agraf, armbru,
	borntraeger, arei.gonglei, stefanha, peter.huangpeng, rth

On Tue, 30 Sep 2014 11:17:29 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 30/09/2014 11:04, Cornelia Huck ha scritto:
> > On Tue, 30 Sep 2014 14:10:38 +0800
> > <arei.gonglei@huawei.com> wrote:
> > 
> >> From: Gonglei <arei.gonglei@huawei.com>
> >>
> >> For better code sharing, add a wrapper help funciton
> > 
> > typo: s/funciton/function/
> > 
> >> for various virtio devices.
> > 
> > s/for various virtio devices/that handles referencing the virtio
> > backend for virtio proxy devices/ ?
> > 
> >>
> >> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> >> Cc: Michael S. Tsirkin <mst@redhat.com>
> >> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> >> ---
> >>  hw/s390x/s390-virtio-bus.c | 42 +++++++++++++++++----------------------
> >>  hw/s390x/virtio-ccw.c      | 42 +++++++++++++++++----------------------
> >>  hw/virtio/virtio-pci.c     | 49 ++++++++++++++++++++--------------------------
> >>  hw/virtio/virtio.c         | 11 +++++++++++
> >>  include/hw/virtio/virtio.h |  3 +++
> >>  5 files changed, 71 insertions(+), 76 deletions(-)
> > 
> > I'm wondering whether we should call the wrapper for virtio-balloon as
> > well. Even if it does not have any properties to alias, calling
> > qdev_alias_all_properties() probably doesn't hurt, does it?
> 
> Actually it has properties to alias: guest-stats and
> guest-stats-polling-interval are effectively aliases,
> so we could remove 30-odd lines of code from virtio-pci
> and virtio-ccw.  It's okay IMO to put it in a separate patch,
> since it is a bit different from the trivial replacement
> done here.

Yup. Let's do the easy changes first.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports
  2014-09-30  8:41 ` Paolo Bonzini
@ 2014-09-30 11:08   ` Michael S. Tsirkin
  0 siblings, 0 replies; 34+ messages in thread
From: Michael S. Tsirkin @ 2014-09-30 11:08 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: agraf, weidong.huang, armbru, luonengjun, peter.huangpeng,
	qemu-devel, borntraeger, arei.gonglei, stefanha, cornelia.huck,
	rth

On Tue, Sep 30, 2014 at 10:41:11AM +0200, Paolo Bonzini wrote:
> Il 30/09/2014 08:10, arei.gonglei@huawei.com ha scritto:
> > From: Gonglei <arei.gonglei@huawei.com>
> > 
> > virtio-$device-{pci, s390, ccw} all duplicate the
> > qdev properties of their virtio child. This approach does
> > not work well with string or pointer properties since we
> > must be careful about leaking or double-freeing them.
> > 
> > Use the QOM alias property to forward property accesses to the
> > VirtIORNG child.  This way no duplication is necessary.
> > 
> > For their child, object_initialize() leaves the object with a refcount of 1.
> > object_property_add_child() adds its own reference which is dropped
> > again when the property is deleted.
> > 
> > The upshot of this is that we always have a refcount >= 1.  Upon hot
> > unplug the virtio-$device child is not finalized!
> > 
> > Drop our reference after the child property has been added to the
> > parent.
> > 
> > Changs since v1:
> >  - add the same handling for virtio-9p-pci device in PATCH 10 and PATCH 11.
> >  - add a wrapper function for better code sharing 
> >    in PATCH 12 (Cornelia/Michael/Paolo)
> 
> I would like to take these patches through the SCSI tree, because of a
> small conflict with virtio-scsi dataplane.  Any objections?
> 
> Paolo

Fine with me.

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCH v2 03/12] virtio/vhost-scsi: use aliases instead of duplicate qdev properties
  2014-09-30  8:43   ` Cornelia Huck
@ 2014-09-30 14:00     ` Cornelia Huck
  0 siblings, 0 replies; 34+ messages in thread
From: Cornelia Huck @ 2014-09-30 14:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: weidong.huang, mst, armbru, luonengjun, agraf, peter.huangpeng,
	borntraeger, arei.gonglei, stefanha, pbonzini, rth

On Tue, 30 Sep 2014 10:43:31 +0200
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:

> On Tue, 30 Sep 2014 14:10:29 +0800
> <arei.gonglei@huawei.com> wrote:
> 
> > From: Gonglei <arei.gonglei@huawei.com>
> > 
> > {virtio, vhost}-scsi-{pci, s390, ccw} all duplicate the
> > qdev properties of their VirtIOSCSI/VHostSCSI child.
> > This approach does not work well with string or pointer
> > properties since we must be careful about leaking or
> > double-freeing them.
> > 
> > Use the QOM alias property to forward property accesses to the
> > VirtIOSCSI/VHostSCSI child. This way no duplication is necessary.
> > 
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > ---
> >  hw/s390x/s390-virtio-bus.c | 4 ++--
> >  hw/s390x/virtio-ccw.c      | 4 ++--
> >  hw/virtio/virtio-pci.c     | 4 ++--
> >  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> I just noticed that qemu-system-s390x -device vhost-scsi-{s390,ccw},?
> segfaults on me - but it also does on master, so that is an orthogonal
> problem I'll look into later. This patch looks fine to me and actually
> also fixes a bug for ccw where vhost-scsi referenced VirtIOSCSICcw
> instead of VHostSCSICcw, so:
> 
> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

...and it turns out that s390-virtio is actually fine, while the
problem in virtio-ccw is trivially fixed by
s/VirtIOSCSICcw/VHostSCSICcw/ in the .instance_size initializer. I'll
queue that up with the next round of s390 patches.

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2014-09-30 14:00 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-30  6:10 [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports arei.gonglei
2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 01/12] virtio-net: use aliases instead of duplicate qdev properties arei.gonglei
2014-09-30  8:18   ` Cornelia Huck
2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 02/12] virtio-net: fix virtio-net child refcount in transports arei.gonglei
2014-09-30  8:19   ` Cornelia Huck
2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 03/12] virtio/vhost-scsi: use aliases instead of duplicate qdev properties arei.gonglei
2014-09-30  8:43   ` Cornelia Huck
2014-09-30 14:00     ` Cornelia Huck
2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 04/12] virtio/vhost-scsi: fix virtio-scsi/vhost-scsi child refcount in transports arei.gonglei
2014-09-30  8:44   ` Cornelia Huck
2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 05/12] virtio-serial: use aliases instead of duplicate qdev properties arei.gonglei
2014-09-30  8:46   ` Cornelia Huck
2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 06/12] virtio-serial: fix virtio-serial child refcount in transports arei.gonglei
2014-09-30  8:47   ` Cornelia Huck
2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 07/12] virtio-rng: use aliases instead of duplicate qdev properties arei.gonglei
2014-09-30  8:50   ` Cornelia Huck
2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 08/12] virtio-rng: fix virtio-rng child refcount in transports arei.gonglei
2014-09-30  8:51   ` Cornelia Huck
2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 09/12] virtio-balloon: fix virtio-balloon " arei.gonglei
2014-09-30  8:52   ` Cornelia Huck
2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 10/12] virtio-9p: use aliases instead of duplicate qdev properties arei.gonglei
2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 11/12] virtio-9p: fix virtio-9p child refcount in transports arei.gonglei
2014-09-30  6:10 ` [Qemu-devel] [PATCH v2 12/12] virtio: add a wrapper for virtio-backend initialization arei.gonglei
2014-09-30  9:04   ` Cornelia Huck
2014-09-30  9:10     ` Paolo Bonzini
2014-09-30  9:45       ` Gonglei (Arei)
2014-09-30  9:17     ` Paolo Bonzini
2014-09-30  9:35       ` Gonglei (Arei)
2014-09-30 10:58       ` Cornelia Huck
2014-09-30 10:51     ` Markus Armbruster
2014-09-30  8:02 ` [Qemu-devel] [PATCH v2 00/12] virtio: fix virtio child recount in transports Cornelia Huck
2014-09-30  8:06   ` Gonglei (Arei)
2014-09-30  8:41 ` Paolo Bonzini
2014-09-30 11:08   ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).