[Qemu-devel] [PATCH RESEND 0/9] virtio: fix virtio child recount in transports

[Qemu-devel] [PATCH RESEND 0/9] virtio: fix virtio child recount in transports

[arei.gonglei]

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.

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 v2 0/7] add description field in ObjectProperty and PropertyInfo struct
 http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg04908.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!

Please review, Thanks a lot!
-Gonglei

Gonglei (9):
  virtio-net: use aliases instead of duplicate qdev properties
  virtio: 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

 hw/s390x/s390-virtio-bus.c | 16 ++++++++++------
 hw/s390x/virtio-ccw.c      | 18 +++++++++++-------
 hw/virtio/virtio-pci.c     | 18 +++++++++++-------
 3 files changed, 32 insertions(+), 20 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [PATCH RESEND 1/9] virtio-net: use aliases instead of duplicate qdev properties

[arei.gonglei]

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

[Qemu-devel] [PATCH RESEND 2/9] virtio: fix virtio-net child refcount in transports

[arei.gonglei]

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

[Qemu-devel] [PATCH RESEND 3/9] virtio/vhost scsi: use aliases instead of duplicate qdev properties

[arei.gonglei]

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

[Qemu-devel] [PATCH RESEND 4/9] virtio/vhost-scsi: fix virtio-scsi/vhost-scsi child refcount in transports

[arei.gonglei]

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

[Qemu-devel] [PATCH RESEND 5/9] virtio-serial: use aliases instead of duplicate qdev properties

[arei.gonglei]

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

[Qemu-devel] [PATCH RESEND 6/9] virtio-serial: fix virtio-serial child refcount in transports

[arei.gonglei]

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

[Qemu-devel] [PATCH RESEND 7/9] virtio-rng: use aliases instead of duplicate qdev properties

[arei.gonglei]

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

[Qemu-devel] [PATCH RESEND 8/9] virtio-rng: fix virtio-rng child refcount in transports

[arei.gonglei]

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

[Qemu-devel] [PATCH RESEND 9/9] virtio-balloon: fix virtio-balloon child refcount in transports

[arei.gonglei]

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

Re: [Qemu-devel] [PATCH RESEND 0/9] virtio: fix virtio child recount in transports

[Cornelia Huck]

On Fri, 26 Sep 2014 16:45:38 +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.
> 
> 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.
> 
> 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 v2 0/7] add description field in ObjectProperty and PropertyInfo struct
>  http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg04908.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!
> 
> Please review, Thanks a lot!

Do you have a git branch for quick testing?

> -Gonglei
> 
> Gonglei (9):
>   virtio-net: use aliases instead of duplicate qdev properties
>   virtio: 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
> 
>  hw/s390x/s390-virtio-bus.c | 16 ++++++++++------
>  hw/s390x/virtio-ccw.c      | 18 +++++++++++-------
>  hw/virtio/virtio-pci.c     | 18 +++++++++++-------
>  3 files changed, 32 insertions(+), 20 deletions(-)

One thing I noticed is that the various devices end up with similar
code in the end:

     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_WHATEVER);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
     object_unref(OBJECT(&dev->vdev));
     qdev_alias_all_properties(DEVICE(&dev->vdev), obj);

Would it make sense to add a helper function for that?

Re: [Qemu-devel] [PATCH RESEND 0/9] virtio: fix virtio child recount in transports

[Gonglei (Arei)]

Hi,

> >
> > 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.
> >
> > 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 v2 0/7] add description field in ObjectProperty and PropertyInfo
> struct
> >  http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg04908.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!
> >
> > Please review, Thanks a lot!
> 
> Do you have a git branch for quick testing?
> 
Sorry, I havn't. :(  

I worked this in the environment of my company,
then send patches.

Would you apply those patches based qemu master by 'git am' ? 
Thanks!

