[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH net-next 1/2] xen-netback: add a vif-is-connected flag
> -----Original Message----- > From: David Vrabel > Sent: 20 September 2013 15:29 > To: Wei Liu > Cc: Paul Durrant; netdev@xxxxxxxxxxxxxxx; xen-devel@xxxxxxxxxxxxx; Ian > Campbell > Subject: Re: [PATCH net-next 1/2] xen-netback: add a vif-is-connected flag > > On 20/09/13 14:31, Wei Liu wrote: > > On Fri, Sep 20, 2013 at 01:56:30PM +0100, Paul Durrant wrote: > >> Having applied my patch to separate vif disconnect and free, I ran into a > >> BUG when testing resume from S3 with a Windows frontend because the > vif task > >> pointer was not cleared by xenvif_disconnect() and so a double call to this > >> function tries to stop the thread twice. > >> Rather than applying a point fix for that issue it seems better to > >> introduce > >> a boolean to indicate whether the vif is connected or not such that > repeated > >> calls to either xenvif_connect() or xenvif_disconnect() behave > appropriately. > > We already have a backend state of CONNECTED/CLOSED/etc. why do we > need > an additional bit of state outside of this? > It's not really additional state; we were essentially inferring connected-ness from the value of tx_irq. This patch really just removes that inference and created something with the intended meaning. > Does something like this in frontend_changed() fix it? > It may well do, but it's a far more invasive change and would require more testing. It certainly sounds like a good thing to do in the longer term. Paul > case XenbusStateClosing: > switch (dev->state) { > case XenbusStateClosed; > break; > case XenbusStateConnected: > disconnect_backend(dev); > /* fall through */ > default: > xenbus_switch_state(dev, XenbusStateClosing); > break; > } > break; > > case XenbusStateClosed: > switch (dev->state) { > case XenbusStateConnected; > disconnect_backend(dev); > /* fall through */ > default: > xenbus_switch_state(dev, XenbusStateClosed); > break; > } > if (xenbus_dev_is_online(dev)) > break; > /* fall through if not online */ > > Can you also remove destroy_backend()? It's not needed any more. > > I'd also recommend waiting a bit for other review feedback before > posting an updated series. > > David _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |