All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio
@ 2013-08-01  2:17 Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro Andreas Färber
                   ` (23 more replies)
  0 siblings, 24 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Crosthwaite, Michael S. Tsirkin, Alexey Kardashevskiy,
	Hu Tao, Jesse Larrew, Hervé Poussineau, Anthony Liguori,
	Paolo Bonzini, Andreas Färber, Frederic Konrad

Hello,

This series converts virtio devices to QOM realize/unrealize.
It is intended as base for fixing virtio-net initialization order issues,
as reported by Jesse. Only partially tested though.

Note that while VirtioDevice was setting a DeviceClass::exit callback
for cleaning up the bus name, this was overwritten by most derived classes.
That is fixed as part of this conversion.

Similarly, virtio_scsi_common_{init,exit} can be moved to VirtIOSCSICommon now.
This has the side-effect that the two SCSI subclasses now perform some
initializations after the common SCSI implementation has invoked
virtio_bus_plug_device().
Paolo, waiting on your review; deferred to last patch so we can easily drop it.

v2 avoids adding class structs to store parent_* fields by introducing an
OBJECT_GET_PARENT_CLASS() macro inspired by Peter C. It does the conversion
in multiple steps based on an approach by Hervé and tries to separate out
refactorings as requested by mst.

Available from:
https://github.com/afaerber/qemu-cpu/commits/realize-virtio.v2
git://github.com/afaerber/qemu-cpu.git realize-virtio.v2

Regards,
Andreas

v1 -> v2:
* Prepended patch introducing OBJECT_GET_PARENT_CLASS() macro (Peter C.).
* Prepended patch allowing empty VirtioDevice::init (Hervé).
* Split VirtioDevice QOM realize conversion per device (mst).
* Split out cleanups (mst).
* Split out propagating unrealization.
* Split out VirtIOSCSICommon QOM conversion.
* Dropped klass -> oc cleanup since no new variables are added any more.
* Split off VirtConsole QOM conversion (an independent type hierarchy).

Cc: Anthony Liguori <anthony@codemonkey.ws>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Cc: Hervé Poussineau <hpoussin@reactos.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
Cc: Frederic Konrad <fred.konrad@greensocs.com>
Cc: Hu Tao <hutao@cn.fujitsu.com>
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>

Andreas Färber (22):
  object: Add OBJECT_GET_PARENT_CLASS() macro
  virtio-console: Use exitfn for virtserialport, too
  virtio-9p-device: Avoid freeing uninitialized memory
  virtio-blk-dataplane: Improve error reporting
  virtio: Allow NULL VirtioDeviceClass::init hook
  virtio-9p: QOM realize preparations
  virtio-9p: Convert to QOM realize
  virtio-blk: QOM realize preparations
  virtio-blk: Convert to QOM realize
  virtio-serial: QOM realize preparations
  virtio-serial: Convert to QOM realize
  virtio-net: QOM realize preparations
  virtio-net: Convert to QOM realize
  virtio-balloon: QOM realize preparations
  virtio-balloon: Convert to QOM realize
  virtio-rng: QOM realize preparations
  virtio-rng: Convert to QOM realize
  virtio-scsi: QOM realize preparations
  virtio-scsi: Convert to QOM realize
  virtio: Convert VirtioDevice to QOM realize
  virtio: Unrealize parent
  virtio-scsi: Convert virtio_scsi_common_[un]realize to QOM realize

 hw/9pfs/virtio-9p-device.c         | 49 ++++++++++++++-------------
 hw/9pfs/virtio-9p.h                |  2 ++
 hw/block/dataplane/virtio-blk.c    | 30 +++++++++--------
 hw/block/dataplane/virtio-blk.h    |  5 +--
 hw/block/virtio-blk.c              | 53 +++++++++++++++++++----------
 hw/char/virtio-console.c           |  1 +
 hw/char/virtio-serial-bus.c        | 39 ++++++++++++---------
 hw/net/virtio-net.c                | 42 ++++++++++++-----------
 hw/scsi/vhost-scsi.c               | 49 +++++++++++++++------------
 hw/scsi/virtio-scsi.c              | 69 ++++++++++++++++++++++----------------
 hw/virtio/virtio-balloon.c         | 40 +++++++++++++---------
 hw/virtio/virtio-rng.c             | 43 +++++++++++++-----------
 hw/virtio/virtio.c                 | 20 ++++-------
 include/hw/virtio/vhost-scsi.h     |  2 ++
 include/hw/virtio/virtio-balloon.h |  2 ++
 include/hw/virtio/virtio-blk.h     |  2 ++
 include/hw/virtio/virtio-net.h     |  2 ++
 include/hw/virtio/virtio-rng.h     |  2 ++
 include/hw/virtio/virtio-scsi.h    |  7 ++--
 include/hw/virtio/virtio-serial.h  |  2 ++
 include/hw/virtio/virtio.h         |  6 ++--
 include/qom/object.h               | 10 ++++++
 22 files changed, 282 insertions(+), 195 deletions(-)

-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-06  5:54   ` Alexey Kardashevskiy
  2013-08-07  1:50   ` Peter Crosthwaite
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 02/22] virtio-console: Use exitfn for virtserialport, too Andreas Färber
                   ` (22 subsequent siblings)
  23 siblings, 2 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Andreas Färber

The object argument is currently unused and may be used to optimize the
class lookup when needed.

Inspired-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 include/qom/object.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/qom/object.h b/include/qom/object.h
index 23fc048..a8e71dc 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -511,6 +511,16 @@ struct TypeInfo
     OBJECT_CLASS_CHECK(class, object_get_class(OBJECT(obj)), name)
 
 /**
+ * OBJECT_GET_PARENT_CLASS:
+ * @obj: The object to obtain the parent class for.
+ * @name: The QOM typename of @obj.
+ *
+ * Returns the parent class for a given object of a specific class.
+ */
+#define OBJECT_GET_PARENT_CLASS(obj, name) \
+    object_class_get_parent(object_class_by_name(name))
+
+/**
  * InterfaceInfo:
  * @type: The name of the interface.
  *
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 02/22] virtio-console: Use exitfn for virtserialport, too
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 03/22] virtio-9p-device: Avoid freeing uninitialized memory Andreas Färber
                   ` (21 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Amit Shah, Anthony Liguori, Andreas Färber, qemu-stable

virtconsole and virtserialport are identical in every other aspect
except for the distinguishing VirtIOSerialPortClass::is_console field.

Cc: qemu-stable@nongnu.org
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/char/virtio-console.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
index 6759e51..2e00ad2 100644
--- a/hw/char/virtio-console.c
+++ b/hw/char/virtio-console.c
@@ -185,6 +185,7 @@ static void virtserialport_class_init(ObjectClass *klass, void *data)
     VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_CLASS(klass);
 
     k->init = virtconsole_initfn;
+    k->exit = virtconsole_exitfn;
     k->have_data = flush_buf;
     k->set_guest_connected = set_guest_connected;
     dc->props = virtserialport_properties;
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 03/22] virtio-9p-device: Avoid freeing uninitialized memory
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 02/22] virtio-console: Use exitfn for virtserialport, too Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 04/22] virtio-blk-dataplane: Improve error reporting Andreas Färber
                   ` (20 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Andreas Färber, Aneesh Kumar K.V

In virtio_9p_device_init() there are 6x goto out that will lead to
v9fs_path_free() attempting to free unitialized path.data field.
Easiest way to trigger is: qemu-system-x86_64 -device virtio-9p-pci

Fix this by moving v9fs_path_init() before any goto out.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/9pfs/virtio-9p-device.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index de6f0fe..f0ffbe8 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -61,6 +61,8 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
 
     s->vq = virtio_add_queue(vdev, MAX_REQ, handle_9p_output);
 
+    v9fs_path_init(&path);
+
     fse = get_fsdev_fsentry(s->fsconf.fsdev_id);
 
     if (!fse) {
@@ -111,7 +113,6 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
      * call back to do that. Since we are in the init path, we don't
      * use co-routines here.
      */
-    v9fs_path_init(&path);
     if (s->ops->name_to_path(&s->ctx, NULL, "/", &path) < 0) {
         fprintf(stderr,
                 "error in converting name to path %s", strerror(errno));
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 04/22] virtio-blk-dataplane: Improve error reporting
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (2 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 03/22] virtio-9p-device: Avoid freeing uninitialized memory Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 05/22] virtio: Allow NULL VirtioDeviceClass::init hook Andreas Färber
                   ` (19 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Anthony Liguori, Andreas Färber, Stefan Hajnoczi

Return an Error so that it can be propagated later.

Tested-by: Stefan Hajnoczi <stefanha@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
[AF: Rebased]
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/block/dataplane/virtio-blk.c | 30 ++++++++++++++++--------------
 hw/block/dataplane/virtio-blk.h |  5 +++--
 hw/block/virtio-blk.c           | 15 +++++++++++++--
 3 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 411becc..fed9231 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -392,8 +392,9 @@ static void start_data_plane_bh(void *opaque)
                        s, QEMU_THREAD_JOINABLE);
 }
 
-bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
-                                  VirtIOBlockDataPlane **dataplane)
+void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
+                                  VirtIOBlockDataPlane **dataplane,
+                                  Error **errp)
 {
     VirtIOBlockDataPlane *s;
     int fd;
@@ -401,33 +402,35 @@ bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
     *dataplane = NULL;
 
     if (!blk->data_plane) {
-        return true;
+        return;
     }
 
     if (blk->scsi) {
-        error_report("device is incompatible with x-data-plane, use scsi=off");
-        return false;
+        error_setg(errp,
+                   "device is incompatible with x-data-plane, use scsi=off");
+        return;
     }
 
     if (blk->config_wce) {
-        error_report("device is incompatible with x-data-plane, "
-                     "use config-wce=off");
-        return false;
+        error_setg(errp, "device is incompatible with x-data-plane, "
+                         "use config-wce=off");
+        return;
     }
 
     /* If dataplane is (re-)enabled while the guest is running there could be
      * block jobs that can conflict.
      */
     if (bdrv_in_use(blk->conf.bs)) {
-        error_report("cannot start dataplane thread while device is in use");
-        return false;
+        error_setg(errp,
+                   "cannot start dataplane thread while device is in use");
+        return;
     }
 
     fd = raw_get_aio_fd(blk->conf.bs);
     if (fd < 0) {
-        error_report("drive is incompatible with x-data-plane, "
-                     "use format=raw,cache=none,aio=native");
-        return false;
+        error_setg(errp, "drive is incompatible with x-data-plane, "
+                         "use format=raw,cache=none,aio=native");
+        return;
     }
 
     s = g_new0(VirtIOBlockDataPlane, 1);
@@ -439,7 +442,6 @@ bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
     bdrv_set_in_use(blk->conf.bs, 1);
 
     *dataplane = s;
-    return true;
 }
 
 void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s)
diff --git a/hw/block/dataplane/virtio-blk.h b/hw/block/dataplane/virtio-blk.h
index c90e99f..1750c99 100644
--- a/hw/block/dataplane/virtio-blk.h
+++ b/hw/block/dataplane/virtio-blk.h
@@ -19,8 +19,9 @@
 
 typedef struct VirtIOBlockDataPlane VirtIOBlockDataPlane;
 
-bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
-                                  VirtIOBlockDataPlane **dataplane);
+void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
+                                  VirtIOBlockDataPlane **dataplane,
+                                  Error **errp);
 void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s);
 void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s);
 void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s);
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index e2f55cc..5fb006d 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -638,6 +638,7 @@ static void virtio_blk_migration_state_changed(Notifier *notifier, void *data)
     VirtIOBlock *s = container_of(notifier, VirtIOBlock,
                                   migration_state_notifier);
     MigrationState *mig = data;
+    Error *err = NULL;
 
     if (migration_in_setup(mig)) {
         if (!s->dataplane) {
@@ -652,7 +653,11 @@ static void virtio_blk_migration_state_changed(Notifier *notifier, void *data)
         }
         bdrv_drain_all(); /* complete in-flight non-dataplane requests */
         virtio_blk_data_plane_create(VIRTIO_DEVICE(s), &s->blk,
-                                     &s->dataplane);
+                                     &s->dataplane, &err);
+        if (err != NULL) {
+            error_report("%s", error_get_pretty(err));
+            error_free(err);
+        }
     }
 }
 #endif /* CONFIG_VIRTIO_BLK_DATA_PLANE */
@@ -662,6 +667,9 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
     DeviceState *qdev = DEVICE(vdev);
     VirtIOBlock *s = VIRTIO_BLK(vdev);
     VirtIOBlkConf *blk = &(s->blk);
+#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
+    Error *err = NULL;
+#endif
     static int virtio_blk_id;
 
     if (!blk->conf.bs) {
@@ -689,7 +697,10 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
 
     s->vq = virtio_add_queue(vdev, 128, virtio_blk_handle_output);
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
-    if (!virtio_blk_data_plane_create(vdev, blk, &s->dataplane)) {
+    virtio_blk_data_plane_create(vdev, blk, &s->dataplane, &err);
+    if (err != NULL) {
+        error_report("%s", error_get_pretty(err));
+        error_free(err);
         virtio_cleanup(vdev);
         return -1;
     }
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 05/22] virtio: Allow NULL VirtioDeviceClass::init hook
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (3 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 04/22] virtio-blk-dataplane: Improve error reporting Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 06/22] virtio-9p: QOM realize preparations Andreas Färber
                   ` (18 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Andreas Färber

This is a temporary measure to allow mixing VirtioDeviceClass::init with
DeviceClass::realize.

Clean up variable naming in preparation for QOM realize.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/virtio/virtio.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 09f62c6..c00c224 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1139,13 +1139,15 @@ void virtio_device_set_child_bus_name(VirtIODevice *vdev, char *bus_name)
     }
 }
 
-static int virtio_device_init(DeviceState *qdev)
+static int virtio_device_init(DeviceState *dev)
 {
-    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev);
-    assert(k->init != NULL);
-    if (k->init(vdev) < 0) {
-        return -1;
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(dev);
+
+    if (vdc->init != NULL) {
+        if (vdc->init(vdev) < 0) {
+            return -1;
+        }
     }
     virtio_bus_plug_device(vdev);
     return 0;
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 06/22] virtio-9p: QOM realize preparations
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (4 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 05/22] virtio: Allow NULL VirtioDeviceClass::init hook Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 07/22] virtio-9p: Convert to QOM realize Andreas Färber
                   ` (17 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Andreas Färber, Aneesh Kumar K.V

Avoid unnecessary VIRTIO_DEVICE().

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/9pfs/virtio-9p-device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index f0ffbe8..ea21655 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -49,7 +49,7 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
     FsDriverEntry *fse;
     V9fsPath path;
 
-    virtio_init(VIRTIO_DEVICE(s), "virtio-9p", VIRTIO_ID_9P,
+    virtio_init(vdev, "virtio-9p", VIRTIO_ID_9P,
                 sizeof(struct virtio_9p_config) + MAX_TAG_LEN);
 
     /* initialize pdu allocator */
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 07/22] virtio-9p: Convert to QOM realize
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (5 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 06/22] virtio-9p: QOM realize preparations Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 08/22] virtio-blk: QOM realize preparations Andreas Färber
                   ` (16 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Andreas Färber, Aneesh Kumar K.V

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/9pfs/virtio-9p-device.c | 44 +++++++++++++++++++++++---------------------
 hw/9pfs/virtio-9p.h        |  2 ++
 2 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index ea21655..7d2990b 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -41,9 +41,11 @@ static void virtio_9p_get_config(VirtIODevice *vdev, uint8_t *config)
     g_free(cfg);
 }
 
