All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Jason Wang <jasowang@redhat.com>
Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org,
	Jesse Larrew <jlarrew@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [PATCH] virtio-net: count VIRTIO_NET_F_MAC when calculating config_len
Date: Thu, 25 Apr 2013 10:06:56 +0300	[thread overview]
Message-ID: <20130425070656.GA7485@redhat.com> (raw)
In-Reply-To: <5178D514.5090600@redhat.com>

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 <jlarrew@linux.vnet.ibm.com>
> >> Signed-off-by: Jason Wang <jasowang@redhat.com>
> >> ---
> >>  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

  reply	other threads:[~2013-04-25  7:07 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-25  6:21 [Qemu-devel] [PATCH] virtio-net: count VIRTIO_NET_F_MAC when calculating config_len Jason Wang
2013-04-25  6:59 ` Michael S. Tsirkin
2013-04-25  7:02   ` Jason Wang
2013-04-25  7:06     ` Michael S. Tsirkin [this message]
2013-04-25  7:52       ` Jason Wang
2013-04-29 14:42   ` Jesse Larrew
2013-04-29 14:55     ` KONRAD Frédéric
2013-04-29 15:14       ` Jesse Larrew
2013-04-29 15:29         ` KONRAD Frédéric
2013-04-29 15:55           ` Jesse Larrew
2013-04-29 16:02             ` Michael S. Tsirkin
2013-04-29 16:14               ` KONRAD Frédéric
2013-04-29 16:21                 ` Michael S. Tsirkin
2013-04-29 16:30                   ` Michael S. Tsirkin
2013-04-29 16:41                     ` KONRAD Frédéric
2013-04-29 17:01                       ` Michael S. Tsirkin
2013-04-29 17:23                         ` KONRAD Frédéric
2013-04-29 17:52                           ` Michael S. Tsirkin
2013-04-29 18:01                             ` KONRAD Frédéric
2013-04-29 18:15                               ` Michael S. Tsirkin
2013-04-29 18:45                                 ` KONRAD Frédéric
2013-04-29 20:09                                   ` Michael S. Tsirkin
2013-04-30  8:47                                     ` KONRAD Frédéric
2013-04-30  9:02                                       ` Michael S. Tsirkin
2013-05-02  9:02                                         ` Cornelia Huck
2013-05-02  8:59                                     ` Cornelia Huck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130425070656.GA7485@redhat.com \
    --to=mst@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=jasowang@redhat.com \
    --cc=jlarrew@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.