[Qemu-devel] Re: [PATCHv2] virtio-net: unify vhost-net start/stop

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Sep 28, 2010 at 09:16:34AM -0600, Alex Williamson wrote:
> On Mon, 2010-09-27 at 23:52 +0200, Michael S. Tsirkin wrote:
> > On Mon, Sep 27, 2010 at 01:43:11PM -0600, Alex Williamson wrote:
> > > On Mon, 2010-09-27 at 18:41 +0200, Michael S. Tsirkin wrote:
> > > > Move all of vhost-net start/stop logic to a single routine,
> > > > and call it from everywhere.
> > > > 
> > > > Additionally, start/stop vhost-net on link up/down:
> > > > we should not transmit anything if user asked us to
> > > > put the link down.
> > > > 
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > ---
> > > > 
> > > > Sent a wrong version, sorry about the noise.
> > > > 
> > > >  hw/virtio-net.c |   89 ++++++++++++++++++++++++++----------------------------
> > > >  1 files changed, 43 insertions(+), 46 deletions(-)
> > > > 
> > > > diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> > > > index 075f72d..f87c219 100644
> > > > --- a/hw/virtio-net.c
> > > > +++ b/hw/virtio-net.c
> > > > @@ -51,6 +51,7 @@ typedef struct VirtIONet
> > > >      uint8_t nouni;
> > > >      uint8_t nobcast;
> > > >      uint8_t vhost_started;
> > > > +    bool vm_running;
> > > >      VMChangeStateEntry *vmstate;
> > > >      struct {
> > > >          int in_use;
> > > > @@ -95,6 +96,38 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
> > > >      }
> > > >  }
> > > >  
> > > > +static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
> > > > +{
> > > > +    VirtIONet *n = to_virtio_net(vdev);
> > > > +    if (!n->nic->nc.peer) {
> > > > +        return;
> > > > +    }
> > > > +    if (n->nic->nc.peer->info->type != NET_CLIENT_TYPE_TAP) {
> > > > +        return;
> > > > +    }
> > > > +
> > > > +    if (!tap_get_vhost_net(n->nic->nc.peer)) {
> > > > +        return;
> > > > +    }
> > > > +    if (!!n->vhost_started == (status & VIRTIO_CONFIG_S_DRIVER_OK) &&
> > > > +                                (n->status & VIRTIO_NET_S_LINK_UP) &&
> > > > +                                n->vm_running) {
> > > 
> > > I don't think this works...
> > > 
> > > #define VIRTIO_CONFIG_S_DRIVER_OK       4
> > > 
> > > !!(1) == (status & 4)?
> > 
> > The && converts it to a boolean.
> 
> Are you missing a set of parens around everything right of the ==?  ie:
> 
> if (!!n->vhost_started == ((status & VIRTIO_CONFIG_S_DRIVER_OK) &&
>                            (n->status & VIRTIO_NET_S_LINK_UP) &&
>                            n->vm_running)) {...

Yes, that's it. Thanks!

-- 
MST