qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH RFT 0/5] QOM realize for virtio
@ 2013-06-07 18:18 Andreas Färber
  2013-06-07 18:18 ` [Qemu-devel] [PATCH RFT 1/5] virtio-blk-dataplane: Improve error reporting Andreas Färber
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Andreas Färber @ 2013-06-07 18:18 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, jlarrew, anthony, Paolo Bonzini,
	Andreas Färber, fred.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().

As a follow-up, VirtIOSerialPort is also converted to QOM realize/unrealize.
As a side-effect, virtio-console realization is changed from in-order to pre-order.

Incidentally I stumbled over a minor cleanup issue with virtserialport.

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

Regards,
Andreas

Cc: Anthony Liguori <anthony@codemonkey.ws>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
Cc: Frederic Konrad <fred.konrad@greensocs.com>

Andreas Färber (5):
  virtio-blk-dataplane: Improve error reporting
  virtio: Convert VirtioDevice to QOM realize/unrealize
  virtio-console: QOM'ify VirtConsole
  virtio-console: Use exitfn for virtserialport, too
  virtio-serial-port: Convert to QOM realize/unrealize

 hw/9pfs/virtio-9p-device.c         | 67 ++++++++++++++------------
 hw/9pfs/virtio-9p.h                | 13 +++++
 hw/block/dataplane/virtio-blk.c    | 25 +++++-----
 hw/block/dataplane/virtio-blk.h    |  5 +-
 hw/block/virtio-blk.c              | 56 +++++++++++++--------
 hw/char/virtio-console.c           | 99 ++++++++++++++++++++++++++------------
 hw/char/virtio-serial-bus.c        | 94 ++++++++++++++++++------------------
 hw/net/virtio-net.c                | 48 ++++++++++--------
 hw/scsi/vhost-scsi.c               | 59 +++++++++++++----------
 hw/scsi/virtio-scsi.c              | 85 ++++++++++++++++++++------------
 hw/virtio/virtio-balloon.c         | 50 +++++++++++--------
 hw/virtio/virtio-rng.c             | 53 +++++++++++---------
 hw/virtio/virtio.c                 | 20 +++-----
 include/hw/virtio/vhost-scsi.h     | 13 +++++
 include/hw/virtio/virtio-balloon.h | 13 +++++
 include/hw/virtio/virtio-blk.h     | 13 +++++
 include/hw/virtio/virtio-net.h     | 13 +++++
 include/hw/virtio/virtio-rng.h     | 13 +++++
 include/hw/virtio/virtio-scsi.h    | 29 +++++++++--
 include/hw/virtio/virtio-serial.h  | 24 ++++-----
 include/hw/virtio/virtio.h         |  6 ++-
 21 files changed, 513 insertions(+), 285 deletions(-)

-- 
1.8.1.4

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

* [Qemu-devel] [PATCH RFT 1/5] virtio-blk-dataplane: Improve error reporting
  2013-06-07 18:18 [Qemu-devel] [PATCH RFT 0/5] QOM realize for virtio Andreas Färber
@ 2013-06-07 18:18 ` Andreas Färber
  2013-06-10 11:39   ` Stefan Hajnoczi
  2013-06-07 18:18 ` [Qemu-devel] [PATCH RFT 2/5] virtio: Convert VirtioDevice to QOM realize/unrealize Andreas Färber
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Andreas Färber @ 2013-06-07 18:18 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Anthony Liguori, anthony, jlarrew, Stefan Hajnoczi,
	Andreas Färber, fred.konrad

Return an Error so that it can be propagated later.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/block/dataplane/virtio-blk.c | 25 +++++++++++++------------
 hw/block/dataplane/virtio-blk.h |  5 +++--
 hw/block/virtio-blk.c           |  8 +++++++-
 3 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 0356665..5ae8d6e 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -395,8 +395,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;
@@ -404,25 +405,26 @@ 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;
     }
 
     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);
@@ -438,7 +440,6 @@ bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
     migrate_add_blocker(s->migration_blocker);
 
     *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 cf12469..8ea1f03 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -633,6 +633,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) {
@@ -660,7 +663,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] 19+ messages in thread

