[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] virtio: make bindings typesafe
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH] virtio: make bindings typesafe |
Date: |
Mon, 17 Dec 2012 22:13:11 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0 |
Am 17.12.2012 21:48, schrieb Michael S. Tsirkin:
> On Mon, Dec 17, 2012 at 07:25:08PM +0100, Andreas Färber wrote:
>> Am 17.12.2012 19:21, schrieb Paolo Bonzini:
>>> 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.
>>
>> VIRTIO_PCI_PROXY(d) would be acceptable, sure. But as-is this patch just
>> pushes unnecessary work on Fred, me, you or anyone else who works with QOM.
>
> What's VIRTIO_PCI_PROXY? Note this is data path we do not want extra
> code.
My complaint is the direct access of pci_dev, qdev, etc. parent fields
in many places as the main change of this patch. Those mean more places
to touch in a future patch.
Use of any new-style macro hiding these - wherever the particular one
suggested may be defined or whether it needs to be added - is better.
If performance of dynamic_cast is an issue - something I'd leave you to
discuss with Anthony - you can just do a C cast directly. Just don't
spread this qdev paradigm further please.
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
- [Qemu-devel] [PATCH] virtio: make bindings typesafe, Michael S. Tsirkin, 2012/12/17
- Re: [Qemu-devel] [PATCH] virtio: make bindings typesafe, Andreas Färber, 2012/12/17
- Re: [Qemu-devel] [PATCH] virtio: make bindings typesafe, Paolo Bonzini, 2012/12/17
- Re: [Qemu-devel] [PATCH] virtio: make bindings typesafe, Andreas Färber, 2012/12/17
- Re: [Qemu-devel] [PATCH] virtio: make bindings typesafe, Michael S. Tsirkin, 2012/12/17
- Re: [Qemu-devel] [PATCH] virtio: make bindings typesafe,
Andreas Färber <=
- Re: [Qemu-devel] [PATCH] virtio: make bindings typesafe, Michael S. Tsirkin, 2012/12/17
- Re: [Qemu-devel] [PATCH] virtio: make bindings typesafe, Andreas Färber, 2012/12/17
- Re: [Qemu-devel] [PATCH] virtio: make bindings typesafe, Michael S. Tsirkin, 2012/12/17
- Re: [Qemu-devel] [PATCH] virtio: make bindings typesafe, Andreas Färber, 2012/12/17
- Re: [Qemu-devel] [PATCH] virtio: make bindings typesafe, Michael S. Tsirkin, 2012/12/17
- Re: [Qemu-devel] [PATCH] virtio: make bindings typesafe, Andreas Färber, 2012/12/17
- Re: [Qemu-devel] [PATCH] virtio: make bindings typesafe, Michael S. Tsirkin, 2012/12/18
Re: [Qemu-devel] [PATCH] virtio: make bindings typesafe, Michael S. Tsirkin, 2012/12/17