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

[Qemu-devel] [PATCH v2 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.

The v1 as below:
 http://lists.gnu.org/archive/html/qemu-devel/2014-09/msg01208.html

Changes since v1:
 1. using alias properties avoid to double-free property.(Stefan)
 2. add handling all other virtio-devices had the same probleam.
 3. same handling for CCW and s390-virito.
 
Acknowledgements:
 I copied Stefan's commit message about virtio-blk which summarized
 reasons very well, I cannot agree more with him. Holp Stefan do not
 mind, thank you so much!

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 v2 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 ddb5da1..6722156 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1414,8 +1414,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(),
 };
 
@@ -1456,6 +1454,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 v2 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 6722156..19c9a6c 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1454,6 +1454,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 v2 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 19c9a6c..2dd8360 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1124,7 +1124,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(),
 };
 
@@ -1176,6 +1175,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 = {
@@ -1192,7 +1192,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(),
 };
 
@@ -1232,6 +1231,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 v2 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 2dd8360..00dbfc9 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1175,6 +1175,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);
 }
 
@@ -1231,6 +1232,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 v2 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 00dbfc9..5a5b534 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1376,7 +1376,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(),
 };
 
@@ -1399,6 +1398,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 v2 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 5a5b534..87ceea3 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1399,6 +1399,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 v2 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 87ceea3..28b9b74 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1472,7 +1472,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(),
 };
 
@@ -1514,6 +1513,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 v2 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 28b9b74..d93ffad 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1514,6 +1514,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 v2 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 d93ffad..e6cdaca 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1314,7 +1314,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 v2 0/9] virtio: fix virtio child recount in transports

[Gonglei (Arei)]

Ping...

Any comments will be appreciated!

Best regards,
-Gonglei


> -----Original Message-----
> From: Gonglei (Arei)
> Sent: Tuesday, September 09, 2014 2:35 PM
> To: qemu-devel@nongnu.org
> Cc: mst@redhat.com; pbonzini@redhat.com; stefanha@redhat.com;
> Huangweidong (C); agraf@suse.de; Huangpeng (Peter); rth@twiddle.net;
> cornelia.huck@de.ibm.com; borntraeger@de.ibm.com; Luonengjun; Gonglei
> (Arei)
> Subject: [PATCH v2 0/9] virtio: fix virtio child recount in transports
> 
> 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.
> 
> The v1 as below:
>  http://lists.gnu.org/archive/html/qemu-devel/2014-09/msg01208.html
> 
> Changes since v1:
>  1. using alias properties avoid to double-free property.(Stefan)
>  2. add handling all other virtio-devices had the same probleam.
>  3. same handling for CCW and s390-virito.
> 
> Acknowledgements:
>  I copied Stefan's commit message about virtio-blk which summarized
>  reasons very well, I cannot agree more with him. Holp Stefan do not
>  mind, thank you so much!
> 
> 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
> 

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

[Michael S. Tsirkin]