* [Qemu-devel] [PATCH RFT 2/5] virtio: Convert VirtioDevice to QOM realize/unrealize
  2013-06-07 18:18 [Qemu-devel] [PATCH RFT 0/5] QOM realize for virtio Andreas Färber
  2013-06-07 18:18 ` [Qemu-devel] [PATCH RFT 1/5] virtio-blk-dataplane: Improve error reporting Andreas Färber
@ 2013-06-07 18:18 ` Andreas Färber
  2013-06-08  2:22   ` Peter Crosthwaite
  2013-06-09 10:36   ` Michael S. Tsirkin
  2013-06-07 18:18 ` [Qemu-devel] [PATCH RFT 3/5] virtio-console: QOM'ify VirtConsole Andreas Färber
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 19+ messages in thread
From: Andreas Färber @ 2013-06-07 18:18 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi, Michael S. Tsirkin,
	jlarrew, Aneesh Kumar K.V, anthony, Amit Shah, Paolo Bonzini,
	Andreas Färber, fred.konrad

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

Note: 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.

While at it, drop duplicate VIRTIO_DEVICE() casts and avoid qdev.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/9pfs/virtio-9p-device.c         | 67 ++++++++++++++++--------------
 hw/9pfs/virtio-9p.h                | 13 ++++++
 hw/block/virtio-blk.c              | 52 ++++++++++++++---------
 hw/char/virtio-serial-bus.c        | 49 ++++++++++++++--------
 hw/net/virtio-net.c                | 48 ++++++++++++---------
 hw/scsi/vhost-scsi.c               | 59 +++++++++++++++-----------
 hw/scsi/virtio-scsi.c              | 85 ++++++++++++++++++++++++--------------
 hw/virtio/virtio-balloon.c         | 50 +++++++++++++---------
 hw/virtio/virtio-rng.c             | 53 ++++++++++++++----------
 hw/virtio/virtio.c                 | 20 ++++-----
 include/hw/virtio/vhost-scsi.h     | 13 ++++++
 include/hw/virtio/virtio-balloon.h | 13 ++++++
 include/hw/virtio/virtio-blk.h     | 13 ++++++
 include/hw/virtio/virtio-net.h     | 13 ++++++
 include/hw/virtio/virtio-rng.h     | 13 ++++++
 include/hw/virtio/virtio-scsi.h    | 29 +++++++++++--
 include/hw/virtio/virtio-serial.h  | 13 ++++++
 include/hw/virtio/virtio.h         |  6 ++-
 18 files changed, 406 insertions(+), 203 deletions(-)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index dc6f4e4..409d315 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -41,15 +41,17 @@ 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);
+    V9fsClass *v9c = VIRTIO_9P_GET_CLASS(dev);
     int i, len;
     struct stat stat;
     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 */
@@ -65,17 +67,17 @@ 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");
-        return -1;
+        error_setg(errp, "Virtio-9p device couldn't find fsdev with the "
+                   "id = %s",
+                   s->fsconf.fsdev_id ? s->fsconf.fsdev_id : "NULL");
+        return;
     }
 
     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);
-        return -1;
+        error_setg(errp, "fsdev with id %s needs mount_tag arguments",
+                   s->fsconf.fsdev_id);
+        return;
     }
 
     s->ctx.export_flags = fse->export_flags;
@@ -83,9 +85,9 @@ 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);
-        return -1;
+        error_setg(errp, "mount tag '%s' (%d bytes) is longer than "
+                   "maximum (%d bytes)", s->fsconf.tag, len, MAX_TAG_LEN - 1);
+        return;
     }
 
     s->tag = strdup(s->fsconf.tag);
@@ -97,13 +99,13 @@ 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);
-        return -1;
+        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);
+        return;
     }
     if (v9fs_init_worker_threads() < 0) {
-        fprintf(stderr, "worker thread initialization failed\n");
-        return -1;
+        error_setg(errp, "worker thread initialization failed");
+        return;
     }
 
     /*
@@ -113,20 +115,20 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
      */
     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));
-        return -1;
+        error_setg(errp,
+                   "error in converting name to path %s", strerror(errno));
+        return;
     }
     if (s->ops->lstat(&s->ctx, &path, &stat)) {
-        fprintf(stderr, "share path %s does not exist\n", fse->path);
-        return -1;
+        error_setg(errp, "share path %s does not exist", fse->path);
+        return;
     } else if (!S_ISDIR(stat.st_mode)) {
-        fprintf(stderr, "share path %s is not a directory\n", fse->path);
-        return -1;
+        error_setg(errp, "share path %s is not a directory", fse->path);
+        return;
     }
     v9fs_path_free(&path);
 
-    return 0;
+    v9c->parent_realize(dev, errp);
 }
 
 /* virtio-9p device */
@@ -136,12 +138,16 @@ static Property virtio_9p_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void virtio_9p_class_init(ObjectClass *klass, void *data)
+static void virtio_9p_class_init(ObjectClass *oc, void *data)
 {
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
+    V9fsClass *v9c = VIRTIO_9P_CLASS(oc);
+
+    v9c->parent_realize = dc->realize;
+    dc->realize = virtio_9p_device_realize;
+
     dc->props = virtio_9p_properties;
-    vdc->init = virtio_9p_device_init;
     vdc->get_features = virtio_9p_get_features;
     vdc->get_config = virtio_9p_get_config;
 }
@@ -151,6 +157,7 @@ static const TypeInfo virtio_device_info = {
     .parent = TYPE_VIRTIO_DEVICE,
     .instance_size = sizeof(V9fsState),
     .class_init = virtio_9p_class_init,
+    .class_size = sizeof(V9fsClass),
 };
 
 static void virtio_9p_register_types(void)
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 1d6eedb..85699a7 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -227,6 +227,15 @@ typedef struct V9fsState
     V9fsConf fsconf;
 } V9fsState;
 
+typedef struct V9fsClass {
+    /*< private >*/
+    VirtioDeviceClass parent_class;
+    /*< public >*/
+
+    DeviceRealize parent_realize;
+} V9fsClass;
+
+
 typedef struct V9fsStatState {
     V9fsPDU *pdu;
     size_t offset;
@@ -404,6 +413,10 @@ 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_CLASS(obj) \
+        OBJECT_GET_CLASS(V9fsClass, (obj), TYPE_VIRTIO_9P)
+#define VIRTIO_9P_CLASS(cls) \
+        OBJECT_CLASS_CHECK(V9fsClass, (cls), TYPE_VIRTIO_9P)
 
 #define DEFINE_VIRTIO_9P_PROPERTIES(_state, _field)             \
         DEFINE_PROP_STRING("mount_tag", _state, _field.tag),    \
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 8ea1f03..88e99a6 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -628,10 +628,11 @@ void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk)
     memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));
 }
 
-static int virtio_blk_device_init(VirtIODevice *vdev)
+static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
 {
-    DeviceState *qdev = DEVICE(vdev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOBlock *s = VIRTIO_BLK(vdev);
+    VirtIOBlockClass *vbc = VIRTIO_BLK_GET_CLASS(dev);
     VirtIOBlkConf *blk = &(s->blk);
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
     Error *err = NULL;
@@ -639,17 +640,18 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
     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,
@@ -665,29 +667,31 @@ 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;
     }
 #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");
-    return 0;
+    add_boot_device_path(s->conf->bootindex, dev, "/disk@0,0");
+
+    vbc->parent_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);
+    VirtIOBlockClass *vbc = VIRTIO_BLK_GET_CLASS(dev);
+
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
     virtio_blk_data_plane_destroy(s->dataplane);
     s->dataplane = NULL;
@@ -696,7 +700,8 @@ 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;
+
+    vbc->parent_unrealize(dev, errp);
 }
 
 static Property virtio_blk_properties[] = {
@@ -704,13 +709,19 @@ static Property virtio_blk_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void virtio_blk_class_init(ObjectClass *klass, void *data)
+static void virtio_blk_class_init(ObjectClass *oc, void *data)
 {
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_blk_device_exit;
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
+    VirtIOBlockClass *vbc = VIRTIO_BLK_CLASS(oc);
+
+    vbc->parent_realize = dc->realize;
+    dc->realize = virtio_blk_device_realize;
+
+    vbc->parent_unrealize = dc->unrealize;
+    dc->unrealize = virtio_blk_device_unrealize;
+
     dc->props = virtio_blk_properties;
-    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;
@@ -723,6 +734,7 @@ static const TypeInfo virtio_device_info = {
     .parent = TYPE_VIRTIO_DEVICE,
     .instance_size = sizeof(VirtIOBlock),
     .class_init = virtio_blk_class_init,
+    .class_size = sizeof(VirtIOBlockClass),
 };
 
 static void virtio_register_types(void)
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index cc3d1dd..1cdd659 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -889,31 +889,35 @@ 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 *qdev = DEVICE(vdev);
-    VirtIOSerial *vser = VIRTIO_SERIAL(vdev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOSerial *vser = VIRTIO_SERIAL(dev);
+    VirtIOSerialClass *vsc = VIRTIO_SERIAL_GET_CLASS(dev);
+    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,
                 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,10 +965,10 @@ 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;
+    vsc->parent_realize(dev, errp);
 }
 
 static void virtio_serial_port_class_init(ObjectClass *klass, void *data)
@@ -986,10 +990,11 @@ 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);
+    VirtIOSerialClass *vsc = VIRTIO_SERIAL_GET_CLASS(dev);
 
     unregister_savevm(dev, "virtio-console", vser);
 
@@ -1003,7 +1008,8 @@ static int virtio_serial_device_exit(DeviceState *dev)
         g_free(vser->post_load);
     }
     virtio_cleanup(vdev);
-    return 0;
+
+    vsc->parent_unrealize(dev, errp);
 }
 
 static Property virtio_serial_properties[] = {
@@ -1011,13 +1017,19 @@ static Property virtio_serial_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void virtio_serial_class_init(ObjectClass *klass, void *data)
+static void virtio_serial_class_init(ObjectClass *oc, void *data)
 {
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_serial_device_exit;
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
+    VirtIOSerialClass *vsc = VIRTIO_SERIAL_CLASS(oc);
+
+    vsc->parent_realize = dc->realize;
+    dc->realize = virtio_serial_device_realize;
+
+    vsc->parent_unrealize = dc->unrealize;
+    dc->unrealize = virtio_serial_device_unrealize;
+
     dc->props = virtio_serial_properties;
-    vdc->init = virtio_serial_device_init;
     vdc->get_features = get_features;
     vdc->get_config = get_config;
     vdc->set_config = set_config;
@@ -1030,6 +1042,7 @@ static const TypeInfo virtio_device_info = {
     .parent = TYPE_VIRTIO_DEVICE,
     .instance_size = sizeof(VirtIOSerial),
     .class_init = virtio_serial_class_init,
+    .class_size = sizeof(VirtIOSerialClass),
 };
 
 static void virtio_serial_register_types(void)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 1ea9556..9a3680c 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1367,15 +1367,16 @@ 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)
 {
     int i;
 
-    DeviceState *qdev = DEVICE(vdev);
-    VirtIONet *n = VIRTIO_NET(vdev);
+    DeviceState *qdev = DEVICE(dev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIONet *n = VIRTIO_NET(dev);
+    VirtIONetClass *vnc = VIRTIO_NET_GET_CLASS(dev);
 
-    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);
@@ -1439,24 +1440,26 @@ static int virtio_net_device_init(VirtIODevice *vdev)
 
     n->vlans = g_malloc0(MAX_VLAN >> 3);
 
-    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");
-    return 0;
+    add_boot_device_path(n->nic_conf.bootindex, dev, "/ethernet-phy@0");
+
+    vnc->parent_realize(dev, errp);
 }
 
-static int virtio_net_device_exit(DeviceState *qdev)
+static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
 {
-    VirtIONet *n = VIRTIO_NET(qdev);
-    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIONet *n = VIRTIO_NET(dev);
+    VirtIONetClass *vnc = VIRTIO_NET_GET_CLASS(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);
@@ -1488,7 +1491,7 @@ static int virtio_net_device_exit(DeviceState *qdev)
     qemu_del_nic(n->nic);
     virtio_cleanup(vdev);
 
-    return 0;
+    vnc->parent_unrealize(dev, errp);
 }
 
 static void virtio_net_instance_init(Object *obj)
@@ -1511,13 +1514,19 @@ static Property virtio_net_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void virtio_net_class_init(ObjectClass *klass, void *data)
+static void virtio_net_class_init(ObjectClass *oc, void *data)
 {
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_net_device_exit;
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
+    VirtIONetClass *vnc = VIRTIO_NET_CLASS(oc);
+
+    vnc->parent_realize = dc->realize;
+    dc->realize = virtio_net_device_realize;
+
+    vnc->parent_unrealize = dc->unrealize;
+    dc->unrealize = virtio_net_device_unrealize;
+
     dc->props = virtio_net_properties;
-    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;
@@ -1535,6 +1544,7 @@ static const TypeInfo virtio_net_info = {
     .instance_size = sizeof(VirtIONet),
     .instance_init = virtio_net_instance_init,
     .class_init = virtio_net_class_init,
+    .class_size = sizeof(VirtIONetClass),
 };
 
 static void virtio_register_types(void)
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index d7a1c33..cacaf64 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -196,29 +196,32 @@ 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);
+    VHostSCSIClass *vsc = VHOST_SCSI_GET_CLASS(dev);
+    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;
+    vsc->parent_realize(dev, &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
     }
 
     s->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
@@ -227,24 +230,22 @@ 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;
 
     error_setg(&s->migration_blocker,
             "vhost-scsi does not support migration");
     migrate_add_blocker(s->migration_blocker);
-
-    return 0;
 }
 
-static int vhost_scsi_exit(DeviceState *qdev)
+static void vhost_scsi_unrealize(DeviceState *dev, Error **errp)
 {
-    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);
+    VHostSCSIClass *vsc = VHOST_SCSI_GET_CLASS(dev);
 
     migrate_del_blocker(s->migration_blocker);
     error_free(s->migration_blocker);
@@ -253,7 +254,8 @@ static int vhost_scsi_exit(DeviceState *qdev)
     vhost_scsi_set_status(vdev, 0);
 
     g_free(s->dev.vqs);
-    return virtio_scsi_common_exit(vs);
+
+    vsc->parent_unrealize(dev, errp);
 }
 
 static Property vhost_scsi_properties[] = {
@@ -261,13 +263,19 @@ static Property vhost_scsi_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void vhost_scsi_class_init(ObjectClass *klass, void *data)
+static void vhost_scsi_class_init(ObjectClass *oc, void *data)
 {
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = vhost_scsi_exit;
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
+    VHostSCSIClass *vsc = VHOST_SCSI_CLASS(oc);
+
+    vsc->parent_realize = dc->realize;
+    dc->realize = vhost_scsi_realize;
+
+    vsc->parent_unrealize = dc->unrealize;
+    dc->unrealize = vhost_scsi_unrealize;
+
     dc->props = vhost_scsi_properties;
-    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;
@@ -278,6 +286,7 @@ static const TypeInfo vhost_scsi_info = {
     .parent = TYPE_VIRTIO_SCSI_COMMON,
     .instance_size = sizeof(VHostSCSI),
     .class_init = vhost_scsi_class_init,
+    .class_size = sizeof(VHostSCSIClass),
 };
 
 static void virtio_register_types(void)
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 08dd3f3..6704f78 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -587,12 +587,14 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
     .load_request = virtio_scsi_load_request,
 };
 
-int virtio_scsi_common_init(VirtIOSCSICommon *s)
+static 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);
+    VirtIOSCSICommonClass *vscc = VIRTIO_SCSI_COMMON_GET_CLASS(dev);
     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 *));
@@ -608,50 +610,53 @@ int virtio_scsi_common_init(VirtIOSCSICommon *s)
                                          virtio_scsi_handle_cmd);
     }
 
-    return 0;
+    vscc->parent_realize(dev, errp);
 }
 
-static int virtio_scsi_device_init(VirtIODevice *vdev)
+static void virtio_scsi_device_realize(DeviceState *dev, Error **errp)
 {
-    DeviceState *qdev = DEVICE(vdev);
-    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
-    VirtIOSCSI *s = VIRTIO_SCSI(vdev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOSCSI *s = VIRTIO_SCSI(dev);
+    VirtIOSCSIClass *vsc = VIRTIO_SCSI_GET_CLASS(dev);
     static int virtio_scsi_id;
-    int ret;
+    Error *err = NULL;
 
-    ret = virtio_scsi_common_init(vs);
-    if (ret < 0) {
-        return ret;
+    vsc->parent_realize(dev, &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
     }
 
-    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);
     }
 
-    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;
 }
 
-int virtio_scsi_common_exit(VirtIOSCSICommon *vs)
+static 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);
+    VirtIOSCSICommonClass *vscc = VIRTIO_SCSI_COMMON_GET_CLASS(dev);
 
     g_free(vs->cmd_vqs);
     virtio_cleanup(vdev);
-    return 0;
+
+    vscc->parent_unrealize(dev, errp);
 }
 
-static int virtio_scsi_device_exit(DeviceState *qdev)
+static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp)
 {
-    VirtIOSCSI *s = VIRTIO_SCSI(qdev);
-    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(qdev);
+    VirtIOSCSI *s = VIRTIO_SCSI(dev);
+    VirtIOSCSIClass *vsc = VIRTIO_SCSI_GET_CLASS(dev);
 
-    unregister_savevm(qdev, "virtio-scsi", s);
-    return virtio_scsi_common_exit(vs);
+    unregister_savevm(dev, "virtio-scsi", s);
+
+    vsc->parent_unrealize(dev, errp);
 }
 
 static Property virtio_scsi_properties[] = {
@@ -659,20 +664,34 @@ static Property virtio_scsi_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void virtio_scsi_common_class_init(ObjectClass *klass, void *data)
+static void virtio_scsi_common_class_init(ObjectClass *oc, void *data)
 {
-    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
+    VirtIOSCSICommonClass *vscc = VIRTIO_SCSI_COMMON_CLASS(oc);
+
+    vscc->parent_realize = dc->realize;
+    dc->realize = virtio_scsi_common_realize;
+
+    vscc->parent_unrealize = dc->unrealize;
+    dc->unrealize = virtio_scsi_common_unrealize;
 
     vdc->get_config = virtio_scsi_get_config;
 }
 
-static void virtio_scsi_class_init(ObjectClass *klass, void *data)
+static void virtio_scsi_class_init(ObjectClass *oc, void *data)
 {
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_scsi_device_exit;
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
+    VirtIOSCSIClass *vsc = VIRTIO_SCSI_CLASS(oc);
+
+    vsc->parent_realize = dc->realize;
+    dc->realize = virtio_scsi_device_realize;
+
+    vsc->parent_unrealize = dc->unrealize;
+    dc->unrealize = virtio_scsi_device_unrealize;
+
     dc->props = virtio_scsi_properties;
-    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;
@@ -683,6 +702,7 @@ static const TypeInfo virtio_scsi_common_info = {
     .parent = TYPE_VIRTIO_DEVICE,
     .instance_size = sizeof(VirtIOSCSICommon),
     .class_init = virtio_scsi_common_class_init,
+    .class_size = sizeof(VirtIOSCSICommonClass),
 };
 
 static const TypeInfo virtio_scsi_info = {
@@ -690,6 +710,7 @@ static const TypeInfo virtio_scsi_info = {
     .parent = TYPE_VIRTIO_SCSI_COMMON,
     .instance_size = sizeof(VirtIOSCSI),
     .class_init = virtio_scsi_class_init,
+    .class_size = sizeof(VirtIOSCSIClass),
 };
 
 static void virtio_register_types(void)
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index d669756..0535062 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -336,10 +336,11 @@ 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 *qdev = DEVICE(vdev);
-    VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOBalloon *s = VIRTIO_BALLOON(dev);
+    VirtIOBalloonClass *vbc = VIRTIO_BALLOON_GET_CLASS(dev);
     int ret;
 
     virtio_init(vdev, "virtio-balloon", VIRTIO_ID_BALLOON, 8);
@@ -348,50 +349,60 @@ static int virtio_balloon_device_init(VirtIODevice *vdev)
                                    virtio_balloon_stat, s);
 
     if (ret < 0) {
-        virtio_cleanup(VIRTIO_DEVICE(s));
-        return -1;
+        error_setg(errp, "Adding balloon handler failed");
+        virtio_cleanup(vdev);
+        return;
     }
 
     s->ivq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
     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;
+
+    vbc->parent_realize(dev, errp);
 }
 
-static int virtio_balloon_device_exit(DeviceState *qdev)
+static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
 {
-    VirtIOBalloon *s = VIRTIO_BALLOON(qdev);
-    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOBalloon *s = VIRTIO_BALLOON(dev);
+    VirtIOBalloonClass *vbc = VIRTIO_BALLOON_GET_CLASS(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;
+
+    vbc->parent_unrealize(dev, errp);
 }
 
 static Property virtio_balloon_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void virtio_balloon_class_init(ObjectClass *klass, void *data)
+static void virtio_balloon_class_init(ObjectClass *oc, void *data)
 {
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_balloon_device_exit;
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
+    VirtIOBalloonClass *vbc = VIRTIO_BALLOON_CLASS(oc);
+
+    vbc->parent_realize = dc->realize;
+    dc->realize = virtio_balloon_device_realize;
+
+    vbc->parent_unrealize = dc->unrealize;
+    dc->unrealize = virtio_balloon_device_unrealize;
+
     dc->props = virtio_balloon_properties;
-    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;
@@ -402,6 +413,7 @@ static const TypeInfo virtio_balloon_info = {
     .parent = TYPE_VIRTIO_DEVICE,
     .instance_size = sizeof(VirtIOBalloon),
     .class_init = virtio_balloon_class_init,
+    .class_size = sizeof(VirtIOBalloonClass),
 };
 
 static void virtio_register_types(void)
diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index cb787c7..108cfc9 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -133,21 +133,22 @@ 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 *qdev = DEVICE(vdev);
-    VirtIORNG *vrng = VIRTIO_RNG(vdev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIORNG *vrng = VIRTIO_RNG(dev);
+    VirtIORNGClass *vrc = VIRTIO_RNG_GET_CLASS(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);
     }
@@ -156,15 +157,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);
@@ -178,22 +178,24 @@ 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;
+    vrc->parent_realize(dev, errp);
 }
 
-static int virtio_rng_device_exit(DeviceState *qdev)
+static void virtio_rng_device_unrealize(DeviceState *dev, Error **errp)
 {
-    VirtIORNG *vrng = VIRTIO_RNG(qdev);
-    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIORNG *vrng = VIRTIO_RNG(dev);
+    VirtIORNGClass *vrc = VIRTIO_RNG_GET_CLASS(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;
+
+    vrc->parent_unrealize(dev, errp);
 }
 
 static Property virtio_rng_properties[] = {
@@ -201,13 +203,19 @@ static Property virtio_rng_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void virtio_rng_class_init(ObjectClass *klass, void *data)
+static void virtio_rng_class_init(ObjectClass *oc, void *data)
 {
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_rng_device_exit;
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
+    VirtIORNGClass *vrc = VIRTIO_RNG_CLASS(oc);
+
+    vrc->parent_realize = dc->realize;
+    dc->realize = virtio_rng_device_realize;
+
+    vrc->parent_unrealize = dc->unrealize;
+    dc->unrealize = virtio_rng_device_unrealize;
+
     dc->props = virtio_rng_properties;
-    vdc->init = virtio_rng_device_init;
     vdc->get_features = get_features;
 }
 
@@ -225,6 +233,7 @@ static const TypeInfo virtio_rng_info = {
     .instance_size = sizeof(VirtIORNG),
     .instance_init = virtio_rng_initfn,
     .class_init = virtio_rng_class_init,
+    .class_size = sizeof(VirtIORNGClass),
 };
 
 static void virtio_register_types(void)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 8176c14..d369f9a 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1105,35 +1105,29 @@ void virtio_device_set_child_bus_name(VirtIODevice *vdev, char *bus_name)
     }
 }
 
-static int virtio_device_init(DeviceState *qdev)
+static void virtio_device_realize(DeviceState *dev, Error **errp)
 {
-    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);
+
     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/vhost-scsi.h b/include/hw/virtio/vhost-scsi.h
index 85cc031..b0d64e4 100644
--- a/include/hw/virtio/vhost-scsi.h
+++ b/include/hw/virtio/vhost-scsi.h
@@ -53,6 +53,10 @@ 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_CLASS(obj) \
+        OBJECT_GET_CLASS(VHostSCSIClass, (obj), TYPE_VHOST_SCSI)
+#define VHOST_SCSI_CLASS(cls) \
+        OBJECT_CLASS_CHECK(VHostSCSIClass, (cls), TYPE_VHOST_SCSI)
 
 typedef struct VHostSCSI {
     VirtIOSCSICommon parent_obj;
@@ -62,6 +66,15 @@ typedef struct VHostSCSI {
     struct vhost_dev dev;
 } VHostSCSI;
 
+typedef struct VHostSCSIClass {
+    /*< private >*/
+    VirtIOSCSICommonClass parent_class;
+    /*< public >*/
+
+    DeviceRealize parent_realize;
+    DeviceUnrealize parent_unrealize;
+} VHostSCSIClass;
+
 #define DEFINE_VHOST_SCSI_PROPERTIES(_state, _conf_field) \
     DEFINE_PROP_STRING("vhostfd", _state, _conf_field.vhostfd), \
     DEFINE_PROP_STRING("wwpn", _state, _conf_field.wwpn), \
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index f863bfe..146fa04 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -21,6 +21,10 @@
 #define TYPE_VIRTIO_BALLOON "virtio-balloon-device"
 #define VIRTIO_BALLOON(obj) \
         OBJECT_CHECK(VirtIOBalloon, (obj), TYPE_VIRTIO_BALLOON)
+#define VIRTIO_BALLOON_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(VirtIOBalloonClass, (obj), TYPE_VIRTIO_BALLOON)
+#define VIRTIO_BALLOON_CLASS(cls) \
+        OBJECT_CLASS_CHECK(VirtIOBalloonClass, (cls), TYPE_VIRTIO_BALLOON)
 
 /* from Linux's linux/virtio_balloon.h */
 
@@ -69,4 +73,13 @@ typedef struct VirtIOBalloon {
     int64_t stats_poll_interval;
 } VirtIOBalloon;
 
+typedef struct VirtIOBalloonClass {
+    /*< private >*/
+    VirtioDeviceClass parent_class;
+    /*< public >*/
+
+    DeviceRealize parent_realize;
+    DeviceUnrealize parent_unrealize;
+} VirtIOBalloonClass;
+
 #endif
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index fc71853..5ed703d 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -20,6 +20,10 @@
 #define TYPE_VIRTIO_BLK "virtio-blk-device"
 #define VIRTIO_BLK(obj) \
         OBJECT_CHECK(VirtIOBlock, (obj), TYPE_VIRTIO_BLK)
+#define VIRTIO_BLK_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(VirtIOBlockClass, (obj), TYPE_VIRTIO_BLK)
+#define VIRTIO_BLK_CLASS(cls) \
+        OBJECT_CLASS_CHECK(VirtIOBlockClass, (cls), TYPE_VIRTIO_BLK)
 
 /* from Linux's linux/virtio_blk.h */
 
@@ -129,6 +133,15 @@ typedef struct VirtIOBlock {
 #endif
 } VirtIOBlock;
 
+typedef struct VirtIOBlockClass {
+    /*< private >*/
+    VirtioDeviceClass parent_class;
+    /*< public >*/
+
+    DeviceRealize parent_realize;
+    DeviceUnrealize parent_unrealize;
+} VirtIOBlockClass;
+
 #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
         DEFINE_VIRTIO_COMMON_FEATURES(_state, _field)
 
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index b315ac9..0b8a06e 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -20,6 +20,10 @@
 #define TYPE_VIRTIO_NET "virtio-net-device"
 #define VIRTIO_NET(obj) \
         OBJECT_CHECK(VirtIONet, (obj), TYPE_VIRTIO_NET)
+#define VIRTIO_NET_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(VirtIONetClass, (obj), TYPE_VIRTIO_NET)
+#define VIRTIO_NET_CLASS(cls) \
+        OBJECT_CLASS_CHECK(VirtIONetClass, (cls), TYPE_VIRTIO_NET)
 
 #define ETH_ALEN    6
 
@@ -195,6 +199,15 @@ typedef struct VirtIONet {
     uint64_t curr_guest_offloads;
 } VirtIONet;
 
+typedef struct VirtIONetClass {
+    /*< private >*/
+    VirtioDeviceClass parent_class;
+    /*< public >*/
+
+    DeviceRealize parent_realize;
+    DeviceUnrealize parent_unrealize;
+} VirtIONetClass;
+
 #define VIRTIO_NET_CTRL_MAC    1
  #define VIRTIO_NET_CTRL_MAC_TABLE_SET        0
  #define VIRTIO_NET_CTRL_MAC_ADDR_SET         1
diff --git a/include/hw/virtio/virtio-rng.h b/include/hw/virtio/virtio-rng.h
index debaa15..528f70c 100644
--- a/include/hw/virtio/virtio-rng.h
+++ b/include/hw/virtio/virtio-rng.h
@@ -18,6 +18,10 @@
 #define TYPE_VIRTIO_RNG "virtio-rng-device"
 #define VIRTIO_RNG(obj) \
         OBJECT_CHECK(VirtIORNG, (obj), TYPE_VIRTIO_RNG)
+#define VIRTIO_RNG_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(VirtIORNGClass, (obj), TYPE_VIRTIO_RNG)
+#define VIRTIO_RNG_CLASS(cls) \
+        OBJECT_CLASS_CHECK(VirtIORNGClass, (cls), TYPE_VIRTIO_RNG)
 
 /* The Virtio ID for the virtio rng device */
 #define VIRTIO_ID_RNG    4
@@ -46,6 +50,15 @@ typedef struct VirtIORNG {
     int64_t quota_remaining;
 } VirtIORNG;
 
+typedef struct VirtIORNGClass {
+    /*< private >*/
+    VirtioDeviceClass parent_class;
+    /*< public >*/
+
+    DeviceRealize parent_realize;
+    DeviceUnrealize parent_unrealize;
+} VirtIORNGClass;
+
 /* Set a default rate limit of 2^47 bytes per minute or roughly 2TB/s.  If
    you have an entropy source capable of generating more entropy than this
    and you can pass it through via virtio-rng, then hats off to you.  Until
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 9a98540..2ce58cd 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -21,10 +21,18 @@
 #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_CLASS(obj) \
+        OBJECT_GET_CLASS(VirtIOSCSICommonClass, (obj), TYPE_VIRTIO_SCSI_COMMON)
+#define VIRTIO_SCSI_COMMON_CLASS(cls) \
+        OBJECT_CHECK(VirtIOSCSICommonClass, (cls), TYPE_VIRTIO_SCSI_COMMON)
 
 #define TYPE_VIRTIO_SCSI "virtio-scsi-device"
 #define VIRTIO_SCSI(obj) \
         OBJECT_CHECK(VirtIOSCSI, (obj), TYPE_VIRTIO_SCSI)
+#define VIRTIO_SCSI_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(VirtIOSCSIClass, (obj), TYPE_VIRTIO_SCSI)
+#define VIRTIO_SCSI_CLASS(cls) \
+        OBJECT_CHECK(VirtIOSCSIClass, (cls), TYPE_VIRTIO_SCSI)
 
 
 /* The ID for virtio_scsi */
@@ -166,6 +174,15 @@ typedef struct VirtIOSCSICommon {
     VirtQueue **cmd_vqs;
 } VirtIOSCSICommon;
 
+typedef struct VirtIOSCSICommonClass {
+    /*< private >*/
+    VirtioDeviceClass parent_class;
+    /*< public >*/
+
+    DeviceRealize parent_realize;
+    DeviceUnrealize parent_unrealize;
+} VirtIOSCSICommonClass;
+
 typedef struct {
     VirtIOSCSICommon parent_obj;
 
@@ -174,6 +191,15 @@ typedef struct {
     bool events_dropped;
 } VirtIOSCSI;
 
+typedef struct VirtIOSCSIClass {
+    /*< private >*/
+    VirtIOSCSICommonClass parent_class;
+    /*< public >*/
+
+    DeviceRealize parent_realize;
+    DeviceUnrealize parent_unrealize;
+} VirtIOSCSIClass;
+
 #define DEFINE_VIRTIO_SCSI_PROPERTIES(_state, _conf_field)                     \
     DEFINE_PROP_UINT32("num_queues", _state, _conf_field.num_queues, 1),       \
     DEFINE_PROP_UINT32("max_sectors", _state, _conf_field.max_sectors, 0xFFFF),\
@@ -186,7 +212,4 @@ 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);
-
 #endif /* _QEMU_VIRTIO_SCSI_H */
diff --git a/include/hw/virtio/virtio-serial.h b/include/hw/virtio/virtio-serial.h
index 1d2040b..49af9e3 100644
--- a/include/hw/virtio/virtio-serial.h
+++ b/include/hw/virtio/virtio-serial.h
@@ -212,6 +212,15 @@ struct VirtIOSerial {
     virtio_serial_conf serial;
 };
 
+typedef struct VirtIOSerialClass {
+    /*< private >*/
+    VirtioDeviceClass parent_class;
+    /*< public >*/
+
+    DeviceRealize parent_realize;
+    DeviceUnrealize parent_unrealize;
+} VirtIOSerialClass;
+
 /* Interface to the virtio-serial bus */
 
 /*
@@ -247,6 +256,10 @@ 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_CLASS(obj) \
+        OBJECT_GET_CLASS(VirtIOSerialClass, (obj), TYPE_VIRTIO_SERIAL)
+#define VIRTIO_SERIAL_CLASS(cls) \
+        OBJECT_CLASS_CHECK(VirtIOSerialClass, (cls), TYPE_VIRTIO_SERIAL)
 
 #define DEFINE_VIRTIO_SERIAL_PROPERTIES(_state, _field) \
         DEFINE_PROP_UINT32("max_ports", _state, _field.max_virtserial_ports, 31)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index a6c5c53..7e1854a 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -122,9 +122,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] 19+ messages in thread

* [Qemu-devel] [PATCH RFT 3/5] virtio-console: QOM'ify VirtConsole
  2013-06-07 18:18 [Qemu-devel] [PATCH RFT 0/5] QOM realize for virtio Andreas Färber
  2013-06-07 18:18 ` [Qemu-devel] [PATCH RFT 1/5] virtio-blk-dataplane: Improve error reporting Andreas Färber
  2013-06-07 18:18 ` [Qemu-devel] [PATCH RFT 2/5] virtio: Convert VirtioDevice to QOM realize/unrealize Andreas Färber
@ 2013-06-07 18:18 ` Andreas Färber
  2013-06-07 18:18 ` [Qemu-devel] [PATCH RFT 4/5] virtio-console: Use exitfn for virtserialport, too Andreas Färber
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Andreas Färber @ 2013-06-07 18:18 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anthony Liguori, jlarrew, anthony, Amit Shah, Andreas Färber,
	fred.konrad

Introduce type constant, cast macro and rename parent field.

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

diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
index 6759e51..7b1a382 100644
--- a/hw/char/virtio-console.c
+++ b/hw/char/virtio-console.c
@@ -15,8 +15,13 @@
 #include "trace.h"
 #include "hw/virtio/virtio-serial.h"
 
+#define TYPE_VIRTIO_CONSOLE "virtconsole"
+#define VIRTIO_CONSOLE(obj) \
+    OBJECT_CHECK(VirtConsole, (obj), TYPE_VIRTIO_CONSOLE)
+
 typedef struct VirtConsole {
-    VirtIOSerialPort port;
+    VirtIOSerialPort parent_obj;
+
     CharDriverState *chr;
     guint watch;
 } VirtConsole;
@@ -31,7 +36,7 @@ static gboolean chr_write_unblocked(GIOChannel *chan, GIOCondition cond,
     VirtConsole *vcon = opaque;
 
     vcon->watch = 0;
-    virtio_serial_throttle_port(&vcon->port, false);
+    virtio_serial_throttle_port(VIRTIO_SERIAL_PORT(vcon), false);
     return FALSE;
 }
 
@@ -39,7 +44,7 @@ static gboolean chr_write_unblocked(GIOChannel *chan, GIOCondition cond,
 static ssize_t flush_buf(VirtIOSerialPort *port,
                          const uint8_t *buf, ssize_t len)
 {
-    VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+    VirtConsole *vcon = VIRTIO_CONSOLE(port);
     ssize_t ret;
 
     if (!vcon->chr) {
@@ -75,7 +80,7 @@ static ssize_t flush_buf(VirtIOSerialPort *port,
 /* Callback function that's called when the guest opens/closes the port */
 static void set_guest_connected(VirtIOSerialPort *port, int guest_connected)
 {
-    VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+    VirtConsole *vcon = VIRTIO_CONSOLE(port);
 
     if (!vcon->chr) {
         return;
@@ -88,40 +93,42 @@ static int chr_can_read(void *opaque)
 {
     VirtConsole *vcon = opaque;
 
-    return virtio_serial_guest_ready(&vcon->port);
+    return virtio_serial_guest_ready(VIRTIO_SERIAL_PORT(vcon));
 }
 
 /* Send data from a char device over to the guest */
 static void chr_read(void *opaque, const uint8_t *buf, int size)
 {
     VirtConsole *vcon = opaque;
+    VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(vcon);
 
-    trace_virtio_console_chr_read(vcon->port.id, size);
-    virtio_serial_write(&vcon->port, buf, size);
+    trace_virtio_console_chr_read(port->id, size);
+    virtio_serial_write(port, buf, size);
 }
 
 static void chr_event(void *opaque, int event)
 {
     VirtConsole *vcon = opaque;
+    VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(vcon);
 
-    trace_virtio_console_chr_event(vcon->port.id, event);
+    trace_virtio_console_chr_event(port->id, event);
     switch (event) {
     case CHR_EVENT_OPENED:
-        virtio_serial_open(&vcon->port);
+        virtio_serial_open(port);
         break;
     case CHR_EVENT_CLOSED:
         if (vcon->watch) {
             g_source_remove(vcon->watch);
             vcon->watch = 0;
         }
-        virtio_serial_close(&vcon->port);
+        virtio_serial_close(port);
         break;
     }
 }
 
 static int virtconsole_initfn(VirtIOSerialPort *port)
 {
-    VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+    VirtConsole *vcon = VIRTIO_CONSOLE(port);
     VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(port);
 
     if (port->id == 0 && !k->is_console) {
@@ -140,7 +147,7 @@ static int virtconsole_initfn(VirtIOSerialPort *port)
 
 static int virtconsole_exitfn(VirtIOSerialPort *port)
 {
-    VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+    VirtConsole *vcon = VIRTIO_CONSOLE(port);
 
     if (vcon->watch) {
         g_source_remove(vcon->watch);
@@ -168,7 +175,7 @@ static void virtconsole_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo virtconsole_info = {
-    .name          = "virtconsole",
+    .name          = TYPE_VIRTIO_CONSOLE,
     .parent        = TYPE_VIRTIO_SERIAL_PORT,
     .instance_size = sizeof(VirtConsole),
     .class_init    = virtconsole_class_init,
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH RFT 4/5] virtio-console: Use exitfn for virtserialport, too
  2013-06-07 18:18 [Qemu-devel] [PATCH RFT 0/5] QOM realize for virtio Andreas Färber
                   ` (2 preceding siblings ...)
  2013-06-07 18:18 ` [Qemu-devel] [PATCH RFT 3/5] virtio-console: QOM'ify VirtConsole Andreas Färber
@ 2013-06-07 18:18 ` Andreas Färber
  2013-07-29 23:25   ` Andreas Färber
  2013-06-07 18:19 ` [Qemu-devel] [PATCH RFT 5/5] virtio-serial-port: Convert to QOM realize/unrealize Andreas Färber
  2013-06-09 10:39 ` [Qemu-devel] [PATCH RFT 0/5] QOM realize for virtio Michael S. Tsirkin
  5 siblings, 1 reply; 19+ messages in thread
From: Andreas Färber @ 2013-06-07 18:18 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anthony Liguori, qemu-stable, jlarrew, anthony, Amit Shah,
	Andreas Färber, fred.konrad

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 7b1a382..73e18f2 100644
--- a/hw/char/virtio-console.c
+++ b/hw/char/virtio-console.c
@@ -192,6 +192,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] 19+ messages in thread

* [Qemu-devel] [PATCH RFT 5/5] virtio-serial-port: Convert to QOM realize/unrealize
  2013-06-07 18:18 [Qemu-devel] [PATCH RFT 0/5] QOM realize for virtio Andreas Färber
                   ` (3 preceding siblings ...)
  2013-06-07 18:18 ` [Qemu-devel] [PATCH RFT 4/5] virtio-console: Use exitfn for virtserialport, too Andreas Färber
@ 2013-06-07 18:19 ` Andreas Färber
  2013-06-09 10:39 ` [Qemu-devel] [PATCH RFT 0/5] QOM realize for virtio Michael S. Tsirkin
  5 siblings, 0 replies; 19+ messages in thread
From: Andreas Färber @ 2013-06-07 18:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anthony Liguori, jlarrew, anthony, Amit Shah, Andreas Färber,
	fred.konrad

Note: virtconsole's/virtserialport's realizefn now registers its
handlers before VirtIOSerialPort's realizefn.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/char/virtio-console.c          | 71 ++++++++++++++++++++++++++++-----------
 hw/char/virtio-serial-bus.c       | 45 ++++++++++---------------
 include/hw/virtio/virtio-serial.h | 11 ------
 3 files changed, 68 insertions(+), 59 deletions(-)

diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
index 73e18f2..ab8b902 100644
--- a/hw/char/virtio-console.c
+++ b/hw/char/virtio-console.c
@@ -18,6 +18,10 @@
 #define TYPE_VIRTIO_CONSOLE "virtconsole"
 #define VIRTIO_CONSOLE(obj) \
     OBJECT_CHECK(VirtConsole, (obj), TYPE_VIRTIO_CONSOLE)
+#define VIRTIO_CONSOLE_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(VirtConsoleClass, (obj), TYPE_VIRTIO_CONSOLE)
+#define VIRTIO_CONSOLE_CLASS(cls) \
+    OBJECT_CLASS_CHECK(VirtConsoleClass, (cls), TYPE_VIRTIO_CONSOLE)
 
 typedef struct VirtConsole {
     VirtIOSerialPort parent_obj;
@@ -26,6 +30,13 @@ typedef struct VirtConsole {
     guint watch;
 } VirtConsole;
 
+typedef struct VirtConsoleClass {
+    VirtIOSerialPortClass parent_class;
+
+    DeviceRealize parent_realize;
+    DeviceUnrealize parent_unrealize;
+} VirtConsoleClass;
+
 /*
  * Callback function that's called from chardevs when backend becomes
  * writable.
@@ -126,14 +137,17 @@ static void chr_event(void *opaque, int event)
     }
 }
 
-static int virtconsole_initfn(VirtIOSerialPort *port)
+static void virtconsole_realize(DeviceState *dev, Error **errp)
 {
-    VirtConsole *vcon = VIRTIO_CONSOLE(port);
-    VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(port);
+    VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(dev);
+    VirtConsole *vcon = VIRTIO_CONSOLE(dev);
+    VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(dev);
+    VirtConsoleClass *vcc = VIRTIO_CONSOLE_GET_CLASS(dev);
 
     if (port->id == 0 && !k->is_console) {
-        error_report("Port number 0 on virtio-serial devices reserved for virtconsole devices for backward compatibility.");
-        return -1;
+        error_setg(errp, "Port number 0 on virtio-serial devices reserved "
+                   "for virtconsole devices for backward compatibility.");
+        return;
     }
 
     if (vcon->chr) {
@@ -142,18 +156,24 @@ static int virtconsole_initfn(VirtIOSerialPort *port)
                               vcon);
     }
 
-    return 0;
+    vcc->parent_realize(dev, errp);
 }
 
-static int virtconsole_exitfn(VirtIOSerialPort *port)
+static void virtconsole_unrealize(DeviceState *dev, Error **errp)
 {
-    VirtConsole *vcon = VIRTIO_CONSOLE(port);
+    VirtConsole *vcon = VIRTIO_CONSOLE(dev);
+    VirtConsoleClass *vcc = VIRTIO_CONSOLE_GET_CLASS(dev);
+    Error *err = NULL;
+
+    vcc->parent_unrealize(dev, &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
 
     if (vcon->watch) {
         g_source_remove(vcon->watch);
     }
-
-    return 0;
 }
 
 static Property virtconsole_properties[] = {
@@ -161,14 +181,19 @@ static Property virtconsole_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void virtconsole_class_init(ObjectClass *klass, void *data)
+static void virtconsole_class_init(ObjectClass *oc, void *data)
 {
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_CLASS(oc);
+    VirtConsoleClass *vcc = VIRTIO_CONSOLE_CLASS(oc);
+
+    vcc->parent_realize = dc->realize;
+    dc->realize = virtconsole_realize;
+
+    vcc->parent_unrealize = dc->unrealize;
+    dc->unrealize = virtconsole_unrealize;
 
     k->is_console = true;
-    k->init = virtconsole_initfn;
-    k->exit = virtconsole_exitfn;
     k->have_data = flush_buf;
     k->set_guest_connected = set_guest_connected;
     dc->props = virtconsole_properties;
@@ -179,6 +204,7 @@ static const TypeInfo virtconsole_info = {
     .parent        = TYPE_VIRTIO_SERIAL_PORT,
     .instance_size = sizeof(VirtConsole),
     .class_init    = virtconsole_class_init,
+    .class_size    = sizeof(VirtConsoleClass),
 };
 
 static Property virtserialport_properties[] = {
@@ -186,13 +212,18 @@ static Property virtserialport_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void virtserialport_class_init(ObjectClass *klass, void *data)
+static void virtserialport_class_init(ObjectClass *oc, void *data)
 {
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_CLASS(oc);
+    VirtConsoleClass *vcc = VIRTIO_CONSOLE_CLASS(oc);
+
+    vcc->parent_realize = dc->realize;
+    dc->realize = virtconsole_realize;
+
+    vcc->parent_unrealize = dc->unrealize;
+    dc->unrealize = virtconsole_unrealize;
 
-    k->init = virtconsole_initfn;
-    k->exit = virtconsole_exitfn;
     k->have_data = flush_buf;
     k->set_guest_connected = set_guest_connected;
     dc->props = virtserialport_properties;
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 1cdd659..90592d8 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -808,12 +808,12 @@ static void remove_port(VirtIOSerial *vser, uint32_t port_id)
     send_control_event(vser, port->id, VIRTIO_CONSOLE_PORT_REMOVE, 1);
 }
 
-static int virtser_port_qdev_init(DeviceState *qdev)
+static void virtser_port_device_realize(DeviceState *dev, Error **errp)
 {
-    VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, qdev);
+    VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(dev);
     VirtIOSerialPortClass *vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
-    VirtIOSerialBus *bus = DO_UPCAST(VirtIOSerialBus, qbus, qdev->parent_bus);
-    int ret, max_nr_ports;
+    VirtIOSerialBus *bus = VIRTIO_SERIAL_BUS(qdev_get_parent_bus(dev));
+    int max_nr_ports;
     bool plugging_port0;
 
     port->vser = bus->vser;
@@ -829,9 +829,9 @@ static int virtser_port_qdev_init(DeviceState *qdev)
     plugging_port0 = vsc->is_console && !find_port_by_id(port->vser, 0);
 
     if (find_port_by_id(port->vser, port->id)) {
-        error_report("virtio-serial-bus: A port already exists at id %u",
-                     port->id);
-        return -1;
+        error_setg(errp, "virtio-serial-bus: A port already exists at id %u",
+                   port->id);
+        return;
     }
 
     if (port->id == VIRTIO_CONSOLE_BAD_ID) {
@@ -840,22 +840,19 @@ static int virtser_port_qdev_init(DeviceState *qdev)
         } else {
             port->id = find_free_port_id(port->vser);
             if (port->id == VIRTIO_CONSOLE_BAD_ID) {
-                error_report("virtio-serial-bus: Maximum port limit for this device reached");
-                return -1;
+                error_setg(errp, "virtio-serial-bus: Maximum port limit for "
+                                 "this device reached");
+                return;
             }
         }
     }
 
     max_nr_ports = tswap32(port->vser->config.max_nr_ports);
     if (port->id >= max_nr_ports) {
-        error_report("virtio-serial-bus: Out-of-range port id specified, max. allowed: %u",
+        error_setg(errp, "virtio-serial-bus: Out-of-range port id specified, "
+                         "max. allowed: %u",
                      max_nr_ports - 1);
-        return -1;
-    }
-
-    ret = vsc->init(port);
-    if (ret) {
-        return ret;
+        return;
     }
 
     port->elem.out_num = 0;
@@ -868,25 +865,17 @@ static int virtser_port_qdev_init(DeviceState *qdev)
 
     /* Send an update to the guest about this new port added */
     virtio_notify_config(VIRTIO_DEVICE(port->vser));
-
-    return ret;
 }
 
-static int virtser_port_qdev_exit(DeviceState *qdev)
+static void virtser_port_device_unrealize(DeviceState *dev, Error **errp)
 {
-    VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, qdev);
-    VirtIOSerialPortClass *vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
+    VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(dev);
     VirtIOSerial *vser = port->vser;
 
     qemu_bh_delete(port->bh);
     remove_port(port->vser, port->id);
 
     QTAILQ_REMOVE(&vser->ports, port, next);
-
-    if (vsc->exit) {
-        vsc->exit(port);
-    }
-    return 0;
 }
 
 static void virtio_serial_device_realize(DeviceState *dev, Error **errp)
@@ -974,9 +963,9 @@ static void virtio_serial_device_realize(DeviceState *dev, Error **errp)
 static void virtio_serial_port_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
-    k->init = virtser_port_qdev_init;
     k->bus_type = TYPE_VIRTIO_SERIAL_BUS;
-    k->exit = virtser_port_qdev_exit;
+    k->realize = virtser_port_device_realize;
+    k->unrealize = virtser_port_device_unrealize;
     k->unplug = qdev_simple_unplug_cb;
     k->props = virtser_props;
 }
diff --git a/include/hw/virtio/virtio-serial.h b/include/hw/virtio/virtio-serial.h
index 49af9e3..8d309a4 100644
--- a/include/hw/virtio/virtio-serial.h
+++ b/include/hw/virtio/virtio-serial.h
@@ -80,17 +80,6 @@ typedef struct VirtIOSerialPortClass {
     /* Is this a device that binds with hvc in the guest? */
     bool is_console;
 
-    /*
-     * The per-port (or per-app) init function that's called when a
-     * new device is found on the bus.
-     */
-    int (*init)(VirtIOSerialPort *port);
-    /*
-     * Per-port exit function that's called when a port gets
-     * hot-unplugged or removed.
-     */
-    int (*exit)(VirtIOSerialPort *port);
-
     /* Callbacks for guest events */
         /* Guest opened/closed device. */
     void (*set_guest_connected)(VirtIOSerialPort *port, int guest_connected);
-- 
1.8.1.4

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

* Re: [Qemu-devel] [PATCH RFT 2/5] virtio: Convert VirtioDevice to QOM realize/unrealize
  2013-06-07 18:18 ` [Qemu-devel] [PATCH RFT 2/5] virtio: Convert VirtioDevice to QOM realize/unrealize Andreas Färber