> > -Gonglei
> >
> > Gonglei (9):
> >   virtio-net: use aliases instead of duplicate qdev properties
> >   virtio: 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
> >
> >  hw/s390x/s390-virtio-bus.c | 16 ++++++++++------
> >  hw/s390x/virtio-ccw.c      | 18 +++++++++++-------
> >  hw/virtio/virtio-pci.c     | 18 +++++++++++-------
> >  3 files changed, 32 insertions(+), 20 deletions(-)
> 
> One thing I noticed is that the various devices end up with similar
> code in the end:
> 
>      object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_WHATEVER);
>      object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev),
> NULL);
>      object_unref(OBJECT(&dev->vdev));
>      qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
> 
> Would it make sense to add a helper function for that?

Yes. I think it make sense.
Maybe I can add a helper function in virtio.c, named
virtio_backend_init(). Agree? Thanks!

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH RESEND 0/9] virtio: fix virtio child recount in transports

[Gonglei (Arei)]

> > >   virtio-net: use aliases instead of duplicate qdev properties
> > >   virtio: 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
> > >
> > >  hw/s390x/s390-virtio-bus.c | 16 ++++++++++------
> > >  hw/s390x/virtio-ccw.c      | 18 +++++++++++-------
> > >  hw/virtio/virtio-pci.c     | 18 +++++++++++-------
> > >  3 files changed, 32 insertions(+), 20 deletions(-)
> >
> > One thing I noticed is that the various devices end up with similar
> > code in the end:
> >
> >      object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_WHATEVER);
> >      object_property_add_child(obj, "virtio-backend",
> OBJECT(&dev->vdev),
> > NULL);
> >      object_unref(OBJECT(&dev->vdev));
> >      qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
> >
> > Would it make sense to add a helper function for that?
> 

Sorry, I'm afraid this is not helpful. Because dev and dev->vdev is different
for different virtio devices, like VirtIOBlkPCI(and its vdev is VirtIOBlock), 
VirtIONetPCI(and its vdev is VirtIONet). They have no the same parameters
for above code segment. :) 

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH RESEND 0/9] virtio: fix virtio child recount in transports

[Cornelia Huck]

On Sat, 27 Sep 2014 10:37:23 +0000
"Gonglei (Arei)" <arei.gonglei@huawei.com> wrote:

> > > One thing I noticed is that the various devices end up with similar
> > > code in the end:
> > >
> > >      object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_WHATEVER);
> > >      object_property_add_child(obj, "virtio-backend",
> > OBJECT(&dev->vdev),
> > > NULL);
> > >      object_unref(OBJECT(&dev->vdev));
> > >      qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
> > >
> > > Would it make sense to add a helper function for that?
> > 
> 
> Sorry, I'm afraid this is not helpful. Because dev and dev->vdev is different
> for different virtio devices, like VirtIOBlkPCI(and its vdev is VirtIOBlock), 
> VirtIONetPCI(and its vdev is VirtIONet). They have no the same parameters
> for above code segment. :) 

Hm...

void virtio_instance_init_common(Object *proxydev,
                                 DeviceState *vdev,
                                 size_t vdevsize,
                                 const char *vdevname)
{
    object_initialize(vdev, vdevsize, vdevname);
    object_property_add_child(proxydev, "virtio-backend", OBJECT(vdev), NULL);
    object_unref(OBJECT(vdev));
    qdev_alias_all_properties(vdev, proxydev);
}

and have the initializers call

virtio_instance_init_common(obj, DEVICE(&dev->vdev), sizeof(dev->vdev), TYPE_WHATEVER);

?

Re: [Qemu-devel] [PATCH RESEND 0/9] virtio: fix virtio child recount in transports

[Gonglei (Arei)]