On Tue, Sep 09, 2014 at 02:35:02PM +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(-)
> 
> 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 ddb5da1..6722156 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1414,8 +1414,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(),
>  };
>  
> @@ -1456,6 +1454,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 = {

Does -device virtio-net-pci,? still list e.g. the mac property with this
patch?


> -- 
> 1.7.12.4
> 

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

[Gonglei (Arei)]

> On Tue, Sep 09, 2014 at 02:35:02PM +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(-)
> >
> > 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 ddb5da1..6722156 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -1414,8 +1414,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(),
> >  };
> >
> > @@ -1456,6 +1454,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 = {
> 
> Does -device virtio-net-pci,? still list e.g. the mac property with this
> patch?
> 
Yes. Please see the properties of virtio-net-pci with this patch:

#./qemu-system-x86_64 -enable-kvm -m 4096 -smp 4 -name redhat6.2 -drive file=/home/redhat6.2, \ 
if=none,id=drive-ide0-0-0 -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
-drive file=/mnt/sdb/gonglei/iso/rhel-server-7.0-x86_64-dvd.iso,if=none,id=drive-ide0-0-1 \
-device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1,bootindex=4 -vnc 0.0.0.0:10 \
-netdev type=user,id=net0 -device virtio-net-pci,netdev=net0,bootindex=3,id=nic1 -monitor stdio
QEMU 2.1.50 monitor - type 'help' for more information
(qemu) qom-list nic1
virtio-net-pci.rom[0] (child<qemu:memory-region>)
virtio-pci[0] (child<qemu:memory-region>)
msix-pba[0] (child<qemu:memory-region>)
msix-table[0] (child<qemu:memory-region>)
virtio-net-pci-msix[0] (child<qemu:memory-region>)
virtio-bus (child<virtio-pci-bus>)
bus master[0] (child<qemu:memory-region>)
bootindex (int)
tx (str)
x-txburst (int32)
x-txtimer (uint32)
netdev (str)
vlan (int32)
mac (str)
virtio-backend (child<virtio-net-device>)
parent_bus (link<bus>)
command_serr_enable (bool)
multifunction (bool)
rombar (uint32)
romfile (str)
addr (int32)
legacy-addr (str)
event_idx (bool)
indirect_desc (bool)
mq (bool)
ctrl_guest_offloads (bool)
ctrl_mac_addr (bool)
ctrl_rx_extra (bool)
ctrl_vlan (bool)
ctrl_rx (bool)
ctrl_vq (bool)
status (bool)
mrg_rxbuf (bool)
host_ufo (bool)
host_ecn (bool)
host_tso6 (bool)
host_tso4 (bool)
guest_announce (bool)
guest_ufo (bool)
guest_ecn (bool)
guest_tso6 (bool)
guest_tso4 (bool)
gso (bool)
guest_csum (bool)
csum (bool)
any_layout (bool)
vectors (uint32)
ioeventfd (bool)
hotplugged (bool)
hotpluggable (bool)
realized (bool)
type (string)
(qemu) qom-get nic1 mac
"52:54:00:12:34:56"
(qemu) qom-get nic1 bootindex
3 (0x3)
(qemu) info network
nic1: index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
 \ net0: index=0,type=user,net=10.0.2.0,restrict=off
(qemu)

Best regards,
-Gonglei

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

[Michael S. Tsirkin]

On Mon, Sep 15, 2014 at 01:03:24AM +0000, Gonglei (Arei) wrote:
> > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > > index ddb5da1..6722156 100644
> > > --- a/hw/virtio/virtio-pci.c
> > > +++ b/hw/virtio/virtio-pci.c
> > > @@ -1414,8 +1414,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(),
> > >  };
> > >
> > > @@ -1456,6 +1454,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 = {
> > 
> > Does -device virtio-net-pci,? still list e.g. the mac property with this
> > patch?
> > 
> Yes. Please see the properties of virtio-net-pci with this patch:
> 
> #./qemu-system-x86_64 -enable-kvm -m 4096 -smp 4 -name redhat6.2 -drive file=/home/redhat6.2, \ 
> if=none,id=drive-ide0-0-0 -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
> -drive file=/mnt/sdb/gonglei/iso/rhel-server-7.0-x86_64-dvd.iso,if=none,id=drive-ide0-0-1 \
> -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1,bootindex=4 -vnc 0.0.0.0:10 \
> -netdev type=user,id=net0 -device virtio-net-pci,netdev=net0,bootindex=3,id=nic1 -monitor stdio
> QEMU 2.1.50 monitor - type 'help' for more information
> (qemu) qom-list nic1
> virtio-net-pci.rom[0] (child<qemu:memory-region>)
> virtio-pci[0] (child<qemu:memory-region>)
> msix-pba[0] (child<qemu:memory-region>)
> msix-table[0] (child<qemu:memory-region>)
> virtio-net-pci-msix[0] (child<qemu:memory-region>)
> virtio-bus (child<virtio-pci-bus>)
> bus master[0] (child<qemu:memory-region>)
> bootindex (int)
> tx (str)
> x-txburst (int32)
> x-txtimer (uint32)
> netdev (str)
> vlan (int32)
> mac (str)

This is a regression though.
Before:

-virtio-net-pci.netdev=netdev
-virtio-net-pci.vlan=vlan
-virtio-net-pci.mac=macaddr

After

+virtio-net-pci.netdev=str
+virtio-net-pci.vlan=int32
+virtio-net-pci.mac=str

Compare to e1000:
e1000.netdev=netdev
e1000.vlan=vlan
e1000.mac=macaddr

-- 
MST

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

[Gonglei (Arei)]

> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Monday, September 15, 2014 1:36 PM
> Subject: Re: [PATCH v2 1/9] virtio-net: use aliases instead of duplicate qdev
> properties
> 
> On Mon, Sep 15, 2014 at 01:03:24AM +0000, Gonglei (Arei) wrote:
> > > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > > > index ddb5da1..6722156 100644
> > > > --- a/hw/virtio/virtio-pci.c
> > > > +++ b/hw/virtio/virtio-pci.c
> > > > @@ -1414,8 +1414,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(),
> > > >  };
> > > >
> > > > @@ -1456,6 +1454,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 = {
> > >
> > > Does -device virtio-net-pci,? still list e.g. the mac property with this
> > > patch?
> > >
> > Yes. Please see the properties of virtio-net-pci with this patch:
> >
> > #./qemu-system-x86_64 -enable-kvm -m 4096 -smp 4 -name redhat6.2 -drive
> file=/home/redhat6.2, \
> > if=none,id=drive-ide0-0-0 -device
> ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
> > -drive
> file=/mnt/sdb/gonglei/iso/rhel-server-7.0-x86_64-dvd.iso,if=none,id=drive-ide0-
> 0-1 \
> > -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1,bootindex=4
> -vnc 0.0.0.0:10 \
> > -netdev type=user,id=net0 -device
> virtio-net-pci,netdev=net0,bootindex=3,id=nic1 -monitor stdio
> > QEMU 2.1.50 monitor - type 'help' for more information
> > (qemu) qom-list nic1
> > virtio-net-pci.rom[0] (child<qemu:memory-region>)
> > virtio-pci[0] (child<qemu:memory-region>)
> > msix-pba[0] (child<qemu:memory-region>)
> > msix-table[0] (child<qemu:memory-region>)
> > virtio-net-pci-msix[0] (child<qemu:memory-region>)
> > virtio-bus (child<virtio-pci-bus>)
> > bus master[0] (child<qemu:memory-region>)
> > bootindex (int)
> > tx (str)
> > x-txburst (int32)
> > x-txtimer (uint32)
> > netdev (str)
> > vlan (int32)
> > mac (str)
> 
> This is a regression though.
> Before:
> 
> -virtio-net-pci.netdev=netdev
> -virtio-net-pci.vlan=vlan
> -virtio-net-pci.mac=macaddr
> 
> After
> 
> +virtio-net-pci.netdev=str
> +virtio-net-pci.vlan=int32
> +virtio-net-pci.mac=str
> 
> Compare to e1000:
> e1000.netdev=netdev
> e1000.vlan=vlan
> e1000.mac=macaddr
> 
> --
> MST

Sorry, I don't understand your meaning. :(

I used 'qom-list' interface is HMP interface which
introduced by Andreas, haven't applied in qemu master.
 http://thread.gmane.org/gmane.comp.emulators.qemu/271513
Those properties of virtio-net-pci are the same with e1000.

Please see details as below:

#./qemu-system-x86_64 -enable-kvm -m 4096 -smp 4 -name redhat6.2 -drive file=/home/redhat6.2, \ 
if=none,id=drive-ide0-0-0 -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
-drive file=/mnt/sdb/gonglei/iso/rhel-server-7.0-x86_64-dvd.iso,if=none,id=drive-ide0-0-1 \
-device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1,bootindex=4 -vnc 0.0.0.0:10 \
-netdev type=user,id=net0 -device virtio-net-pci,netdev=net0,bootindex=3,id=nic1 -monitor stdio \
-netdev type=user,id=net1 -device e1000,netdev=net1,bootindex=2,id=nic
QEMU 2.1.50 monitor - type 'help' for more information
(qemu) qom-list nic1
virtio-net-pci.rom[0] (child<qemu:memory-region>)
virtio-pci[0] (child<qemu:memory-region>)
msix-pba[0] (child<qemu:memory-region>)
msix-table[0] (child<qemu:memory-region>)
virtio-net-pci-msix[0] (child<qemu:memory-region>)
virtio-bus (child<virtio-pci-bus>)
bus master[0] (child<qemu:memory-region>)
bootindex (int)
tx (str)
x-txburst (int32)
x-txtimer (uint32)
netdev (str)
vlan (int32)
mac (str)
virtio-backend (child<virtio-net-device>)
parent_bus (link<bus>)
command_serr_enable (bool)
multifunction (bool)
rombar (uint32)
romfile (str)
addr (int32)
legacy-addr (str)
event_idx (bool)
indirect_desc (bool)
mq (bool)
ctrl_guest_offloads (bool)
ctrl_mac_addr (bool)
ctrl_rx_extra (bool)
ctrl_vlan (bool)
ctrl_rx (bool)
ctrl_vq (bool)
status (bool)
mrg_rxbuf (bool)
host_ufo (bool)
host_ecn (bool)
host_tso6 (bool)
host_tso4 (bool)
guest_announce (bool)
guest_ufo (bool)
guest_ecn (bool)
guest_tso6 (bool)
guest_tso4 (bool)
gso (bool)
guest_csum (bool)
csum (bool)
any_layout (bool)
vectors (uint32)
ioeventfd (bool)
hotplugged (bool)
hotpluggable (bool)
realized (bool)
type (string)
(qemu) qom-get nic1 mac
"52:54:00:12:34:56"
(qemu) qom-get nic1 bootindex
3 (0x3)
(qemu) qom-list /machine/peripheral/nic1/virtio-backend
bootindex (int)
parent_bus (link<bus>)
tx (str)
x-txburst (int32)
x-txtimer (uint32)
netdev (str)
vlan (int32)
legacy-vlan (str)
mac (str)
hotplugged (bool)
hotpluggable (bool)
realized (bool)
type (string)
(qemu) qom-list nic
e1000.rom[0] (child<qemu:memory-region>)
e1000-io[0] (child<qemu:memory-region>)
e1000-mmio[0] (child<qemu:memory-region>)
bus master[0] (child<qemu:memory-region>)
bootindex (int)
parent_bus (link<bus>)
command_serr_enable (bool)
multifunction (bool)
rombar (uint32)
romfile (str)
addr (int32)
legacy-addr (str)
mitigation (bool)
autonegotiation (bool)
netdev (str)
vlan (int32)
legacy-vlan (str)
mac (str)
hotplugged (bool)
hotpluggable (bool)
realized (bool)
type (string)
(qemu) info network
nic1: index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
 \ net0: index=0,type=user,net=10.0.2.0,restrict=off
nic: index=0,type=nic,model=e1000,macaddr=52:54:00:12:34:57
 \ net1: index=0,type=user,net=10.0.2.0,restrict=off
(qemu)

Best regards,
-Gonglei

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

[Michael S. Tsirkin]

On Mon, Sep 15, 2014 at 05:49:42AM +0000, Gonglei (Arei) wrote:
> > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > Sent: Monday, September 15, 2014 1:36 PM
> > Subject: Re: [PATCH v2 1/9] virtio-net: use aliases instead of duplicate qdev
> > properties
> > 
> > On Mon, Sep 15, 2014 at 01:03:24AM +0000, Gonglei (Arei) wrote:
> > > > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > > > > index ddb5da1..6722156 100644
> > > > > --- a/hw/virtio/virtio-pci.c
> > > > > +++ b/hw/virtio/virtio-pci.c
> > > > > @@ -1414,8 +1414,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(),
> > > > >  };
> > > > >
> > > > > @@ -1456,6 +1454,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 = {
> > > >
> > > > Does -device virtio-net-pci,? still list e.g. the mac property with this
> > > > patch?
> > > >
> > > Yes. Please see the properties of virtio-net-pci with this patch:
> > >
> > > #./qemu-system-x86_64 -enable-kvm -m 4096 -smp 4 -name redhat6.2 -drive
> > file=/home/redhat6.2, \
> > > if=none,id=drive-ide0-0-0 -device
> > ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
> > > -drive
> > file=/mnt/sdb/gonglei/iso/rhel-server-7.0-x86_64-dvd.iso,if=none,id=drive-ide0-
> > 0-1 \
> > > -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1,bootindex=4
> > -vnc 0.0.0.0:10 \
> > > -netdev type=user,id=net0 -device
> > virtio-net-pci,netdev=net0,bootindex=3,id=nic1 -monitor stdio
> > > QEMU 2.1.50 monitor - type 'help' for more information
> > > (qemu) qom-list nic1
> > > virtio-net-pci.rom[0] (child<qemu:memory-region>)
> > > virtio-pci[0] (child<qemu:memory-region>)
> > > msix-pba[0] (child<qemu:memory-region>)
> > > msix-table[0] (child<qemu:memory-region>)
> > > virtio-net-pci-msix[0] (child<qemu:memory-region>)
> > > virtio-bus (child<virtio-pci-bus>)
> > > bus master[0] (child<qemu:memory-region>)
> > > bootindex (int)
> > > tx (str)
> > > x-txburst (int32)
> > > x-txtimer (uint32)
> > > netdev (str)
> > > vlan (int32)
> > > mac (str)
> > 
> > This is a regression though.
> > Before:
> > 
> > -virtio-net-pci.netdev=netdev
> > -virtio-net-pci.vlan=vlan
> > -virtio-net-pci.mac=macaddr
> > 
> > After
> > 
> > +virtio-net-pci.netdev=str
> > +virtio-net-pci.vlan=int32
> > +virtio-net-pci.mac=str
> > 
> > Compare to e1000:
> > e1000.netdev=netdev
> > e1000.vlan=vlan
> > e1000.mac=macaddr
> > 
> > --
> > MST
> 
> Sorry, I don't understand your meaning. :(

I applied your patch, and looked at the property types.

They should be netdev/vlan/macaddr.
Your patch makes them str/int32/str

-- 
MST

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

[Gonglei (Arei)]

> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Monday, September 15, 2014 2:41 PM
> Subject: Re: [PATCH v2 1/9] virtio-net: use aliases instead of duplicate qdev
> properties
> 
> On Mon, Sep 15, 2014 at 05:49:42AM +0000, Gonglei (Arei) wrote:
> > > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > > Sent: Monday, September 15, 2014 1:36 PM
> > > Subject: Re: [PATCH v2 1/9] virtio-net: use aliases instead of duplicate qdev
> > > properties
> > >
> > > On Mon, Sep 15, 2014 at 01:03:24AM +0000, Gonglei (Arei) wrote:
> > > > > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > > > > > index ddb5da1..6722156 100644
> > > > > > --- a/hw/virtio/virtio-pci.c
> > > > > > +++ b/hw/virtio/virtio-pci.c
> > > > > > @@ -1414,8 +1414,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(),
> > > > > >  };
> > > > > >
> > > > > > @@ -1456,6 +1454,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 = {
> > > > >
> > > > > Does -device virtio-net-pci,? still list e.g. the mac property with this
> > > > > patch?
> > > > >
> > > > Yes. Please see the properties of virtio-net-pci with this patch:
> > > >
> > > > #./qemu-system-x86_64 -enable-kvm -m 4096 -smp 4 -name redhat6.2
> -drive
> > > file=/home/redhat6.2, \
> > > > if=none,id=drive-ide0-0-0 -device
> > > ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
> > > > -drive
> > >
> file=/mnt/sdb/gonglei/iso/rhel-server-7.0-x86_64-dvd.iso,if=none,id=drive-ide0-
> > > 0-1 \
> > > > -device
> ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1,bootindex=4
> > > -vnc 0.0.0.0:10 \
> > > > -netdev type=user,id=net0 -device
> > > virtio-net-pci,netdev=net0,bootindex=3,id=nic1 -monitor stdio
> > > > QEMU 2.1.50 monitor - type 'help' for more information
> > > > (qemu) qom-list nic1
> > > > virtio-net-pci.rom[0] (child<qemu:memory-region>)
> > > > virtio-pci[0] (child<qemu:memory-region>)
> > > > msix-pba[0] (child<qemu:memory-region>)
> > > > msix-table[0] (child<qemu:memory-region>)
> > > > virtio-net-pci-msix[0] (child<qemu:memory-region>)
> > > > virtio-bus (child<virtio-pci-bus>)
> > > > bus master[0] (child<qemu:memory-region>)
> > > > bootindex (int)
> > > > tx (str)
> > > > x-txburst (int32)
> > > > x-txtimer (uint32)
> > > > netdev (str)
> > > > vlan (int32)
> > > > mac (str)
> > >
> > > This is a regression though.
> > > Before:
> > >
> > > -virtio-net-pci.netdev=netdev
> > > -virtio-net-pci.vlan=vlan
> > > -virtio-net-pci.mac=macaddr
> > >
> > > After
> > >
> > > +virtio-net-pci.netdev=str
> > > +virtio-net-pci.vlan=int32
> > > +virtio-net-pci.mac=str
> > >
> > > Compare to e1000:
> > > e1000.netdev=netdev
> > > e1000.vlan=vlan
> > > e1000.mac=macaddr
> > >
> > > --
> > > MST
> >
> > Sorry, I don't understand your meaning. :(
> 
> I applied your patch, and looked at the property types.
> 
Could you tell me your method looked at properties type? 

> They should be netdev/vlan/macaddr.
> Your patch makes them str/int32/str
> 
Does those changes have any side-effects? Thanks!
The similar changes for virtio-blk had applied in qemu master. 

Best regards,
-Gonglei

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

[Michael S. Tsirkin]

On Mon, Sep 15, 2014 at 07:12:59AM +0000, Gonglei (Arei) wrote:
> > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > Sent: Monday, September 15, 2014 2:41 PM
> > Subject: Re: [PATCH v2 1/9] virtio-net: use aliases instead of duplicate qdev
> > properties
> > 
> > On Mon, Sep 15, 2014 at 05:49:42AM +0000, Gonglei (Arei) wrote:
> > > > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > > > Sent: Monday, September 15, 2014 1:36 PM
> > > > Subject: Re: [PATCH v2 1/9] virtio-net: use aliases instead of duplicate qdev
> > > > properties
> > > >
> > > > On Mon, Sep 15, 2014 at 01:03:24AM +0000, Gonglei (Arei) wrote:
> > > > > > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > > > > > > index ddb5da1..6722156 100644
> > > > > > > --- a/hw/virtio/virtio-pci.c
> > > > > > > +++ b/hw/virtio/virtio-pci.c
> > > > > > > @@ -1414,8 +1414,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(),
> > > > > > >  };
> > > > > > >
> > > > > > > @@ -1456,6 +1454,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 = {
> > > > > >
> > > > > > Does -device virtio-net-pci,? still list e.g. the mac property with this
> > > > > > patch?
> > > > > >
> > > > > Yes. Please see the properties of virtio-net-pci with this patch:
> > > > >
> > > > > #./qemu-system-x86_64 -enable-kvm -m 4096 -smp 4 -name redhat6.2
> > -drive
> > > > file=/home/redhat6.2, \
> > > > > if=none,id=drive-ide0-0-0 -device
> > > > ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
> > > > > -drive
> > > >
> > file=/mnt/sdb/gonglei/iso/rhel-server-7.0-x86_64-dvd.iso,if=none,id=drive-ide0-
> > > > 0-1 \
> > > > > -device
> > ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1,bootindex=4
> > > > -vnc 0.0.0.0:10 \
> > > > > -netdev type=user,id=net0 -device
> > > > virtio-net-pci,netdev=net0,bootindex=3,id=nic1 -monitor stdio
> > > > > QEMU 2.1.50 monitor - type 'help' for more information
> > > > > (qemu) qom-list nic1
> > > > > virtio-net-pci.rom[0] (child<qemu:memory-region>)
> > > > > virtio-pci[0] (child<qemu:memory-region>)
> > > > > msix-pba[0] (child<qemu:memory-region>)
> > > > > msix-table[0] (child<qemu:memory-region>)
> > > > > virtio-net-pci-msix[0] (child<qemu:memory-region>)
> > > > > virtio-bus (child<virtio-pci-bus>)
> > > > > bus master[0] (child<qemu:memory-region>)
> > > > > bootindex (int)
> > > > > tx (str)
> > > > > x-txburst (int32)
> > > > > x-txtimer (uint32)
> > > > > netdev (str)
> > > > > vlan (int32)
> > > > > mac (str)
> > > >
> > > > This is a regression though.
> > > > Before:
> > > >
> > > > -virtio-net-pci.netdev=netdev
> > > > -virtio-net-pci.vlan=vlan
> > > > -virtio-net-pci.mac=macaddr
> > > >
> > > > After
> > > >
> > > > +virtio-net-pci.netdev=str
> > > > +virtio-net-pci.vlan=int32
> > > > +virtio-net-pci.mac=str
> > > >
> > > > Compare to e1000:
> > > > e1000.netdev=netdev
> > > > e1000.vlan=vlan
> > > > e1000.mac=macaddr
> > > >
> > > > --
> > > > MST
> > >
> > > Sorry, I don't understand your meaning. :(
> > 
> > I applied your patch, and looked at the property types.
> > 
> Could you tell me your method looked at properties type? 

You can use an HMP command, or you can use
-device virtio-net-pci,?
changes must not modify output except for adding
new commands, or fixing bugs (with care).

> > They should be netdev/vlan/macaddr.
> > Your patch makes them str/int32/str
> > 
> Does those changes have any side-effects? Thanks!

Yes.  If the types are just "int" and "str" then you
have no way to know they should have a specific format,
such as a name of a netdev, valid mac address, or a vlan.
In fact, vlan=int is a bug since vlan names aren't numbers.

> The similar changes for virtio-blk had applied in qemu master. 
> 
> Best regards,
> -Gonglei

Which commit? Did output change? If yes it's a bug.

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

[Gonglei (Arei)]

> > > >
> > > > Sorry, I don't understand your meaning. :(
> > >
> > > I applied your patch, and looked at the property types.
> > >
> > Could you tell me your method looked at properties type?
> 
> You can use an HMP command, or you can use
> -device virtio-net-pci,?

OK. Good method.

> changes must not modify output except for adding
> new commands, or fixing bugs (with care).
> 
OK. 

> > > They should be netdev/vlan/macaddr.
> > > Your patch makes them str/int32/str
> > >
> > Does those changes have any side-effects? Thanks!
> 
> Yes.  If the types are just "int" and "str" then you
> have no way to know they should have a specific format,
> such as a name of a netdev, valid mac address, or a vlan.
> In fact, vlan=int is a bug since vlan names aren't numbers.
> 
Got it. 

> > The similar changes for virtio-blk had applied in qemu master.
> >
> > Best regards,
> > -Gonglei
> 
> Which commit? Did output change? If yes it's a bug.

No, I checked it again, virtio-blk-pci's properties are 'int' type as
the same as virtio-blk device, so it will have no problem on this point. 

I will rework this patch series. Thanks a lot for review! :)