@ 2013-06-08  2:22   ` Peter Crosthwaite
  2013-06-08  9:55     ` Andreas Färber
  2013-06-09 10:36   ` Michael S. Tsirkin
  1 sibling, 1 reply; 19+ messages in thread
From: Peter Crosthwaite @ 2013-06-08  2:22 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Kevin Wolf, Anthony Liguori, anthony, Michael S. Tsirkin, jlarrew,
	qemu-devel, Aneesh Kumar K.V, Stefan Hajnoczi, Amit Shah,
	Paolo Bonzini, fred.konrad

Hi Andreas,

On Sat, Jun 8, 2013 at 4:18 AM, Andreas Färber <afaerber@suse.de> wrote:
> VirtioDevice's DeviceClass::exit code cleaning up bus_name is no longer
> overwritten by virtio-{blk,serial,net,scsi,balloon,rng} and vhost-scsi.
>
> Note: 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.
>
> While at it, drop duplicate VIRTIO_DEVICE() casts and avoid qdev.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  hw/9pfs/virtio-9p-device.c         | 67 ++++++++++++++++--------------
>  hw/9pfs/virtio-9p.h                | 13 ++++++
>  hw/block/virtio-blk.c              | 52 ++++++++++++++---------
>  hw/char/virtio-serial-bus.c        | 49 ++++++++++++++--------
>  hw/net/virtio-net.c                | 48 ++++++++++++---------
>  hw/scsi/vhost-scsi.c               | 59 +++++++++++++++-----------
>  hw/scsi/virtio-scsi.c              | 85 ++++++++++++++++++++++++--------------
>  hw/virtio/virtio-balloon.c         | 50 +++++++++++++---------
>  hw/virtio/virtio-rng.c             | 53 ++++++++++++++----------
>  hw/virtio/virtio.c                 | 20 ++++-----
>  include/hw/virtio/vhost-scsi.h     | 13 ++++++
>  include/hw/virtio/virtio-balloon.h | 13 ++++++
>  include/hw/virtio/virtio-blk.h     | 13 ++++++
>  include/hw/virtio/virtio-net.h     | 13 ++++++
>  include/hw/virtio/virtio-rng.h     | 13 ++++++
>  include/hw/virtio/virtio-scsi.h    | 29 +++++++++++--
>  include/hw/virtio/virtio-serial.h  | 13 ++++++
>  include/hw/virtio/virtio.h         |  6 ++-
>  18 files changed, 406 insertions(+), 203 deletions(-)
>
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index dc6f4e4..409d315 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -41,15 +41,17 @@ 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);
> +    V9fsClass *v9c = VIRTIO_9P_GET_CLASS(dev);
>      int i, len;
>      struct stat stat;
>      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 */
> @@ -65,17 +67,17 @@ 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");
> -        return -1;
> +        error_setg(errp, "Virtio-9p device couldn't find fsdev with the "
> +                   "id = %s",
> +                   s->fsconf.fsdev_id ? s->fsconf.fsdev_id : "NULL");
> +        return;
>      }
>
>      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);
> -        return -1;
> +        error_setg(errp, "fsdev with id %s needs mount_tag arguments",
> +                   s->fsconf.fsdev_id);
> +        return;
>      }
>
>      s->ctx.export_flags = fse->export_flags;
> @@ -83,9 +85,9 @@ 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);
> -        return -1;
> +        error_setg(errp, "mount tag '%s' (%d bytes) is longer than "
> +                   "maximum (%d bytes)", s->fsconf.tag, len, MAX_TAG_LEN - 1);
> +        return;
>      }
>
>      s->tag = strdup(s->fsconf.tag);
> @@ -97,13 +99,13 @@ 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);
> -        return -1;
> +        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);
> +        return;
>      }
>      if (v9fs_init_worker_threads() < 0) {
> -        fprintf(stderr, "worker thread initialization failed\n");
> -        return -1;
> +        error_setg(errp, "worker thread initialization failed");
> +        return;
>      }
>
>      /*
> @@ -113,20 +115,20 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
>       */
>      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));
> -        return -1;
> +        error_setg(errp,
> +                   "error in converting name to path %s", strerror(errno));
> +        return;
>      }
>      if (s->ops->lstat(&s->ctx, &path, &stat)) {
> -        fprintf(stderr, "share path %s does not exist\n", fse->path);
> -        return -1;
> +        error_setg(errp, "share path %s does not exist", fse->path);
> +        return;
>      } else if (!S_ISDIR(stat.st_mode)) {
> -        fprintf(stderr, "share path %s is not a directory\n", fse->path);
> -        return -1;
> +        error_setg(errp, "share path %s is not a directory", fse->path);
> +        return;
>      }
>      v9fs_path_free(&path);
>
> -    return 0;
> +    v9c->parent_realize(dev, errp);
>  }
>
>  /* virtio-9p device */
> @@ -136,12 +138,16 @@ static Property virtio_9p_properties[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> -static void virtio_9p_class_init(ObjectClass *klass, void *data)
> +static void virtio_9p_class_init(ObjectClass *oc, void *data)
>  {
> -    DeviceClass *dc = DEVICE_CLASS(klass);
> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
> +    V9fsClass *v9c = VIRTIO_9P_CLASS(oc);
> +
> +    v9c->parent_realize = dc->realize;
> +    dc->realize = virtio_9p_device_realize;
> +
>      dc->props = virtio_9p_properties;
> -    vdc->init = virtio_9p_device_init;
>      vdc->get_features = virtio_9p_get_features;
>      vdc->get_config = virtio_9p_get_config;
>  }
> @@ -151,6 +157,7 @@ static const TypeInfo virtio_device_info = {
>      .parent = TYPE_VIRTIO_DEVICE,
>      .instance_size = sizeof(V9fsState),
>      .class_init = virtio_9p_class_init,
> +    .class_size = sizeof(V9fsClass),
>  };
>
>  static void virtio_9p_register_types(void)
> diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
> index 1d6eedb..85699a7 100644
> --- a/hw/9pfs/virtio-9p.h
> +++ b/hw/9pfs/virtio-9p.h
> @@ -227,6 +227,15 @@ typedef struct V9fsState
>      V9fsConf fsconf;
>  } V9fsState;
>
> +typedef struct V9fsClass {
> +    /*< private >*/
> +    VirtioDeviceClass parent_class;
> +    /*< public >*/
> +
> +    DeviceRealize parent_realize;
> +} V9fsClass;
> +
> +

If applied tree-wide this change pattern is going to add a lot of
boiler-plate to all devices. There is capability in QOM to access the
overridden parent class functions already, so it can be made to work
without every class having to do this save-and-call trick with
overridden realize (and friends). How about this:

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 9190a7e..696702a 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -37,6 +37,18 @@ int qdev_hotplug = 0;
 static bool qdev_hot_added = false;
 static bool qdev_hot_removed = false;

+void device_parent_realize(DeviceState *dev, Error **errp)
+{
+    ObjectClass *class = object_get_class(dev);
+    DeviceClass *dc;
+
+    class = object_class_get_parent(dc);
+    assert(class);
+    dc = DEVICE_CLASS(class);
+
+    dc->realize(dev, errp);
+}
+

And child class realize fns can call this to realize themselves as the
parent would. Ditto for reset and unrealize. Then you would only need
to define struct FooClass when creating new abstractions (or virtual
functions if your C++).

Regards,
Peter

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

* Re: [Qemu-devel] [PATCH RFT 2/5] virtio: Convert VirtioDevice to QOM realize/unrealize
  2013-06-08  2:22   ` Peter Crosthwaite
