Re: [Qemu-devel] [PATCH] virtio-net: count VIRTIO_NET_F_MAC when calculating config_len

[Michael S. Tsirkin]

On Thu, Apr 25, 2013 at 03:02:44PM +0800, Jason Wang wrote:
> On 04/25/2013 02:59 PM, Michael S. Tsirkin wrote:
> > On Thu, Apr 25, 2013 at 02:21:29PM +0800, Jason Wang wrote:
> >> Commit 14f9b664 (hw/virtio-net.c: set config size using host features) 
> >> tries to
> >> calculate config size based on the host features. But it forgets the
> >> VIRTIO_NET_F_MAC were always set for qemu later. This will lead a zero 
> >> config
> >> len for virtio-net device when both VIRTIO_NET_F_STATUS and 
> >> VIRTIO_NET_F_MQ were
> >> disabled form command line. Then qemu will crash when user tries to read 
> >> the
> >> config of virtio-net.
> >>
> >> Fix this by counting VIRTIO_NET_F_MAC and make sure the config at least 
> >> contains
> >> the mac address.
> >>
> >> Cc: Jesse Larrew <address@hidden>
> >> Signed-off-by: Jason Wang <address@hidden>
> >> ---
> >>  hw/net/virtio-net.c |    3 ++-
> >>  1 files changed, 2 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >> index 70c8fce..33a70ef 100644
> >> --- a/hw/net/virtio-net.c
> >> +++ b/hw/net/virtio-net.c
> >> @@ -1264,7 +1264,8 @@ static void 
> >> virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx,
> >>  
> >>  void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features)
> >>  {
> >> -    int i, config_size = 0;
> >> +    /* VIRTIO_NET_F_MAC can't be disabled from qemu side */
> >> +    int i, config_size = feature_sizes[0].end;
> > This would be cleaner:
> >     host_features |= (1 << VIRTIO_NET_F_MAC);
> >
> > no need for a comment then.
> 
> Sure, will send V2.

Maybe ass assert(config_size) in core just to make sure
other devices do not have this bug.

> >>      for (i = 0; feature_sizes[i].flags != 0; i++) {
> >>          if (host_features & feature_sizes[i].flags) {
> >>              config_size = MAX(feature_sizes[i].end, config_size);
> >> -- 
> >> 1.7.1