Best regards,
-Gonglei

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

[Gonglei (Arei)]

> -----Original Message-----
> From: Gonglei (Arei)
> Sent: Monday, September 15, 2014 5:06 PM
> To: 'Michael S. Tsirkin'
> Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; stefanha@redhat.com;
> Huangweidong (C); agraf@suse.de; Huangpeng (Peter); rth@twiddle.net;
> cornelia.huck@de.ibm.com; borntraeger@de.ibm.com; Luonengjun;
> peter.crosthwaite@xilinx.com; afaerber@suse.de
> Subject: RE: [PATCH v2 1/9] virtio-net: use aliases instead of duplicate qdev
> properties
> 
> > > > >
> > > > > Sorry, I don't understand your meaning. :(
> > > >
> > > > I applied your patch, and looked at the property types.
> > > >
> > > Could you tell me your method looked at properties type?
> >
> > You can use an HMP command, or you can use
> > -device virtio-net-pci,?
> 
> OK. Good method.
> 
> > changes must not modify output except for adding
> > new commands, or fixing bugs (with care).
> >
> OK.
> 
> > > > They should be netdev/vlan/macaddr.
> > > > Your patch makes them str/int32/str
> > > >
> > > Does those changes have any side-effects? Thanks!
> >
> > Yes.  If the types are just "int" and "str" then you
> > have no way to know they should have a specific format,
> > such as a name of a netdev, valid mac address, or a vlan.
> > In fact, vlan=int is a bug since vlan names aren't numbers.
> >
> Got it.
> 
> > > The similar changes for virtio-blk had applied in qemu master.
> > >
> > > Best regards,
> > > -Gonglei
> >
> > Which commit? Did output change? If yes it's a bug.
> 
> No, I checked it again, virtio-blk-pci's properties are 'int' type as
> the same as virtio-blk device, so it will have no problem on this point.
> 
Sorry. There are two properties are different
 (commit caffdac363801cd2cf2bf01ad013a8c1e1e43800)