@ 2013-06-08  9:55     ` Andreas Färber
  2013-06-08 12:32       ` Peter Crosthwaite
  0 siblings, 1 reply; 19+ messages in thread
From: Andreas Färber @ 2013-06-08  9:55 UTC (permalink / raw)
  To: Peter Crosthwaite
  Cc: Kevin Wolf, Anthony Liguori, anthony, Michael S. Tsirkin, jlarrew,
	qemu-devel, Aneesh Kumar K.V, Stefan Hajnoczi, Amit Shah,
	Paolo Bonzini, fred.konrad

Hi,

Am 08.06.2013 04:22, schrieb Peter Crosthwaite:
> On Sat, Jun 8, 2013 at 4:18 AM, Andreas Färber <afaerber@suse.de> wrote:
>> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
>> index dc6f4e4..409d315 100644
>> --- a/hw/9pfs/virtio-9p-device.c
>> +++ b/hw/9pfs/virtio-9p-device.c
[...]
>> @@ -136,12 +138,16 @@ static Property virtio_9p_properties[] = {
>>      DEFINE_PROP_END_OF_LIST(),
>>  };
>>
>> -static void virtio_9p_class_init(ObjectClass *klass, void *data)
>> +static void virtio_9p_class_init(ObjectClass *oc, void *data)
>>  {
>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
>> +    DeviceClass *dc = DEVICE_CLASS(oc);
>> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
>> +    V9fsClass *v9c = VIRTIO_9P_CLASS(oc);
>> +
>> +    v9c->parent_realize = dc->realize;
>> +    dc->realize = virtio_9p_device_realize;
>> +
>>      dc->props = virtio_9p_properties;
>> -    vdc->init = virtio_9p_device_init;
>>      vdc->get_features = virtio_9p_get_features;
>>      vdc->get_config = virtio_9p_get_config;
>>  }
>> @@ -151,6 +157,7 @@ static const TypeInfo virtio_device_info = {
>>      .parent = TYPE_VIRTIO_DEVICE,
>>      .instance_size = sizeof(V9fsState),
>>      .class_init = virtio_9p_class_init,
>> +    .class_size = sizeof(V9fsClass),
>>  };
>>
>>  static void virtio_9p_register_types(void)
>> diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
>> index 1d6eedb..85699a7 100644
>> --- a/hw/9pfs/virtio-9p.h
>> +++ b/hw/9pfs/virtio-9p.h
>> @@ -227,6 +227,15 @@ typedef struct V9fsState
>>      V9fsConf fsconf;
>>  } V9fsState;
>>
>> +typedef struct V9fsClass {
>> +    /*< private >*/
>> +    VirtioDeviceClass parent_class;
>> +    /*< public >*/
>> +
>> +    DeviceRealize parent_realize;
>> +} V9fsClass;
>> +
>> +
> 
> If applied tree-wide this change pattern is going to add a lot of
> boiler-plate to all devices. There is capability in QOM to access the
> overridden parent class functions already, so it can be made to work
> without every class having to do this save-and-call trick with
> overridden realize (and friends). How about this:
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 9190a7e..696702a 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -37,6 +37,18 @@ int qdev_hotplug = 0;
>  static bool qdev_hot_added = false;
>  static bool qdev_hot_removed = false;
> 
> +void device_parent_realize(DeviceState *dev, Error **errp)
> +{
> +    ObjectClass *class = object_get_class(dev);
> +    DeviceClass *dc;
> +
> +    class = object_class_get_parent(dc);
> +    assert(class);
> +    dc = DEVICE_CLASS(class);
> +
> +    dc->realize(dev, errp);
> +}
> +
> 
> And child class realize fns can call this to realize themselves as the
> parent would. Ditto for reset and unrealize. Then you would only need
> to define struct FooClass when creating new abstractions (or virtual
> functions if your C++).

Indeed, if you check the archives you will find that I suggested the
same in the context of ISA i8254/i8259 or CPUState. ;) Yet Anthony
specifically instructed me to do it this way, referring to GObject.
I then documented the expected process in qdev-core.h and object.h.

Regards,
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] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFT 2/5] virtio: Convert VirtioDevice to QOM realize/unrealize
  2013-06-08  9:55     ` Andreas Färber
@ 2013-06-08 12:32       ` Peter Crosthwaite
  2013-06-10  2:08         ` Anthony Liguori
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Crosthwaite @ 2013-06-08 12:32 UTC (permalink / raw)
  To: Andreas Färber, Anthony Liguori, Anthony Liguori
  Cc: Kevin Wolf, Michael S. Tsirkin, qemu-devel, jlarrew,
	Aneesh Kumar K.V, Stefan Hajnoczi, Amit Shah, Paolo Bonzini,
	fred.konrad

Hi Andreas,

On Sat, Jun 8, 2013 at 7:55 PM, Andreas Färber <afaerber@suse.de> wrote:
> Hi,
>
> Am 08.06.2013 04:22, schrieb Peter Crosthwaite:
>> On Sat, Jun 8, 2013 at 4:18 AM, Andreas Färber <afaerber@suse.de> wrote:
>>> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
>>> index dc6f4e4..409d315 100644
>>> --- a/hw/9pfs/virtio-9p-device.c
>>> +++ b/hw/9pfs/virtio-9p-device.c
> [...]
>>> @@ -136,12 +138,16 @@ static Property virtio_9p_properties[] = {
>>>      DEFINE_PROP_END_OF_LIST(),
>>>  };
>>>
>>> -static void virtio_9p_class_init(ObjectClass *klass, void *data)
>>> +static void virtio_9p_class_init(ObjectClass *oc, void *data)
>>>  {
>>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>>> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
>>> +    DeviceClass *dc = DEVICE_CLASS(oc);
>>> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
>>> +    V9fsClass *v9c = VIRTIO_9P_CLASS(oc);
>>> +
>>> +    v9c->parent_realize = dc->realize;
>>> +    dc->realize = virtio_9p_device_realize;
>>> +
>>>      dc->props = virtio_9p_properties;
>>> -    vdc->init = virtio_9p_device_init;
>>>      vdc->get_features = virtio_9p_get_features;
>>>      vdc->get_config = virtio_9p_get_config;
>>>  }
>>> @@ -151,6 +157,7 @@ static const TypeInfo virtio_device_info = {
>>>      .parent = TYPE_VIRTIO_DEVICE,
>>>      .instance_size = sizeof(V9fsState),
>>>      .class_init = virtio_9p_class_init,
>>> +    .class_size = sizeof(V9fsClass),
>>>  };
>>>
>>>  static void virtio_9p_register_types(void)
>>> diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
>>> index 1d6eedb..85699a7 100644
>>> --- a/hw/9pfs/virtio-9p.h
>>> +++ b/hw/9pfs/virtio-9p.h
>>> @@ -227,6 +227,15 @@ typedef struct V9fsState
>>>      V9fsConf fsconf;
>>>  } V9fsState;
>>>
>>> +typedef struct V9fsClass {
>>> +    /*< private >*/
>>> +    VirtioDeviceClass parent_class;
>>> +    /*< public >*/
>>> +
>>> +    DeviceRealize parent_realize;
>>> +} V9fsClass;
>>> +
>>> +
>>
>> If applied tree-wide this change pattern is going to add a lot of
>> boiler-plate to all devices. There is capability in QOM to access the
>> overridden parent class functions already, so it can be made to work
>> without every class having to do this save-and-call trick with
>> overridden realize (and friends). How about this:
>>
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>> index 9190a7e..696702a 100644
>> --- a/hw/core/qdev.c
>> +++ b/hw/core/qdev.c
>> @@ -37,6 +37,18 @@ int qdev_hotplug = 0;
>>  static bool qdev_hot_added = false;
>>  static bool qdev_hot_removed = false;
>>
>> +void device_parent_realize(DeviceState *dev, Error **errp)
>> +{
>> +    ObjectClass *class = object_get_class(dev);
>> +    DeviceClass *dc;
>> +
>> +    class = object_class_get_parent(dc);
>> +    assert(class);
>> +    dc = DEVICE_CLASS(class);
>> +
>> +    dc->realize(dev, errp);
>> +}
>> +
>>
>> And child class realize fns can call this to realize themselves as the
>> parent would. Ditto for reset and unrealize. Then you would only need
>> to define struct FooClass when creating new abstractions (or virtual
>> functions if your C++).
>
> Indeed, if you check the archives you will find that I suggested the
> same in the context of ISA i8254/i8259 or CPUState. ;) Yet Anthony
> specifically instructed me to do it this way, referring to GObject.
> I then documented the expected process in qdev-core.h and object.h.
>

Thanks for the history. I found this:

Jan 18 2013 Anthony Liguori wrote:

It's idiomatic from GObject.

I'm not sure I can come up with a concrete example but in the absense of
a compelling reason to shift from the idiom, I'd strongly suggest not.

Regards,

Anthony Liguori

------------------------------------

The additive diff on this series would take a massive nosedive - is
that a compelling reason? It is very unfortunate that pretty much
every class inheriting from device is going to have to define a class
struct just for the sake of multi-level realization.

There is roughly 15 or so lines of boiler plate added to every class,
and in just the cleanup you have done this week you have about 10 or
so instances of this change pattern. Under the
child-access-to-parent-version proposal those 15 lines become 1.

I don't see the fundamental reason why child classes shouldnt be able
to access parent implementations of virtualised functions. Many OO
oriented languages indeed explicitly build this capability into the
syntax. Examples include Java with "super.foo()" accesses and C++ via
the parent class namespace:

class Bar : public Foo {
  // ...

  void printStuff() {
    Foo::printStuff(); // calls base class' function
  }
};

Anthony is it possible to consider loosening this restriction?

Regards,
Peter

> Regards,
> 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] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFT 2/5] virtio: Convert VirtioDevice to QOM realize/unrealize
  2013-06-07 18:18 ` [Qemu-devel] [PATCH RFT 2/5] virtio: Convert VirtioDevice to QOM realize/unrealize Andreas Färber
  2013-06-08  2:22   ` Peter Crosthwaite
@ 2013-06-09 10:36   ` Michael S. Tsirkin
  1 sibling, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2013-06-09 10:36 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi, qemu-devel, jlarrew,
	Aneesh Kumar K.V, anthony, Amit Shah, Paolo Bonzini, fred.konrad

On Fri, Jun 07, 2013 at 08:18:57PM +0200, Andreas Färber wrote:
> VirtioDevice's DeviceClass::exit code cleaning up bus_name is no longer
> overwritten by virtio-{blk,serial,net,scsi,balloon,rng} and vhost-scsi.
> 
> Note: 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.
> 
> While at it, drop duplicate VIRTIO_DEVICE() casts and avoid qdev.

There are also improvements to error handling here, they can
be done by a separate patch.

Please don't mix refactoring and functional changes -
this patch is huge as it is.