-static int virtio_9p_device_init(VirtIODevice *vdev)
+static void virtio_9p_device_realize(DeviceState *dev, Error **errp)
 {
-    V9fsState *s = VIRTIO_9P(vdev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    V9fsState *s = VIRTIO_9P(dev);
+    DeviceClass *parent_dc = DEVICE_CLASS(VIRTIO_9P_GET_PARENT_CLASS(dev));
     int i, len;
     struct stat stat;
     FsDriverEntry *fse;
@@ -67,16 +69,16 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
 
     if (!fse) {
         /* We don't have a fsdev identified by fsdev_id */
-        fprintf(stderr, "Virtio-9p device couldn't find fsdev with the "
-                "id = %s\n",
-                s->fsconf.fsdev_id ? s->fsconf.fsdev_id : "NULL");
+        error_setg(errp, "Virtio-9p device couldn't find fsdev with the "
+                   "id = %s",
+                   s->fsconf.fsdev_id ? s->fsconf.fsdev_id : "NULL");
         goto out;
     }
 
     if (!s->fsconf.tag) {
         /* we haven't specified a mount_tag */
-        fprintf(stderr, "fsdev with id %s needs mount_tag arguments\n",
-                s->fsconf.fsdev_id);
+        error_setg(errp, "fsdev with id %s needs mount_tag arguments",
+                   s->fsconf.fsdev_id);
         goto out;
     }
 
@@ -85,8 +87,8 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
     s->ctx.exops.get_st_gen = NULL;
     len = strlen(s->fsconf.tag);
     if (len > MAX_TAG_LEN - 1) {
-        fprintf(stderr, "mount tag '%s' (%d bytes) is longer than "
-                "maximum (%d bytes)", s->fsconf.tag, len, MAX_TAG_LEN - 1);
+        error_setg(errp, "mount tag '%s' (%d bytes) is longer than "
+                   "maximum (%d bytes)", s->fsconf.tag, len, MAX_TAG_LEN - 1);
         goto out;
     }
 
@@ -99,12 +101,12 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
     qemu_co_rwlock_init(&s->rename_lock);
 
     if (s->ops->init(&s->ctx) < 0) {
-        fprintf(stderr, "Virtio-9p Failed to initialize fs-driver with id:%s"
-                " and export path:%s\n", s->fsconf.fsdev_id, s->ctx.fs_root);
+        error_setg(errp, "Virtio-9p Failed to initialize fs-driver with id:%s"
+                   " and export path:%s", s->fsconf.fsdev_id, s->ctx.fs_root);
         goto out;
     }
     if (v9fs_init_worker_threads() < 0) {
-        fprintf(stderr, "worker thread initialization failed\n");
+        error_setg(errp, "worker thread initialization failed");
         goto out;
     }
 
@@ -114,28 +116,27 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
      * use co-routines here.
      */
     if (s->ops->name_to_path(&s->ctx, NULL, "/", &path) < 0) {
-        fprintf(stderr,
-                "error in converting name to path %s", strerror(errno));
+        error_setg(errp,
+                   "error in converting name to path %s", strerror(errno));
         goto out;
     }
     if (s->ops->lstat(&s->ctx, &path, &stat)) {
-        fprintf(stderr, "share path %s does not exist\n", fse->path);
+        error_setg(errp, "share path %s does not exist", fse->path);
         goto out;
     } else if (!S_ISDIR(stat.st_mode)) {
-        fprintf(stderr, "share path %s is not a directory\n", fse->path);
+        error_setg(errp, "share path %s is not a directory", fse->path);
         goto out;
     }
     v9fs_path_free(&path);
 
-    return 0;
+    parent_dc->realize(dev, errp);
+
+    return;
 out:
     g_free(s->ctx.fs_root);
     g_free(s->tag);
     virtio_cleanup(vdev);
     v9fs_path_free(&path);
-
-    return -1;
-
 }
 
 /* virtio-9p device */
@@ -149,9 +150,10 @@ static void virtio_9p_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+
+    dc->realize = virtio_9p_device_realize;
     dc->props = virtio_9p_properties;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-    vdc->init = virtio_9p_device_init;
     vdc->get_features = virtio_9p_get_features;
     vdc->get_config = virtio_9p_get_config;
 }
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 1d6eedb..51df6c8 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -404,6 +404,8 @@ extern int v9fs_name_to_path(V9fsState *s, V9fsPath *dirpath,
 #define TYPE_VIRTIO_9P "virtio-9p-device"
 #define VIRTIO_9P(obj) \
         OBJECT_CHECK(V9fsState, (obj), TYPE_VIRTIO_9P)
+#define VIRTIO_9P_GET_PARENT_CLASS(obj) \
+        OBJECT_GET_PARENT_CLASS(obj, TYPE_VIRTIO_9P)
 
 #define DEFINE_VIRTIO_9P_PROPERTIES(_state, _field)             \
         DEFINE_PROP_STRING("mount_tag", _state, _field.tag),    \
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 08/22] virtio-blk: QOM realize preparations
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (6 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 07/22] virtio-9p: Convert to QOM realize Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 09/22] virtio-blk: Convert to QOM realize Andreas Färber
                   ` (15 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Anthony Liguori, Andreas Färber, Stefan Hajnoczi

Rename variable qdev -> dev since that's what realize's argument is called
by convention.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/block/virtio-blk.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 5fb006d..9efc8b7 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -664,8 +664,8 @@ static void virtio_blk_migration_state_changed(Notifier *notifier, void *data)
 
 static int virtio_blk_device_init(VirtIODevice *vdev)
 {
-    DeviceState *qdev = DEVICE(vdev);
-    VirtIOBlock *s = VIRTIO_BLK(vdev);
+    DeviceState *dev = DEVICE(vdev);
+    VirtIOBlock *s = VIRTIO_BLK(dev);
     VirtIOBlkConf *blk = &(s->blk);
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
     Error *err = NULL;
@@ -709,14 +709,14 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
 #endif
 
     s->change = qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s);
-    register_savevm(qdev, "virtio-blk", virtio_blk_id++, 2,
+    register_savevm(dev, "virtio-blk", virtio_blk_id++, 2,
                     virtio_blk_save, virtio_blk_load, s);
     bdrv_set_dev_ops(s->bs, &virtio_block_ops, s);
     bdrv_set_buffer_alignment(s->bs, s->conf->logical_block_size);
 
     bdrv_iostatus_enable(s->bs);
 
-    add_boot_device_path(s->conf->bootindex, qdev, "/disk@0,0");
+    add_boot_device_path(s->conf->bootindex, dev, "/disk@0,0");
     return 0;
 }
 
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 09/22] virtio-blk: Convert to QOM realize
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (7 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 08/22] virtio-blk: QOM realize preparations Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 10/22] virtio-serial: QOM realize preparations Andreas Färber
                   ` (14 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Anthony Liguori, Andreas Färber, Stefan Hajnoczi

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/block/virtio-blk.c          | 32 +++++++++++++++++---------------
 include/hw/virtio/virtio-blk.h |  2 ++
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 9efc8b7..18fb020 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -662,28 +662,30 @@ static void virtio_blk_migration_state_changed(Notifier *notifier, void *data)
 }
 #endif /* CONFIG_VIRTIO_BLK_DATA_PLANE */
 
-static int virtio_blk_device_init(VirtIODevice *vdev)
+static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
 {
-    DeviceState *dev = DEVICE(vdev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOBlock *s = VIRTIO_BLK(dev);
     VirtIOBlkConf *blk = &(s->blk);
+    DeviceClass *parent_dc = DEVICE_CLASS(VIRTIO_BLK_GET_PARENT_CLASS(dev));
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
     Error *err = NULL;
 #endif
     static int virtio_blk_id;
 
     if (!blk->conf.bs) {
-        error_report("drive property not set");
-        return -1;
+        error_setg(errp, "drive property not set");
+        return;
     }
     if (!bdrv_is_inserted(blk->conf.bs)) {
-        error_report("Device needs media, but drive is empty");
-        return -1;
+        error_setg(errp, "Device needs media, but drive is empty");
+        return;
     }
 
     blkconf_serial(&blk->conf, &blk->serial);
     if (blkconf_geometry(&blk->conf, NULL, 65535, 255, 255) < 0) {
-        return -1;
+        error_setg(errp, "Error setting geometry");
+        return;
     }
 
     virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
@@ -699,10 +701,9 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
     virtio_blk_data_plane_create(vdev, blk, &s->dataplane, &err);
     if (err != NULL) {
-        error_report("%s", error_get_pretty(err));
-        error_free(err);
+        error_propagate(errp, err);
         virtio_cleanup(vdev);
-        return -1;
+        return;
     }
     s->migration_state_notifier.notify = virtio_blk_migration_state_changed;
     add_migration_state_change_notifier(&s->migration_state_notifier);
@@ -717,10 +718,11 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
     bdrv_iostatus_enable(s->bs);
 
     add_boot_device_path(s->conf->bootindex, dev, "/disk@0,0");
-    return 0;
+
+    parent_dc->realize(dev, errp);
 }
 
-static int virtio_blk_device_exit(DeviceState *dev)
+static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOBlock *s = VIRTIO_BLK(dev);
@@ -733,7 +735,6 @@ static int virtio_blk_device_exit(DeviceState *dev)
     unregister_savevm(dev, "virtio-blk", s);
     blockdev_mark_auto_del(s->bs);
     virtio_cleanup(vdev);
-    return 0;
 }
 
 static Property virtio_blk_properties[] = {
@@ -745,10 +746,11 @@ static void virtio_blk_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_blk_device_exit;
+
+    dc->realize = virtio_blk_device_realize;
+    dc->unrealize = virtio_blk_device_unrealize;
     dc->props = virtio_blk_properties;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-    vdc->init = virtio_blk_device_init;
     vdc->get_config = virtio_blk_update_config;
     vdc->set_config = virtio_blk_set_config;
     vdc->get_features = virtio_blk_get_features;
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index b87cf49..201531d 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -20,6 +20,8 @@
 #define TYPE_VIRTIO_BLK "virtio-blk-device"
 #define VIRTIO_BLK(obj) \
         OBJECT_CHECK(VirtIOBlock, (obj), TYPE_VIRTIO_BLK)
+#define VIRTIO_BLK_GET_PARENT_CLASS(obj) \
+        OBJECT_GET_PARENT_CLASS(obj, TYPE_VIRTIO_BLK)
 
 /* from Linux's linux/virtio_blk.h */
 
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 10/22] virtio-serial: QOM realize preparations
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (8 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 09/22] virtio-blk: Convert to QOM realize Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 11/22] virtio-serial: Convert to QOM realize Andreas Färber
                   ` (13 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Amit Shah, Anthony Liguori, Andreas Färber

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/char/virtio-serial-bus.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index da417c7..a82786b 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -891,8 +891,9 @@ static int virtser_port_qdev_exit(DeviceState *qdev)
 
 static int virtio_serial_device_init(VirtIODevice *vdev)
 {
-    DeviceState *qdev = DEVICE(vdev);
-    VirtIOSerial *vser = VIRTIO_SERIAL(vdev);
+    DeviceState *dev = DEVICE(vdev);
+    VirtIOSerial *vser = VIRTIO_SERIAL(dev);
+    BusState *bus;
     uint32_t i, max_supported_ports;
 
     if (!vser->serial.max_virtserial_ports) {
@@ -911,9 +912,10 @@ static int virtio_serial_device_init(VirtIODevice *vdev)
                 sizeof(struct virtio_console_config));
 
     /* Spawn a new virtio-serial bus on which the ports will ride as devices */
-    qbus_create_inplace(&vser->bus.qbus, TYPE_VIRTIO_SERIAL_BUS, qdev,
+    qbus_create_inplace(&vser->bus, TYPE_VIRTIO_SERIAL_BUS, dev,
                         vdev->bus_name);
-    vser->bus.qbus.allow_hotplug = 1;
+    bus = BUS(&vser->bus);
+    bus->allow_hotplug = 1;
     vser->bus.vser = vser;
     QTAILQ_INIT(&vser->ports);
 
@@ -961,7 +963,7 @@ static int virtio_serial_device_init(VirtIODevice *vdev)
      * Register for the savevm section with the virtio-console name
      * to preserve backward compat
      */
-    register_savevm(qdev, "virtio-console", -1, 3, virtio_serial_save,
+    register_savevm(dev, "virtio-console", -1, 3, virtio_serial_save,
                     virtio_serial_load, vser);
 
     return 0;
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 11/22] virtio-serial: Convert to QOM realize
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (9 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 10/22] virtio-serial: QOM realize preparations Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 12/22] virtio-net: QOM realize preparations Andreas Färber
                   ` (12 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Amit Shah, Anthony Liguori, Andreas Färber

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/char/virtio-serial-bus.c       | 25 ++++++++++++++-----------
 include/hw/virtio/virtio-serial.h |  2 ++
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index a82786b..1bed93f 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -889,23 +889,26 @@ static int virtser_port_qdev_exit(DeviceState *qdev)
     return 0;
 }
 
-static int virtio_serial_device_init(VirtIODevice *vdev)
+static void virtio_serial_device_realize(DeviceState *dev, Error **errp)
 {
-    DeviceState *dev = DEVICE(vdev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOSerial *vser = VIRTIO_SERIAL(dev);
+    ObjectClass *parent_oc = VIRTIO_SERIAL_GET_PARENT_CLASS(dev);
+    DeviceClass *parent_dc = DEVICE_CLASS(parent_oc);
     BusState *bus;
     uint32_t i, max_supported_ports;
 
     if (!vser->serial.max_virtserial_ports) {
-        return -1;
+        error_setg(errp, "Maximum number of serial ports not specified");
+        return;
     }
 
     /* Each port takes 2 queues, and one pair is for the control queue */
     max_supported_ports = VIRTIO_PCI_QUEUE_MAX / 2 - 1;
 
     if (vser->serial.max_virtserial_ports > max_supported_ports) {
-        error_report("maximum ports supported: %u", max_supported_ports);
-        return -1;
+        error_setg(errp, "maximum ports supported: %u", max_supported_ports);
+        return;
     }
 
     virtio_init(vdev, "virtio-serial", VIRTIO_ID_CONSOLE,
@@ -966,7 +969,7 @@ static int virtio_serial_device_init(VirtIODevice *vdev)
     register_savevm(dev, "virtio-console", -1, 3, virtio_serial_save,
                     virtio_serial_load, vser);
 
-    return 0;
+    parent_dc->realize(dev, errp);
 }
 
 static void virtio_serial_port_class_init(ObjectClass *klass, void *data)
@@ -989,10 +992,10 @@ static const TypeInfo virtio_serial_port_type_info = {
     .class_init = virtio_serial_port_class_init,
 };
 
-static int virtio_serial_device_exit(DeviceState *dev)
+static void virtio_serial_device_unrealize(DeviceState *dev, Error **errp)
 {
-    VirtIOSerial *vser = VIRTIO_SERIAL(dev);
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOSerial *vser = VIRTIO_SERIAL(dev);
 
     unregister_savevm(dev, "virtio-console", vser);
 
@@ -1006,7 +1009,6 @@ static int virtio_serial_device_exit(DeviceState *dev)
         g_free(vser->post_load);
     }
     virtio_cleanup(vdev);
-    return 0;
 }
 
 static Property virtio_serial_properties[] = {
@@ -1018,10 +1020,11 @@ static void virtio_serial_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_serial_device_exit;
+
+    dc->realize = virtio_serial_device_realize;
+    dc->unrealize = virtio_serial_device_unrealize;
     dc->props = virtio_serial_properties;
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
-    vdc->init = virtio_serial_device_init;
     vdc->get_features = get_features;
     vdc->get_config = get_config;
     vdc->set_config = set_config;
diff --git a/include/hw/virtio/virtio-serial.h b/include/hw/virtio/virtio-serial.h
index 1d2040b..9a94abd 100644
--- a/include/hw/virtio/virtio-serial.h
+++ b/include/hw/virtio/virtio-serial.h
@@ -247,6 +247,8 @@ void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle);
 #define TYPE_VIRTIO_SERIAL "virtio-serial-device"
 #define VIRTIO_SERIAL(obj) \
         OBJECT_CHECK(VirtIOSerial, (obj), TYPE_VIRTIO_SERIAL)
+#define VIRTIO_SERIAL_GET_PARENT_CLASS(obj) \
+        OBJECT_GET_PARENT_CLASS(obj, TYPE_VIRTIO_SERIAL)
 
 #define DEFINE_VIRTIO_SERIAL_PROPERTIES(_state, _field) \
         DEFINE_PROP_UINT32("max_ports", _state, _field.max_virtserial_ports, 31)
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 12/22] virtio-net: QOM realize preparations
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (10 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 11/22] virtio-serial: Convert to QOM realize Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 13/22] virtio-net: Convert to QOM realize Andreas Färber
                   ` (11 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Andreas Färber

Rename variable qdev -> dev since that's what realize's argument is
called by convention.

Avoid duplicate VIRTIO_DEVICE() cast.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/net/virtio-net.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index aa1880c..05ad605 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1486,14 +1486,12 @@ void virtio_net_set_netclient_name(VirtIONet *n, const char *name,
 
 static int virtio_net_device_init(VirtIODevice *vdev)
 {
-    int i;
-
-    DeviceState *qdev = DEVICE(vdev);
-    VirtIONet *n = VIRTIO_NET(vdev);
+    DeviceState *dev = DEVICE(vdev);
+    VirtIONet *n = VIRTIO_NET(dev);
     NetClientState *nc;
+    int i;
 
-    virtio_init(VIRTIO_DEVICE(n), "virtio-net", VIRTIO_ID_NET,
-                                  n->config_size);
+    virtio_init(vdev, "virtio-net", VIRTIO_ID_NET, n->config_size);
 
     n->max_queues = MAX(n->nic_conf.queues, 1);
     n->vqs = g_malloc0(sizeof(VirtIONetQueue) * n->max_queues);
@@ -1533,7 +1531,7 @@ static int virtio_net_device_init(VirtIODevice *vdev)
                               n->netclient_type, n->netclient_name, n);
     } else {
         n->nic = qemu_new_nic(&net_virtio_info, &n->nic_conf,
-                              object_get_typename(OBJECT(qdev)), qdev->id, n);
+                              object_get_typename(OBJECT(dev)), dev->id, n);
     }
 
     peer_test_vnet_hdr(n);
@@ -1560,24 +1558,24 @@ static int virtio_net_device_init(VirtIODevice *vdev)
     nc = qemu_get_queue(n->nic);
     nc->rxfilter_notify_enabled = 1;
 
-    n->qdev = qdev;
-    register_savevm(qdev, "virtio-net", -1, VIRTIO_NET_VM_VERSION,
+    n->qdev = dev;
+    register_savevm(dev, "virtio-net", -1, VIRTIO_NET_VM_VERSION,
                     virtio_net_save, virtio_net_load, n);
 
-    add_boot_device_path(n->nic_conf.bootindex, qdev, "/ethernet-phy@0");
+    add_boot_device_path(n->nic_conf.bootindex, dev, "/ethernet-phy@0");
     return 0;
 }
 
-static int virtio_net_device_exit(DeviceState *qdev)
+static int virtio_net_device_exit(DeviceState *dev)
 {
-    VirtIONet *n = VIRTIO_NET(qdev);
-    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIONet *n = VIRTIO_NET(dev);
     int i;
 
     /* This will stop vhost backend if appropriate. */
     virtio_net_set_status(vdev, 0);
 
-    unregister_savevm(qdev, "virtio-net", n);
+    unregister_savevm(dev, "virtio-net", n);
 
     if (n->netclient_name) {
         g_free(n->netclient_name);
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 13/22] virtio-net: Convert to QOM realize
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (11 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 12/22] virtio-net: QOM realize preparations Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 14/22] virtio-balloon: QOM realize preparations Andreas Färber
                   ` (10 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Andreas Färber

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/net/virtio-net.c            | 18 ++++++++++--------
 include/hw/virtio/virtio-net.h |  2 ++
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 05ad605..16c230f 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1484,10 +1484,12 @@ void virtio_net_set_netclient_name(VirtIONet *n, const char *name,
     n->netclient_type = g_strdup(type);
 }
 
-static int virtio_net_device_init(VirtIODevice *vdev)
+static void virtio_net_device_realize(DeviceState *dev, Error **errp)
 {
-    DeviceState *dev = DEVICE(vdev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIONet *n = VIRTIO_NET(dev);
+    ObjectClass *parent_oc = VIRTIO_NET_GET_PARENT_CLASS(dev);
+    DeviceClass *parent_dc = DEVICE_CLASS(parent_oc);
     NetClientState *nc;
     int i;
 
@@ -1563,10 +1565,11 @@ static int virtio_net_device_init(VirtIODevice *vdev)
                     virtio_net_save, virtio_net_load, n);
 
     add_boot_device_path(n->nic_conf.bootindex, dev, "/ethernet-phy@0");
-    return 0;
+
+    parent_dc->realize(dev, errp);
 }
 
-static int virtio_net_device_exit(DeviceState *dev)
+static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIONet *n = VIRTIO_NET(dev);
@@ -1606,8 +1609,6 @@ static int virtio_net_device_exit(DeviceState *dev)
     g_free(n->vqs);
     qemu_del_nic(n->nic);
     virtio_cleanup(vdev);
-
-    return 0;
 }
 
 static void virtio_net_instance_init(Object *obj)