Before:

virtio-blk-pci.physical_block_size=blocksize
virtio-blk-pci.logical_block_size=blocksize

After commit caffdac363801cd2cf2bf01ad013a8c1e1e43800 :

virtio-blk-pci.physical_block_size=uint16
virtio-blk-pci.logical_block_size=uint16

> I will rework this patch series. Thanks a lot for review! :)
> 
> Best regards,
> -Gonglei

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

[Gonglei (Arei)]

> > Subject: RE: [PATCH v2 1/9] virtio-net: use aliases instead of duplicate qdev
> > properties
> >
> > > > > >
> > > > > > Sorry, I don't understand your meaning. :(
> > > > >
> > > > > I applied your patch, and looked at the property types.
> > > > >
> > > > Could you tell me your method looked at properties type?
> > >
> > > You can use an HMP command, or you can use
> > > -device virtio-net-pci,?
> >
> > OK. Good method.
> >
> > > changes must not modify output except for adding
> > > new commands, or fixing bugs (with care).
> > >
> > OK.
> >
> > > > > They should be netdev/vlan/macaddr.
> > > > > Your patch makes them str/int32/str
> > > > >
> > > > Does those changes have any side-effects? Thanks!
> > >
> > > Yes.  If the types are just "int" and "str" then you
> > > have no way to know they should have a specific format,
> > > such as a name of a netdev, valid mac address, or a vlan.
> > > In fact, vlan=int is a bug since vlan names aren't numbers.
> > >
> > Got it.
> >
> > > > The similar changes for virtio-blk had applied in qemu master.
> > > >
> > > > Best regards,
> > > > -Gonglei
> > >
> > > Which commit? Did output change? If yes it's a bug.
> >
> > No, I checked it again, virtio-blk-pci's properties are 'int' type as
> > the same as virtio-blk device, so it will have no problem on this point.
> >
> Sorry. There are two properties are different
>  (commit caffdac363801cd2cf2bf01ad013a8c1e1e43800)
> 
> Before:
> 
> virtio-blk-pci.physical_block_size=blocksize
> virtio-blk-pci.logical_block_size=blocksize
> 
> After commit caffdac363801cd2cf2bf01ad013a8c1e1e43800 :
> 
> virtio-blk-pci.physical_block_size=uint16
> virtio-blk-pci.logical_block_size=uint16
> 
And