Also, could changes to each device be done in a separate patch?
This last might be hard, not a must IMO.

> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  hw/9pfs/virtio-9p-device.c         | 67 ++++++++++++++++--------------
>  hw/9pfs/virtio-9p.h                | 13 ++++++
>  hw/block/virtio-blk.c              | 52 ++++++++++++++---------
>  hw/char/virtio-serial-bus.c        | 49 ++++++++++++++--------
>  hw/net/virtio-net.c                | 48 ++++++++++++---------
>  hw/scsi/vhost-scsi.c               | 59 +++++++++++++++-----------
>  hw/scsi/virtio-scsi.c              | 85 ++++++++++++++++++++++++--------------
>  hw/virtio/virtio-balloon.c         | 50 +++++++++++++---------
>  hw/virtio/virtio-rng.c             | 53 ++++++++++++++----------
>  hw/virtio/virtio.c                 | 20 ++++-----
>  include/hw/virtio/vhost-scsi.h     | 13 ++++++
>  include/hw/virtio/virtio-balloon.h | 13 ++++++
>  include/hw/virtio/virtio-blk.h     | 13 ++++++
>  include/hw/virtio/virtio-net.h     | 13 ++++++
>  include/hw/virtio/virtio-rng.h     | 13 ++++++
>  include/hw/virtio/virtio-scsi.h    | 29 +++++++++++--
>  include/hw/virtio/virtio-serial.h  | 13 ++++++
>  include/hw/virtio/virtio.h         |  6 ++-
>  18 files changed, 406 insertions(+), 203 deletions(-)
> 
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index dc6f4e4..409d315 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -41,15 +41,17 @@ 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);
> +    V9fsClass *v9c = VIRTIO_9P_GET_CLASS(dev);
>      int i, len;
>      struct stat stat;
>      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 */
> @@ -65,17 +67,17 @@ 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");
> -        return -1;
> +        error_setg(errp, "Virtio-9p device couldn't find fsdev with the "
> +                   "id = %s",
> +                   s->fsconf.fsdev_id ? s->fsconf.fsdev_id : "NULL");
> +        return;
>      }
>  
>      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);
> -        return -1;
> +        error_setg(errp, "fsdev with id %s needs mount_tag arguments",
> +                   s->fsconf.fsdev_id);
> +        return;
>      }
>  
>      s->ctx.export_flags = fse->export_flags;
> @@ -83,9 +85,9 @@ 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);
> -        return -1;
> +        error_setg(errp, "mount tag '%s' (%d bytes) is longer than "
> +                   "maximum (%d bytes)", s->fsconf.tag, len, MAX_TAG_LEN - 1);
> +        return;
>      }
>  
>      s->tag = strdup(s->fsconf.tag);
> @@ -97,13 +99,13 @@ 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);
> -        return -1;
> +        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);
> +        return;
>      }
>      if (v9fs_init_worker_threads() < 0) {
> -        fprintf(stderr, "worker thread initialization failed\n");
> -        return -1;
> +        error_setg(errp, "worker thread initialization failed");
> +        return;
>      }
>  
>      /*
> @@ -113,20 +115,20 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
>       */
>      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));
> -        return -1;
> +        error_setg(errp,
> +                   "error in converting name to path %s", strerror(errno));
> +        return;
>      }
>      if (s->ops->lstat(&s->ctx, &path, &stat)) {
> -        fprintf(stderr, "share path %s does not exist\n", fse->path);
> -        return -1;
> +        error_setg(errp, "share path %s does not exist", fse->path);
> +        return;
>      } else if (!S_ISDIR(stat.st_mode)) {
> -        fprintf(stderr, "share path %s is not a directory\n", fse->path);
> -        return -1;
> +        error_setg(errp, "share path %s is not a directory", fse->path);
> +        return;
>      }
>      v9fs_path_free(&path);
>  
> -    return 0;
> +    v9c->parent_realize(dev, errp);
>  }
>  
>  /* virtio-9p device */
> @@ -136,12 +138,16 @@ static Property virtio_9p_properties[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> -static void virtio_9p_class_init(ObjectClass *klass, void *data)
> +static void virtio_9p_class_init(ObjectClass *oc, void *data)
>  {
> -    DeviceClass *dc = DEVICE_CLASS(klass);
> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
> +    V9fsClass *v9c = VIRTIO_9P_CLASS(oc);
> +
> +    v9c->parent_realize = dc->realize;
> +    dc->realize = virtio_9p_device_realize;
> +
>      dc->props = virtio_9p_properties;
> -    vdc->init = virtio_9p_device_init;
>      vdc->get_features = virtio_9p_get_features;
>      vdc->get_config = virtio_9p_get_config;
>  }
> @@ -151,6 +157,7 @@ static const TypeInfo virtio_device_info = {
>      .parent = TYPE_VIRTIO_DEVICE,
>      .instance_size = sizeof(V9fsState),
>      .class_init = virtio_9p_class_init,
> +    .class_size = sizeof(V9fsClass),
>  };
>  
>  static void virtio_9p_register_types(void)
> diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
> index 1d6eedb..85699a7 100644
> --- a/hw/9pfs/virtio-9p.h
> +++ b/hw/9pfs/virtio-9p.h
> @@ -227,6 +227,15 @@ typedef struct V9fsState
>      V9fsConf fsconf;
>  } V9fsState;
>  
> +typedef struct V9fsClass {
> +    /*< private >*/
> +    VirtioDeviceClass parent_class;
> +    /*< public >*/
> +
> +    DeviceRealize parent_realize;
> +} V9fsClass;
> +
> +

One empty line please.

