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

[Anthony Liguori]

Konrad Frederic <address@hidden> writes:

> On 26/11/2012 17:59, Anthony Liguori wrote:
>> Peter Maydell<address@hidden>  writes:
>>
>>> On 26 November 2012 14:33, Anthony Liguori<address@hidden>  wrote:
>>>> VirtioBusInfo is not a great name.  This is a proxy class that allows
>>>> for a device to implement the virtio bus interface.
>>>>
>>>> This could be done as an interface but since nothing else uses
>>>> interfaces, I'm okay with something like this.  But the first argument
>>>> ought to be an opaque for all methods.
>>> We have at least one user of Interface in the tree IIRC.
>>> I'd much rather we did this the right way -- the only reason
>>> it's the way Fred has coded it is that there's no obvious
>>> body of code in the tree to copy, so we're thrashing around
>>> a bit. If you tell us what the correct set of structs/classes/
>>> interfaces/etc is then we can implement it :-)
>> I really think extending virtio-bus to a virtio-pci-bus and then
>> initializing it with a link to the PCI device is the best approach.
>>
>> It's by far the simpliest approach in terms of coding.
>>
>> Did I explain it adequately?  To recap:
>>
>> virtio-bus extends bus-state
>>   - implements everything that VirtIOBindings implements as methods
>>
>> virtio-pci-bus extends virtio-bus
>>   - is constructed with a pointer to a PCIDevice
>>   - implements the methods necessary to be a virtio bus
> I still have trouble with that ^^.
>
> virtio-pci-bus extends virtio-bus so I put something like that :
>
> static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
> {
>      BusClass *bc = BUS_CLASS(klass);
>      VirtioBusClass *k = VIRTIO_BUS_CLASS(klass);
>      /* VirtIOBindings */
>      k->notify = virtio_pci_notify;
>      k->save_config = virtio_pci_save_config;
>      k->load_config = virtio_pci_load_config;
>      k->save_queue = virtio_pci_save_queue;
>      k->load_queue = virtio_pci_load_queue;
>      k->get_features = virtio_pci_get_features;
>      k->query_guest_notifiers = virtio_pci_query_guest_notifiers;
>      k->set_host_notifier = virtio_pci_set_host_notifier;
>      k->set_guest_notifiers = virtio_pci_set_guest_notifiers;
>      k->vmstate_change = virtio_pci_vmstate_change;
>      /*
>       * TODO : Init and exit function.
>       * void (*init)(void *opaque);
>       * void (*exit)(void *opaque);
>       */
> }
>
> static TypeInfo virtio_pci_bus_info = {
>      .name          = TYPE_VIRTIO_PCI_BUS,
>      .parent        = TYPE_VIRTIO_BUS,
>      .class_init    = virtio_pci_bus_class_init,
> };
>
> and I have VirtioDevice which extends DeviceState like that :
>
> static void virtio_device_class_init(ObjectClass *klass, void *data)
> {
>      /* Set the default value here. */
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      dc->bus_type = TYPE_VIRTIO_BUS;
>      dc->init = virtio_device_init;
> }
>
> static TypeInfo virtio_device_info = {
>      .name = TYPE_VIRTIO_DEVICE,
>      .parent = TYPE_DEVICE,
>      .instance_size = sizeof(VirtioDeviceState),
>      /* Abstract the virtio-device */
>      .class_init = virtio_device_class_init,
>      .abstract = true,
>      .class_size = sizeof(VirtioDeviceClass),
> };
>
> The problem is that the virtio devices can't be connected to the 
> virtio-pci-bus even if it extends virtio-bus because TYPE_VIRTIO_BUS != 
> TYPE_VIRTIO_PCI_BUS.

That's just a bug.  See the patch I just sent out to fix it.

The type check is too strict in qdev_device_add().

Regards,

Anthony Liguori

>
> Did I miss something ?
>
> Thanks,
>
> Fred
>
>
>> virtio-device extends device-state
>>   - implements methods used by virtio-bus
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>> -- PMM