@@ -1634,10 +1635,11 @@ static void virtio_net_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_net_device_exit;
+
+    dc->realize = virtio_net_device_realize;
+    dc->unrealize = virtio_net_device_unrealize;
     dc->props = virtio_net_properties;
     set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
-    vdc->init = virtio_net_device_init;
     vdc->get_config = virtio_net_get_config;
     vdc->set_config = virtio_net_set_config;
     vdc->get_features = virtio_net_get_features;
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index df60f16..f1efed2 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -20,6 +20,8 @@
 #define TYPE_VIRTIO_NET "virtio-net-device"
 #define VIRTIO_NET(obj) \
         OBJECT_CHECK(VirtIONet, (obj), TYPE_VIRTIO_NET)
+#define VIRTIO_NET_GET_PARENT_CLASS(obj) \
+        OBJECT_GET_PARENT_CLASS(obj, TYPE_VIRTIO_NET)
 
 #define ETH_ALEN    6
 
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 14/22] virtio-balloon: QOM realize preparations
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (12 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 13/22] virtio-net: Convert to QOM realize Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 15/22] virtio-balloon: Convert to QOM realize Andreas Färber
                   ` (9 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Andreas Färber

Rename qdev -> dev since that's what realize's argument is called by
convention. No need to keep more "qdev" around than necessary.

Avoid duplicate VIRTIO_DEVICE() cast.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/virtio/virtio-balloon.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index aac7f83..3bab5db 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -339,8 +339,8 @@ static int virtio_balloon_load(QEMUFile *f, void *opaque, int version_id)
 
 static int virtio_balloon_device_init(VirtIODevice *vdev)
 {
-    DeviceState *qdev = DEVICE(vdev);
-    VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
+    DeviceState *dev = DEVICE(vdev);
+    VirtIOBalloon *s = VIRTIO_BALLOON(dev);
     int ret;
 
     virtio_init(vdev, "virtio-balloon", VIRTIO_ID_BALLOON, 8);
@@ -349,7 +349,7 @@ static int virtio_balloon_device_init(VirtIODevice *vdev)
                                    virtio_balloon_stat, s);
 
     if (ret < 0) {
-        virtio_cleanup(VIRTIO_DEVICE(s));
+        virtio_cleanup(vdev);
         return -1;
     }
 
@@ -357,27 +357,27 @@ static int virtio_balloon_device_init(VirtIODevice *vdev)
     s->dvq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
     s->svq = virtio_add_queue(vdev, 128, virtio_balloon_receive_stats);
 
-    register_savevm(qdev, "virtio-balloon", -1, 1,
+    register_savevm(dev, "virtio-balloon", -1, 1,
                     virtio_balloon_save, virtio_balloon_load, s);
 
-    object_property_add(OBJECT(qdev), "guest-stats", "guest statistics",
+    object_property_add(OBJECT(dev), "guest-stats", "guest statistics",
                         balloon_stats_get_all, NULL, NULL, s, NULL);
 
-    object_property_add(OBJECT(qdev), "guest-stats-polling-interval", "int",
+    object_property_add(OBJECT(dev), "guest-stats-polling-interval", "int",
                         balloon_stats_get_poll_interval,
                         balloon_stats_set_poll_interval,
                         NULL, s, NULL);
     return 0;
 }
 
-static int virtio_balloon_device_exit(DeviceState *qdev)
+static int virtio_balloon_device_exit(DeviceState *dev)
 {
-    VirtIOBalloon *s = VIRTIO_BALLOON(qdev);
-    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+    VirtIOBalloon *s = VIRTIO_BALLOON(dev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
 
     balloon_stats_destroy_timer(s);
     qemu_remove_balloon_handler(s);
-    unregister_savevm(qdev, "virtio-balloon", s);
+    unregister_savevm(dev, "virtio-balloon", s);
     virtio_cleanup(vdev);
     return 0;
 }
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 15/22] virtio-balloon: Convert to QOM realize
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (13 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 14/22] virtio-balloon: QOM realize preparations Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 16/22] virtio-rng: QOM realize preparations Andreas Färber
                   ` (8 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Andreas Färber

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/virtio/virtio-balloon.c         | 22 +++++++++++++---------
 include/hw/virtio/virtio-balloon.h |  2 ++
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 3bab5db..9b65732 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -337,10 +337,12 @@ static int virtio_balloon_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
-static int virtio_balloon_device_init(VirtIODevice *vdev)
+static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
 {
-    DeviceState *dev = DEVICE(vdev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOBalloon *s = VIRTIO_BALLOON(dev);
+    ObjectClass *parent_oc = VIRTIO_BALLOON_GET_PARENT_CLASS(dev);
+    DeviceClass *parent_dc = DEVICE_CLASS(parent_oc);
     int ret;
 
     virtio_init(vdev, "virtio-balloon", VIRTIO_ID_BALLOON, 8);
@@ -349,8 +351,9 @@ static int virtio_balloon_device_init(VirtIODevice *vdev)
                                    virtio_balloon_stat, s);
 
     if (ret < 0) {
+        error_setg(errp, "Adding balloon handler failed");
         virtio_cleanup(vdev);
-        return -1;
+        return;
     }
 
     s->ivq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
@@ -367,19 +370,19 @@ static int virtio_balloon_device_init(VirtIODevice *vdev)
                         balloon_stats_get_poll_interval,
                         balloon_stats_set_poll_interval,
                         NULL, s, NULL);
-    return 0;
+
+    parent_dc->realize(dev, errp);
 }
 
-static int virtio_balloon_device_exit(DeviceState *dev)
+static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
 {
-    VirtIOBalloon *s = VIRTIO_BALLOON(dev);
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOBalloon *s = VIRTIO_BALLOON(dev);
 
     balloon_stats_destroy_timer(s);
     qemu_remove_balloon_handler(s);
     unregister_savevm(dev, "virtio-balloon", s);
     virtio_cleanup(vdev);
-    return 0;
 }
 
 static Property virtio_balloon_properties[] = {
@@ -390,10 +393,11 @@ static void virtio_balloon_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_balloon_device_exit;
+
+    dc->realize = virtio_balloon_device_realize;
+    dc->unrealize = virtio_balloon_device_unrealize;
     dc->props = virtio_balloon_properties;
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
-    vdc->init = virtio_balloon_device_init;
     vdc->get_config = virtio_balloon_get_config;
     vdc->set_config = virtio_balloon_set_config;
     vdc->get_features = virtio_balloon_get_features;
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index f863bfe..bf3846c 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -21,6 +21,8 @@
 #define TYPE_VIRTIO_BALLOON "virtio-balloon-device"
 #define VIRTIO_BALLOON(obj) \
         OBJECT_CHECK(VirtIOBalloon, (obj), TYPE_VIRTIO_BALLOON)
+#define VIRTIO_BALLOON_GET_PARENT_CLASS(obj) \
+        OBJECT_GET_PARENT_CLASS(obj, TYPE_VIRTIO_BALLOON)
 
 /* from Linux's linux/virtio_balloon.h */
 
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 16/22] virtio-rng: QOM realize preparations
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (14 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 15/22] virtio-balloon: Convert to QOM realize Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 17/22] virtio-rng: Convert to QOM realize Andreas Färber
                   ` (7 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Andreas Färber

Rename qdev -> dev because that's what realize's argument is called by
convention. No need to keep more "qdev" around than necessary.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/virtio/virtio-rng.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index bac8421..0accc9e 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -135,19 +135,19 @@ static void check_rate_limit(void *opaque)
 
 static int virtio_rng_device_init(VirtIODevice *vdev)
 {
-    DeviceState *qdev = DEVICE(vdev);
-    VirtIORNG *vrng = VIRTIO_RNG(vdev);
+    DeviceState *dev = DEVICE(vdev);
+    VirtIORNG *vrng = VIRTIO_RNG(dev);
     Error *local_err = NULL;
 
     if (vrng->conf.rng == NULL) {
         vrng->conf.default_backend = RNG_RANDOM(object_new(TYPE_RNG_RANDOM));
 
-        object_property_add_child(OBJECT(qdev),
+        object_property_add_child(OBJECT(dev),
                                   "default-backend",
                                   OBJECT(vrng->conf.default_backend),
                                   NULL);
 
-        object_property_set_link(OBJECT(qdev),
+        object_property_set_link(OBJECT(dev),
                                  OBJECT(vrng->conf.default_backend),
                                  "rng", NULL);
     }
@@ -178,20 +178,20 @@ static int virtio_rng_device_init(VirtIODevice *vdev)
     qemu_mod_timer(vrng->rate_limit_timer,
                    qemu_get_clock_ms(vm_clock) + vrng->conf.period_ms);
 
-    register_savevm(qdev, "virtio-rng", -1, 1, virtio_rng_save,
+    register_savevm(dev, "virtio-rng", -1, 1, virtio_rng_save,
                     virtio_rng_load, vrng);
 
     return 0;
 }
 
-static int virtio_rng_device_exit(DeviceState *qdev)
+static int virtio_rng_device_exit(DeviceState *dev)
 {
-    VirtIORNG *vrng = VIRTIO_RNG(qdev);
-    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+    VirtIORNG *vrng = VIRTIO_RNG(dev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
 
     qemu_del_timer(vrng->rate_limit_timer);
     qemu_free_timer(vrng->rate_limit_timer);
-    unregister_savevm(qdev, "virtio-rng", vrng);
+    unregister_savevm(dev, "virtio-rng", vrng);
     virtio_cleanup(vdev);
     return 0;
 }
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 17/22] virtio-rng: Convert to QOM realize
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (15 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 16/22] virtio-rng: QOM realize preparations Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 18/22] virtio-scsi: QOM realize preparations Andreas Färber
                   ` (6 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Andreas Färber

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/virtio/virtio-rng.c         | 27 ++++++++++++++-------------
 include/hw/virtio/virtio-rng.h |  2 ++
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index 0accc9e..7c4526a 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -133,10 +133,12 @@ static void check_rate_limit(void *opaque)
                    qemu_get_clock_ms(vm_clock) + vrng->conf.period_ms);
 }
 
