Re: [Qemu-devel] [PATCH v5 05/10] hw/vfio/pci: split vfio_get_device

[Eric Auger <eric.auger@linaro.org>]

On 08/13/2014 05:32 AM, David Gibson wrote:
> On Tue, Aug 12, 2014 at 08:54:34AM +0200, Eric Auger wrote:
>> On 08/12/2014 04:41 AM, David Gibson wrote:
>>> On Sat, Aug 09, 2014 at 03:25:44PM +0100, Eric Auger wrote:
>>>> vfio_get_device now takes a VFIODevice as argument. The function is split
>>>> into 4 functional parts: dev_info query, device check, region populate
>>>> and interrupt populate. the last 3 are specialized by parent device and
>>>> are added into DeviceOps.
>>>
>>> Why is splitting these up into 4 stages useful, rather than having a
>>> single sub-class specific callback?
>>
>> Hi David,
>>
>> VFIOPlatformDevice already inherits from SysBusDevice and hence cannot
>> inherit from another VFIODevice. Same for VFIOPCIDevice that inherits
>> from PCIDevice. This is why I created this non QOM struct. But did you
>> mean something else?
> 
> Ah, yes, sorry, I missed that, though it's obvious now I think about
> it.
> 
>> Then splitting into 4: This was to share some code between platform and
>> PCI (dev_info query) and vfio_get_device was quite big already. I
>> thought it makes sense to split it into functional parts.
> 
> Hm, ok.  So splitting out dev_info_query certainly makes sense then.
> But does splitting the two populate sections make sense?  Is it
> plausible that two different VFIO capable busses would share one of
> these functions but not the other?

Hi David,

Coming back to you on that topic. There is no other justification for
splitting the code into 3 functions except than having shorter functions
with reduced functionality. But I acknowledge it would simplify the diff
between original code and new one so I intend to keep a single
specialized functions instead of 3.

Best Regards

Eric

>