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

[Andrew Baumann]

> From: Jason Wang [mailto:address@hidden
> Sent: Tuesday, 28 March 2017 19:39
> 
> 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>
[...]
> >> 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);
>   }
> 

Yep, this works. Thanks!

Andrew