-static int virtio_rng_device_init(VirtIODevice *vdev)
+static void virtio_rng_device_realize(DeviceState *dev, Error **errp)
 {
-    DeviceState *dev = DEVICE(vdev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIORNG *vrng = VIRTIO_RNG(dev);
+    ObjectClass *parent_oc = VIRTIO_RNG_GET_PARENT_CLASS(dev);
+    DeviceClass *parent_dc = DEVICE_CLASS(parent_oc);
     Error *local_err = NULL;
 
     if (vrng->conf.rng == NULL) {
@@ -156,15 +158,14 @@ static int virtio_rng_device_init(VirtIODevice *vdev)
 
     vrng->rng = vrng->conf.rng;
     if (vrng->rng == NULL) {
-        qerror_report(QERR_INVALID_PARAMETER_VALUE, "rng", "a valid object");
-        return -1;
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "rng", "a valid object");
+        return;
     }
 
     rng_backend_open(vrng->rng, &local_err);
     if (local_err) {
-        qerror_report_err(local_err);
-        error_free(local_err);
-        return -1;
+        error_propagate(errp, local_err);
+        return;
     }
 
     vrng->vq = virtio_add_queue(vdev, 8, handle_input);
@@ -181,19 +182,18 @@ static int virtio_rng_device_init(VirtIODevice *vdev)
     register_savevm(dev, "virtio-rng", -1, 1, virtio_rng_save,
                     virtio_rng_load, vrng);
 
-    return 0;
+    parent_dc->realize(dev, errp);
 }
 
-static int virtio_rng_device_exit(DeviceState *dev)
+static void virtio_rng_device_unrealize(DeviceState *dev, Error **errp)
 {
-    VirtIORNG *vrng = VIRTIO_RNG(dev);
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIORNG *vrng = VIRTIO_RNG(dev);
 
     qemu_del_timer(vrng->rate_limit_timer);
     qemu_free_timer(vrng->rate_limit_timer);
     unregister_savevm(dev, "virtio-rng", vrng);
     virtio_cleanup(vdev);
-    return 0;
 }
 
 static Property virtio_rng_properties[] = {
@@ -205,10 +205,11 @@ static void virtio_rng_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_rng_device_exit;
+
+    dc->realize = virtio_rng_device_realize;
+    dc->unrealize = virtio_rng_device_unrealize;
     dc->props = virtio_rng_properties;
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
-    vdc->init = virtio_rng_device_init;
     vdc->get_features = get_features;
 }
 
diff --git a/include/hw/virtio/virtio-rng.h b/include/hw/virtio/virtio-rng.h
index debaa15..14e85a5 100644
--- a/include/hw/virtio/virtio-rng.h
+++ b/include/hw/virtio/virtio-rng.h
@@ -18,6 +18,8 @@
 #define TYPE_VIRTIO_RNG "virtio-rng-device"
 #define VIRTIO_RNG(obj) \
         OBJECT_CHECK(VirtIORNG, (obj), TYPE_VIRTIO_RNG)
+#define VIRTIO_RNG_GET_PARENT_CLASS(obj) \
+        OBJECT_GET_PARENT_CLASS(obj, TYPE_VIRTIO_RNG)
 
 /* The Virtio ID for the virtio rng device */
 #define VIRTIO_ID_RNG    4
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 18/22] virtio-scsi: QOM realize preparations
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (16 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 17/22] virtio-rng: Convert to QOM realize Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 19/22] virtio-scsi: Convert to QOM realize Andreas Färber
                   ` (5 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Anthony Liguori, Andreas Färber, Michael S. Tsirkin

Rename qdev -> dev since that's what realize's argument is called by
convention. No need to keep more "qdev" around than necessary.

Avoid duplicate VIRTIO_DEVICE() cast.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/scsi/vhost-scsi.c  |  8 ++++----
 hw/scsi/virtio-scsi.c | 22 +++++++++++-----------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 9e770fb..72a726d 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -240,11 +240,11 @@ static int vhost_scsi_init(VirtIODevice *vdev)
     return 0;
 }
 
-static int vhost_scsi_exit(DeviceState *qdev)
+static int vhost_scsi_exit(DeviceState *dev)
 {
-    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
-    VHostSCSI *s = VHOST_SCSI(qdev);
-    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(qdev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VHostSCSI *s = VHOST_SCSI(dev);
+    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
 
     migrate_del_blocker(s->migration_blocker);
     error_free(s->migration_blocker);
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 05da56b..1d8869d 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -594,7 +594,7 @@ int virtio_scsi_common_init(VirtIOSCSICommon *s)
     VirtIODevice *vdev = VIRTIO_DEVICE(s);
     int i;
 
-    virtio_init(VIRTIO_DEVICE(s), "virtio-scsi", VIRTIO_ID_SCSI,
+    virtio_init(vdev, "virtio-scsi", VIRTIO_ID_SCSI,
                 sizeof(VirtIOSCSIConfig));
 
     s->cmd_vqs = g_malloc0(s->conf.num_queues * sizeof(VirtQueue *));
@@ -615,9 +615,9 @@ int virtio_scsi_common_init(VirtIOSCSICommon *s)
 
 static int virtio_scsi_device_init(VirtIODevice *vdev)
 {
-    DeviceState *qdev = DEVICE(vdev);
-    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
-    VirtIOSCSI *s = VIRTIO_SCSI(vdev);
+    DeviceState *dev = DEVICE(vdev);
+    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
+    VirtIOSCSI *s = VIRTIO_SCSI(dev);
     static int virtio_scsi_id;
     Error *err = NULL;
     int ret;
@@ -627,9 +627,9 @@ static int virtio_scsi_device_init(VirtIODevice *vdev)
         return ret;
     }
 
-    scsi_bus_new(&s->bus, qdev, &virtio_scsi_scsi_info, vdev->bus_name);
+    scsi_bus_new(&s->bus, dev, &virtio_scsi_scsi_info, vdev->bus_name);
 
-    if (!qdev->hotplugged) {
+    if (!dev->hotplugged) {
         scsi_bus_legacy_handle_cmdline(&s->bus, &err);
         if (err != NULL) {
             error_free(err);
@@ -637,7 +637,7 @@ static int virtio_scsi_device_init(VirtIODevice *vdev)
         }
     }
 
-    register_savevm(qdev, "virtio-scsi", virtio_scsi_id++, 1,
+    register_savevm(dev, "virtio-scsi", virtio_scsi_id++, 1,
                     virtio_scsi_save, virtio_scsi_load, s);
 
     return 0;
@@ -652,12 +652,12 @@ int virtio_scsi_common_exit(VirtIOSCSICommon *vs)
     return 0;
 }
 
-static int virtio_scsi_device_exit(DeviceState *qdev)
+static int virtio_scsi_device_exit(DeviceState *dev)
 {
-    VirtIOSCSI *s = VIRTIO_SCSI(qdev);
-    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(qdev);
+    VirtIOSCSI *s = VIRTIO_SCSI(dev);
+    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
 
-    unregister_savevm(qdev, "virtio-scsi", s);
+    unregister_savevm(dev, "virtio-scsi", s);
     return virtio_scsi_common_exit(vs);
 }
 
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 19/22] virtio-scsi: Convert to QOM realize
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (17 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 18/22] virtio-scsi: QOM realize preparations Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 20/22] virtio: Convert VirtioDevice " Andreas Färber
                   ` (4 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Anthony Liguori, Andreas Färber, Michael S. Tsirkin

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/scsi/vhost-scsi.c            | 43 ++++++++++++++++++++++-----------------
 hw/scsi/virtio-scsi.c           | 45 +++++++++++++++++++++--------------------
 include/hw/virtio/vhost-scsi.h  |  2 ++
 include/hw/virtio/virtio-scsi.h |  6 ++++--
 4 files changed, 53 insertions(+), 43 deletions(-)

diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 72a726d..6e495b8 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -196,29 +196,33 @@ static void vhost_scsi_set_status(VirtIODevice *vdev, uint8_t val)
     }
 }
 
-static int vhost_scsi_init(VirtIODevice *vdev)
+static void vhost_scsi_realize(DeviceState *dev, Error **errp)
 {
-    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
-    VHostSCSI *s = VHOST_SCSI(vdev);
+    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
+    VHostSCSI *s = VHOST_SCSI(dev);
+    ObjectClass *parent_oc = VHOST_SCSI_GET_PARENT_CLASS(dev);
+    DeviceClass *parent_dc = DEVICE_CLASS(parent_oc);
+    Error *err = NULL;
     int vhostfd = -1;
     int ret;
 
     if (!vs->conf.wwpn) {
-        error_report("vhost-scsi: missing wwpn\n");
-        return -EINVAL;
+        error_setg(errp, "vhost-scsi: missing wwpn");
+        return;
     }
 
     if (vs->conf.vhostfd) {
         vhostfd = monitor_handle_fd_param(cur_mon, vs->conf.vhostfd);
         if (vhostfd == -1) {
-            error_report("vhost-scsi: unable to parse vhostfd\n");
-            return -EINVAL;
+            error_setg(errp, "vhost-scsi: unable to parse vhostfd");
+            return;
         }
     }
 
-    ret = virtio_scsi_common_init(vs);
-    if (ret < 0) {
-        return ret;
+    virtio_scsi_common_realize(dev, &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
     }
 
     s->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
@@ -227,9 +231,9 @@ static int vhost_scsi_init(VirtIODevice *vdev)
 
     ret = vhost_dev_init(&s->dev, vhostfd, "/dev/vhost-scsi", true);
     if (ret < 0) {
-        error_report("vhost-scsi: vhost initialization failed: %s\n",
-                strerror(-ret));
-        return ret;
+        error_setg(errp, "vhost-scsi: vhost initialization failed: %s",
+                   strerror(-ret));
+        return;
     }
     s->dev.backend_features = 0;
 
@@ -237,14 +241,13 @@ static int vhost_scsi_init(VirtIODevice *vdev)
             "vhost-scsi does not support migration");
     migrate_add_blocker(s->migration_blocker);
 
-    return 0;
+    parent_dc->realize(dev, errp);
 }
 
-static int vhost_scsi_exit(DeviceState *dev)
+static void vhost_scsi_unrealize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VHostSCSI *s = VHOST_SCSI(dev);
-    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
 
     migrate_del_blocker(s->migration_blocker);
     error_free(s->migration_blocker);
@@ -253,7 +256,8 @@ static int vhost_scsi_exit(DeviceState *dev)
     vhost_scsi_set_status(vdev, 0);
 
     g_free(s->dev.vqs);
-    return virtio_scsi_common_exit(vs);
+
+    virtio_scsi_common_unrealize(dev, errp);
 }
 
 static Property vhost_scsi_properties[] = {
@@ -265,10 +269,11 @@ static void vhost_scsi_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = vhost_scsi_exit;
+
+    dc->realize = vhost_scsi_realize;
+    dc->unrealize = vhost_scsi_unrealize;
     dc->props = vhost_scsi_properties;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-    vdc->init = vhost_scsi_init;
     vdc->get_features = vhost_scsi_get_features;
     vdc->set_config = vhost_scsi_set_config;
     vdc->set_status = vhost_scsi_set_status;
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 1d8869d..9bddd3a 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -589,9 +589,10 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
     .load_request = virtio_scsi_load_request,
 };
 
-int virtio_scsi_common_init(VirtIOSCSICommon *s)
+void virtio_scsi_common_realize(DeviceState *dev, Error **errp)
 {
-    VirtIODevice *vdev = VIRTIO_DEVICE(s);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOSCSICommon *s = VIRTIO_SCSI_COMMON(dev);
     int i;
 
     virtio_init(vdev, "virtio-scsi", VIRTIO_ID_SCSI,
@@ -609,22 +610,21 @@ int virtio_scsi_common_init(VirtIOSCSICommon *s)
         s->cmd_vqs[i] = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE,
                                          virtio_scsi_handle_cmd);
     }
-
-    return 0;
 }
 
-static int virtio_scsi_device_init(VirtIODevice *vdev)
+static void virtio_scsi_device_realize(DeviceState *dev, Error **errp)
 {
-    DeviceState *dev = DEVICE(vdev);
-    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOSCSI *s = VIRTIO_SCSI(dev);
+    ObjectClass *parent_oc = VIRTIO_SCSI_GET_PARENT_CLASS(dev);
+    DeviceClass *parent_dc = DEVICE_CLASS(parent_oc);
     static int virtio_scsi_id;
     Error *err = NULL;
-    int ret;
 
-    ret = virtio_scsi_common_init(vs);
-    if (ret < 0) {
-        return ret;
+    virtio_scsi_common_realize(dev, &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
     }
 
     scsi_bus_new(&s->bus, dev, &virtio_scsi_scsi_info, vdev->bus_name);
@@ -632,33 +632,33 @@ static int virtio_scsi_device_init(VirtIODevice *vdev)
     if (!dev->hotplugged) {
         scsi_bus_legacy_handle_cmdline(&s->bus, &err);
         if (err != NULL) {
-            error_free(err);
-            return -1;
+            error_propagate(errp, err);
+            return;
         }
     }
 
     register_savevm(dev, "virtio-scsi", virtio_scsi_id++, 1,
                     virtio_scsi_save, virtio_scsi_load, s);
 
-    return 0;
+    parent_dc->realize(dev, errp);
 }
 
-int virtio_scsi_common_exit(VirtIOSCSICommon *vs)
+void virtio_scsi_common_unrealize(DeviceState *dev, Error **errp)
 {
-    VirtIODevice *vdev = VIRTIO_DEVICE(vs);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
 
     g_free(vs->cmd_vqs);
     virtio_cleanup(vdev);
-    return 0;
 }
 
-static int virtio_scsi_device_exit(DeviceState *dev)
+static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp)
 {
     VirtIOSCSI *s = VIRTIO_SCSI(dev);
-    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
 
     unregister_savevm(dev, "virtio-scsi", s);
-    return virtio_scsi_common_exit(vs);
+
+    virtio_scsi_common_unrealize(dev, errp);
 }
 
 static Property virtio_scsi_properties[] = {
@@ -679,10 +679,11 @@ static void virtio_scsi_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_scsi_device_exit;
+
+    dc->realize = virtio_scsi_device_realize;
+    dc->unrealize = virtio_scsi_device_unrealize;
     dc->props = virtio_scsi_properties;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-    vdc->init = virtio_scsi_device_init;
     vdc->set_config = virtio_scsi_set_config;
     vdc->get_features = virtio_scsi_get_features;
     vdc->reset = virtio_scsi_reset;
diff --git a/include/hw/virtio/vhost-scsi.h b/include/hw/virtio/vhost-scsi.h
index 85cc031..c034bc5 100644
--- a/include/hw/virtio/vhost-scsi.h
+++ b/include/hw/virtio/vhost-scsi.h
@@ -53,6 +53,8 @@ enum vhost_scsi_vq_list {
 #define TYPE_VHOST_SCSI "vhost-scsi"
 #define VHOST_SCSI(obj) \
         OBJECT_CHECK(VHostSCSI, (obj), TYPE_VHOST_SCSI)
+#define VHOST_SCSI_GET_PARENT_CLASS(obj) \
+        OBJECT_GET_PARENT_CLASS(obj, TYPE_VHOST_SCSI)
 
 typedef struct VHostSCSI {
     VirtIOSCSICommon parent_obj;
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 9a98540..c56292b 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -25,6 +25,8 @@
 #define TYPE_VIRTIO_SCSI "virtio-scsi-device"
 #define VIRTIO_SCSI(obj) \
         OBJECT_CHECK(VirtIOSCSI, (obj), TYPE_VIRTIO_SCSI)
+#define VIRTIO_SCSI_GET_PARENT_CLASS(obj) \
+        OBJECT_GET_PARENT_CLASS(obj, TYPE_VIRTIO_SCSI)
 
 
 /* The ID for virtio_scsi */
@@ -186,7 +188,7 @@ typedef struct {
     DEFINE_PROP_BIT("param_change", _state, _feature_field,                    \
                                             VIRTIO_SCSI_F_CHANGE, true)
 
-int virtio_scsi_common_init(VirtIOSCSICommon *vs);
-int virtio_scsi_common_exit(VirtIOSCSICommon *vs);
+void virtio_scsi_common_realize(DeviceState *dev, Error **errp);
+void virtio_scsi_common_unrealize(DeviceState *dev, Error **errp);
 
 #endif /* _QEMU_VIRTIO_SCSI_H */
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 20/22] virtio: Convert VirtioDevice to QOM realize
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (18 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 19/22] virtio-scsi: Convert to QOM realize Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 21/22] virtio: Unrealize parent Andreas Färber
                   ` (3 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Andreas Färber

Drop VirtioDevice::init.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/virtio/virtio.c         | 18 +++++-------------
 include/hw/virtio/virtio.h |  6 ++++--
 2 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index c00c224..8f19095 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1139,37 +1139,29 @@ void virtio_device_set_child_bus_name(VirtIODevice *vdev, char *bus_name)
     }
 }
 
-static int virtio_device_init(DeviceState *dev)
+static void virtio_device_realize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(dev);
 
-    if (vdc->init != NULL) {
-        if (vdc->init(vdev) < 0) {
-            return -1;
-        }
-    }
     virtio_bus_plug_device(vdev);
-    return 0;
 }
 
-static int virtio_device_exit(DeviceState *qdev)
+static void virtio_device_unrealize(DeviceState *dev, Error **errp)
 {
-    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
 
     if (vdev->bus_name) {
         g_free(vdev->bus_name);
         vdev->bus_name = NULL;
     }
-    return 0;
 }
 
 static void virtio_device_class_init(ObjectClass *klass, void *data)
 {
     /* Set the default value here. */
     DeviceClass *dc = DEVICE_CLASS(klass);
-    dc->init = virtio_device_init;
-    dc->exit = virtio_device_exit;
+    dc->realize = virtio_device_realize;
+    dc->unrealize = virtio_device_unrealize;
     dc->bus_type = TYPE_VIRTIO_BUS;
 }
 
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index d7e9e0f..89b1793 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -124,9 +124,11 @@ struct VirtIODevice
 };
 
 typedef struct VirtioDeviceClass {
-    /* This is what a VirtioDevice must implement */
+    /*< private >*/
     DeviceClass parent;
-    int (*init)(VirtIODevice *vdev);
+    /*< public >*/
+
+    /* This is what a VirtioDevice must implement */
     uint32_t (*get_features)(VirtIODevice *vdev, uint32_t requested_features);
     uint32_t (*bad_features)(VirtIODevice *vdev);
     void (*set_features)(VirtIODevice *vdev, uint32_t val);
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH for-next v2 21/22] virtio: Unrealize parent
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (19 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 20/22] virtio: Convert VirtioDevice " Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:17 ` [Qemu-devel] [PATCH RFC for-next v2 22/22] virtio-scsi: Convert virtio_scsi_common_[un]realize to QOM realize Andreas Färber
                   ` (2 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Anthony Liguori, Michael S. Tsirkin, Stefan Hajnoczi,
	Amit Shah, Paolo Bonzini, Andreas Färber

VirtioDevice's DeviceClass::exit code cleaning up bus_name was
overwritten by virtio-{blk,serial,net,scsi,balloon,rng} and vhost-scsi.

Call their parent's DeviceClass::unrealize to fix this.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/block/virtio-blk.c       | 4 ++++
 hw/char/virtio-serial-bus.c | 4 ++++
 hw/net/virtio-net.c         | 4 ++++
 hw/scsi/vhost-scsi.c        | 4 ++++
 hw/scsi/virtio-scsi.c       | 4 ++++
 hw/virtio/virtio-balloon.c  | 4 ++++
 hw/virtio/virtio-rng.c      | 4 ++++
 7 files changed, 28 insertions(+)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 18fb020..0e45370 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -726,6 +726,8 @@ static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOBlock *s = VIRTIO_BLK(dev);
+    DeviceClass *parent_dc = DEVICE_CLASS(VIRTIO_BLK_GET_PARENT_CLASS(dev));
+
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
     remove_migration_state_change_notifier(&s->migration_state_notifier);
     virtio_blk_data_plane_destroy(s->dataplane);
@@ -735,6 +737,8 @@ static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp)
     unregister_savevm(dev, "virtio-blk", s);
     blockdev_mark_auto_del(s->bs);
     virtio_cleanup(vdev);
+
+    parent_dc->unrealize(dev, errp);
 }
 
 static Property virtio_blk_properties[] = {
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 1bed93f..ba221bf 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -996,6 +996,8 @@ static void virtio_serial_device_unrealize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOSerial *vser = VIRTIO_SERIAL(dev);
+    ObjectClass *parent_oc = VIRTIO_SERIAL_GET_PARENT_CLASS(dev);
+    DeviceClass *parent_dc = DEVICE_CLASS(parent_oc);
 
     unregister_savevm(dev, "virtio-console", vser);
 
@@ -1009,6 +1011,8 @@ static void virtio_serial_device_unrealize(DeviceState *dev, Error **errp)
         g_free(vser->post_load);
     }
     virtio_cleanup(vdev);
+
+    parent_dc->unrealize(dev, errp);
 }
 
 static Property virtio_serial_properties[] = {
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 16c230f..55d404e 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1573,6 +1573,8 @@ static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIONet *n = VIRTIO_NET(dev);
+    ObjectClass *parent_oc = VIRTIO_NET_GET_PARENT_CLASS(dev);
+    DeviceClass *parent_dc = DEVICE_CLASS(parent_oc);
     int i;
 
     /* This will stop vhost backend if appropriate. */
@@ -1609,6 +1611,8 @@ static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
     g_free(n->vqs);
     qemu_del_nic(n->nic);
     virtio_cleanup(vdev);
+
+    parent_dc->unrealize(dev, errp);
 }
 
 static void virtio_net_instance_init(Object *obj)
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 6e495b8..ad7cc9b 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -248,6 +248,8 @@ static void vhost_scsi_unrealize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VHostSCSI *s = VHOST_SCSI(dev);
+    ObjectClass *parent_oc = VHOST_SCSI_GET_PARENT_CLASS(dev);
+    DeviceClass *parent_dc = DEVICE_CLASS(parent_oc);
 
     migrate_del_blocker(s->migration_blocker);
     error_free(s->migration_blocker);
@@ -258,6 +260,8 @@ static void vhost_scsi_unrealize(DeviceState *dev, Error **errp)
     g_free(s->dev.vqs);
 
     virtio_scsi_common_unrealize(dev, errp);
+
+    parent_dc->unrealize(dev, errp);
 }
 
 static Property vhost_scsi_properties[] = {
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 9bddd3a..ac0ecdf 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -655,10 +655,14 @@ void virtio_scsi_common_unrealize(DeviceState *dev, Error **errp)
 static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp)
 {
     VirtIOSCSI *s = VIRTIO_SCSI(dev);
+    ObjectClass *parent_oc = VIRTIO_SCSI_GET_PARENT_CLASS(dev);
+    DeviceClass *parent_dc = DEVICE_CLASS(parent_oc);
 
     unregister_savevm(dev, "virtio-scsi", s);
 
     virtio_scsi_common_unrealize(dev, errp);
+
+    parent_dc->unrealize(dev, errp);
 }
 
 static Property virtio_scsi_properties[] = {
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 9b65732..861dc3d 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -378,11 +378,15 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOBalloon *s = VIRTIO_BALLOON(dev);
+    ObjectClass *parent_oc = VIRTIO_BALLOON_GET_PARENT_CLASS(dev);
+    DeviceClass *parent_dc = DEVICE_CLASS(parent_oc);
 
     balloon_stats_destroy_timer(s);
     qemu_remove_balloon_handler(s);
     unregister_savevm(dev, "virtio-balloon", s);
     virtio_cleanup(vdev);
+
+    parent_dc->unrealize(dev, errp);
 }
 
 static Property virtio_balloon_properties[] = {
diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index 7c4526a..7d207a8 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -189,11 +189,15 @@ static void virtio_rng_device_unrealize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIORNG *vrng = VIRTIO_RNG(dev);
+    ObjectClass *parent_oc = VIRTIO_RNG_GET_PARENT_CLASS(dev);
+    DeviceClass *parent_dc = DEVICE_CLASS(parent_oc);
 
     qemu_del_timer(vrng->rate_limit_timer);
     qemu_free_timer(vrng->rate_limit_timer);
     unregister_savevm(dev, "virtio-rng", vrng);
     virtio_cleanup(vdev);
+
+    parent_dc->unrealize(dev, errp);
 }
 
 static Property virtio_rng_properties[] = {
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH RFC for-next v2 22/22] virtio-scsi: Convert virtio_scsi_common_[un]realize to QOM realize
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (20 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 21/22] virtio: Unrealize parent Andreas Färber
@ 2013-08-01  2:17 ` Andreas Färber
  2013-08-01  2:22 ` [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
  2013-08-14 16:28 ` Anthony Liguori
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Anthony Liguori, Andreas Färber, Michael S. Tsirkin

VirtIOSCSI and VHostSCSI now perform some initializations after
VirtIOSCSICommon's realize calls virtio_bus_plug_device(), namely
creating the SCSIBus and initializing /dev/vhost-scsi respectively.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/scsi/vhost-scsi.c            |  6 +-----
 hw/scsi/virtio-scsi.c           | 20 +++++++++++++-------
 include/hw/virtio/virtio-scsi.h |  5 ++---
 3 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index ad7cc9b..f4546da 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -219,7 +219,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
         }
     }
 
