Re: [Qemu-devel] [RFC PATCH v2 1/3] virtio-bus : Introduce VirtioBus.

[Stefan Hajnoczi]

On Thu, Nov 22, 2012 at 03:50:50PM +0100, address@hidden wrote:
> +/* Bind the VirtIODevice to the VirtioBus. */
> +void virtio_bus_bind_device(VirtioBus *bus)
> +{
> +    BusState *qbus = BUS(bus);
> +    assert(bus != NULL);
> +    assert(bus->vdev != NULL);
> +    virtio_bind_device(bus->vdev, &(bus->info->virtio_bindings), 
> qbus->parent);
> +}

Should plug and bind be together in a single function?  Binding the
device seems like an internal step that the bus takes when you plug the
device.

> +struct VirtioBusInfo {

This is defining an ad-hoc interface.  QOM has support for interfaces so
that a virtio-pci adapter brovides a VirtioBindingInterface which
VirtioBus can talk to intead of using VirtioBusInfo.

> +    void (*init_cb)(DeviceState *dev);
> +    void (*exit_cb)(DeviceState *dev);

Can _cb be dropped from the name?  Structs with function pointers always
provide "callbacks" so the _cb is unnecessary.  For example, QOM methods
like BusClass->reset() don't include _cb either.

> +    VirtIOBindings virtio_bindings;
> +};
> +
> +struct VirtioBus {
> +    BusState qbus;
> +    bool bus_in_use;

Should bus_in_use basically be bus->vdev != NULL?