> From: Cornelia Huck [mailto:cornelia.huck@de.ibm.com]
> Sent: Monday, September 29, 2014 6:53 PM
> Subject: Re: [PATCH RESEND 0/9] virtio: fix virtio child recount in transports
> 
> On Sat, 27 Sep 2014 10:37:23 +0000
> "Gonglei (Arei)" <arei.gonglei@huawei.com> wrote:
> 
> > > > One thing I noticed is that the various devices end up with similar
> > > > code in the end:
> > > >
> > > >      object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_WHATEVER);
> > > >      object_property_add_child(obj, "virtio-backend",
> > > OBJECT(&dev->vdev),
> > > > NULL);
> > > >      object_unref(OBJECT(&dev->vdev));
> > > >      qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
> > > >
> > > > Would it make sense to add a helper function for that?
> > >
> >
> > Sorry, I'm afraid this is not helpful. Because dev and dev->vdev is different
> > for different virtio devices, like VirtIOBlkPCI(and its vdev is VirtIOBlock),
> > VirtIONetPCI(and its vdev is VirtIONet). They have no the same parameters
> > for above code segment. :)
> 
> Hm...
> 
> void virtio_instance_init_common(Object *proxydev,
>                                  DeviceState *vdev,
>                                  size_t vdevsize,
>                                  const char *vdevname)
> {
>     object_initialize(vdev, vdevsize, vdevname);
>     object_property_add_child(proxydev, "virtio-backend", OBJECT(vdev),
> NULL);
>     object_unref(OBJECT(vdev));
>     qdev_alias_all_properties(vdev, proxydev);
> }
> 
> and have the initializers call
> 
> virtio_instance_init_common(obj, DEVICE(&dev->vdev), sizeof(dev->vdev),
> TYPE_WHATEVER);
> 
> ?

OK, it looks good that pass all parameters to one wrapper function. 
Will do this in next version.

Thanks, Cornelia. :)

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH RESEND 0/9] virtio: fix virtio child recount in transports

[Michael S. Tsirkin]

On Mon, Sep 29, 2014 at 12:09:06PM +0000, Gonglei (Arei) wrote:
> > From: Cornelia Huck [mailto:cornelia.huck@de.ibm.com]
> > Sent: Monday, September 29, 2014 6:53 PM
> > Subject: Re: [PATCH RESEND 0/9] virtio: fix virtio child recount in transports
> > 
> > On Sat, 27 Sep 2014 10:37:23 +0000
> > "Gonglei (Arei)" <arei.gonglei@huawei.com> wrote:
> > 
> > > > > One thing I noticed is that the various devices end up with similar
> > > > > code in the end:
> > > > >
> > > > >      object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_WHATEVER);
> > > > >      object_property_add_child(obj, "virtio-backend",
> > > > OBJECT(&dev->vdev),
> > > > > NULL);
> > > > >      object_unref(OBJECT(&dev->vdev));
> > > > >      qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
> > > > >
> > > > > Would it make sense to add a helper function for that?
> > > >
> > >
> > > Sorry, I'm afraid this is not helpful. Because dev and dev->vdev is different
> > > for different virtio devices, like VirtIOBlkPCI(and its vdev is VirtIOBlock),
> > > VirtIONetPCI(and its vdev is VirtIONet). They have no the same parameters
> > > for above code segment. :)
> > 
> > Hm...
> > 
> > void virtio_instance_init_common(Object *proxydev,
> >                                  DeviceState *vdev,
> >                                  size_t vdevsize,
> >                                  const char *vdevname)
> > {
> >     object_initialize(vdev, vdevsize, vdevname);
> >     object_property_add_child(proxydev, "virtio-backend", OBJECT(vdev),
> > NULL);
> >     object_unref(OBJECT(vdev));
> >     qdev_alias_all_properties(vdev, proxydev);
> > }
> > 
> > and have the initializers call
> > 
> > virtio_instance_init_common(obj, DEVICE(&dev->vdev), sizeof(dev->vdev),
> > TYPE_WHATEVER);
> > 
> > ?
> 
> OK, it looks good that pass all parameters to one wrapper function. 
> Will do this in next version.
> 
> Thanks, Cornelia. :)
> 
> Best regards,
> -Gonglei

I'm fine with doing the cleanup as a patch on top.
Cornelia, fine with you?

Re: [Qemu-devel] [PATCH RESEND 0/9] virtio: fix virtio child recount in transports

[Cornelia Huck]