-    virtio_scsi_common_realize(dev, &err);
+    parent_dc->realize(dev, &err);
     if (err != NULL) {
         error_propagate(errp, err);
         return;
@@ -240,8 +240,6 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
     error_setg(&s->migration_blocker,
             "vhost-scsi does not support migration");
     migrate_add_blocker(s->migration_blocker);
-
-    parent_dc->realize(dev, errp);
 }
 
 static void vhost_scsi_unrealize(DeviceState *dev, Error **errp)
@@ -259,8 +257,6 @@ static void vhost_scsi_unrealize(DeviceState *dev, Error **errp)
 
     g_free(s->dev.vqs);
 
-    virtio_scsi_common_unrealize(dev, errp);
-
     parent_dc->unrealize(dev, errp);
 }
 
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index ac0ecdf..3a4ec97 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -589,10 +589,12 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
     .load_request = virtio_scsi_load_request,
 };
 
-void virtio_scsi_common_realize(DeviceState *dev, Error **errp)
+static void virtio_scsi_common_realize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOSCSICommon *s = VIRTIO_SCSI_COMMON(dev);
+    ObjectClass *parent_oc = VIRTIO_SCSI_COMMON_GET_PARENT_CLASS(dev);
+    DeviceClass *parent_dc = DEVICE_CLASS(parent_oc);
     int i;
 
     virtio_init(vdev, "virtio-scsi", VIRTIO_ID_SCSI,
@@ -610,6 +612,8 @@ void virtio_scsi_common_realize(DeviceState *dev, Error **errp)
         s->cmd_vqs[i] = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE,
                                          virtio_scsi_handle_cmd);
     }
+
+    parent_dc->realize(dev, errp);
 }
 
 static void virtio_scsi_device_realize(DeviceState *dev, Error **errp)
@@ -621,7 +625,7 @@ static void virtio_scsi_device_realize(DeviceState *dev, Error **errp)
     static int virtio_scsi_id;
     Error *err = NULL;
 
-    virtio_scsi_common_realize(dev, &err);
+    parent_dc->realize(dev, &err);
     if (err != NULL) {
         error_propagate(errp, err);
         return;
@@ -639,17 +643,19 @@ static void virtio_scsi_device_realize(DeviceState *dev, Error **errp)
 
     register_savevm(dev, "virtio-scsi", virtio_scsi_id++, 1,
                     virtio_scsi_save, virtio_scsi_load, s);
-
-    parent_dc->realize(dev, errp);
 }
 
-void virtio_scsi_common_unrealize(DeviceState *dev, Error **errp)
+static void virtio_scsi_common_unrealize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
+    ObjectClass *parent_oc = VIRTIO_SCSI_COMMON_GET_PARENT_CLASS(dev);
+    DeviceClass *parent_dc = DEVICE_CLASS(parent_oc);
 
     g_free(vs->cmd_vqs);
     virtio_cleanup(vdev);
+
+    parent_dc->unrealize(dev, errp);
 }
 
 static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp)
@@ -660,8 +666,6 @@ static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp)
 
     unregister_savevm(dev, "virtio-scsi", s);
 
-    virtio_scsi_common_unrealize(dev, errp);
-
     parent_dc->unrealize(dev, errp);
 }
 
@@ -675,6 +679,8 @@ static void virtio_scsi_common_class_init(ObjectClass *klass, void *data)
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
+    dc->realize = virtio_scsi_common_realize;
+    dc->unrealize = virtio_scsi_common_unrealize;
     vdc->get_config = virtio_scsi_get_config;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
 }
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index c56292b..a9fac74 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -21,6 +21,8 @@
 #define TYPE_VIRTIO_SCSI_COMMON "virtio-scsi-common"
 #define VIRTIO_SCSI_COMMON(obj) \
         OBJECT_CHECK(VirtIOSCSICommon, (obj), TYPE_VIRTIO_SCSI_COMMON)
+#define VIRTIO_SCSI_COMMON_GET_PARENT_CLASS(obj) \
+        OBJECT_GET_PARENT_CLASS(obj, TYPE_VIRTIO_SCSI_COMMON)
 
 #define TYPE_VIRTIO_SCSI "virtio-scsi-device"
 #define VIRTIO_SCSI(obj) \
