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

[Christian Borntraeger]

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<address@hidden>
> 
> 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 <address@hidden>
Date:   Tue Feb 5 17:47:15 2013 -0600

    virtio-net: pass host features to virtio_net_init
    
    Signed-off-by: Anthony Liguori <address@hidden>

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 :-)