Before:
  virtio-blk-pci.drive=drive

After:
 virtio-blk-pci.drive=str

> > I will rework this patch series. Thanks a lot for review! :)
> >
> > Best regards,
> > -Gonglei

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

[Michael S. Tsirkin]

On Mon, Sep 15, 2014 at 09:23:26AM +0000, Gonglei (Arei) wrote:
> 
> 
> 
> 
> 
> 
> 
> > -----Original Message-----
> > From: Gonglei (Arei)
> > Sent: Monday, September 15, 2014 5:06 PM
> > To: 'Michael S. Tsirkin'
> > Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; stefanha@redhat.com;
> > Huangweidong (C); agraf@suse.de; Huangpeng (Peter); rth@twiddle.net;
> > cornelia.huck@de.ibm.com; borntraeger@de.ibm.com; Luonengjun;
> > peter.crosthwaite@xilinx.com; afaerber@suse.de
> > Subject: RE: [PATCH v2 1/9] virtio-net: use aliases instead of duplicate qdev
> > properties
> > 
> > > > > >
> > > > > > Sorry, I don't understand your meaning. :(
> > > > >
> > > > > I applied your patch, and looked at the property types.
> > > > >
> > > > Could you tell me your method looked at properties type?
> > >
> > > You can use an HMP command, or you can use
> > > -device virtio-net-pci,?
> > 
> > OK. Good method.
> > 
> > > changes must not modify output except for adding
> > > new commands, or fixing bugs (with care).
> > >
> > OK.
> > 
> > > > > They should be netdev/vlan/macaddr.
> > > > > Your patch makes them str/int32/str
> > > > >
> > > > Does those changes have any side-effects? Thanks!
> > >
> > > Yes.  If the types are just "int" and "str" then you
> > > have no way to know they should have a specific format,
> > > such as a name of a netdev, valid mac address, or a vlan.
> > > In fact, vlan=int is a bug since vlan names aren't numbers.
> > >
> > Got it.
> > 
> > > > The similar changes for virtio-blk had applied in qemu master.
> > > >
> > > > Best regards,
> > > > -Gonglei
> > >
> > > Which commit? Did output change? If yes it's a bug.
> > 
> > No, I checked it again, virtio-blk-pci's properties are 'int' type as
> > the same as virtio-blk device, so it will have no problem on this point.
> > 
> Sorry. There are two properties are different
>  (commit caffdac363801cd2cf2bf01ad013a8c1e1e43800)
> 
> Before:
> 
> virtio-blk-pci.physical_block_size=blocksize
> virtio-blk-pci.logical_block_size=blocksize
> 
> After commit caffdac363801cd2cf2bf01ad013a8c1e1e43800 :
> 
> virtio-blk-pci.physical_block_size=uint16
> virtio-blk-pci.logical_block_size=uint16

