Re: [Qemu-devel] [PATCH 3/4] hw/net/virtio-net: Move DEFINE_VIRTIO_NET_FEATURES to virtio-net

[Peter Maydell]

On 17 April 2015 at 13:13, Shannon Zhao <address@hidden> wrote:
> Move DEFINE_VIRTIO_NET_FEATURES to the backend virtio-net.
> The transports just sync the host features from backend.
>
> Signed-off-by: Shannon Zhao <address@hidden>
> Signed-off-by: Shannon Zhao <address@hidden>
> ---
>  hw/net/virtio-net.c            | 4 ++++
>  hw/s390x/s390-virtio-bus.c     | 1 -
>  hw/s390x/virtio-ccw.c          | 1 -
>  hw/virtio/virtio-pci.c         | 1 -
>  include/hw/virtio/virtio-net.h | 1 +
>  5 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 27adcc5..5d72e2d 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -446,6 +446,9 @@ static uint32_t virtio_net_get_features(VirtIODevice 
> *vdev, uint32_t features)
>      VirtIONet *n = VIRTIO_NET(vdev);
>      NetClientState *nc = qemu_get_queue(n->nic);
>
> +    /* First sync all virtio-net possible supported features */
> +    features |= n->host_features;
> +
>      virtio_add_feature(&features, VIRTIO_NET_F_MAC);
>
>      if (!peer_has_vnet_hdr(n)) {
> @@ -1714,6 +1717,7 @@ static void virtio_net_instance_init(Object *obj)
>  }
>
>  static Property virtio_net_properties[] = {
> +    DEFINE_VIRTIO_NET_FEATURES(VirtIONet, host_features),
>      DEFINE_NIC_PROPERTIES(VirtIONet, nic_conf),
>      DEFINE_PROP_UINT32("x-txtimer", VirtIONet, net_conf.txtimer,
>                                                 TX_TIMER_INTERVAL),
> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
> index 31fdf94..49c13e2 100644
> --- a/hw/s390x/s390-virtio-bus.c
> +++ b/hw/s390x/s390-virtio-bus.c
> @@ -517,7 +517,6 @@ static void virtio_s390_device_plugged(DeviceState *d)
>
>  static Property s390_virtio_net_properties[] = {
>      DEFINE_VIRTIO_COMMON_FEATURES(VirtIOS390Device, host_features),
> -    DEFINE_VIRTIO_NET_FEATURES(VirtIOS390Device, host_features),
>      DEFINE_PROP_END_OF_LIST(),
>  };

I'm confused. This seems to be the same "breaks backwards compatibility"
implementation you suggested to me off-list and which I said
wouldn't work.

thanks
-- PMM