Re: [Qemu-devel] [PATCH] virtio: make bindings typesafe

[Paolo Bonzini <pbonzini@redhat.com>]

Il 17/12/2012 18:55, Andreas Färber ha scritto:
> Am 17.12.2012 16:45, schrieb Michael S. Tsirkin:
>> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
>> index 3ea4140..63ae888 100644
>> --- a/hw/virtio-pci.c
>> +++ b/hw/virtio-pci.c
>> @@ -98,34 +98,34 @@ bool virtio_is_big_endian(void);
>>  
>>  /* virtio device */
>>  
>> -static void virtio_pci_notify(void *opaque, uint16_t vector)
>> +static void virtio_pci_notify(DeviceState *d, uint16_t vector)
>>  {
>> -    VirtIOPCIProxy *proxy = opaque;
>> +    VirtIOPCIProxy *proxy = container_of(d, VirtIOPCIProxy, pci_dev.qdev);
> 
> Nack. This is going the wrong direction QOM-wise and you among all
> others know that from PCI host bridges!

Well, that's just a difference of VIRTIO_PCI_PROXY(d) vs. container_of.

But the patch is still an improvement.  In fact it should a logical
first step even in Peter's refactoring (where the devicestate is
replaced by d->parent_bus or d->parent_bus->parent).

Paolo

> A core issue being addressed here is that virtio devices are modelled
> neither in the regular qdev way nor the QOM way. They don't inherit
> correctly and use their own set of common-init functions - one side
> effect of Fred's series was to make them first-class QOM citizens. QOM
> like qdev doesn't support multi-inheritence, so the discussed approach
> Fred is trying to implement is to have both a VirtioDevice as base class
> for Virtio{Block,...}Device and PCIDevice/SysBusDevice/... as base class
> for a virtio bridge device with a virtio-bus, on which only virtio is
> spoken and pure VirtioDevices can sit (which as you say have device IDs
> but not all PCI properties).
> What remained under discussion AFAIU was how to expose this modelling
> construct to the user - Peter aiming to expose this to the user and me
> proposing to hide this (for PCI) as an internal implementation detail.
> 
> Whether DeviceState or a new VirtioDevice or something else is being
> used in the API is a different issue that I don't really mind.
> 
> Andreas
>