So that's a regresion in 2.1.0
Would be nice to fix in stable.

> > I will rework this patch series. Thanks a lot for review! :)
> > 
> > Best regards,
> > -Gonglei

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

[Michael S. Tsirkin]

On Mon, Sep 15, 2014 at 09:29:26AM +0000, Gonglei (Arei) wrote:
> > > Subject: RE: [PATCH v2 1/9] virtio-net: use aliases instead of duplicate qdev
> > > properties
> > >
> > > > > > >
> > > > > > > Sorry, I don't understand your meaning. :(
> > > > > >
> > > > > > I applied your patch, and looked at the property types.
> > > > > >
> > > > > Could you tell me your method looked at properties type?
> > > >
> > > > You can use an HMP command, or you can use
> > > > -device virtio-net-pci,?
> > >
> > > OK. Good method.
> > >
> > > > changes must not modify output except for adding
> > > > new commands, or fixing bugs (with care).
> > > >
> > > OK.
> > >
> > > > > > They should be netdev/vlan/macaddr.
> > > > > > Your patch makes them str/int32/str
> > > > > >
> > > > > Does those changes have any side-effects? Thanks!
> > > >
> > > > Yes.  If the types are just "int" and "str" then you
> > > > have no way to know they should have a specific format,
> > > > such as a name of a netdev, valid mac address, or a vlan.
> > > > In fact, vlan=int is a bug since vlan names aren't numbers.
> > > >
> > > Got it.
> > >
> > > > > The similar changes for virtio-blk had applied in qemu master.
> > > > >
> > > > > Best regards,
> > > > > -Gonglei
> > > >
> > > > Which commit? Did output change? If yes it's a bug.
> > >
> > > No, I checked it again, virtio-blk-pci's properties are 'int' type as
> > > the same as virtio-blk device, so it will have no problem on this point.
> > >
> > Sorry. There are two properties are different
> >  (commit caffdac363801cd2cf2bf01ad013a8c1e1e43800)
> > 
> > Before:
> > 
> > virtio-blk-pci.physical_block_size=blocksize
> > virtio-blk-pci.logical_block_size=blocksize
> > 
> > After commit caffdac363801cd2cf2bf01ad013a8c1e1e43800 :
> > 
> > virtio-blk-pci.physical_block_size=uint16
> > virtio-blk-pci.logical_block_size=uint16
> > 
> And
> 
> Before:
>   virtio-blk-pci.drive=drive
> 
> After:
>  virtio-blk-pci.drive=str

That's a bug. Need to fix that.

> > > I will rework this patch series. Thanks a lot for review! :)
> > >
> > > Best regards,
> > > -Gonglei

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

[Gonglei (Arei)]

> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Monday, September 15, 2014 5:59 PM
> > > >
> > > > > > > >
> > > > > > > > Sorry, I don't understand your meaning. :(
> > > > > > >
> > > > > > > I applied your patch, and looked at the property types.
> > > > > > >
> > > > > > Could you tell me your method looked at properties type?
> > > > >
> > > > > You can use an HMP command, or you can use
> > > > > -device virtio-net-pci,?
> > > >
> > > > OK. Good method.
> > > >
> > > > > changes must not modify output except for adding
> > > > > new commands, or fixing bugs (with care).
> > > > >
> > > > OK.
> > > >
> > > > > > > They should be netdev/vlan/macaddr.
> > > > > > > Your patch makes them str/int32/str
> > > > > > >
> > > > > > Does those changes have any side-effects? Thanks!
> > > > >
> > > > > Yes.  If the types are just "int" and "str" then you
> > > > > have no way to know they should have a specific format,
> > > > > such as a name of a netdev, valid mac address, or a vlan.
> > > > > In fact, vlan=int is a bug since vlan names aren't numbers.
> > > > >
> > > > Got it.
> > > >
> > > > > > The similar changes for virtio-blk had applied in qemu master.
> > > > > >
> > > > > > Best regards,
> > > > > > -Gonglei
> > > > >
> > > > > Which commit? Did output change? If yes it's a bug.
> > > >
> > > > No, I checked it again, virtio-blk-pci's properties are 'int' type as
> > > > the same as virtio-blk device, so it will have no problem on this point.
> > > >
> > > Sorry. There are two properties are different
> > >  (commit caffdac363801cd2cf2bf01ad013a8c1e1e43800)
> > >
> > > Before:
> > >
> > > virtio-blk-pci.physical_block_size=blocksize
> > > virtio-blk-pci.logical_block_size=blocksize
> > >
> > > After commit caffdac363801cd2cf2bf01ad013a8c1e1e43800 :
> > >
> > > virtio-blk-pci.physical_block_size=uint16
> > > virtio-blk-pci.logical_block_size=uint16
> > >
> > And
> >
> > Before:
> >   virtio-blk-pci.drive=drive
> >
> > After:
> >  virtio-blk-pci.drive=str
> 
> That's a bug. Need to fix that.
> 
Hi, Michael
I have debugged this. And I found this just a qmp's output problem on
make_device_property_info(). 

