[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for 2.9?] tap-win32: don't abort in tap_enable()
From: |
Andrew Baumann |
Subject: |
Re: [Qemu-devel] [PATCH for 2.9?] tap-win32: don't abort in tap_enable(); enables -netdev tap |
Date: |
Wed, 29 Mar 2017 04:13:53 +0000 |
> 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