Re: [Qemu-devel] [PATCH for 2.9?] tap-win32: don't abort in tap_enable(); enables -netdev tap

[Jason Wang]

On 2017年03月29日 02:55, Andrew Baumann wrote:
> > From: Stefan Weil [mailto:address@hidden
> > Sent: Tuesday, 28 March 2017 11:28
> > Am 25.03.2017 um 00:46 schrieb Andrew Baumann:
> > > The docs generally steer users away from using the legacy -net
> > > parameter, however on win32 attempting to enable a tap device using
> > > -netdev tap fails at an abort() in tap_enable(). Removing the abort()s
> > > seems to be enough to get everything working, so do that.
> > >
> > > Signed-off-by: Andrew Baumann <address@hidden>
> > > ---
> > >   net/tap-win32.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/net/tap-win32.c b/net/tap-win32.c
> > > index 662f9b6..3620843 100644
> > > --- a/net/tap-win32.c
> > > +++ b/net/tap-win32.c
> > > @@ -811,10 +811,10 @@ int net_init_tap(const Netdev *netdev, const char
> > *name,
> > >   int tap_enable(NetClientState *nc)
> > >   {
> > > -    abort();
> > > +    return 0;
> > >   }
> > >
> > >   int tap_disable(NetClientState *nc)
> > >   {
> > > -    abort();
> > > +    return 0;
> > >   }
> > As I never worked with TAP on Windows, I cannot say much to this fix.
> >
> > Jason, what is the use of tap_enable, tap_disable?

It should be only used when we want to enable and disable a specific 
queue of a multiqueue supported tap.

> >   Is it fine
> > to simply do nothing on Windows here?

Unless windows support multiqueue tap, we should keep the assert here.

> I was also hoping for a review -- I'm no expert on this stuff either, but my 
> quick reading of those code paths is that they issue ioctls to enable/disable 
> packet reception on the underlying tap device. As win32 TAP is implemented, 
> that is already enabled from start of day.
>
> It's possible this patch still does not permit dynamic reconfiguration of tap 
> devices (e.g. from the monitor console). However, it does work with the -netdev 
> tap option on the command-line.
>
> > And is this something for QEMU‌ 2.9 (I added question to subject line)?
> Ideally, yes. If not, -netdev tap will continue to blow up in the abort as it 
> does today...
>
> Andrew

Yes, so the problem is we should prevent tap_enable() and tap_disable() 
from being called if multiqueue is disabled.

I believe the following patch can fix this issue, could you give a try 
on this?

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index c321680..7d091c9 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -510,6 +510,10 @@ static int peer_attach(VirtIONet *n, int index)
         return 0;
     }

+    if (n->max_queues == 1) {
+        return 0;
+    }
+
     return tap_enable(nc->peer);
 }