When set a device's alias properties,
the setter/getter properties functions will be used the same functions as 
target_object. See please:

static void property_set_alias(Object *obj, struct Visitor *v, void *opaque,
                               const char *name, Error **errp)
{
    AliasProperty *prop = opaque;

    object_property_set(prop->target_obj, v, prop->target_name, errp);
}

static Object *property_resolve_alias(Object *obj, void *opaque,
                                      const gchar *part)
{
    AliasProperty *prop = opaque;

    return object_resolve_path_component(prop->target_obj, prop->target_name);
}

I will post a simple patch series fix the output issue for alias properties. :)

Best regards,
-Gonglei

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

[Gonglei (Arei)]

Hi, Michael, Paolo and Markus

This patch series fix an obvious resource leak issue about virtio devices.
Maybe those should be merged in qemu-stable tree IMHO.
But the patch serial using property_alias function which 
introduce a regression (-device FOO,?) such as:

before:
virtio-blk-pci.physical_block_size=blocksize
virtio-blk-pci.logical_block_size=blocksize
virtio-blk-pci.drive=drive
virtio-net-pci.netdev=netdev
virtio-net-pci.vlan=vlan
virtio-net-pci.mac=macaddr

after:
virtio-blk-pci.physical_block_size=uint16
virtio-blk-pci.logical_block_size=uint16
virtio-blk-pci.drive=str
virtio-net-pci.netdev=str
virtio-net-pci.vlan=int32
virtio-net-pci.mac=str

So, I posted a patch series:
 [PATCH v2 0/3] Fix confused output for alias properties

which to fix the output regression. But Paolo said this fixing way
is layering violation. Now, it seems that the both seriaes are stagnant.

What's your opinion? Thanks a lot!
Does this patch series can be merged firstly?

Best regards,
-Gonglei


> -----Original Message-----
> From: Gonglei (Arei)
> Sent: Tuesday, September 09, 2014 2:35 PM
> To: qemu-devel@nongnu.org
> Cc: mst@redhat.com; pbonzini@redhat.com; stefanha@redhat.com;
> Huangweidong (C); agraf@suse.de; Huangpeng (Peter); rth@twiddle.net;
> cornelia.huck@de.ibm.com; borntraeger@de.ibm.com; Luonengjun; Gonglei
> (Arei)
> Subject: [PATCH v2 0/9] virtio: fix virtio child recount in transports
> 
> 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.
> 
> The v1 as below:
>  http://lists.gnu.org/archive/html/qemu-devel/2014-09/msg01208.html
> 
> Changes since v1:
>  1. using alias properties avoid to double-free property.(Stefan)
>  2. add handling all other virtio-devices had the same probleam.
>  3. same handling for CCW and s390-virito.
> 
> Acknowledgements:
>  I copied Stefan's commit message about virtio-blk which summarized
>  reasons very well, I cannot agree more with him. Holp Stefan do not
>  mind, thank you so much!
> 
> 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
>