On 06/04/2013 12:35 PM, Andreas Färber wrote:
Hi,
Hi Andreas!
Thanks for the review. :)
Am 04.06.2013 18:22, schrieb Jesse Larrew:
Virtio devices are initialized prior to plugging them into a bus. However,
other initializations (such as host_features) don't occur until after the
device is plugged into the bus. If a device needs to modify it's
configuration based on host_features, then it needs to be notified when the
bus is attached and host_features is available for use.
This patch extends struct VirtioDeviceClass to add a bus_plugged() method.
If implemented by a device, it will be called after the device is attached
to a bus.
Signed-off-by: Jesse Larrew <address@hidden>
I think this is backwards...
First of all, why is host_features not available before?
A hook on the bus makes sense because it allows central handling for any
devices on that bus.
However for a device, first TypeInfo::instance_init is run, then
qdev_set_parent_bus() connects the bus and finally DeviceClass::realize
is run - and we want to postpone realize further in the future.
Yes! This would do perfectly.
So why can't this be in VirtioDevice's or VirtIONet's realize method? At
realize time we should definitely be on the bus in this case. I.e.,
create vdev->config only after we know how large it needs to be rather
than creating and later resizing it, which might fail.
It appears that virtio hasn't been completely converted to use realize() yet.
Currently, device_realize() in virtio.c simply calls virtio_device_init(),
which looks like this:
static int virtio_device_init(DeviceState *qdev)
{
VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev);
assert(k->init != NULL);
if (k->init(vdev) < 0) {
return -1;
}
virtio_bus_plug_device(vdev);
return 0;
}
VirtioDeviceClass::init() calls virtio_init(), which allocates the config
struct. Then virtio_bus_plug_device() is called to attach the bus (and to
populate host_features). I wonder if it's safe to call
virtio_bus_plug_device() sooner...