* [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
* 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 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
* [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
* 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-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 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 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
* [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
* 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
* [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 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
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).