Re: [Qemu-devel] Re: [PATCH] qemu/virtio: move features to an inline function

[Michael S. Tsirkin]

On Mon, Aug 10, 2009 at 03:33:59PM -0500, Anthony Liguori wrote:
> Michael S. Tsirkin wrote:
>>> Normally, the common features are transport features and the devices  
>>> should have absolutely no knowledge of transport feature (since 
>>> they're  transport dependent).
>>>     
>>
>> Good point. But
>>
>> 1. note that with my patch they don't. They call
>> virtio_get_common_features and that's all.
>>
>> 2. some features may not make sense for some devices. For example, it is
>> quite possible that indirect ring entries feature improves performance
>> on block but hurts on net, as net has a similar merged buffers feature.
>> Someone should try benchmarking with it disabled, and it becomes
>> possible with my patch.
>>   
>
> I don't necessarily disagree but I think your patch goes about it the  
> wrong way.
>
> There ought to be a way to layer qdev properties that achieves this goal  
> so that when you create a virtio-pci-block device, you have the ability  
> to turn off indirect sg without virtio-block having to know what that is.

I don't understand, sorry. Why do you insist on involving pci here?
ring layout has nothing to do with pci, does it?  With my patch,
virtio-block does not know what indirect sg is. It just does
enable/disable.

virtio net has device-specific feature that overlaps with
indirect entries. So insisting that devices should just ignore
transport does not make sense, to me.

> For your use-case, I wonder if you're integrating at the wrong level.   

Forget about that for now. Let's solve the generic problem.

> Regards,
>
> Anthony Liguori