>  typedef struct V9fsStatState {
>      V9fsPDU *pdu;
>      size_t offset;
> @@ -404,6 +413,10 @@ 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_CLASS(obj) \
> +        OBJECT_GET_CLASS(V9fsClass, (obj), TYPE_VIRTIO_9P)
> +#define VIRTIO_9P_CLASS(cls) \
> +        OBJECT_CLASS_CHECK(V9fsClass, (cls), TYPE_VIRTIO_9P)
>  
>  #define DEFINE_VIRTIO_9P_PROPERTIES(_state, _field)             \
>          DEFINE_PROP_STRING("mount_tag", _state, _field.tag),    \
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 8ea1f03..88e99a6 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -628,10 +628,11 @@ void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk)
>      memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));
>  }
>  
> -static int virtio_blk_device_init(VirtIODevice *vdev)
> +static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
>  {
> -    DeviceState *qdev = DEVICE(vdev);
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>      VirtIOBlock *s = VIRTIO_BLK(vdev);
> +    VirtIOBlockClass *vbc = VIRTIO_BLK_GET_CLASS(dev);
>      VirtIOBlkConf *blk = &(s->blk);
>  #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
>      Error *err = NULL;
> @@ -639,17 +640,18 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
>      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,
> @@ -665,29 +667,31 @@ 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;
>      }
>  #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");
> -    return 0;
> +    add_boot_device_path(s->conf->bootindex, dev, "/disk@0,0");
> +
> +    vbc->parent_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);
> +    VirtIOBlockClass *vbc = VIRTIO_BLK_GET_CLASS(dev);
> +
>  #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
>      virtio_blk_data_plane_destroy(s->dataplane);
>      s->dataplane = NULL;
> @@ -696,7 +700,8 @@ 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;
> +
> +    vbc->parent_unrealize(dev, errp);
>  }
>  
>  static Property virtio_blk_properties[] = {
> @@ -704,13 +709,19 @@ static Property virtio_blk_properties[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> -static void virtio_blk_class_init(ObjectClass *klass, void *data)
> +static void virtio_blk_class_init(ObjectClass *oc, void *data)
>  {
> -    DeviceClass *dc = DEVICE_CLASS(klass);
> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> -    dc->exit = virtio_blk_device_exit;
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
> +    VirtIOBlockClass *vbc = VIRTIO_BLK_CLASS(oc);
> +
> +    vbc->parent_realize = dc->realize;
> +    dc->realize = virtio_blk_device_realize;
> +
> +    vbc->parent_unrealize = dc->unrealize;
> +    dc->unrealize = virtio_blk_device_unrealize;
> +
>      dc->props = virtio_blk_properties;
> -    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;
> @@ -723,6 +734,7 @@ static const TypeInfo virtio_device_info = {
>      .parent = TYPE_VIRTIO_DEVICE,
>      .instance_size = sizeof(VirtIOBlock),
>      .class_init = virtio_blk_class_init,
> +    .class_size = sizeof(VirtIOBlockClass),
>  };
>  
>  static void virtio_register_types(void)
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index cc3d1dd..1cdd659 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -889,31 +889,35 @@ 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 *qdev = DEVICE(vdev);
> -    VirtIOSerial *vser = VIRTIO_SERIAL(vdev);
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    VirtIOSerial *vser = VIRTIO_SERIAL(dev);
> +    VirtIOSerialClass *vsc = VIRTIO_SERIAL_GET_CLASS(dev);
> +    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,
>                  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,10 +965,10 @@ 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;
> +    vsc->parent_realize(dev, errp);
>  }
>  
>  static void virtio_serial_port_class_init(ObjectClass *klass, void *data)
> @@ -986,10 +990,11 @@ 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);
> +    VirtIOSerialClass *vsc = VIRTIO_SERIAL_GET_CLASS(dev);
>  
>      unregister_savevm(dev, "virtio-console", vser);
>  
> @@ -1003,7 +1008,8 @@ static int virtio_serial_device_exit(DeviceState *dev)
>          g_free(vser->post_load);
>      }
>      virtio_cleanup(vdev);
> -    return 0;
> +
> +    vsc->parent_unrealize(dev, errp);
>  }
>  
>  static Property virtio_serial_properties[] = {
> @@ -1011,13 +1017,19 @@ static Property virtio_serial_properties[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> -static void virtio_serial_class_init(ObjectClass *klass, void *data)
> +static void virtio_serial_class_init(ObjectClass *oc, void *data)
>  {
> -    DeviceClass *dc = DEVICE_CLASS(klass);
> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> -    dc->exit = virtio_serial_device_exit;
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
> +    VirtIOSerialClass *vsc = VIRTIO_SERIAL_CLASS(oc);
> +
> +    vsc->parent_realize = dc->realize;
> +    dc->realize = virtio_serial_device_realize;
> +
> +    vsc->parent_unrealize = dc->unrealize;
> +    dc->unrealize = virtio_serial_device_unrealize;
> +
>      dc->props = virtio_serial_properties;
> -    vdc->init = virtio_serial_device_init;
>      vdc->get_features = get_features;
>      vdc->get_config = get_config;
>      vdc->set_config = set_config;
> @@ -1030,6 +1042,7 @@ static const TypeInfo virtio_device_info = {
>      .parent = TYPE_VIRTIO_DEVICE,
>      .instance_size = sizeof(VirtIOSerial),
>      .class_init = virtio_serial_class_init,
> +    .class_size = sizeof(VirtIOSerialClass),
>  };
>  
>  static void virtio_serial_register_types(void)
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 1ea9556..9a3680c 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1367,15 +1367,16 @@ 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)
>  {
>      int i;
>  
> -    DeviceState *qdev = DEVICE(vdev);
> -    VirtIONet *n = VIRTIO_NET(vdev);
> +    DeviceState *qdev = DEVICE(dev);
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    VirtIONet *n = VIRTIO_NET(dev);
> +    VirtIONetClass *vnc = VIRTIO_NET_GET_CLASS(dev);
>  
> -    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);
> @@ -1439,24 +1440,26 @@ static int virtio_net_device_init(VirtIODevice *vdev)
>  
>      n->vlans = g_malloc0(MAX_VLAN >> 3);
>  
> -    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");
> -    return 0;
> +    add_boot_device_path(n->nic_conf.bootindex, dev, "/ethernet-phy@0");
> +
> +    vnc->parent_realize(dev, errp);
>  }
>  
> -static int virtio_net_device_exit(DeviceState *qdev)
> +static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
>  {
> -    VirtIONet *n = VIRTIO_NET(qdev);
> -    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    VirtIONet *n = VIRTIO_NET(dev);
> +    VirtIONetClass *vnc = VIRTIO_NET_GET_CLASS(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);
> @@ -1488,7 +1491,7 @@ static int virtio_net_device_exit(DeviceState *qdev)
>      qemu_del_nic(n->nic);
>      virtio_cleanup(vdev);
>  
> -    return 0;
> +    vnc->parent_unrealize(dev, errp);
>  }
>  
>  static void virtio_net_instance_init(Object *obj)
> @@ -1511,13 +1514,19 @@ static Property virtio_net_properties[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> -static void virtio_net_class_init(ObjectClass *klass, void *data)
> +static void virtio_net_class_init(ObjectClass *oc, void *data)
>  {
> -    DeviceClass *dc = DEVICE_CLASS(klass);
> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> -    dc->exit = virtio_net_device_exit;
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
> +    VirtIONetClass *vnc = VIRTIO_NET_CLASS(oc);
> +
> +    vnc->parent_realize = dc->realize;
> +    dc->realize = virtio_net_device_realize;
> +
> +    vnc->parent_unrealize = dc->unrealize;
> +    dc->unrealize = virtio_net_device_unrealize;
> +
>      dc->props = virtio_net_properties;
> -    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;
> @@ -1535,6 +1544,7 @@ static const TypeInfo virtio_net_info = {
>      .instance_size = sizeof(VirtIONet),
>      .instance_init = virtio_net_instance_init,
>      .class_init = virtio_net_class_init,
> +    .class_size = sizeof(VirtIONetClass),
>  };
>  
>  static void virtio_register_types(void)
> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> index d7a1c33..cacaf64 100644
> --- a/hw/scsi/vhost-scsi.c
> +++ b/hw/scsi/vhost-scsi.c
> @@ -196,29 +196,32 @@ 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);
> +    VHostSCSIClass *vsc = VHOST_SCSI_GET_CLASS(dev);
> +    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;
> +    vsc->parent_realize(dev, &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
>      }
>  
>      s->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
> @@ -227,24 +230,22 @@ 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;
>  
>      error_setg(&s->migration_blocker,
>              "vhost-scsi does not support migration");
>      migrate_add_blocker(s->migration_blocker);
> -
> -    return 0;
>  }
>  
> -static int vhost_scsi_exit(DeviceState *qdev)
> +static void vhost_scsi_unrealize(DeviceState *dev, Error **errp)
>  {
> -    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);
> +    VHostSCSIClass *vsc = VHOST_SCSI_GET_CLASS(dev);
>  
>      migrate_del_blocker(s->migration_blocker);
>      error_free(s->migration_blocker);
> @@ -253,7 +254,8 @@ static int vhost_scsi_exit(DeviceState *qdev)
>      vhost_scsi_set_status(vdev, 0);
>  
>      g_free(s->dev.vqs);
> -    return virtio_scsi_common_exit(vs);
> +
> +    vsc->parent_unrealize(dev, errp);
>  }
>  
>  static Property vhost_scsi_properties[] = {
> @@ -261,13 +263,19 @@ static Property vhost_scsi_properties[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> -static void vhost_scsi_class_init(ObjectClass *klass, void *data)
> +static void vhost_scsi_class_init(ObjectClass *oc, void *data)
>  {
> -    DeviceClass *dc = DEVICE_CLASS(klass);
> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> -    dc->exit = vhost_scsi_exit;
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
> +    VHostSCSIClass *vsc = VHOST_SCSI_CLASS(oc);
> +
> +    vsc->parent_realize = dc->realize;
> +    dc->realize = vhost_scsi_realize;
> +
> +    vsc->parent_unrealize = dc->unrealize;
> +    dc->unrealize = vhost_scsi_unrealize;
> +
>      dc->props = vhost_scsi_properties;
> -    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;
> @@ -278,6 +286,7 @@ static const TypeInfo vhost_scsi_info = {
>      .parent = TYPE_VIRTIO_SCSI_COMMON,
>      .instance_size = sizeof(VHostSCSI),
>      .class_init = vhost_scsi_class_init,
> +    .class_size = sizeof(VHostSCSIClass),
>  };
>  
>  static void virtio_register_types(void)
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 08dd3f3..6704f78 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -587,12 +587,14 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
>      .load_request = virtio_scsi_load_request,
>  };
>  
> -int virtio_scsi_common_init(VirtIOSCSICommon *s)
> +static 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);
> +    VirtIOSCSICommonClass *vscc = VIRTIO_SCSI_COMMON_GET_CLASS(dev);
>      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 *));
> @@ -608,50 +610,53 @@ int virtio_scsi_common_init(VirtIOSCSICommon *s)
>                                           virtio_scsi_handle_cmd);
>      }
>  
> -    return 0;
> +    vscc->parent_realize(dev, errp);
>  }
>  
> -static int virtio_scsi_device_init(VirtIODevice *vdev)
> +static void virtio_scsi_device_realize(DeviceState *dev, Error **errp)
>  {
> -    DeviceState *qdev = DEVICE(vdev);
> -    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
> -    VirtIOSCSI *s = VIRTIO_SCSI(vdev);
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    VirtIOSCSI *s = VIRTIO_SCSI(dev);
> +    VirtIOSCSIClass *vsc = VIRTIO_SCSI_GET_CLASS(dev);
>      static int virtio_scsi_id;
> -    int ret;
> +    Error *err = NULL;
>  
> -    ret = virtio_scsi_common_init(vs);
> -    if (ret < 0) {
> -        return ret;
> +    vsc->parent_realize(dev, &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
>      }
>  
> -    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);
>      }
>  
> -    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;
>  }
>  
> -int virtio_scsi_common_exit(VirtIOSCSICommon *vs)
> +static 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);
> +    VirtIOSCSICommonClass *vscc = VIRTIO_SCSI_COMMON_GET_CLASS(dev);
>  
>      g_free(vs->cmd_vqs);
>      virtio_cleanup(vdev);
> -    return 0;
> +
> +    vscc->parent_unrealize(dev, errp);
>  }
>  
> -static int virtio_scsi_device_exit(DeviceState *qdev)
> +static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp)
>  {
> -    VirtIOSCSI *s = VIRTIO_SCSI(qdev);
> -    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(qdev);
> +    VirtIOSCSI *s = VIRTIO_SCSI(dev);
> +    VirtIOSCSIClass *vsc = VIRTIO_SCSI_GET_CLASS(dev);
>  
> -    unregister_savevm(qdev, "virtio-scsi", s);
> -    return virtio_scsi_common_exit(vs);
> +    unregister_savevm(dev, "virtio-scsi", s);
> +
> +    vsc->parent_unrealize(dev, errp);
>  }
>  
>  static Property virtio_scsi_properties[] = {
> @@ -659,20 +664,34 @@ static Property virtio_scsi_properties[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> -static void virtio_scsi_common_class_init(ObjectClass *klass, void *data)
> +static void virtio_scsi_common_class_init(ObjectClass *oc, void *data)
>  {
> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
> +    VirtIOSCSICommonClass *vscc = VIRTIO_SCSI_COMMON_CLASS(oc);
> +
> +    vscc->parent_realize = dc->realize;
> +    dc->realize = virtio_scsi_common_realize;
> +
> +    vscc->parent_unrealize = dc->unrealize;
> +    dc->unrealize = virtio_scsi_common_unrealize;
>  
>      vdc->get_config = virtio_scsi_get_config;
>  }
>  
> -static void virtio_scsi_class_init(ObjectClass *klass, void *data)
> +static void virtio_scsi_class_init(ObjectClass *oc, void *data)
>  {
> -    DeviceClass *dc = DEVICE_CLASS(klass);
> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> -    dc->exit = virtio_scsi_device_exit;
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
> +    VirtIOSCSIClass *vsc = VIRTIO_SCSI_CLASS(oc);
> +
> +    vsc->parent_realize = dc->realize;
> +    dc->realize = virtio_scsi_device_realize;
> +
> +    vsc->parent_unrealize = dc->unrealize;
> +    dc->unrealize = virtio_scsi_device_unrealize;
> +
>      dc->props = virtio_scsi_properties;
> -    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;
> @@ -683,6 +702,7 @@ static const TypeInfo virtio_scsi_common_info = {
>      .parent = TYPE_VIRTIO_DEVICE,
>      .instance_size = sizeof(VirtIOSCSICommon),
>      .class_init = virtio_scsi_common_class_init,
> +    .class_size = sizeof(VirtIOSCSICommonClass),
>  };
>  
>  static const TypeInfo virtio_scsi_info = {
> @@ -690,6 +710,7 @@ static const TypeInfo virtio_scsi_info = {
>      .parent = TYPE_VIRTIO_SCSI_COMMON,
>      .instance_size = sizeof(VirtIOSCSI),
>      .class_init = virtio_scsi_class_init,
> +    .class_size = sizeof(VirtIOSCSIClass),
>  };
>  
>  static void virtio_register_types(void)
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index d669756..0535062 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -336,10 +336,11 @@ 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 *qdev = DEVICE(vdev);
> -    VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    VirtIOBalloon *s = VIRTIO_BALLOON(dev);
> +    VirtIOBalloonClass *vbc = VIRTIO_BALLOON_GET_CLASS(dev);
>      int ret;
>  
>      virtio_init(vdev, "virtio-balloon", VIRTIO_ID_BALLOON, 8);
> @@ -348,50 +349,60 @@ static int virtio_balloon_device_init(VirtIODevice *vdev)
>                                     virtio_balloon_stat, s);
>  
>      if (ret < 0) {
> -        virtio_cleanup(VIRTIO_DEVICE(s));
> -        return -1;
> +        error_setg(errp, "Adding balloon handler failed");
> +        virtio_cleanup(vdev);
> +        return;
>      }
>  
>      s->ivq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
>      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;
> +
> +    vbc->parent_realize(dev, errp);
>  }
>  
> -static int virtio_balloon_device_exit(DeviceState *qdev)
> +static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
>  {
> -    VirtIOBalloon *s = VIRTIO_BALLOON(qdev);
> -    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    VirtIOBalloon *s = VIRTIO_BALLOON(dev);
> +    VirtIOBalloonClass *vbc = VIRTIO_BALLOON_GET_CLASS(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;
> +
> +    vbc->parent_unrealize(dev, errp);
>  }
>  
>  static Property virtio_balloon_properties[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> -static void virtio_balloon_class_init(ObjectClass *klass, void *data)
> +static void virtio_balloon_class_init(ObjectClass *oc, void *data)
>  {
> -    DeviceClass *dc = DEVICE_CLASS(klass);
> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> -    dc->exit = virtio_balloon_device_exit;
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
> +    VirtIOBalloonClass *vbc = VIRTIO_BALLOON_CLASS(oc);
> +
> +    vbc->parent_realize = dc->realize;
> +    dc->realize = virtio_balloon_device_realize;
> +
> +    vbc->parent_unrealize = dc->unrealize;
> +    dc->unrealize = virtio_balloon_device_unrealize;
> +
>      dc->props = virtio_balloon_properties;
> -    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;
> @@ -402,6 +413,7 @@ static const TypeInfo virtio_balloon_info = {
>      .parent = TYPE_VIRTIO_DEVICE,
>      .instance_size = sizeof(VirtIOBalloon),
>      .class_init = virtio_balloon_class_init,
> +    .class_size = sizeof(VirtIOBalloonClass),
>  };
>  
>  static void virtio_register_types(void)
> diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
> index cb787c7..108cfc9 100644
> --- a/hw/virtio/virtio-rng.c
> +++ b/hw/virtio/virtio-rng.c
> @@ -133,21 +133,22 @@ 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 *qdev = DEVICE(vdev);
> -    VirtIORNG *vrng = VIRTIO_RNG(vdev);
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    VirtIORNG *vrng = VIRTIO_RNG(dev);
> +    VirtIORNGClass *vrc = VIRTIO_RNG_GET_CLASS(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);
>      }
> @@ -156,15 +157,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);
> @@ -178,22 +178,24 @@ 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;
> +    vrc->parent_realize(dev, errp);
>  }
>  
> -static int virtio_rng_device_exit(DeviceState *qdev)
> +static void virtio_rng_device_unrealize(DeviceState *dev, Error **errp)
>  {
> -    VirtIORNG *vrng = VIRTIO_RNG(qdev);
> -    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    VirtIORNG *vrng = VIRTIO_RNG(dev);
> +    VirtIORNGClass *vrc = VIRTIO_RNG_GET_CLASS(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;
> +
> +    vrc->parent_unrealize(dev, errp);
>  }
>  
>  static Property virtio_rng_properties[] = {
> @@ -201,13 +203,19 @@ static Property virtio_rng_properties[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> -static void virtio_rng_class_init(ObjectClass *klass, void *data)
> +static void virtio_rng_class_init(ObjectClass *oc, void *data)
>  {
> -    DeviceClass *dc = DEVICE_CLASS(klass);
> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> -    dc->exit = virtio_rng_device_exit;
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
> +    VirtIORNGClass *vrc = VIRTIO_RNG_CLASS(oc);
> +
> +    vrc->parent_realize = dc->realize;
> +    dc->realize = virtio_rng_device_realize;
> +
> +    vrc->parent_unrealize = dc->unrealize;
> +    dc->unrealize = virtio_rng_device_unrealize;
> +
>      dc->props = virtio_rng_properties;
> -    vdc->init = virtio_rng_device_init;
>      vdc->get_features = get_features;
>  }
>  
> @@ -225,6 +233,7 @@ static const TypeInfo virtio_rng_info = {
>      .instance_size = sizeof(VirtIORNG),
>      .instance_init = virtio_rng_initfn,
>      .class_init = virtio_rng_class_init,
> +    .class_size = sizeof(VirtIORNGClass),
>  };
>  
>  static void virtio_register_types(void)
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 8176c14..d369f9a 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1105,35 +1105,29 @@ void virtio_device_set_child_bus_name(VirtIODevice *vdev, char *bus_name)
>      }
>  }
>  
> -static int virtio_device_init(DeviceState *qdev)
> +static void virtio_device_realize(DeviceState *dev, Error **errp)
>  {
> -    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);
> +
>      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/vhost-scsi.h b/include/hw/virtio/vhost-scsi.h
> index 85cc031..b0d64e4 100644
> --- a/include/hw/virtio/vhost-scsi.h
> +++ b/include/hw/virtio/vhost-scsi.h
> @@ -53,6 +53,10 @@ 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_CLASS(obj) \
> +        OBJECT_GET_CLASS(VHostSCSIClass, (obj), TYPE_VHOST_SCSI)
> +#define VHOST_SCSI_CLASS(cls) \
> +        OBJECT_CLASS_CHECK(VHostSCSIClass, (cls), TYPE_VHOST_SCSI)
>  
>  typedef struct VHostSCSI {
>      VirtIOSCSICommon parent_obj;
> @@ -62,6 +66,15 @@ typedef struct VHostSCSI {
>      struct vhost_dev dev;
>  } VHostSCSI;
>  
> +typedef struct VHostSCSIClass {
> +    /*< private >*/
> +    VirtIOSCSICommonClass parent_class;
> +    /*< public >*/
> +
> +    DeviceRealize parent_realize;
> +    DeviceUnrealize parent_unrealize;
> +} VHostSCSIClass;
> +
>  #define DEFINE_VHOST_SCSI_PROPERTIES(_state, _conf_field) \
>      DEFINE_PROP_STRING("vhostfd", _state, _conf_field.vhostfd), \
>      DEFINE_PROP_STRING("wwpn", _state, _conf_field.wwpn), \
> diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
> index f863bfe..146fa04 100644
> --- a/include/hw/virtio/virtio-balloon.h
> +++ b/include/hw/virtio/virtio-balloon.h
> @@ -21,6 +21,10 @@
>  #define TYPE_VIRTIO_BALLOON "virtio-balloon-device"
>  #define VIRTIO_BALLOON(obj) \
>          OBJECT_CHECK(VirtIOBalloon, (obj), TYPE_VIRTIO_BALLOON)
> +#define VIRTIO_BALLOON_GET_CLASS(obj) \
> +        OBJECT_GET_CLASS(VirtIOBalloonClass, (obj), TYPE_VIRTIO_BALLOON)
> +#define VIRTIO_BALLOON_CLASS(cls) \
> +        OBJECT_CLASS_CHECK(VirtIOBalloonClass, (cls), TYPE_VIRTIO_BALLOON)
>  
>  /* from Linux's linux/virtio_balloon.h */
>  
> @@ -69,4 +73,13 @@ typedef struct VirtIOBalloon {
>      int64_t stats_poll_interval;
>  } VirtIOBalloon;
>  
> +typedef struct VirtIOBalloonClass {
> +    /*< private >*/
> +    VirtioDeviceClass parent_class;
> +    /*< public >*/
> +
> +    DeviceRealize parent_realize;
> +    DeviceUnrealize parent_unrealize;
> +} VirtIOBalloonClass;
> +
>  #endif
> diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
> index fc71853..5ed703d 100644
> --- a/include/hw/virtio/virtio-blk.h
> +++ b/include/hw/virtio/virtio-blk.h
> @@ -20,6 +20,10 @@
>  #define TYPE_VIRTIO_BLK "virtio-blk-device"
>  #define VIRTIO_BLK(obj) \
>          OBJECT_CHECK(VirtIOBlock, (obj), TYPE_VIRTIO_BLK)
> +#define VIRTIO_BLK_GET_CLASS(obj) \
> +        OBJECT_GET_CLASS(VirtIOBlockClass, (obj), TYPE_VIRTIO_BLK)
> +#define VIRTIO_BLK_CLASS(cls) \
> +        OBJECT_CLASS_CHECK(VirtIOBlockClass, (cls), TYPE_VIRTIO_BLK)
>  
>  /* from Linux's linux/virtio_blk.h */
>  
> @@ -129,6 +133,15 @@ typedef struct VirtIOBlock {
>  #endif
>  } VirtIOBlock;
>  
> +typedef struct VirtIOBlockClass {
> +    /*< private >*/
> +    VirtioDeviceClass parent_class;
> +    /*< public >*/
> +
> +    DeviceRealize parent_realize;
> +    DeviceUnrealize parent_unrealize;
> +} VirtIOBlockClass;
> +
>  #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
>          DEFINE_VIRTIO_COMMON_FEATURES(_state, _field)
>  
> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
> index b315ac9..0b8a06e 100644
> --- a/include/hw/virtio/virtio-net.h
> +++ b/include/hw/virtio/virtio-net.h
> @@ -20,6 +20,10 @@
>  #define TYPE_VIRTIO_NET "virtio-net-device"
>  #define VIRTIO_NET(obj) \
>          OBJECT_CHECK(VirtIONet, (obj), TYPE_VIRTIO_NET)
> +#define VIRTIO_NET_GET_CLASS(obj) \
> +        OBJECT_GET_CLASS(VirtIONetClass, (obj), TYPE_VIRTIO_NET)
> +#define VIRTIO_NET_CLASS(cls) \
> +        OBJECT_CLASS_CHECK(VirtIONetClass, (cls), TYPE_VIRTIO_NET)
>  
>  #define ETH_ALEN    6
>  
> @@ -195,6 +199,15 @@ typedef struct VirtIONet {
>      uint64_t curr_guest_offloads;
>  } VirtIONet;
>  
> +typedef struct VirtIONetClass {
> +    /*< private >*/
> +    VirtioDeviceClass parent_class;
> +    /*< public >*/
> +
> +    DeviceRealize parent_realize;
> +    DeviceUnrealize parent_unrealize;
> +} VirtIONetClass;
> +
>  #define VIRTIO_NET_CTRL_MAC    1
>   #define VIRTIO_NET_CTRL_MAC_TABLE_SET        0
>   #define VIRTIO_NET_CTRL_MAC_ADDR_SET         1
> diff --git a/include/hw/virtio/virtio-rng.h b/include/hw/virtio/virtio-rng.h
> index debaa15..528f70c 100644
> --- a/include/hw/virtio/virtio-rng.h
> +++ b/include/hw/virtio/virtio-rng.h
> @@ -18,6 +18,10 @@
>  #define TYPE_VIRTIO_RNG "virtio-rng-device"
>  #define VIRTIO_RNG(obj) \
>          OBJECT_CHECK(VirtIORNG, (obj), TYPE_VIRTIO_RNG)
> +#define VIRTIO_RNG_GET_CLASS(obj) \
> +        OBJECT_GET_CLASS(VirtIORNGClass, (obj), TYPE_VIRTIO_RNG)
> +#define VIRTIO_RNG_CLASS(cls) \
> +        OBJECT_CLASS_CHECK(VirtIORNGClass, (cls), TYPE_VIRTIO_RNG)
>  
>  /* The Virtio ID for the virtio rng device */
>  #define VIRTIO_ID_RNG    4
> @@ -46,6 +50,15 @@ typedef struct VirtIORNG {
>      int64_t quota_remaining;
>  } VirtIORNG;
>  
> +typedef struct VirtIORNGClass {
> +    /*< private >*/
> +    VirtioDeviceClass parent_class;
> +    /*< public >*/
> +
> +    DeviceRealize parent_realize;
> +    DeviceUnrealize parent_unrealize;
> +} VirtIORNGClass;
> +
>  /* Set a default rate limit of 2^47 bytes per minute or roughly 2TB/s.  If
>     you have an entropy source capable of generating more entropy than this
>     and you can pass it through via virtio-rng, then hats off to you.  Until
> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
> index 9a98540..2ce58cd 100644
> --- a/include/hw/virtio/virtio-scsi.h
> +++ b/include/hw/virtio/virtio-scsi.h
> @@ -21,10 +21,18 @@
>  #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_CLASS(obj) \
> +        OBJECT_GET_CLASS(VirtIOSCSICommonClass, (obj), TYPE_VIRTIO_SCSI_COMMON)
> +#define VIRTIO_SCSI_COMMON_CLASS(cls) \
> +        OBJECT_CHECK(VirtIOSCSICommonClass, (cls), TYPE_VIRTIO_SCSI_COMMON)
>  
>  #define TYPE_VIRTIO_SCSI "virtio-scsi-device"
>  #define VIRTIO_SCSI(obj) \
>          OBJECT_CHECK(VirtIOSCSI, (obj), TYPE_VIRTIO_SCSI)
> +#define VIRTIO_SCSI_GET_CLASS(obj) \
> +        OBJECT_GET_CLASS(VirtIOSCSIClass, (obj), TYPE_VIRTIO_SCSI)
> +#define VIRTIO_SCSI_CLASS(cls) \
> +        OBJECT_CHECK(VirtIOSCSIClass, (cls), TYPE_VIRTIO_SCSI)
>  
>  
>  /* The ID for virtio_scsi */
> @@ -166,6 +174,15 @@ typedef struct VirtIOSCSICommon {
>      VirtQueue **cmd_vqs;
>  } VirtIOSCSICommon;
>  
> +typedef struct VirtIOSCSICommonClass {
> +    /*< private >*/
> +    VirtioDeviceClass parent_class;
> +    /*< public >*/
> +
> +    DeviceRealize parent_realize;
> +    DeviceUnrealize parent_unrealize;
> +} VirtIOSCSICommonClass;
> +
>  typedef struct {
>      VirtIOSCSICommon parent_obj;
>  
> @@ -174,6 +191,15 @@ typedef struct {
>      bool events_dropped;
>  } VirtIOSCSI;
>  
> +typedef struct VirtIOSCSIClass {
> +    /*< private >*/
> +    VirtIOSCSICommonClass parent_class;
> +    /*< public >*/
> +
> +    DeviceRealize parent_realize;
> +    DeviceUnrealize parent_unrealize;
> +} VirtIOSCSIClass;
> +
>  #define DEFINE_VIRTIO_SCSI_PROPERTIES(_state, _conf_field)                     \
>      DEFINE_PROP_UINT32("num_queues", _state, _conf_field.num_queues, 1),       \
>      DEFINE_PROP_UINT32("max_sectors", _state, _conf_field.max_sectors, 0xFFFF),\
> @@ -186,7 +212,4 @@ 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);
> -
>  #endif /* _QEMU_VIRTIO_SCSI_H */
> diff --git a/include/hw/virtio/virtio-serial.h b/include/hw/virtio/virtio-serial.h
> index 1d2040b..49af9e3 100644
> --- a/include/hw/virtio/virtio-serial.h
> +++ b/include/hw/virtio/virtio-serial.h
> @@ -212,6 +212,15 @@ struct VirtIOSerial {
>      virtio_serial_conf serial;
>  };
>  
> +typedef struct VirtIOSerialClass {
> +    /*< private >*/
> +    VirtioDeviceClass parent_class;
> +    /*< public >*/
> +
> +    DeviceRealize parent_realize;
> +    DeviceUnrealize parent_unrealize;
> +} VirtIOSerialClass;
> +
>  /* Interface to the virtio-serial bus */
>  
>  /*
> @@ -247,6 +256,10 @@ 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_CLASS(obj) \
> +        OBJECT_GET_CLASS(VirtIOSerialClass, (obj), TYPE_VIRTIO_SERIAL)
> +#define VIRTIO_SERIAL_CLASS(cls) \
> +        OBJECT_CLASS_CHECK(VirtIOSerialClass, (cls), TYPE_VIRTIO_SERIAL)
>  
>  #define DEFINE_VIRTIO_SERIAL_PROPERTIES(_state, _field) \
>          DEFINE_PROP_UINT32("max_ports", _state, _field.max_virtserial_ports, 31)
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index a6c5c53..7e1854a 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -122,9 +122,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	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFT 0/5] QOM realize for virtio
  2013-06-07 18:18 [Qemu-devel] [PATCH RFT 0/5] QOM realize for virtio Andreas Färber
                   ` (4 preceding siblings ...)
  2013-06-07 18:19 ` [Qemu-devel] [PATCH RFT 5/5] virtio-serial-port: Convert to QOM realize/unrealize Andreas Färber
@ 2013-06-09 10:39 ` Michael S. Tsirkin
  5 siblings, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2013-06-09 10:39 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Paolo Bonzini, fred.konrad, qemu-devel, anthony, jlarrew

On Fri, Jun 07, 2013 at 08:18:55PM +0200, Andreas Färber wrote:
> 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().
> 
> As a follow-up, VirtIOSerialPort is also converted to QOM realize/unrealize.
> As a side-effect, virtio-console realization is changed from in-order to pre-order.
> 
> Incidentally I stumbled over a minor cleanup issue with virtserialport.
> 
> Available from:
> https://github.com/afaerber/qemu-cpu/commits/realize-virtio.v1
> git://github.com/afaerber/qemu-cpu.git realize-virtio.v1
> 
> Regards,
> Andreas


I think it's a good idea overall.
A bit busy with other things now so only had
time to glance over this quickly, I sent some
minor comments separately.

More review hopefully later this week.

> Cc: Anthony Liguori <anthony@codemonkey.ws>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
> Cc: Frederic Konrad <fred.konrad@greensocs.com>
> 
> Andreas Färber (5):
>   virtio-blk-dataplane: Improve error reporting
>   virtio: Convert VirtioDevice to QOM realize/unrealize
>   virtio-console: QOM'ify VirtConsole
>   virtio-console: Use exitfn for virtserialport, too
>   virtio-serial-port: Convert to QOM realize/unrealize
> 
>  hw/9pfs/virtio-9p-device.c         | 67 ++++++++++++++------------
>  hw/9pfs/virtio-9p.h                | 13 +++++
>  hw/block/dataplane/virtio-blk.c    | 25 +++++-----
>  hw/block/dataplane/virtio-blk.h    |  5 +-
>  hw/block/virtio-blk.c              | 56 +++++++++++++--------
>  hw/char/virtio-console.c           | 99 ++++++++++++++++++++++++++------------
>  hw/char/virtio-serial-bus.c        | 94 ++++++++++++++++++------------------
>  hw/net/virtio-net.c                | 48 ++++++++++--------
>  hw/scsi/vhost-scsi.c               | 59 +++++++++++++----------
>  hw/scsi/virtio-scsi.c              | 85 ++++++++++++++++++++------------
>  hw/virtio/virtio-balloon.c         | 50 +++++++++++--------
>  hw/virtio/virtio-rng.c             | 53 +++++++++++---------
>  hw/virtio/virtio.c                 | 20 +++-----
>  include/hw/virtio/vhost-scsi.h     | 13 +++++
>  include/hw/virtio/virtio-balloon.h | 13 +++++
>  include/hw/virtio/virtio-blk.h     | 13 +++++
>  include/hw/virtio/virtio-net.h     | 13 +++++
>  include/hw/virtio/virtio-rng.h     | 13 +++++
>  include/hw/virtio/virtio-scsi.h    | 29 +++++++++--
>  include/hw/virtio/virtio-serial.h  | 24 ++++-----
>  include/hw/virtio/virtio.h         |  6 ++-
>  21 files changed, 513 insertions(+), 285 deletions(-)
> 
> -- 
> 1.8.1.4

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

* Re: [Qemu-devel] [PATCH RFT 2/5] virtio: Convert VirtioDevice to QOM realize/unrealize
  2013-06-08 12:32       ` Peter Crosthwaite
@ 2013-06-10  2:08         ` Anthony Liguori
  2013-06-10  6:30           ` Michael S. Tsirkin
  2013-06-12  9:15           ` Andreas Färber
  0 siblings, 2 replies; 19+ messages in thread
From: Anthony Liguori @ 2013-06-10  2:08 UTC (permalink / raw)
  To: Peter Crosthwaite, Andreas Färber
  Cc: Kevin Wolf, Michael S. Tsirkin, qemu-devel, jlarrew,
	Aneesh Kumar K.V, Stefan Hajnoczi, Amit Shah, Paolo Bonzini,
	fred.konrad

Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:

> Hi Andreas,
>
> On Sat, Jun 8, 2013 at 7:55 PM, Andreas Färber <afaerber@suse.de> wrote:
>> Hi,
>>
>> Am 08.06.2013 04:22, schrieb Peter Crosthwaite:
>>> On Sat, Jun 8, 2013 at 4:18 AM, Andreas Färber <afaerber@suse.de> wrote:
>>>> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
>>>> index dc6f4e4..409d315 100644
>>>> --- a/hw/9pfs/virtio-9p-device.c
>>>> +++ b/hw/9pfs/virtio-9p-device.c
>> [...]
>>>> @@ -136,12 +138,16 @@ static Property virtio_9p_properties[] = {
>>>>      DEFINE_PROP_END_OF_LIST(),
>>>>  };
>>>>
>>>> -static void virtio_9p_class_init(ObjectClass *klass, void *data)
>>>> +static void virtio_9p_class_init(ObjectClass *oc, void *data)
>>>>  {
>>>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>>>> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
>>>> +    DeviceClass *dc = DEVICE_CLASS(oc);
>>>> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
>>>> +    V9fsClass *v9c = VIRTIO_9P_CLASS(oc);
>>>> +
>>>> +    v9c->parent_realize = dc->realize;
>>>> +    dc->realize = virtio_9p_device_realize;
>>>> +
>>>>      dc->props = virtio_9p_properties;
>>>> -    vdc->init = virtio_9p_device_init;
>>>>      vdc->get_features = virtio_9p_get_features;
>>>>      vdc->get_config = virtio_9p_get_config;
>>>>  }
>>>> @@ -151,6 +157,7 @@ static const TypeInfo virtio_device_info = {
>>>>      .parent = TYPE_VIRTIO_DEVICE,
>>>>      .instance_size = sizeof(V9fsState),
>>>>      .class_init = virtio_9p_class_init,
>>>> +    .class_size = sizeof(V9fsClass),
>>>>  };
>>>>
>>>>  static void virtio_9p_register_types(void)
>>>> diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
>>>> index 1d6eedb..85699a7 100644
>>>> --- a/hw/9pfs/virtio-9p.h
>>>> +++ b/hw/9pfs/virtio-9p.h
>>>> @@ -227,6 +227,15 @@ typedef struct V9fsState
>>>>      V9fsConf fsconf;
>>>>  } V9fsState;
>>>>
>>>> +typedef struct V9fsClass {
>>>> +    /*< private >*/
>>>> +    VirtioDeviceClass parent_class;
>>>> +    /*< public >*/
>>>> +
>>>> +    DeviceRealize parent_realize;
>>>> +} V9fsClass;
>>>> +
>>>> +
>>>
>>> If applied tree-wide this change pattern is going to add a lot of
>>> boiler-plate to all devices. There is capability in QOM to access the
>>> overridden parent class functions already, so it can be made to work
>>> without every class having to do this save-and-call trick with
>>> overridden realize (and friends). How about this:
>>>
>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>> index 9190a7e..696702a 100644
>>> --- a/hw/core/qdev.c
>>> +++ b/hw/core/qdev.c
>>> @@ -37,6 +37,18 @@ int qdev_hotplug = 0;
>>>  static bool qdev_hot_added = false;
>>>  static bool qdev_hot_removed = false;
>>>
>>> +void device_parent_realize(DeviceState *dev, Error **errp)
>>> +{
>>> +    ObjectClass *class = object_get_class(dev);
>>> +    DeviceClass *dc;
>>> +
>>> +    class = object_class_get_parent(dc);
>>> +    assert(class);
>>> +    dc = DEVICE_CLASS(class);
>>> +
>>> +    dc->realize(dev, errp);
>>> +}

What's weird about this is that you aren't necessarily calling
Device::realize() here, you're really calling super()::realize().

I really don't know whether it's a better approach or not.

Another option is to have a VirtioDevice::realize() that virtio devices
overload such that you don't have to explicitly call the super()
version.  The advantage of this approach is that you don't have to
explicitly call the super version.

Regards,

Anthony Liguori

>>> +
>>>
>>> And child class realize fns can call this to realize themselves as the
>>> parent would. Ditto for reset and unrealize. Then you would only need
>>> to define struct FooClass when creating new abstractions (or virtual
>>> functions if your C++).
>>
>> Indeed, if you check the archives you will find that I suggested the
>> same in the context of ISA i8254/i8259 or CPUState. ;) Yet Anthony
>> specifically instructed me to do it this way, referring to GObject.
>> I then documented the expected process in qdev-core.h and object.h.
>>
>
> Thanks for the history. I found this:
>
> Jan 18 2013 Anthony Liguori wrote:
>
> It's idiomatic from GObject.
>
> I'm not sure I can come up with a concrete example but in the absense of
> a compelling reason to shift from the idiom, I'd strongly suggest not.
>
> Regards,
>
> Anthony Liguori
>
> ------------------------------------
>
> The additive diff on this series would take a massive nosedive - is
> that a compelling reason? It is very unfortunate that pretty much
> every class inheriting from device is going to have to define a class
> struct just for the sake of multi-level realization.
>
> There is roughly 15 or so lines of boiler plate added to every class,
> and in just the cleanup you have done this week you have about 10 or
> so instances of this change pattern. Under the
> child-access-to-parent-version proposal those 15 lines become 1.
>
> I don't see the fundamental reason why child classes shouldnt be able
> to access parent implementations of virtualised functions. Many OO
> oriented languages indeed explicitly build this capability into the
> syntax. Examples include Java with "super.foo()" accesses and C++ via
> the parent class namespace:
>
> class Bar : public Foo {
>   // ...
>
>   void printStuff() {
>     Foo::printStuff(); // calls base class' function
>   }
> };
>
> Anthony is it possible to consider loosening this restriction?



>
> Regards,
> Peter
>
>> Regards,
>> 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] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFT 2/5] virtio: Convert VirtioDevice to QOM realize/unrealize
  2013-06-10  2:08         ` Anthony Liguori
@ 2013-06-10  6:30           ` Michael S. Tsirkin
  2013-06-12  9:15           ` Andreas Färber
  1 sibling, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2013-06-10  6:30 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: Kevin Wolf, Peter Crosthwaite, qemu-devel, jlarrew,
	Aneesh Kumar K.V, Stefan Hajnoczi, Amit Shah, Paolo Bonzini,
	Andreas Färber, fred.konrad

On Sun, Jun 09, 2013 at 09:08:15PM -0500, Anthony Liguori wrote:
> Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:
> 
> > Hi Andreas,
> >
> > On Sat, Jun 8, 2013 at 7:55 PM, Andreas Färber <afaerber@suse.de> wrote:
> >> Hi,
> >>
> >> Am 08.06.2013 04:22, schrieb Peter Crosthwaite:
> >>> On Sat, Jun 8, 2013 at 4:18 AM, Andreas Färber <afaerber@suse.de> wrote:
> >>>> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> >>>> index dc6f4e4..409d315 100644
> >>>> --- a/hw/9pfs/virtio-9p-device.c
> >>>> +++ b/hw/9pfs/virtio-9p-device.c
> >> [...]
> >>>> @@ -136,12 +138,16 @@ static Property virtio_9p_properties[] = {
> >>>>      DEFINE_PROP_END_OF_LIST(),
> >>>>  };
> >>>>
> >>>> -static void virtio_9p_class_init(ObjectClass *klass, void *data)
> >>>> +static void virtio_9p_class_init(ObjectClass *oc, void *data)
> >>>>  {
> >>>> -    DeviceClass *dc = DEVICE_CLASS(klass);
> >>>> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> >>>> +    DeviceClass *dc = DEVICE_CLASS(oc);
> >>>> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
> >>>> +    V9fsClass *v9c = VIRTIO_9P_CLASS(oc);
> >>>> +
> >>>> +    v9c->parent_realize = dc->realize;
> >>>> +    dc->realize = virtio_9p_device_realize;
> >>>> +
> >>>>      dc->props = virtio_9p_properties;
> >>>> -    vdc->init = virtio_9p_device_init;
> >>>>      vdc->get_features = virtio_9p_get_features;
> >>>>      vdc->get_config = virtio_9p_get_config;
> >>>>  }
> >>>> @@ -151,6 +157,7 @@ static const TypeInfo virtio_device_info = {
> >>>>      .parent = TYPE_VIRTIO_DEVICE,
> >>>>      .instance_size = sizeof(V9fsState),
> >>>>      .class_init = virtio_9p_class_init,
> >>>> +    .class_size = sizeof(V9fsClass),
> >>>>  };
> >>>>
> >>>>  static void virtio_9p_register_types(void)
> >>>> diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
> >>>> index 1d6eedb..85699a7 100644
> >>>> --- a/hw/9pfs/virtio-9p.h
> >>>> +++ b/hw/9pfs/virtio-9p.h
> >>>> @@ -227,6 +227,15 @@ typedef struct V9fsState
> >>>>      V9fsConf fsconf;
> >>>>  } V9fsState;
> >>>>
> >>>> +typedef struct V9fsClass {
> >>>> +    /*< private >*/
> >>>> +    VirtioDeviceClass parent_class;
> >>>> +    /*< public >*/
> >>>> +
> >>>> +    DeviceRealize parent_realize;
> >>>> +} V9fsClass;
> >>>> +
> >>>> +
> >>>
> >>> If applied tree-wide this change pattern is going to add a lot of
> >>> boiler-plate to all devices. There is capability in QOM to access the
> >>> overridden parent class functions already, so it can be made to work
> >>> without every class having to do this save-and-call trick with
> >>> overridden realize (and friends). How about this:
> >>>
> >>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> >>> index 9190a7e..696702a 100644
> >>> --- a/hw/core/qdev.c
> >>> +++ b/hw/core/qdev.c
> >>> @@ -37,6 +37,18 @@ int qdev_hotplug = 0;
> >>>  static bool qdev_hot_added = false;
> >>>  static bool qdev_hot_removed = false;
> >>>
> >>> +void device_parent_realize(DeviceState *dev, Error **errp)
> >>> +{
> >>> +    ObjectClass *class = object_get_class(dev);
> >>> +    DeviceClass *dc;
> >>> +
> >>> +    class = object_class_get_parent(dc);
> >>> +    assert(class);
> >>> +    dc = DEVICE_CLASS(class);
> >>> +
> >>> +    dc->realize(dev, errp);
> >>> +}
> 
> What's weird about this is that you aren't necessarily calling
> Device::realize() here, you're really calling super()::realize().
> 
> I really don't know whether it's a better approach or not.
> 
> Another option is to have a VirtioDevice::realize() that virtio devices
> overload such that you don't have to explicitly call the super()
> version.  The advantage of this approach is that you don't have to
> explicitly call the super version.
> 
> Regards,
> 
> Anthony Liguori

Nod. Since all realize calls get same parameters not calling
parent's constructor automatically seems strange.

> >>> +
> >>>
> >>> And child class realize fns can call this to realize themselves as the
> >>> parent would. Ditto for reset and unrealize. Then you would only need
> >>> to define struct FooClass when creating new abstractions (or virtual
> >>> functions if your C++).
> >>
> >> Indeed, if you check the archives you will find that I suggested the
> >> same in the context of ISA i8254/i8259 or CPUState. ;) Yet Anthony
> >> specifically instructed me to do it this way, referring to GObject.
> >> I then documented the expected process in qdev-core.h and object.h.
> >>
> >
> > Thanks for the history. I found this:
> >
> > Jan 18 2013 Anthony Liguori wrote:
> >
> > It's idiomatic from GObject.
> >
> > I'm not sure I can come up with a concrete example but in the absense of
> > a compelling reason to shift from the idiom, I'd strongly suggest not.
> >
> > Regards,
> >
> > Anthony Liguori
> >
> > ------------------------------------
> >
> > The additive diff on this series would take a massive nosedive - is
> > that a compelling reason? It is very unfortunate that pretty much
> > every class inheriting from device is going to have to define a class
> > struct just for the sake of multi-level realization.
> >
> > There is roughly 15 or so lines of boiler plate added to every class,
> > and in just the cleanup you have done this week you have about 10 or
> > so instances of this change pattern. Under the
> > child-access-to-parent-version proposal those 15 lines become 1.
> >
> > I don't see the fundamental reason why child classes shouldnt be able
> > to access parent implementations of virtualised functions. Many OO
> > oriented languages indeed explicitly build this capability into the
> > syntax. Examples include Java with "super.foo()" accesses and C++ via
> > the parent class namespace:
> >
> > class Bar : public Foo {
> >   // ...
> >
> >   void printStuff() {
> >     Foo::printStuff(); // calls base class' function
> >   }
> > };
> >
> > Anthony is it possible to consider loosening this restriction?
> 
> 
> 
> >
> > Regards,
> > Peter
> >
> >> Regards,
> >> 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] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFT 1/5] virtio-blk-dataplane: Improve error reporting
  2013-06-07 18:18 ` [Qemu-devel] [PATCH RFT 1/5] virtio-blk-dataplane: Improve error reporting Andreas Färber