On Mon, 29 Sep 2014 15:33:32 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Sep 29, 2014 at 12:09:06PM +0000, Gonglei (Arei) wrote:
> > > From: Cornelia Huck [mailto:cornelia.huck@de.ibm.com]
> > > Sent: Monday, September 29, 2014 6:53 PM
> > > Subject: Re: [PATCH RESEND 0/9] virtio: fix virtio child recount in transports
> > > 
> > > On Sat, 27 Sep 2014 10:37:23 +0000
> > > "Gonglei (Arei)" <arei.gonglei@huawei.com> wrote:
> > > 
> > > > > > One thing I noticed is that the various devices end up with similar
> > > > > > code in the end:
> > > > > >
> > > > > >      object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_WHATEVER);
> > > > > >      object_property_add_child(obj, "virtio-backend",
> > > > > OBJECT(&dev->vdev),
> > > > > > NULL);
> > > > > >      object_unref(OBJECT(&dev->vdev));
> > > > > >      qdev_alias_all_properties(DEVICE(&dev->vdev), obj);
> > > > > >
> > > > > > Would it make sense to add a helper function for that?
> > > > >
> > > >
> > > > Sorry, I'm afraid this is not helpful. Because dev and dev->vdev is different
> > > > for different virtio devices, like VirtIOBlkPCI(and its vdev is VirtIOBlock),
> > > > VirtIONetPCI(and its vdev is VirtIONet). They have no the same parameters
> > > > for above code segment. :)
> > > 
> > > Hm...
> > > 
> > > void virtio_instance_init_common(Object *proxydev,
> > >                                  DeviceState *vdev,
> > >                                  size_t vdevsize,
> > >                                  const char *vdevname)
> > > {
> > >     object_initialize(vdev, vdevsize, vdevname);
> > >     object_property_add_child(proxydev, "virtio-backend", OBJECT(vdev),
> > > NULL);
> > >     object_unref(OBJECT(vdev));
> > >     qdev_alias_all_properties(vdev, proxydev);
> > > }
> > > 
> > > and have the initializers call
> > > 
> > > virtio_instance_init_common(obj, DEVICE(&dev->vdev), sizeof(dev->vdev),
> > > TYPE_WHATEVER);
> > > 
> > > ?
> > 
> > OK, it looks good that pass all parameters to one wrapper function. 
> > Will do this in next version.
> > 
> > Thanks, Cornelia. :)
> > 
> > Best regards,
> > -Gonglei
> 
> I'm fine with doing the cleanup as a patch on top.
> Cornelia, fine with you?
> 

Yes, sounds good to me.

Re: [Qemu-devel] [PATCH RESEND 0/9] virtio: fix virtio child recount in transports

[Gonglei (Arei)]

> > > >
> > > > Hm...
> > > >
> > > > void virtio_instance_init_common(Object *proxydev,
> > > >                                  DeviceState *vdev,
> > > >                                  size_t vdevsize,
> > > >                                  const char *vdevname)
> > > > {
> > > >     object_initialize(vdev, vdevsize, vdevname);
> > > >     object_property_add_child(proxydev, "virtio-backend",
> OBJECT(vdev),
> > > > NULL);
> > > >     object_unref(OBJECT(vdev));
> > > >     qdev_alias_all_properties(vdev, proxydev);
> > > > }
> > > >
> > > > and have the initializers call
> > > >
> > > > virtio_instance_init_common(obj, DEVICE(&dev->vdev),
> sizeof(dev->vdev),
> > > > TYPE_WHATEVER);
> > > >
> > > > ?
> > >
> > > OK, it looks good that pass all parameters to one wrapper function.
> > > Will do this in next version.
> > >
> > > Thanks, Cornelia. :)
> > >
> > > Best regards,
> > > -Gonglei
> >
> > I'm fine with doing the cleanup as a patch on top.
> > Cornelia, fine with you?
> >
> 
> Yes, sounds good to me.

Will do, thanks you both!

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH RESEND 0/9] virtio: fix virtio child recount in transports

