Re: [Qemu-devel] [PATCH 2/3] hw/virtio-net.c: set config size using host features

[Christian Borntraeger <borntraeger@de.ibm.com>]

Ping.
Anthony, Jesse,

how is this supposed to work?


Christian
On 05/03/13 18:03, Christian Borntraeger wrote:
> On 05/03/13 17:48, Alexander Graf wrote:
>> On 02/06/2013 12:47 AM, Jesse Larrew wrote:
>>> Currently, the config size for virtio devices is hard coded. When a new
>>> feature is added that changes the config size, drivers that assume a static
>>> config size will break. For purposes of backward compatibility, there needs
>>> to be a way to inform drivers of the config size needed to accommodate the
>>> set of features enabled.
>>>
>>> Signed-off-by: Jesse Larrew<jlarrew@linux.vnet.ibm.com>
>>
>> The following patch gets my s390 virtio guest working again, but I doubt it's the right fix.
>>
>> What is the expected dependency chain of feature calls?
>>
>>
>> Alex
>>
>> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
>> index 089ed92..81be971 100644
>> --- a/hw/s390x/s390-virtio-bus.c
>> +++ b/hw/s390x/s390-virtio-bus.c
>> @@ -154,7 +154,7 @@ static int s390_virtio_net_init(VirtIOS390Device *dev)
>>      VirtIODevice *vdev;
>>
>>      vdev = virtio_net_init((DeviceState *)dev, &dev->nic, &dev->net,
>> -                           dev->host_features);
>> +                           dev->host_features | (1 << VIRTIO_NET_F_MAC));
>>      if (!vdev) {
>>          return -1;
>>      }
>>
>>
> 
> Actually this goes back to
> 
> commit 1e89ad5b00ba0426d4e949c9e6ce2926c15b81b7
> Author: Anthony Liguori <aliguori@us.ibm.com>
> Date:   Tue Feb 5 17:47:15 2013 -0600
> 
>     virtio-net: pass host features to virtio_net_init
>     
>     Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> virtio-s390 calls  virtio_net_init  before it actually queries the dev->features into
> the host_features. virtio-ccw does the same, but it does not BUG. (Its still wrong IMHO)
> 
> Same for virtio-pci:
> 
> 
> static int virtio_net_init_pci(PCIDevice *pci_dev)
> {
>     VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
>     VirtIODevice *vdev;
> 
>     vdev = virtio_net_init(&pci_dev->qdev, &proxy->nic, &proxy->net,
>                            proxy->host_features);   <--- use host_feature
> 
>     vdev->nvectors = proxy->nvectors;
>     virtio_init_pci(proxy, vdev);   <------------- actually gets host_feature (!)
> [..]
> 
> 
> Good that my old rusty virtio-ccw transport has lots of BUG_ONS :-)
> 
> 
>