@ 2013-06-10 11:39   ` Stefan Hajnoczi
  2013-07-29 20:19     ` Andreas Färber
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Hajnoczi @ 2013-06-10 11:39 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Kevin Wolf, Anthony Liguori, qemu-devel, jlarrew, anthony,
	fred.konrad

On Fri, Jun 07, 2013 at 08:18:56PM +0200, Andreas Färber wrote:
> Return an Error so that it can be propagated later.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  hw/block/dataplane/virtio-blk.c | 25 +++++++++++++------------
>  hw/block/dataplane/virtio-blk.h |  5 +++--
>  hw/block/virtio-blk.c           |  8 +++++++-
>  3 files changed, 23 insertions(+), 15 deletions(-)

Tested-by: Stefan Hajnoczi <stefanha@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

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

* Re: [Qemu-devel] [PATCH RFT 2/5] virtio: Convert VirtioDevice to QOM realize/unrealize
  2013-06-10  2:08         ` Anthony Liguori
  2013-06-10  6:30           ` Michael S. Tsirkin
@ 2013-06-12  9:15           ` Andreas Färber
  2013-06-13  1:48             ` Peter Crosthwaite
  2013-06-18  9:57             ` Peter Crosthwaite
  1 sibling, 2 replies; 19+ messages in thread
From: Andreas Färber @ 2013-06-12  9:15 UTC (permalink / raw)
  To: Anthony Liguori, Michael S. Tsirkin
  Cc: Kevin Wolf, Peter Crosthwaite, qemu-devel, jlarrew,
	Aneesh Kumar K.V, Stefan Hajnoczi, Amit Shah, Paolo Bonzini,
	fred.konrad

Am 10.06.2013 04:08, schrieb Anthony Liguori:
> Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:
>> On Sat, Jun 8, 2013 at 7:55 PM, Andreas Färber <afaerber@suse.de> wrote:
>>> Am 08.06.2013 04:22, schrieb Peter Crosthwaite:
>>>> On Sat, Jun 8, 2013 at 4:18 AM, Andreas Färber <afaerber@suse.de> wrote:
>>>>> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
>>>>> index dc6f4e4..409d315 100644
>>>>> --- a/hw/9pfs/virtio-9p-device.c
>>>>> +++ b/hw/9pfs/virtio-9p-device.c
>>> [...]
>>>>> @@ -136,12 +138,16 @@ static Property virtio_9p_properties[] = {
>>>>>      DEFINE_PROP_END_OF_LIST(),
>>>>>  };
>>>>>
>>>>> -static void virtio_9p_class_init(ObjectClass *klass, void *data)
>>>>> +static void virtio_9p_class_init(ObjectClass *oc, void *data)
>>>>>  {
>>>>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>>>>> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
>>>>> +    DeviceClass *dc = DEVICE_CLASS(oc);
>>>>> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
>>>>> +    V9fsClass *v9c = VIRTIO_9P_CLASS(oc);
>>>>> +
>>>>> +    v9c->parent_realize = dc->realize;
>>>>> +    dc->realize = virtio_9p_device_realize;
>>>>> +
>>>>>      dc->props = virtio_9p_properties;
>>>>> -    vdc->init = virtio_9p_device_init;
>>>>>      vdc->get_features = virtio_9p_get_features;
>>>>>      vdc->get_config = virtio_9p_get_config;
>>>>>  }
>>>>> @@ -151,6 +157,7 @@ static const TypeInfo virtio_device_info = {
>>>>>      .parent = TYPE_VIRTIO_DEVICE,
>>>>>      .instance_size = sizeof(V9fsState),
>>>>>      .class_init = virtio_9p_class_init,
>>>>> +    .class_size = sizeof(V9fsClass),
>>>>>  };
>>>>>
>>>>>  static void virtio_9p_register_types(void)
>>>>> diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
>>>>> index 1d6eedb..85699a7 100644
>>>>> --- a/hw/9pfs/virtio-9p.h
>>>>> +++ b/hw/9pfs/virtio-9p.h
>>>>> @@ -227,6 +227,15 @@ typedef struct V9fsState
>>>>>      V9fsConf fsconf;
>>>>>  } V9fsState;
>>>>>
>>>>> +typedef struct V9fsClass {
>>>>> +    /*< private >*/
>>>>> +    VirtioDeviceClass parent_class;
>>>>> +    /*< public >*/
>>>>> +
>>>>> +    DeviceRealize parent_realize;
>>>>> +} V9fsClass;
>>>>> +
>>>>> +
>>>>
>>>> If applied tree-wide this change pattern is going to add a lot of
>>>> boiler-plate to all devices. There is capability in QOM to access the
>>>> overridden parent class functions already, so it can be made to work
>>>> without every class having to do this save-and-call trick with
>>>> overridden realize (and friends). How about this:
>>>>
>>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>>> index 9190a7e..696702a 100644
>>>> --- a/hw/core/qdev.c
>>>> +++ b/hw/core/qdev.c
>>>> @@ -37,6 +37,18 @@ int qdev_hotplug = 0;
>>>>  static bool qdev_hot_added = false;
>>>>  static bool qdev_hot_removed = false;
>>>>
>>>> +void device_parent_realize(DeviceState *dev, Error **errp)
>>>> +{
>>>> +    ObjectClass *class = object_get_class(dev);
>>>> +    DeviceClass *dc;
>>>> +
>>>> +    class = object_class_get_parent(dc);
>>>> +    assert(class);
>>>> +    dc = DEVICE_CLASS(class);
>>>> +
>>>> +    dc->realize(dev, errp);
>>>> +}
> 
> What's weird about this is that you aren't necessarily calling
> Device::realize() here, you're really calling super()::realize().

We can certainly fix that by passing ObjectClass *oc argument instead of
DeviceState *dev and using object_class_get_parent(oc) - dc is used
uninitialized above.

> I really don't know whether it's a better approach or not.

It does save LOCs and should work with sane class_inits.

> Another option is to have a VirtioDevice::realize() that virtio devices
> overload such that you don't have to explicitly call the super()
> version.  The advantage of this approach is that you don't have to
> explicitly call the super version.

The disadvantage is that we then have no chance to solve Jesse's
virtio-net issue this way (cf. cover letter), the only improvement would
be Error propagation.

Just let me know which path to pursue here. We could start by converting
*::init to realize signature and then follow up with either conversion.
Would that be acceptable to move forward?
Long-term a VirtioDevice::realize() would be blurring semantics though.

Partially affects pending ISA series as well (PIT/PIC).

Regards,
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] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFT 2/5] virtio: Convert VirtioDevice to QOM realize/unrealize
  2013-06-12  9:15           ` Andreas Färber