[Paolo Bonzini]

Il 29/09/2014 12:53, Cornelia Huck ha scritto:
> void virtio_instance_init_common(Object *proxydev,
>                                  DeviceState *vdev,
>                                  size_t vdevsize,
>                                  const char *vdevname)
> {
>     object_initialize(vdev, vdevsize, vdevname);
>     object_property_add_child(proxydev, "virtio-backend", OBJECT(vdev), NULL);
>     object_unref(OBJECT(vdev));
>     qdev_alias_all_properties(vdev, proxydev);
> }
> 
> and have the initializers call
> 
> virtio_instance_init_common(obj, DEVICE(&dev->vdev), sizeof(dev->vdev), TYPE_WHATEVER);

You cannot use DEVICE() here because dev->vdev has not been initialized
yet.  But virtio_instance_init_common could just take a void*.

Paolo

Re: [Qemu-devel] [PATCH RESEND 0/9] virtio: fix virtio child recount in transports

[Gonglei (Arei)]

> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Sent: Monday, September 29, 2014 8:32 PM
> Subject: Re: [PATCH RESEND 0/9] virtio: fix virtio child recount in transports
> 
> Il 29/09/2014 12:53, Cornelia Huck ha scritto:
> > void virtio_instance_init_common(Object *proxydev,
> >                                  DeviceState *vdev,
> >                                  size_t vdevsize,
> >                                  const char *vdevname)
> > {
> >     object_initialize(vdev, vdevsize, vdevname);
> >     object_property_add_child(proxydev, "virtio-backend", OBJECT(vdev),
> NULL);
> >     object_unref(OBJECT(vdev));
> >     qdev_alias_all_properties(vdev, proxydev);
> > }
> >
> > and have the initializers call
> >
> > virtio_instance_init_common(obj, DEVICE(&dev->vdev), sizeof(dev->vdev),
> TYPE_WHATEVER);
> 
> You cannot use DEVICE() here because dev->vdev has not been initialized
> yet.  But virtio_instance_init_common could just take a void*.
> 
> Paolo

Good catch and suggestion, Thanks!

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH RESEND 0/9] virtio: fix virtio child recount in transports

[Cornelia Huck]

On Fri, 26 Sep 2014 16:45:38 +0800
<arei.gonglei@huawei.com> wrote:

> Gonglei (9):
>   virtio-net: use aliases instead of duplicate qdev properties
>   virtio: 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
> 
>  hw/s390x/s390-virtio-bus.c | 16 ++++++++++------
>  hw/s390x/virtio-ccw.c      | 18 +++++++++++-------
>  hw/virtio/virtio-pci.c     | 18 +++++++++++-------
>  3 files changed, 32 insertions(+), 20 deletions(-)
> 

The patchset passes a small snifftest for s390-virtio and virtio-ccw:
boots, device_add and device_del work as expected.

Re: [Qemu-devel] [PATCH RESEND 0/9] virtio: fix virtio child recount in transports

[Gonglei (Arei)]

> From: Cornelia Huck [mailto:cornelia.huck@de.ibm.com]
> Sent: Monday, September 29, 2014 7:33 PM
> Subject: Re: [PATCH RESEND 0/9] virtio: fix virtio child recount in transports
> 
> On Fri, 26 Sep 2014 16:45:38 +0800
> <arei.gonglei@huawei.com> wrote:
> 
> > Gonglei (9):
> >   virtio-net: use aliases instead of duplicate qdev properties
> >   virtio: 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
> >
> >  hw/s390x/s390-virtio-bus.c | 16 ++++++++++------
> >  hw/s390x/virtio-ccw.c      | 18 +++++++++++-------
> >  hw/virtio/virtio-pci.c     | 18 +++++++++++-------
> >  3 files changed, 32 insertions(+), 20 deletions(-)
> >
> 
> The patchset passes a small snifftest for s390-virtio and virtio-ccw:
> boots, device_add and device_del work as expected.

Great appreciate for your testing!  :)

Best regards,
-Gonglei