Am 10.06.2013 04:08, schrieb Anthony Liguori: > Peter Crosthwaite <peter.crosthwa...@xilinx.com> writes: >> On Sat, Jun 8, 2013 at 7:55 PM, Andreas Färber <afaer...@suse.de> wrote: >>> Am 08.06.2013 04:22, schrieb Peter Crosthwaite: >>>> On Sat, Jun 8, 2013 at 4:18 AM, Andreas Färber <afaer...@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