@ 2013-06-13  1:48             ` Peter Crosthwaite
  2013-06-18  9:57             ` Peter Crosthwaite
  1 sibling, 0 replies; 19+ messages in thread
From: Peter Crosthwaite @ 2013-06-13  1:48 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Kevin Wolf, Anthony Liguori, Michael S. Tsirkin, jlarrew,
	qemu-devel, Aneesh Kumar K.V, Stefan Hajnoczi, Amit Shah,
	Paolo Bonzini, fred.konrad

Hi Andreas,

On Wed, Jun 12, 2013 at 7:15 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 10.06.2013 04:08, schrieb Anthony Liguori:
>> Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:
>>> On Sat, Jun 8, 2013 at 7:55 PM, Andreas Färber <afaerber@suse.de> wrote:
>>>> Am 08.06.2013 04:22, schrieb Peter Crosthwaite:
>>>>> On Sat, Jun 8, 2013 at 4:18 AM, Andreas Färber <afaerber@suse.de> wrote:
>>>>>> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
>>>>>> index dc6f4e4..409d315 100644
>>>>>> --- a/hw/9pfs/virtio-9p-device.c
>>>>>> +++ b/hw/9pfs/virtio-9p-device.c
>>>> [...]
>>>>>> @@ -136,12 +138,16 @@ static Property virtio_9p_properties[] = {
>>>>>>      DEFINE_PROP_END_OF_LIST(),
>>>>>>  };
>>>>>>
>>>>>> -static void virtio_9p_class_init(ObjectClass *klass, void *data)
>>>>>> +static void virtio_9p_class_init(ObjectClass *oc, void *data)
>>>>>>  {
>>>>>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>>>>>> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
>>>>>> +    DeviceClass *dc = DEVICE_CLASS(oc);
>>>>>> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
>>>>>> +    V9fsClass *v9c = VIRTIO_9P_CLASS(oc);
>>>>>> +
>>>>>> +    v9c->parent_realize = dc->realize;
>>>>>> +    dc->realize = virtio_9p_device_realize;
>>>>>> +
>>>>>>      dc->props = virtio_9p_properties;
>>>>>> -    vdc->init = virtio_9p_device_init;
>>>>>>      vdc->get_features = virtio_9p_get_features;
>>>>>>      vdc->get_config = virtio_9p_get_config;
>>>>>>  }
>>>>>> @@ -151,6 +157,7 @@ static const TypeInfo virtio_device_info = {
>>>>>>      .parent = TYPE_VIRTIO_DEVICE,
>>>>>>      .instance_size = sizeof(V9fsState),
>>>>>>      .class_init = virtio_9p_class_init,
>>>>>> +    .class_size = sizeof(V9fsClass),
>>>>>>  };
>>>>>>
>>>>>>  static void virtio_9p_register_types(void)
>>>>>> diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
>>>>>> index 1d6eedb..85699a7 100644
>>>>>> --- a/hw/9pfs/virtio-9p.h
>>>>>> +++ b/hw/9pfs/virtio-9p.h
>>>>>> @@ -227,6 +227,15 @@ typedef struct V9fsState
>>>>>>      V9fsConf fsconf;
>>>>>>  } V9fsState;
>>>>>>
>>>>>> +typedef struct V9fsClass {
>>>>>> +    /*< private >*/
>>>>>> +    VirtioDeviceClass parent_class;
>>>>>> +    /*< public >*/
>>>>>> +
>>>>>> +    DeviceRealize parent_realize;
>>>>>> +} V9fsClass;
>>>>>> +
>>>>>> +
>>>>>
>>>>> If applied tree-wide this change pattern is going to add a lot of
>>>>> boiler-plate to all devices. There is capability in QOM to access the
>>>>> overridden parent class functions already, so it can be made to work
>>>>> without every class having to do this save-and-call trick with
>>>>> overridden realize (and friends). How about this:
>>>>>
>>>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>>>> index 9190a7e..696702a 100644
>>>>> --- a/hw/core/qdev.c
>>>>> +++ b/hw/core/qdev.c
>>>>> @@ -37,6 +37,18 @@ int qdev_hotplug = 0;
>>>>>  static bool qdev_hot_added = false;
>>>>>  static bool qdev_hot_removed = false;
>>>>>
>>>>> +void device_parent_realize(DeviceState *dev, Error **errp)
>>>>> +{
>>>>> +    ObjectClass *class = object_get_class(dev);
>>>>> +    DeviceClass *dc;
>>>>> +
>>>>> +    class = object_class_get_parent(dc);
>>>>> +    assert(class);
>>>>> +    dc = DEVICE_CLASS(class);
>>>>> +
>>>>> +    dc->realize(dev, errp);
>>>>> +}
>>
>> What's weird about this is that you aren't necessarily calling
>> Device::realize() here, you're really calling super()::realize().
>
> We can certainly fix that by passing ObjectClass *oc argument instead of
> DeviceState *dev and using object_class_get_parent(oc)

That is slightly more defensive but does cost you a line of code on
every usage (to do the DEVICE_GET_CLASS(dev)). Im happy either way as
its the need for the FooClass definition that really bloats the code.
Under this modified proposal its a two line change rather than the one
which is not so bad.

> - dc is used
> uninitialized above.
>

Typo on my part. should be "class"

>> I really don't know whether it's a better approach or not.
>
> It does save LOCs and should work with sane class_inits.
>
>> Another option is to have a VirtioDevice::realize() that virtio devices
>> overload such that you don't have to explicitly call the super()
>> version.  The advantage of this approach is that you don't have to
>> explicitly call the super version.
>
> The disadvantage is that we then have no chance to solve Jesse's
> virtio-net issue this way (cf. cover letter), the only improvement would
> be Error propagation.
>

+1. I thought we were trying to get away from abstractions
(SYS_BUS_DEVICE, I2C_DEVICE etc, and friends) defining their own
abstract inititialization fns (init or realize) so this would be a
step backwards. It also makes the code base very inconsistent with
itself. Devices which inherit from an  abstraction that already have a
realize need to use the specific realize while those that dont would
use the generic one (DeviceClass::realize). Also makes life very hard
when someone wants to add common realize functionality to an
abstraction (e.g. implemenent a non-trivial SysBusDevice::Realize) as
all the concrete classes would then need conversion.

> Just let me know which path to pursue here. We could start by converting
> *::init to realize signature and then follow up with either conversion.
> Would that be acceptable to move forward?

Sounds good to me, and makes it easier to disentangle this particular
issue with the rest of your changes.

Regards,
Peter

> Long-term a VirtioDevice::realize() would be blurring semantics though.
>
> Partially affects pending ISA series as well (PIT/PIC).
>
> Regards,
> 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] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFT 2/5] virtio: Convert VirtioDevice to QOM realize/unrealize
  2013-06-12  9:15           ` Andreas Färber
  2013-06-13  1:48             ` Peter Crosthwaite
@ 2013-06-18  9:57             ` Peter Crosthwaite
  1 sibling, 0 replies; 19+ messages in thread
From: Peter Crosthwaite @ 2013-06-18  9:57 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Kevin Wolf, Anthony Liguori, Michael S. Tsirkin, jlarrew,
	qemu-devel, Aneesh Kumar K.V, Stefan Hajnoczi, Amit Shah,
	Paolo Bonzini, fred.konrad

Hi All,

On Wed, Jun 12, 2013 at 7:15 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 10.06.2013 04:08, schrieb Anthony Liguori:
>> Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:
>>
>> What's weird about this is that you aren't necessarily calling
>> Device::realize() here, you're really calling super()::realize().
>
> We can certainly fix that by passing ObjectClass *oc argument instead of
> DeviceState *dev and using object_class_get_parent(oc) - dc is used
> uninitialized above.
>
>> I really don't know whether it's a better approach or not.
>
> It does save LOCs and should work with sane class_inits.
>
>> Another option is to have a VirtioDevice::realize() that virtio devices
>> overload such that you don't have to explicitly call the super()
>> version.  The advantage of this approach is that you don't have to
>> explicitly call the super version.
>
> The disadvantage is that we then have no chance to solve Jesse's
> virtio-net issue this way (cf. cover letter), the only improvement would
> be Error propagation.
>
> Just let me know which path to pursue here. We could start by converting
> *::init to realize signature and then follow up with either conversion.
> Would that be acceptable to move forward?
> Long-term a VirtioDevice::realize() would be blurring semantics though.
>
> Partially affects pending ISA series as well (PIT/PIC).
>

This got merged, so I took the opportunity to workshop changing one of
them to use this super idea for the sake of discussion. Patches on
list. Please review in the context of this discussion.

Regards,
Peter

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

* Re: [Qemu-devel] [PATCH RFT 1/5] virtio-blk-dataplane: Improve error reporting
  2013-06-10 11:39   ` Stefan Hajnoczi
@ 2013-07-29 20:19     ` Andreas Färber
  0 siblings, 0 replies; 19+ messages in thread
From: Andreas Färber @ 2013-07-29 20:19 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Kevin Wolf, Anthony Liguori, qemu-devel, jlarrew, anthony,
	fred.konrad

Am 10.06.2013 13:39, schrieb Stefan Hajnoczi:
> On Fri, Jun 07, 2013 at 08:18:56PM +0200, Andreas Färber wrote:
>> Return an Error so that it can be propagated later.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>>  hw/block/dataplane/virtio-blk.c | 25 +++++++++++++------------
>>  hw/block/dataplane/virtio-blk.h |  5 +++--
>>  hw/block/virtio-blk.c           |  8 +++++++-
>>  3 files changed, 23 insertions(+), 15 deletions(-)
> 
> Tested-by: Stefan Hajnoczi <stefanha@redhat.com>
> Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

Needs some more trivial changes since today.

Andreas

diff --git a/hw/block/dataplane/virtio-blk.c
b/hw/block/dataplane/virtio-blk.c
index ae82aea..fed9231 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -421,8 +421,9 @@ void virtio_blk_data_plane_create(VirtIODevice
*vdev, VirtIO
BlkConf *blk,
      * 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);
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 14f292b..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 */


-- 
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 related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH RFT 4/5] virtio-console: Use exitfn for virtserialport, too
  2013-06-07 18:18 ` [Qemu-devel] [PATCH RFT 4/5] virtio-console: Use exitfn for virtserialport, too Andreas Färber
@ 2013-07-29 23:25   ` Andreas Färber
  0 siblings, 0 replies; 19+ messages in thread
From: Andreas Färber @ 2013-07-29 23:25 UTC (permalink / raw)
  To: Anthony Liguori, Michael S. Tsirkin
  Cc: Anthony Liguori, qemu-stable, jlarrew, qemu-devel, Amit Shah,
	fred.konrad

Michael, Anthony,

Am 07.06.2013 20:18, schrieb Andreas Färber:
> 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 7b1a382..73e18f2 100644
> --- a/hw/char/virtio-console.c
> +++ b/hw/char/virtio-console.c
> @@ -192,6 +192,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;

Ping! Is this a fix we should get into 1.6? Or does it not matter in
practice for some reason?

Regards,
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] 19+ messages in thread

end of thread, other threads:[~2013-07-29 23:26 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-07 18:18 [Qemu-devel] [PATCH RFT 0/5] QOM realize for virtio Andreas Färber
2013-06-07 18:18 ` [Qemu-devel] [PATCH RFT 1/5] virtio-blk-dataplane: Improve error reporting Andreas Färber
2013-06-10 11:39   ` Stefan Hajnoczi
2013-07-29 20:19     ` Andreas Färber
2013-06-07 18:18 ` [Qemu-devel] [PATCH RFT 2/5] virtio: Convert VirtioDevice to QOM realize/unrealize Andreas Färber
2013-06-08  2:22   ` Peter Crosthwaite
2013-06-08  9:55     ` Andreas Färber
2013-06-08 12:32       ` Peter Crosthwaite
2013-06-10  2:08         ` Anthony Liguori
2013-06-10  6:30           ` Michael S. Tsirkin
2013-06-12  9:15           ` Andreas Färber
2013-06-13  1:48             ` Peter Crosthwaite
2013-06-18  9:57             ` Peter Crosthwaite
2013-06-09 10:36   ` Michael S. Tsirkin
2013-06-07 18:18 ` [Qemu-devel] [PATCH RFT 3/5] virtio-console: QOM'ify VirtConsole Andreas Färber
2013-06-07 18:18 ` [Qemu-devel] [PATCH RFT 4/5] virtio-console: Use exitfn for virtserialport, too Andreas Färber
2013-07-29 23:25   ` Andreas Färber
2013-06-07 18:19 ` [Qemu-devel] [PATCH RFT 5/5] virtio-serial-port: Convert to QOM realize/unrealize Andreas Färber
2013-06-09 10:39 ` [Qemu-devel] [PATCH RFT 0/5] QOM realize for virtio Michael S. Tsirkin

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