@@ -188,7 +190,4 @@ typedef struct {
     DEFINE_PROP_BIT("param_change", _state, _feature_field,                    \
                                             VIRTIO_SCSI_F_CHANGE, true)
 
-void virtio_scsi_common_realize(DeviceState *dev, Error **errp);
-void virtio_scsi_common_unrealize(DeviceState *dev, Error **errp);
-
 #endif /* _QEMU_VIRTIO_SCSI_H */
-- 
1.8.1.4

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

* Re: [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (21 preceding siblings ...)
  2013-08-01  2:17 ` [Qemu-devel] [PATCH RFC for-next v2 22/22] virtio-scsi: Convert virtio_scsi_common_[un]realize to QOM realize Andreas Färber
@ 2013-08-01  2:22 ` Andreas Färber
  2013-08-14 16:28 ` Anthony Liguori
  23 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-01  2:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Crosthwaite, Michael S. Tsirkin, Alexey Kardashevskiy,
	Hu Tao, Jesse Larrew, Hervé Poussineau, Anthony Liguori,
	Paolo Bonzini, Frederic Konrad

Am 01.08.2013 04:17, schrieb Andreas Färber:
> Andreas Färber (22):
>   object: Add OBJECT_GET_PARENT_CLASS() macro

>   virtio-console: Use exitfn for virtserialport, too
>   virtio-9p-device: Avoid freeing uninitialized memory

Sorry, managed to mess up my branches: These two were sent out for 1.6
already. Please ignore these two here and comment on those instead:

http://patchwork.ozlabs.org/patch/263840/
http://patchwork.ozlabs.org/patch/263841/

>   virtio-blk-dataplane: Improve error reporting
>   virtio: Allow NULL VirtioDeviceClass::init hook
>   virtio-9p: QOM realize preparations
>   virtio-9p: Convert to QOM realize
>   virtio-blk: QOM realize preparations
>   virtio-blk: Convert to QOM realize
>   virtio-serial: QOM realize preparations
>   virtio-serial: Convert to QOM realize
>   virtio-net: QOM realize preparations
>   virtio-net: Convert to QOM realize
>   virtio-balloon: QOM realize preparations
>   virtio-balloon: Convert to QOM realize
>   virtio-rng: QOM realize preparations
>   virtio-rng: Convert to QOM realize
>   virtio-scsi: QOM realize preparations
>   virtio-scsi: Convert to QOM realize
>   virtio: Convert VirtioDevice to QOM realize
>   virtio: Unrealize parent
>   virtio-scsi: Convert virtio_scsi_common_[un]realize to QOM realize

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro Andreas Färber
@ 2013-08-06  5:54   ` Alexey Kardashevskiy
  2013-08-06  8:33     ` Andreas Färber
  2013-08-07  1:50   ` Peter Crosthwaite
  1 sibling, 1 reply; 38+ messages in thread
From: Alexey Kardashevskiy @ 2013-08-06  5:54 UTC (permalink / raw)
  To: Andreas Färber; +Cc: qemu-devel

On 08/01/2013 12:17 PM, Andreas Färber wrote:
> The object argument is currently unused and may be used to optimize the
> class lookup when needed.
> 
> Inspired-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  include/qom/object.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 23fc048..a8e71dc 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -511,6 +511,16 @@ struct TypeInfo
>      OBJECT_CLASS_CHECK(class, object_get_class(OBJECT(obj)), name)
>  
>  /**
> + * OBJECT_GET_PARENT_CLASS:
> + * @obj: The object to obtain the parent class for.
> + * @name: The QOM typename of @obj.
> + *
> + * Returns the parent class for a given object of a specific class.
> + */
> +#define OBJECT_GET_PARENT_CLASS(obj, name) \
> +    object_class_get_parent(object_class_by_name(name))
> +
> +/**
>   * InterfaceInfo:
>   * @type: The name of the interface.
>   *
> 

Has anyone ever tried to use this macro?



-- 
Alexey

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

* Re: [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro
  2013-08-06  5:54   ` Alexey Kardashevskiy
@ 2013-08-06  8:33     ` Andreas Färber
  2013-08-07  3:36       ` Alexey Kardashevskiy
  0 siblings, 1 reply; 38+ messages in thread
From: Andreas Färber @ 2013-08-06  8:33 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: Peter Crosthwaite, qemu-devel, Anthony Liguori

Am 06.08.2013 07:54, schrieb Alexey Kardashevskiy:
> On 08/01/2013 12:17 PM, Andreas Färber wrote:
>> The object argument is currently unused and may be used to optimize the
>> class lookup when needed.
>>
>> Inspired-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>>  include/qom/object.h | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/include/qom/object.h b/include/qom/object.h
>> index 23fc048..a8e71dc 100644
>> --- a/include/qom/object.h
>> +++ b/include/qom/object.h
>> @@ -511,6 +511,16 @@ struct TypeInfo
>>      OBJECT_CLASS_CHECK(class, object_get_class(OBJECT(obj)), name)
>>  
>>  /**
>> + * OBJECT_GET_PARENT_CLASS:
>> + * @obj: The object to obtain the parent class for.
>> + * @name: The QOM typename of @obj.
>> + *
>> + * Returns the parent class for a given object of a specific class.
>> + */
>> +#define OBJECT_GET_PARENT_CLASS(obj, name) \
>> +    object_class_get_parent(object_class_by_name(name))
>> +
>> +/**
>>   * InterfaceInfo:
>>   * @type: The name of the interface.
>>   *
>>
> 
> Has anyone ever tried to use this macro?

Since you're asking me, obviously later in this virtio series it's used
and in the IndustryPack series as well.

I'm not aware of anyone else having used it yet - I'm still waiting for
review feedback from Peter Cr. and/or Anthony (or you!) before I put it
on qom-next.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro
  2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro Andreas Färber
  2013-08-06  5:54   ` Alexey Kardashevskiy
@ 2013-08-07  1:50   ` Peter Crosthwaite
  1 sibling, 0 replies; 38+ messages in thread
From: Peter Crosthwaite @ 2013-08-07  1:50 UTC (permalink / raw)
  To: Andreas Färber; +Cc: qemu-devel

On Thu, Aug 1, 2013 at 12:17 PM, Andreas Färber <afaerber@suse.de> wrote:
> The object argument is currently unused and may be used to optimize the
> class lookup when needed.
>
> Inspired-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

One minor nit about comments below but im happy as is. Thanks.

> ---
>  include/qom/object.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 23fc048..a8e71dc 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -511,6 +511,16 @@ struct TypeInfo
>      OBJECT_CLASS_CHECK(class, object_get_class(OBJECT(obj)), name)
>
>  /**
> + * OBJECT_GET_PARENT_CLASS:
> + * @obj: The object to obtain the parent class for.
> + * @name: The QOM typename of @obj.

This is actually a little more complicated than this comment suggests.
You are actually obtaining the parent class of @name for use by
object@ obj. @name only needs to be an ancestor of @obj's concrete
type and not necessarily the concrete class.

Regards,
Peter

> + *
> + * Returns the parent class for a given object of a specific class.
> + */
> +#define OBJECT_GET_PARENT_CLASS(obj, name) \
> +    object_class_get_parent(object_class_by_name(name))
> +
> +/**
>   * InterfaceInfo:
>   * @type: The name of the interface.
>   *
> --
> 1.8.1.4
>
>

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

* Re: [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro
  2013-08-06  8:33     ` Andreas Färber
@ 2013-08-07  3:36       ` Alexey Kardashevskiy
  2013-08-07  4:20         ` Peter Crosthwaite
  2013-08-07  5:58         ` Andreas Färber
  0 siblings, 2 replies; 38+ messages in thread
From: Alexey Kardashevskiy @ 2013-08-07  3:36 UTC (permalink / raw)
  To: Andreas Färber; +Cc: Peter Crosthwaite, qemu-devel, Anthony Liguori

On 08/06/2013 06:33 PM, Andreas Färber wrote:
> Am 06.08.2013 07:54, schrieb Alexey Kardashevskiy:
>> On 08/01/2013 12:17 PM, Andreas Färber wrote:
>>> The object argument is currently unused and may be used to optimize the
>>> class lookup when needed.
>>>
>>> Inspired-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>> ---
>>>  include/qom/object.h | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/include/qom/object.h b/include/qom/object.h
>>> index 23fc048..a8e71dc 100644
>>> --- a/include/qom/object.h
>>> +++ b/include/qom/object.h
>>> @@ -511,6 +511,16 @@ struct TypeInfo
>>>      OBJECT_CLASS_CHECK(class, object_get_class(OBJECT(obj)), name)
>>>  
>>>  /**
>>> + * OBJECT_GET_PARENT_CLASS:
>>> + * @obj: The object to obtain the parent class for.
>>> + * @name: The QOM typename of @obj.
>>> + *
>>> + * Returns the parent class for a given object of a specific class.
>>> + */
>>> +#define OBJECT_GET_PARENT_CLASS(obj, name) \
>>> +    object_class_get_parent(object_class_by_name(name))
>>> +
>>> +/**
>>>   * InterfaceInfo:
>>>   * @type: The name of the interface.
>>>   *
>>>
>>
>> Has anyone ever tried to use this macro?
> 
> Since you're asking me, obviously later in this virtio series it's used
> and in the IndustryPack series as well.
> 
> I'm not aware of anyone else having used it yet - I'm still waiting for
> review feedback from Peter Cr. and/or Anthony (or you!) before I put it
> on qom-next.


Still do not understand what "obj" is doing here.
This what I would suggest:

#define OBJECT_GET_PARENT_CLASS(obj, name) \
    object_class_get_parent(OBJECT_GET_CLASS(ObjectClass, (obj), (name)))

@name here is just to make sure we are at the right level of the class
hierarchy.

And use it like this:

static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
{
    XICSStateClass *xsc = XICS_COMMON_CLASS(
		OBJECT_GET_PARENT_CLASS(OBJECT(icp), TYPE_KVM_XICS));
...

Here both source and destination classes are checked, everyone must be happy :)


-- 
Alexey

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

* Re: [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro
  2013-08-07  3:36       ` Alexey Kardashevskiy
@ 2013-08-07  4:20         ` Peter Crosthwaite
  2013-08-07  5:38           ` Alexey Kardashevskiy
  2013-08-07  5:58         ` Andreas Färber
  1 sibling, 1 reply; 38+ messages in thread
From: Peter Crosthwaite @ 2013-08-07  4:20 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: Andreas Färber, Anthony Liguori, qemu-devel

Hi,

On Wed, Aug 7, 2013 at 1:36 PM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> On 08/06/2013 06:33 PM, Andreas Färber wrote:
>> Am 06.08.2013 07:54, schrieb Alexey Kardashevskiy:
>>> On 08/01/2013 12:17 PM, Andreas Färber wrote:
>>>> The object argument is currently unused and may be used to optimize the
>>>> class lookup when needed.
>>>>
>>>> Inspired-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>>> ---
>>>>  include/qom/object.h | 10 ++++++++++
>>>>  1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/include/qom/object.h b/include/qom/object.h
>>>> index 23fc048..a8e71dc 100644
>>>> --- a/include/qom/object.h
>>>> +++ b/include/qom/object.h
>>>> @@ -511,6 +511,16 @@ struct TypeInfo
>>>>      OBJECT_CLASS_CHECK(class, object_get_class(OBJECT(obj)), name)
>>>>
>>>>  /**
>>>> + * OBJECT_GET_PARENT_CLASS:
>>>> + * @obj: The object to obtain the parent class for.
>>>> + * @name: The QOM typename of @obj.
>>>> + *
>>>> + * Returns the parent class for a given object of a specific class.
>>>> + */
>>>> +#define OBJECT_GET_PARENT_CLASS(obj, name) \
>>>> +    object_class_get_parent(object_class_by_name(name))
>>>> +
>>>> +/**
>>>>   * InterfaceInfo:
>>>>   * @type: The name of the interface.
>>>>   *
>>>>
>>>
>>> Has anyone ever tried to use this macro?
>>
>> Since you're asking me, obviously later in this virtio series it's used
>> and in the IndustryPack series as well.
>>
>> I'm not aware of anyone else having used it yet - I'm still waiting for
>> review feedback from Peter Cr. and/or Anthony (or you!) before I put it
>> on qom-next.
>
>
> Still do not understand what "obj" is doing here.

The object is currently where cast cache optimizations are
implemented. So having a handle to it is useful if ever these
get-parent operations end up in fast paths and we need to do one of
Anthony's caching tricks.

> This what I would suggest:
>
> #define OBJECT_GET_PARENT_CLASS(obj, name) \
>     object_class_get_parent(OBJECT_GET_CLASS(ObjectClass, (obj), (name)))
>

You have changed the semantic from what Andreas has implemented. This
will always return the parent of the concrete class, whereas to solve
the save-override-call problem you need to get the parent of abstract
level that is overriding the function (not always the concrete class).

> @name here is just to make sure we are at the right level of the class
> hierarchy.
>

Its @name that is actually important. Its more than just assertive,
variation of @name for the same object will and should return
different results (aside from just checking). The demonstrative case
for this requirement is TYPE_ARM_CPU, which has a realize fn that
needs to call the parent version as implemented by TYPE_CPU. ARM CPUs
however have a whole bunch of concrete classes inheriting from
TYPE_ARM_CPU. So using your macro, TYPE_ARM_CPU's realize fn will only
be able to get a handle to the parent of the concrete class (i.e.
TYPE_ARM_CPU) and not the parent of TYPE_ARM_CPU (i.e. TYPE_CPU) as
intended.

Regards,
Peter

> And use it like this:
>
> static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
> {
>     XICSStateClass *xsc = XICS_COMMON_CLASS(
>                 OBJECT_GET_PARENT_CLASS(OBJECT(icp), TYPE_KVM_XICS));
> ...
>
> Here both source and destination classes are checked, everyone must be happy :)
>
>
> --
> Alexey
>

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

* Re: [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro
  2013-08-07  4:20         ` Peter Crosthwaite
@ 2013-08-07  5:38           ` Alexey Kardashevskiy
  2013-08-07  5:45             ` Andreas Färber
  0 siblings, 1 reply; 38+ messages in thread
From: Alexey Kardashevskiy @ 2013-08-07  5:38 UTC (permalink / raw)
  To: Peter Crosthwaite; +Cc: Andreas Färber, Anthony Liguori, qemu-devel

On 08/07/2013 02:20 PM, Peter Crosthwaite wrote:
> Hi,
> 
> On Wed, Aug 7, 2013 at 1:36 PM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> On 08/06/2013 06:33 PM, Andreas Färber wrote:
>>> Am 06.08.2013 07:54, schrieb Alexey Kardashevskiy:
>>>> On 08/01/2013 12:17 PM, Andreas Färber wrote:
>>>>> The object argument is currently unused and may be used to optimize the
>>>>> class lookup when needed.
>>>>>
>>>>> Inspired-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>>>> ---
>>>>>  include/qom/object.h | 10 ++++++++++
>>>>>  1 file changed, 10 insertions(+)
>>>>>
>>>>> diff --git a/include/qom/object.h b/include/qom/object.h
>>>>> index 23fc048..a8e71dc 100644
>>>>> --- a/include/qom/object.h
>>>>> +++ b/include/qom/object.h
>>>>> @@ -511,6 +511,16 @@ struct TypeInfo
>>>>>      OBJECT_CLASS_CHECK(class, object_get_class(OBJECT(obj)), name)
>>>>>
>>>>>  /**
>>>>> + * OBJECT_GET_PARENT_CLASS:
>>>>> + * @obj: The object to obtain the parent class for.
>>>>> + * @name: The QOM typename of @obj.
>>>>> + *
>>>>> + * Returns the parent class for a given object of a specific class.
>>>>> + */
>>>>> +#define OBJECT_GET_PARENT_CLASS(obj, name) \
>>>>> +    object_class_get_parent(object_class_by_name(name))
>>>>> +
>>>>> +/**
>>>>>   * InterfaceInfo:
>>>>>   * @type: The name of the interface.
>>>>>   *
>>>>>
>>>>
>>>> Has anyone ever tried to use this macro?
>>>
>>> Since you're asking me, obviously later in this virtio series it's used
>>> and in the IndustryPack series as well.
>>>
>>> I'm not aware of anyone else having used it yet - I'm still waiting for
>>> review feedback from Peter Cr. and/or Anthony (or you!) before I put it
>>> on qom-next.
>>
>>
>> Still do not understand what "obj" is doing here.
> 
> The object is currently where cast cache optimizations are
> implemented. So having a handle to it is useful if ever these
> get-parent operations end up in fast paths and we need to do one of
> Anthony's caching tricks.
> 
>> This what I would suggest:
>>
>> #define OBJECT_GET_PARENT_CLASS(obj, name) \
>>     object_class_get_parent(OBJECT_GET_CLASS(ObjectClass, (obj), (name)))
>>
> 
> You have changed the semantic from what Andreas has implemented. This
> will always return the parent of the concrete class, whereas to solve
> the save-override-call problem you need to get the parent of abstract
> level that is overriding the function (not always the concrete class).
> 
>> @name here is just to make sure we are at the right level of the class
>> hierarchy.
>>
> 
> Its @name that is actually important. Its more than just assertive,
> variation of @name for the same object will and should return
> different results (aside from just checking). The demonstrative case
> for this requirement is TYPE_ARM_CPU, which has a realize fn that
> needs to call the parent version as implemented by TYPE_CPU. ARM CPUs
> however have a whole bunch of concrete classes inheriting from
> TYPE_ARM_CPU. So using your macro, TYPE_ARM_CPU's realize fn will only
> be able to get a handle to the parent of the concrete class (i.e.
> TYPE_ARM_CPU) 

This is what I would really (really) expect in this situation.

> and not the parent of TYPE_ARM_CPU (i.e. TYPE_CPU) as
> intended.

Oh. Then it is not get_parent() at all, this is get_grand_parent.

realize() of TYPE_ARM_CPU_CORTEXTA9 (example) calls its parent
(TYPE_ARM_CPU) realize() and that realize() calls its parent (TYPE_CPU)
realize.

It some level (for example, TYPE_ARM_CPU) does not want to implement
realize(), then pointer to realize() from the upper class should be copied
into TYPE_ARM_CPU's type info struct. class_init() callbacks are called for
every class in the chain, so if some class_init() won't write to realize()
pointer, it should be what the parent initialized it to, no?

What do I miss here?



> Regards,
> Peter
> 
>> And use it like this:
>>
>> static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
>> {
>>     XICSStateClass *xsc = XICS_COMMON_CLASS(
>>                 OBJECT_GET_PARENT_CLASS(OBJECT(icp), TYPE_KVM_XICS));
>> ...
>>
>> Here both source and destination classes are checked, everyone must be happy :)
>>
>>
>> --
>> Alexey
>>


-- 
Alexey

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

* Re: [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro
  2013-08-07  5:38           ` Alexey Kardashevskiy
@ 2013-08-07  5:45             ` Andreas Färber
  2013-08-07  5:53               ` Alexey Kardashevskiy
  0 siblings, 1 reply; 38+ messages in thread
From: Andreas Färber @ 2013-08-07  5:45 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: Peter Crosthwaite, qemu-devel, Anthony Liguori

Am 07.08.2013 07:38, schrieb Alexey Kardashevskiy:
> On 08/07/2013 02:20 PM, Peter Crosthwaite wrote:
>> Hi,
>>
>> On Wed, Aug 7, 2013 at 1:36 PM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>> On 08/06/2013 06:33 PM, Andreas Färber wrote:
>>>> Am 06.08.2013 07:54, schrieb Alexey Kardashevskiy:
>>>>> On 08/01/2013 12:17 PM, Andreas Färber wrote:
>>>>>> The object argument is currently unused and may be used to optimize the
>>>>>> class lookup when needed.
>>>>>>
>>>>>> Inspired-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>>>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>>>>> ---
>>>>>>  include/qom/object.h | 10 ++++++++++
>>>>>>  1 file changed, 10 insertions(+)
>>>>>>
>>>>>> diff --git a/include/qom/object.h b/include/qom/object.h
>>>>>> index 23fc048..a8e71dc 100644
>>>>>> --- a/include/qom/object.h
>>>>>> +++ b/include/qom/object.h
>>>>>> @@ -511,6 +511,16 @@ struct TypeInfo
>>>>>>      OBJECT_CLASS_CHECK(class, object_get_class(OBJECT(obj)), name)
>>>>>>
>>>>>>  /**
>>>>>> + * OBJECT_GET_PARENT_CLASS:
>>>>>> + * @obj: The object to obtain the parent class for.
>>>>>> + * @name: The QOM typename of @obj.
>>>>>> + *
>>>>>> + * Returns the parent class for a given object of a specific class.
>>>>>> + */
>>>>>> +#define OBJECT_GET_PARENT_CLASS(obj, name) \
>>>>>> +    object_class_get_parent(object_class_by_name(name))
>>>>>> +
>>>>>> +/**
>>>>>>   * InterfaceInfo:
>>>>>>   * @type: The name of the interface.
>>>>>>   *
>>>>>>
>>>>>
>>>>> Has anyone ever tried to use this macro?
>>>>
>>>> Since you're asking me, obviously later in this virtio series it's used
>>>> and in the IndustryPack series as well.
>>>>
>>>> I'm not aware of anyone else having used it yet - I'm still waiting for
>>>> review feedback from Peter Cr. and/or Anthony (or you!) before I put it
>>>> on qom-next.
>>>
>>>
>>> Still do not understand what "obj" is doing here.
>>
>> The object is currently where cast cache optimizations are
>> implemented. So having a handle to it is useful if ever these
>> get-parent operations end up in fast paths and we need to do one of
>> Anthony's caching tricks.
>>
>>> This what I would suggest:
>>>
>>> #define OBJECT_GET_PARENT_CLASS(obj, name) \
>>>     object_class_get_parent(OBJECT_GET_CLASS(ObjectClass, (obj), (name)))
>>>
>>
>> You have changed the semantic from what Andreas has implemented. This
>> will always return the parent of the concrete class, whereas to solve
>> the save-override-call problem you need to get the parent of abstract
>> level that is overriding the function (not always the concrete class).
>>
>>> @name here is just to make sure we are at the right level of the class
>>> hierarchy.
>>>
>>
>> Its @name that is actually important. Its more than just assertive,
>> variation of @name for the same object will and should return
>> different results (aside from just checking). The demonstrative case
>> for this requirement is TYPE_ARM_CPU, which has a realize fn that
>> needs to call the parent version as implemented by TYPE_CPU. ARM CPUs
>> however have a whole bunch of concrete classes inheriting from
>> TYPE_ARM_CPU. So using your macro, TYPE_ARM_CPU's realize fn will only
>> be able to get a handle to the parent of the concrete class (i.e.
>> TYPE_ARM_CPU) 
> 
> This is what I would really (really) expect in this situation.
> 
>> and not the parent of TYPE_ARM_CPU (i.e. TYPE_CPU) as
>> intended.
> 
> Oh. Then it is not get_parent() at all, this is get_grand_parent.

No. It is parent of TYPE_ARM_CPU, as specified by the name argument.
obj serves for possible optimization, as demonstrated by an earlier
patch by Peter that avoids the hashtable lookup by iterating over obj's
parent classes. If we pass obj down through our macros then such changes
can be done in one central place rather than reviewing and changing the
whole code base.

> realize() of TYPE_ARM_CPU_CORTEXTA9 (example) calls its parent
> (TYPE_ARM_CPU) realize() and that realize() calls its parent (TYPE_CPU)
> realize.
> 
> It some level (for example, TYPE_ARM_CPU) does not want to implement
> realize(), then pointer to realize() from the upper class should be copied
> into TYPE_ARM_CPU's type info struct. class_init() callbacks are called for
> every class in the chain, so if some class_init() won't write to realize()
> pointer, it should be what the parent initialized it to, no?
> 
> What do I miss here?

You are missing that we do implement realize for TYPE_ARM_CPU, not for
its derived types. So we want to call TYPE_CPU's realize from there, not
some random realize determined by what type dev happens to have.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro
  2013-08-07  5:45             ` Andreas Färber
@ 2013-08-07  5:53               ` Alexey Kardashevskiy
  2013-08-07  6:10                 ` Peter Crosthwaite
  2013-08-07  6:28                 ` Andreas Färber
  0 siblings, 2 replies; 38+ messages in thread
From: Alexey Kardashevskiy @ 2013-08-07  5:53 UTC (permalink / raw)
  To: Andreas Färber; +Cc: Peter Crosthwaite, qemu-devel, Anthony Liguori

On 08/07/2013 03:45 PM, Andreas Färber wrote:
> Am 07.08.2013 07:38, schrieb Alexey Kardashevskiy:
>> On 08/07/2013 02:20 PM, Peter Crosthwaite wrote:
>>> Hi,
>>>
>>> On Wed, Aug 7, 2013 at 1:36 PM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>> On 08/06/2013 06:33 PM, Andreas Färber wrote:
>>>>> Am 06.08.2013 07:54, schrieb Alexey Kardashevskiy:
>>>>>> On 08/01/2013 12:17 PM, Andreas Färber wrote:
>>>>>>> The object argument is currently unused and may be used to optimize the
>>>>>>> class lookup when needed.
>>>>>>>
>>>>>>> Inspired-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>>>>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>>>>>> ---
>>>>>>>  include/qom/object.h | 10 ++++++++++
>>>>>>>  1 file changed, 10 insertions(+)
>>>>>>>
>>>>>>> diff --git a/include/qom/object.h b/include/qom/object.h
>>>>>>> index 23fc048..a8e71dc 100644
>>>>>>> --- a/include/qom/object.h
>>>>>>> +++ b/include/qom/object.h
>>>>>>> @@ -511,6 +511,16 @@ struct TypeInfo
>>>>>>>      OBJECT_CLASS_CHECK(class, object_get_class(OBJECT(obj)), name)
>>>>>>>
>>>>>>>  /**
>>>>>>> + * OBJECT_GET_PARENT_CLASS:
>>>>>>> + * @obj: The object to obtain the parent class for.
>>>>>>> + * @name: The QOM typename of @obj.
>>>>>>> + *
>>>>>>> + * Returns the parent class for a given object of a specific class.
>>>>>>> + */
>>>>>>> +#define OBJECT_GET_PARENT_CLASS(obj, name) \
>>>>>>> +    object_class_get_parent(object_class_by_name(name))
>>>>>>> +
>>>>>>> +/**
>>>>>>>   * InterfaceInfo:
>>>>>>>   * @type: The name of the interface.
>>>>>>>   *
>>>>>>>
>>>>>>
>>>>>> Has anyone ever tried to use this macro?
>>>>>
>>>>> Since you're asking me, obviously later in this virtio series it's used
>>>>> and in the IndustryPack series as well.
>>>>>
>>>>> I'm not aware of anyone else having used it yet - I'm still waiting for
>>>>> review feedback from Peter Cr. and/or Anthony (or you!) before I put it
>>>>> on qom-next.
>>>>
>>>>
>>>> Still do not understand what "obj" is doing here.
>>>
>>> The object is currently where cast cache optimizations are
>>> implemented. So having a handle to it is useful if ever these
>>> get-parent operations end up in fast paths and we need to do one of
>>> Anthony's caching tricks.
>>>
>>>> This what I would suggest:
>>>>
>>>> #define OBJECT_GET_PARENT_CLASS(obj, name) \
>>>>     object_class_get_parent(OBJECT_GET_CLASS(ObjectClass, (obj), (name)))
>>>>
>>>
>>> You have changed the semantic from what Andreas has implemented. This
>>> will always return the parent of the concrete class, whereas to solve
>>> the save-override-call problem you need to get the parent of abstract
>>> level that is overriding the function (not always the concrete class).
>>>
>>>> @name here is just to make sure we are at the right level of the class
>>>> hierarchy.
>>>>
>>>
>>> Its @name that is actually important. Its more than just assertive,
>>> variation of @name for the same object will and should return
>>> different results (aside from just checking). The demonstrative case
>>> for this requirement is TYPE_ARM_CPU, which has a realize fn that
>>> needs to call the parent version as implemented by TYPE_CPU. ARM CPUs
>>> however have a whole bunch of concrete classes inheriting from
>>> TYPE_ARM_CPU. So using your macro, TYPE_ARM_CPU's realize fn will only
>>> be able to get a handle to the parent of the concrete class (i.e.
>>> TYPE_ARM_CPU) 
>>
>> This is what I would really (really) expect in this situation.
>>
>>> and not the parent of TYPE_ARM_CPU (i.e. TYPE_CPU) as
>>> intended.
>>
>> Oh. Then it is not get_parent() at all, this is get_grand_parent.
> 
> No. It is parent of TYPE_ARM_CPU, as specified by the name argument.

Here I lost you. You really want finalize() of TYPE_CPU, no matter how many
classes are between TYPE_CPU and your final ARM CPU class. So call it
directly, why do you need this workaround with get_parent if it is not
really a parent of the current obj and you do not trust to what you get in
finalize callback field of all intermediate classes (TYPE_CPU_ARM in this
example)?

Again, I am not arguing, I really that dump and do not understand :)


> obj serves for possible optimization, as demonstrated by an earlier
> patch by Peter that avoids the hashtable lookup by iterating over obj's
> parent classes. If we pass obj down through our macros then such changes
> can be done in one central place rather than reviewing and changing the
> whole code base.

>> realize() of TYPE_ARM_CPU_CORTEXTA9 (example) calls its parent
>> (TYPE_ARM_CPU) realize() and that realize() calls its parent (TYPE_CPU)
>> realize.
>>
>> It some level (for example, TYPE_ARM_CPU) does not want to implement
>> realize(), then pointer to realize() from the upper class should be copied
>> into TYPE_ARM_CPU's type info struct. class_init() callbacks are called for
>> every class in the chain, so if some class_init() won't write to realize()
>> pointer, it should be what the parent initialized it to, no?
>>
>> What do I miss here?
> 
> You are missing that we do implement realize for TYPE_ARM_CPU, not for
> its derived types. So we want to call TYPE_CPU's realize from there, not
> some random realize determined by what type dev happens to have.

How is it random? Inheritance is strict. It is CPU -> ARM_CPU -> A9_ARM_CPU
so their finalize() should point to something reasonable or inherit from a
parent.


-- 
Alexey

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

* Re: [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro
  2013-08-07  3:36       ` Alexey Kardashevskiy
  2013-08-07  4:20         ` Peter Crosthwaite
@ 2013-08-07  5:58         ` Andreas Färber
  1 sibling, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-07  5:58 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: Peter Crosthwaite, qemu-devel, Anthony Liguori

Am 07.08.2013 05:36, schrieb Alexey Kardashevskiy:
> This what I would suggest:
> 
> #define OBJECT_GET_PARENT_CLASS(obj, name) \
>     object_class_get_parent(OBJECT_GET_CLASS(ObjectClass, (obj), (name)))
> 
> @name here is just to make sure we are at the right level of the class
> hierarchy.
> 
> And use it like this:
> 
> static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
> {
>     XICSStateClass *xsc = XICS_COMMON_CLASS(
> 		OBJECT_GET_PARENT_CLASS(OBJECT(icp), TYPE_KVM_XICS));
> ...
> 
> Here both source and destination classes are checked, everyone must be happy :)

In addition to what Peter has explained, I recommend against using my
macro inline, just like we don't use OBJECT_CHECK() inline. Doing so
keeps the uses of TYPE_KVM_XICS together and lets us exchange the
implementation more easily should the need arise.

Btw COMMON_XICS_CLASS() would be a more specific name. (Yes, ARM GIC is
a counter-example.)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro
  2013-08-07  5:53               ` Alexey Kardashevskiy
@ 2013-08-07  6:10                 ` Peter Crosthwaite
  2013-08-07  6:38                   ` Alexey Kardashevskiy
  2013-08-07  6:28                 ` Andreas Färber
  1 sibling, 1 reply; 38+ messages in thread
From: Peter Crosthwaite @ 2013-08-07  6:10 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: Andreas Färber, Anthony Liguori, qemu-devel

On Wed, Aug 7, 2013 at 3:53 PM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> On 08/07/2013 03:45 PM, Andreas Färber wrote:
>> Am 07.08.2013 07:38, schrieb Alexey Kardashevskiy:
>>> On 08/07/2013 02:20 PM, Peter Crosthwaite wrote:
>>>> Hi,
>>>>
>>>> On Wed, Aug 7, 2013 at 1:36 PM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>> On 08/06/2013 06:33 PM, Andreas Färber wrote:
>>>>>> Am 06.08.2013 07:54, schrieb Alexey Kardashevskiy:
>>>>>>> On 08/01/2013 12:17 PM, Andreas Färber wrote:
>>>>>>>> The object argument is currently unused and may be used to optimize the
>>>>>>>> class lookup when needed.
>>>>>>>>
>>>>>>>> Inspired-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>>>>>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>>>>>>> ---
>>>>>>>>  include/qom/object.h | 10 ++++++++++
>>>>>>>>  1 file changed, 10 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/include/qom/object.h b/include/qom/object.h
>>>>>>>> index 23fc048..a8e71dc 100644
>>>>>>>> --- a/include/qom/object.h
>>>>>>>> +++ b/include/qom/object.h
>>>>>>>> @@ -511,6 +511,16 @@ struct TypeInfo
>>>>>>>>      OBJECT_CLASS_CHECK(class, object_get_class(OBJECT(obj)), name)
>>>>>>>>
>>>>>>>>  /**
>>>>>>>> + * OBJECT_GET_PARENT_CLASS:
>>>>>>>> + * @obj: The object to obtain the parent class for.
>>>>>>>> + * @name: The QOM typename of @obj.
>>>>>>>> + *
>>>>>>>> + * Returns the parent class for a given object of a specific class.
>>>>>>>> + */
>>>>>>>> +#define OBJECT_GET_PARENT_CLASS(obj, name) \
>>>>>>>> +    object_class_get_parent(object_class_by_name(name))
>>>>>>>> +
>>>>>>>> +/**
>>>>>>>>   * InterfaceInfo:
>>>>>>>>   * @type: The name of the interface.
>>>>>>>>   *
>>>>>>>>
>>>>>>>
>>>>>>> Has anyone ever tried to use this macro?
>>>>>>
>>>>>> Since you're asking me, obviously later in this virtio series it's used
>>>>>> and in the IndustryPack series as well.
>>>>>>
>>>>>> I'm not aware of anyone else having used it yet - I'm still waiting for
>>>>>> review feedback from Peter Cr. and/or Anthony (or you!) before I put it
>>>>>> on qom-next.
>>>>>
>>>>>
>>>>> Still do not understand what "obj" is doing here.
>>>>
>>>> The object is currently where cast cache optimizations are
>>>> implemented. So having a handle to it is useful if ever these
>>>> get-parent operations end up in fast paths and we need to do one of
>>>> Anthony's caching tricks.
>>>>
>>>>> This what I would suggest:
>>>>>
>>>>> #define OBJECT_GET_PARENT_CLASS(obj, name) \
>>>>>     object_class_get_parent(OBJECT_GET_CLASS(ObjectClass, (obj), (name)))
>>>>>
>>>>
>>>> You have changed the semantic from what Andreas has implemented. This
>>>> will always return the parent of the concrete class, whereas to solve
>>>> the save-override-call problem you need to get the parent of abstract
>>>> level that is overriding the function (not always the concrete class).
>>>>
>>>>> @name here is just to make sure we are at the right level of the class
>>>>> hierarchy.
>>>>>
>>>>
>>>> Its @name that is actually important. Its more than just assertive,
>>>> variation of @name for the same object will and should return
>>>> different results (aside from just checking). The demonstrative case
>>>> for this requirement is TYPE_ARM_CPU, which has a realize fn that
>>>> needs to call the parent version as implemented by TYPE_CPU. ARM CPUs
>>>> however have a whole bunch of concrete classes inheriting from
>>>> TYPE_ARM_CPU. So using your macro, TYPE_ARM_CPU's realize fn will only
>>>> be able to get a handle to the parent of the concrete class (i.e.
>>>> TYPE_ARM_CPU)
>>>
>>> This is what I would really (really) expect in this situation.
>>>
>>>> and not the parent of TYPE_ARM_CPU (i.e. TYPE_CPU) as
>>>> intended.
>>>
>>> Oh. Then it is not get_parent() at all, this is get_grand_parent.
>>
>> No. It is parent of TYPE_ARM_CPU, as specified by the name argument.
>
> Here I lost you. You really want finalize() of TYPE_CPU, no matter how many
> classes are between TYPE_CPU and your final ARM CPU class.

No, we want the immediate parent of the TYPE_ARM_CPU class which is
not set in stone. Directly refing TYPE_CPU makes it difficult for
anyone trying to re-organise the QOM heirachy. The idea behind this
approach was that TYPE_ARM_CPU::realize does not make assumptions
about the classes above it (except that someone in the ancestry is
TYPE_DEVICE for the definition of realize) nor the classes below it
(as already discussed).

For example, if someone decides to implement TYPE_CPU_FOO (abstract
child of TYPE_CPU) and reparents TYPE_ARM_CPU to this, then without
need-for-change TYPE_ARM_CPU will now call TYPE_CPU_FOO:realize rather
than inadvertently skipping over levels to a grandparent
implementation. It can be done your proposed way, but re-organising
the heirachy will potentially require change patterns that are avoided
with this approach.

So call it
> directly, why do you need this workaround with get_parent if it is not
> really a parent of the current obj and you do not trust to what you get in

This is probably terminology confusion. Its a parent of a class not an
object. The documentation and naming maybe needs work?

Regards,
Peter

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

* Re: [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro
  2013-08-07  5:53               ` Alexey Kardashevskiy
  2013-08-07  6:10                 ` Peter Crosthwaite
@ 2013-08-07  6:28                 ` Andreas Färber
  1 sibling, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-07  6:28 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: Peter Crosthwaite, Peter Chubb, qemu-devel, Anthony Liguori

Am 07.08.2013 07:53, schrieb Alexey Kardashevskiy:
> On 08/07/2013 03:45 PM, Andreas Färber wrote:
>> Am 07.08.2013 07:38, schrieb Alexey Kardashevskiy:
>>> On 08/07/2013 02:20 PM, Peter Crosthwaite wrote:
>>>> Hi,
>>>>
>>>> On Wed, Aug 7, 2013 at 1:36 PM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>> On 08/06/2013 06:33 PM, Andreas Färber wrote:
>>>>>> Am 06.08.2013 07:54, schrieb Alexey Kardashevskiy:
>>>>>>> On 08/01/2013 12:17 PM, Andreas Färber wrote:
>>>>>>>> The object argument is currently unused and may be used to optimize the
>>>>>>>> class lookup when needed.
>>>>>>>>
>>>>>>>> Inspired-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>>>>>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>>>>>>> ---
>>>>>>>>  include/qom/object.h | 10 ++++++++++
>>>>>>>>  1 file changed, 10 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/include/qom/object.h b/include/qom/object.h
>>>>>>>> index 23fc048..a8e71dc 100644
>>>>>>>> --- a/include/qom/object.h
>>>>>>>> +++ b/include/qom/object.h
>>>>>>>> @@ -511,6 +511,16 @@ struct TypeInfo
>>>>>>>>      OBJECT_CLASS_CHECK(class, object_get_class(OBJECT(obj)), name)
>>>>>>>>
>>>>>>>>  /**
>>>>>>>> + * OBJECT_GET_PARENT_CLASS:
>>>>>>>> + * @obj: The object to obtain the parent class for.
>>>>>>>> + * @name: The QOM typename of @obj.
>>>>>>>> + *
>>>>>>>> + * Returns the parent class for a given object of a specific class.
>>>>>>>> + */
>>>>>>>> +#define OBJECT_GET_PARENT_CLASS(obj, name) \
>>>>>>>> +    object_class_get_parent(object_class_by_name(name))
>>>>>>>> +
>>>>>>>> +/**
>>>>>>>>   * InterfaceInfo:
>>>>>>>>   * @type: The name of the interface.
>>>>>>>>   *
>>>>>>>>
>>>>>>>
>>>>>>> Has anyone ever tried to use this macro?
>>>>>>
>>>>>> Since you're asking me, obviously later in this virtio series it's used
>>>>>> and in the IndustryPack series as well.
>>>>>>
>>>>>> I'm not aware of anyone else having used it yet - I'm still waiting for
>>>>>> review feedback from Peter Cr. and/or Anthony (or you!) before I put it
>>>>>> on qom-next.
>>>>>
>>>>>
>>>>> Still do not understand what "obj" is doing here.
>>>>
>>>> The object is currently where cast cache optimizations are
>>>> implemented. So having a handle to it is useful if ever these
>>>> get-parent operations end up in fast paths and we need to do one of
>>>> Anthony's caching tricks.
>>>>
>>>>> This what I would suggest:
>>>>>
>>>>> #define OBJECT_GET_PARENT_CLASS(obj, name) \
>>>>>     object_class_get_parent(OBJECT_GET_CLASS(ObjectClass, (obj), (name)))
>>>>>
>>>>
>>>> You have changed the semantic from what Andreas has implemented. This
>>>> will always return the parent of the concrete class, whereas to solve
>>>> the save-override-call problem you need to get the parent of abstract
>>>> level that is overriding the function (not always the concrete class).
>>>>
>>>>> @name here is just to make sure we are at the right level of the class
>>>>> hierarchy.
>>>>>
>>>>
>>>> Its @name that is actually important. Its more than just assertive,
>>>> variation of @name for the same object will and should return
>>>> different results (aside from just checking). The demonstrative case
>>>> for this requirement is TYPE_ARM_CPU, which has a realize fn that
>>>> needs to call the parent version as implemented by TYPE_CPU. ARM CPUs
>>>> however have a whole bunch of concrete classes inheriting from
>>>> TYPE_ARM_CPU. So using your macro, TYPE_ARM_CPU's realize fn will only
>>>> be able to get a handle to the parent of the concrete class (i.e.
>>>> TYPE_ARM_CPU) 
>>>
>>> This is what I would really (really) expect in this situation.
>>>
>>>> and not the parent of TYPE_ARM_CPU (i.e. TYPE_CPU) as
>>>> intended.
>>>
>>> Oh. Then it is not get_parent() at all, this is get_grand_parent.
>>
>> No. It is parent of TYPE_ARM_CPU, as specified by the name argument.
> 
> Here I lost you. You really want finalize() of TYPE_CPU, no matter how many
> classes are between TYPE_CPU and your final ARM CPU class.

Correct.

> So call it
> directly, why do you need this workaround with get_parent if it is not
> really a parent of the current obj and you do not trust to what you get in
> finalize callback field of all intermediate classes (TYPE_CPU_ARM in this
> example)?
> 
> Again, I am not arguing, I really that dump and do not understand :)

This macro represents Java's "super" or C#'s "base" (which refer to the
method's parent type, not the object pointer's) rather than C++'s
SomeType:: (which refers to some concrete type down the
multi-inheritence chain and is compile-checked in C++ but is independent
of whether it's an immediate or indirect parent type). I.e.,

class ArmCpu {
   protected override void realize(DeviceState dev, ref Error err) {
       base.realize(dev, err);
       ...
   }
}

rather than

void ARMCPU::realize(DeviceState *dev, Error &err)
{
    CPU::realize(dev, err); // what if we get a 32bitCPU parent?
    ...
}

>>> realize() of TYPE_ARM_CPU_CORTEXTA9 (example) calls its parent
>>> (TYPE_ARM_CPU) realize() and that realize() calls its parent (TYPE_CPU)
>>> realize.
>>>
>>> It some level (for example, TYPE_ARM_CPU) does not want to implement
>>> realize(), then pointer to realize() from the upper class should be copied
>>> into TYPE_ARM_CPU's type info struct. class_init() callbacks are called for
>>> every class in the chain, so if some class_init() won't write to realize()
>>> pointer, it should be what the parent initialized it to, no?
>>>
>>> What do I miss here?
>>
>> You are missing that we do implement realize for TYPE_ARM_CPU, not for
>> its derived types. So we want to call TYPE_CPU's realize from there, not
>> some random realize determined by what type dev happens to have.
> 
> How is it random? Inheritance is strict. It is CPU -> ARM_CPU -> A9_ARM_CPU
> so their finalize() should point to something reasonable or inherit from a
> parent.

Inheritence can change by modifying .parent in TypeInfo. I have done so
several times for 1.6 to allow proper FOO() casts for types sharing a
state struct and you have done it for common XICS, too, so it is not
just theory. If that happens, not relying on the parent type's constant
across code is beneficial; as long as the type a function is for does
not change (i.e., not xics_realize -> common_xics_realize) then the
macro can be reused unchanged. If you hardcode TYPE_* inline, then
someone needs to go through and check all that code, making review and
changes harder.

In case you were not aware, read the documentation Peter Ch. just fixed
on overriding QOM methods in include/qom/object.h to see how Anthony
originally asked us to do things. This macro (and its predecessors) try
to simplify that process by avoiding adding *Class classes per type
GObject-style, which does not scale well for leaf types (such as
cortex-a9-arm-cpu).
If Anthony does not object to this approach, then we should update the
documentation and I should pick up and queue Peter Cr.'s ISA and ARM
patches.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro
  2013-08-07  6:10                 ` Peter Crosthwaite
@ 2013-08-07  6:38                   ` Alexey Kardashevskiy
  0 siblings, 0 replies; 38+ messages in thread
From: Alexey Kardashevskiy @ 2013-08-07  6:38 UTC (permalink / raw)
  To: Peter Crosthwaite; +Cc: Andreas Färber, Anthony Liguori, qemu-devel

On 08/07/2013 04:10 PM, Peter Crosthwaite wrote:
> On Wed, Aug 7, 2013 at 3:53 PM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> On 08/07/2013 03:45 PM, Andreas Färber wrote:
>>> Am 07.08.2013 07:38, schrieb Alexey Kardashevskiy:
>>>> On 08/07/2013 02:20 PM, Peter Crosthwaite wrote:
>>>>> Hi,
>>>>>
>>>>> On Wed, Aug 7, 2013 at 1:36 PM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>> On 08/06/2013 06:33 PM, Andreas Färber wrote:
>>>>>>> Am 06.08.2013 07:54, schrieb Alexey Kardashevskiy:
>>>>>>>> On 08/01/2013 12:17 PM, Andreas Färber wrote:
>>>>>>>>> The object argument is currently unused and may be used to optimize the
>>>>>>>>> class lookup when needed.
>>>>>>>>>
>>>>>>>>> Inspired-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>>>>>>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>>>>>>>> ---
>>>>>>>>>  include/qom/object.h | 10 ++++++++++
>>>>>>>>>  1 file changed, 10 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/include/qom/object.h b/include/qom/object.h
>>>>>>>>> index 23fc048..a8e71dc 100644
>>>>>>>>> --- a/include/qom/object.h
>>>>>>>>> +++ b/include/qom/object.h
>>>>>>>>> @@ -511,6 +511,16 @@ struct TypeInfo
>>>>>>>>>      OBJECT_CLASS_CHECK(class, object_get_class(OBJECT(obj)), name)
>>>>>>>>>
>>>>>>>>>  /**
>>>>>>>>> + * OBJECT_GET_PARENT_CLASS:
>>>>>>>>> + * @obj: The object to obtain the parent class for.
>>>>>>>>> + * @name: The QOM typename of @obj.
>>>>>>>>> + *
>>>>>>>>> + * Returns the parent class for a given object of a specific class.
>>>>>>>>> + */
>>>>>>>>> +#define OBJECT_GET_PARENT_CLASS(obj, name) \
>>>>>>>>> +    object_class_get_parent(object_class_by_name(name))
>>>>>>>>> +
>>>>>>>>> +/**
>>>>>>>>>   * InterfaceInfo:
>>>>>>>>>   * @type: The name of the interface.
>>>>>>>>>   *
>>>>>>>>>
>>>>>>>>
>>>>>>>> Has anyone ever tried to use this macro?
>>>>>>>
>>>>>>> Since you're asking me, obviously later in this virtio series it's used
>>>>>>> and in the IndustryPack series as well.
>>>>>>>
>>>>>>> I'm not aware of anyone else having used it yet - I'm still waiting for
>>>>>>> review feedback from Peter Cr. and/or Anthony (or you!) before I put it
>>>>>>> on qom-next.
>>>>>>
>>>>>>
>>>>>> Still do not understand what "obj" is doing here.
>>>>>
>>>>> The object is currently where cast cache optimizations are
>>>>> implemented. So having a handle to it is useful if ever these
>>>>> get-parent operations end up in fast paths and we need to do one of
>>>>> Anthony's caching tricks.
>>>>>
>>>>>> This what I would suggest:
>>>>>>
>>>>>> #define OBJECT_GET_PARENT_CLASS(obj, name) \
>>>>>>     object_class_get_parent(OBJECT_GET_CLASS(ObjectClass, (obj), (name)))
>>>>>>
>>>>>
>>>>> You have changed the semantic from what Andreas has implemented. This
>>>>> will always return the parent of the concrete class, whereas to solve
>>>>> the save-override-call problem you need to get the parent of abstract
>>>>> level that is overriding the function (not always the concrete class).
>>>>>
>>>>>> @name here is just to make sure we are at the right level of the class
>>>>>> hierarchy.
>>>>>>
>>>>>
>>>>> Its @name that is actually important. Its more than just assertive,
>>>>> variation of @name for the same object will and should return
>>>>> different results (aside from just checking). The demonstrative case
>>>>> for this requirement is TYPE_ARM_CPU, which has a realize fn that
>>>>> needs to call the parent version as implemented by TYPE_CPU. ARM CPUs
>>>>> however have a whole bunch of concrete classes inheriting from
>>>>> TYPE_ARM_CPU. So using your macro, TYPE_ARM_CPU's realize fn will only
>>>>> be able to get a handle to the parent of the concrete class (i.e.
>>>>> TYPE_ARM_CPU)
>>>>
>>>> This is what I would really (really) expect in this situation.
>>>>
>>>>> and not the parent of TYPE_ARM_CPU (i.e. TYPE_CPU) as
>>>>> intended.
>>>>
>>>> Oh. Then it is not get_parent() at all, this is get_grand_parent.
>>>
>>> No. It is parent of TYPE_ARM_CPU, as specified by the name argument.
>>
>> Here I lost you. You really want finalize() of TYPE_CPU, no matter how many
>> classes are between TYPE_CPU and your final ARM CPU class.
> 
> No, we want the immediate parent of the TYPE_ARM_CPU class which is
> not set in stone.
>
> Directly refing TYPE_CPU makes it difficult for
> anyone trying to re-organise the QOM heirachy. The idea behind this
> approach was that TYPE_ARM_CPU::realize does not make assumptions
> about the classes above it (except that someone in the ancestry is
> TYPE_DEVICE for the definition of realize) nor the classes below it
> (as already discussed).
> 
> For example, if someone decides to implement TYPE_CPU_FOO (abstract
> child of TYPE_CPU) and reparents TYPE_ARM_CPU to this, then without
> need-for-change TYPE_ARM_CPU will now call TYPE_CPU_FOO:realize rather
> than inadvertently skipping over levels to a grandparent
> implementation. It can be done your proposed way, but re-organising
> the heirachy will potentially require change patterns that are avoided
> with this approach.
> 
> So call it
>> directly, why do you need this workaround with get_parent if it is not
>> really a parent of the current obj and you do not trust to what you get in
> 
> This is probably terminology confusion. Its a parent of a class not an
> object. The documentation and naming maybe needs work?


Oh. Right. It is confusion. I get it now. For some reason I decided that a
specific ARM CPU has its own realize() but it is defined in TYPE_CPU_ARM.
Sorry.


-- 
Alexey

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

* Re: [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio
  2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
                   ` (22 preceding siblings ...)
  2013-08-01  2:22 ` [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
@ 2013-08-14 16:28 ` Anthony Liguori
  2013-08-14 16:49   ` Andreas Färber
  23 siblings, 1 reply; 38+ messages in thread
From: Anthony Liguori @ 2013-08-14 16:28 UTC (permalink / raw)
  To: None, qemu-devel
  Cc: Peter Crosthwaite, Michael S. Tsirkin, Alexey Kardashevskiy,
	Hu Tao, Jesse Larrew, None, Anthony Liguori, Paolo Bonzini,
	Frederic Konrad

Applied.  Thanks.

Regards,

Anthony Liguori

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

* Re: [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio
  2013-08-14 16:28 ` Anthony Liguori
@ 2013-08-14 16:49   ` Andreas Färber
  0 siblings, 0 replies; 38+ messages in thread
From: Andreas Färber @ 2013-08-14 16:49 UTC (permalink / raw)
  To: Michael S. Tsirkin, Anthony Liguori
  Cc: Peter Crosthwaite, qemu-devel, Alexey Kardashevskiy, Hu Tao,
	Alberto Garcia, Anthony Liguori, Jesse Larrew, None,
	Paolo Bonzini, Frederic Konrad

Am 14.08.2013 18:28, schrieb Anthony Liguori:
> Applied.  Thanks.

For the record, only patches 2-3 were applied (which had accidentally
ended up also being included in this v2 despite submitted separately).

Series is still in need of review for 1.7.

In particular I'm still hoping for second opinions - including Anthony's
- on patch 1, which is blocking other series such as IndustryPack [1].

Regards,
Andreas

[1] http://lists.nongnu.org/archive/html/qemu-devel/2013-08/msg00228.html

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

end of thread, other threads:[~2013-08-14 16:49 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-01  2:17 [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 01/22] object: Add OBJECT_GET_PARENT_CLASS() macro Andreas Färber
2013-08-06  5:54   ` Alexey Kardashevskiy
2013-08-06  8:33     ` Andreas Färber
2013-08-07  3:36       ` Alexey Kardashevskiy
2013-08-07  4:20         ` Peter Crosthwaite
2013-08-07  5:38           ` Alexey Kardashevskiy
2013-08-07  5:45             ` Andreas Färber
2013-08-07  5:53               ` Alexey Kardashevskiy
2013-08-07  6:10                 ` Peter Crosthwaite
2013-08-07  6:38                   ` Alexey Kardashevskiy
2013-08-07  6:28                 ` Andreas Färber
2013-08-07  5:58         ` Andreas Färber
2013-08-07  1:50   ` Peter Crosthwaite
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 02/22] virtio-console: Use exitfn for virtserialport, too Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 03/22] virtio-9p-device: Avoid freeing uninitialized memory Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 04/22] virtio-blk-dataplane: Improve error reporting Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 05/22] virtio: Allow NULL VirtioDeviceClass::init hook Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 06/22] virtio-9p: QOM realize preparations Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 07/22] virtio-9p: Convert to QOM realize Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 08/22] virtio-blk: QOM realize preparations Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 09/22] virtio-blk: Convert to QOM realize Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 10/22] virtio-serial: QOM realize preparations Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 11/22] virtio-serial: Convert to QOM realize Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 12/22] virtio-net: QOM realize preparations Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 13/22] virtio-net: Convert to QOM realize Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 14/22] virtio-balloon: QOM realize preparations Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 15/22] virtio-balloon: Convert to QOM realize Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 16/22] virtio-rng: QOM realize preparations Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 17/22] virtio-rng: Convert to QOM realize Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 18/22] virtio-scsi: QOM realize preparations Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 19/22] virtio-scsi: Convert to QOM realize Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 20/22] virtio: Convert VirtioDevice " Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH for-next v2 21/22] virtio: Unrealize parent Andreas Färber
2013-08-01  2:17 ` [Qemu-devel] [PATCH RFC for-next v2 22/22] virtio-scsi: Convert virtio_scsi_common_[un]realize to QOM realize Andreas Färber
2013-08-01  2:22 ` [Qemu-devel] [PATCH for-next v2 00/22] QOM realize for virtio Andreas Färber
2013-08-14 16:28 ` Anthony Liguori
2013-08-14 16:49   ` Andreas Färber

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.