* [PATCHv1 net] xen-netback: transition to CLOSED when removing a VIF
@ 2013-10-07 12:55 David Vrabel
2013-10-07 13:43 ` Wei Liu
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: David Vrabel @ 2013-10-07 12:55 UTC (permalink / raw)
To: xen-devel
Cc: David Vrabel, Konrad Rzeszutek Wilk, Boris Ostrovsky, netdev,
Ian Campbell, Wei Liu, Paul Durrant
From: David Vrabel <david.vrabel@citrix.com>
If a guest is destroyed without transitioning its frontend to CLOSED,
the domain becomes a zombie as netback was not grant unmapping the
shared rings.
When removing a VIF, transition the backend to CLOSED so the VIF is
disconnected if necessary (which will unmap the shared rings etc).
This fixes a regression introduced by
279f438e36c0a70b23b86d2090aeec50155034a9 (xen-netback: Don't destroy
the netdev until the vif is shut down).
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Paul Durrant <Paul.Durrant@citrix.com>
---
drivers/net/xen-netback/xenbus.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index b45bce2..1b08d87 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -39,11 +39,15 @@ static int connect_rings(struct backend_info *);
static void connect(struct backend_info *);
static void backend_create_xenvif(struct backend_info *be);
static void unregister_hotplug_status_watch(struct backend_info *be);
+static void set_backend_state(struct backend_info *be,
+ enum xenbus_state state);
static int netback_remove(struct xenbus_device *dev)
{
struct backend_info *be = dev_get_drvdata(&dev->dev);
+ set_backend_state(be, XenbusStateClosed);
+
unregister_hotplug_status_watch(be);
if (be->vif) {
kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
--
1.7.2.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCHv1 net] xen-netback: transition to CLOSED when removing a VIF
2013-10-07 12:55 [PATCHv1 net] xen-netback: transition to CLOSED when removing a VIF David Vrabel
@ 2013-10-07 13:43 ` Wei Liu
2013-10-07 13:57 ` David Vrabel
2013-10-07 14:28 ` Paul Durrant
2013-10-09 1:34 ` David Miller
2 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2013-10-07 13:43 UTC (permalink / raw)
To: David Vrabel
Cc: xen-devel, Konrad Rzeszutek Wilk, Boris Ostrovsky, netdev,
Ian Campbell, Wei Liu, Paul Durrant
On Mon, Oct 07, 2013 at 01:55:19PM +0100, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> If a guest is destroyed without transitioning its frontend to CLOSED,
> the domain becomes a zombie as netback was not grant unmapping the
> shared rings.
>
> When removing a VIF, transition the backend to CLOSED so the VIF is
> disconnected if necessary (which will unmap the shared rings etc).
>
> This fixes a regression introduced by
> 279f438e36c0a70b23b86d2090aeec50155034a9 (xen-netback: Don't destroy
> the netdev until the vif is shut down).
>
Is this regression solely caused by 279f438e36c or caused by both
ea732dff5c and 279f438e36c? I ask because you make use of the new state
machine introduced in ea732dff5c. Or are you simply using the new state
machine to fix the regression instead of going back to old code?
Wei.
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Paul Durrant <Paul.Durrant@citrix.com>
> ---
> drivers/net/xen-netback/xenbus.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
> index b45bce2..1b08d87 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -39,11 +39,15 @@ static int connect_rings(struct backend_info *);
> static void connect(struct backend_info *);
> static void backend_create_xenvif(struct backend_info *be);
> static void unregister_hotplug_status_watch(struct backend_info *be);
> +static void set_backend_state(struct backend_info *be,
> + enum xenbus_state state);
>
> static int netback_remove(struct xenbus_device *dev)
> {
> struct backend_info *be = dev_get_drvdata(&dev->dev);
>
> + set_backend_state(be, XenbusStateClosed);
> +
> unregister_hotplug_status_watch(be);
> if (be->vif) {
> kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
> --
> 1.7.2.5
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv1 net] xen-netback: transition to CLOSED when removing a VIF
2013-10-07 13:43 ` Wei Liu
@ 2013-10-07 13:57 ` David Vrabel
2013-10-07 14:15 ` Wei Liu
0 siblings, 1 reply; 6+ messages in thread
From: David Vrabel @ 2013-10-07 13:57 UTC (permalink / raw)
To: Wei Liu
Cc: xen-devel, Konrad Rzeszutek Wilk, Boris Ostrovsky, netdev,
Ian Campbell, Paul Durrant
On 07/10/13 14:43, Wei Liu wrote:
> On Mon, Oct 07, 2013 at 01:55:19PM +0100, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> If a guest is destroyed without transitioning its frontend to CLOSED,
>> the domain becomes a zombie as netback was not grant unmapping the
>> shared rings.
>>
>> When removing a VIF, transition the backend to CLOSED so the VIF is
>> disconnected if necessary (which will unmap the shared rings etc).
>>
>> This fixes a regression introduced by
>> 279f438e36c0a70b23b86d2090aeec50155034a9 (xen-netback: Don't destroy
>> the netdev until the vif is shut down).
>>
>
> Is this regression solely caused by 279f438e36c or caused by both
> ea732dff5c and 279f438e36c? I ask because you make use of the new state
> machine introduced in ea732dff5c. Or are you simply using the new state
> machine to fix the regression instead of going back to old code?
I bisected it to 279f438. I'm just using the handy new state machine to
fix it.
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv1 net] xen-netback: transition to CLOSED when removing a VIF
2013-10-07 13:57 ` David Vrabel
@ 2013-10-07 14:15 ` Wei Liu
0 siblings, 0 replies; 6+ messages in thread
From: Wei Liu @ 2013-10-07 14:15 UTC (permalink / raw)
To: David Vrabel
Cc: Wei Liu, xen-devel, Konrad Rzeszutek Wilk, Boris Ostrovsky,
netdev, Ian Campbell, Paul Durrant
On Mon, Oct 07, 2013 at 02:57:37PM +0100, David Vrabel wrote:
> On 07/10/13 14:43, Wei Liu wrote:
> > On Mon, Oct 07, 2013 at 01:55:19PM +0100, David Vrabel wrote:
> >> From: David Vrabel <david.vrabel@citrix.com>
> >>
> >> If a guest is destroyed without transitioning its frontend to CLOSED,
> >> the domain becomes a zombie as netback was not grant unmapping the
> >> shared rings.
> >>
> >> When removing a VIF, transition the backend to CLOSED so the VIF is
> >> disconnected if necessary (which will unmap the shared rings etc).
> >>
> >> This fixes a regression introduced by
> >> 279f438e36c0a70b23b86d2090aeec50155034a9 (xen-netback: Don't destroy
> >> the netdev until the vif is shut down).
> >>
> >
> > Is this regression solely caused by 279f438e36c or caused by both
> > ea732dff5c and 279f438e36c? I ask because you make use of the new state
> > machine introduced in ea732dff5c. Or are you simply using the new state
> > machine to fix the regression instead of going back to old code?
>
> I bisected it to 279f438. I'm just using the handy new state machine to
> fix it.
>
Thanks for the explanation.
Acked-by: Wei Liu <wei.liu2@citrix.com>
Wei.
> David
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCHv1 net] xen-netback: transition to CLOSED when removing a VIF
2013-10-07 12:55 [PATCHv1 net] xen-netback: transition to CLOSED when removing a VIF David Vrabel
2013-10-07 13:43 ` Wei Liu
@ 2013-10-07 14:28 ` Paul Durrant
2013-10-09 1:34 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: Paul Durrant @ 2013-10-07 14:28 UTC (permalink / raw)
To: David Vrabel, xen-devel
Cc: David Vrabel, Konrad Rzeszutek Wilk, Boris Ostrovsky, netdev,
Ian Campbell, Wei Liu
> -----Original Message-----
> From: David Vrabel [mailto:david.vrabel@citrix.com]
> Sent: 07 October 2013 13:55
> To: xen-devel@lists.xen.org
> Cc: David Vrabel; Konrad Rzeszutek Wilk; Boris Ostrovsky;
> netdev@vger.kernel.org; Ian Campbell; Wei Liu; Paul Durrant
> Subject: [PATCHv1 net] xen-netback: transition to CLOSED when removing a
> VIF
>
> From: David Vrabel <david.vrabel@citrix.com>
>
> If a guest is destroyed without transitioning its frontend to CLOSED,
> the domain becomes a zombie as netback was not grant unmapping the
> shared rings.
>
> When removing a VIF, transition the backend to CLOSED so the VIF is
> disconnected if necessary (which will unmap the shared rings etc).
>
> This fixes a regression introduced by
> 279f438e36c0a70b23b86d2090aeec50155034a9 (xen-netback: Don't destroy
> the netdev until the vif is shut down).
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Paul Durrant <Paul.Durrant@citrix.com>
> ---
> drivers/net/xen-netback/xenbus.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-
> netback/xenbus.c
> index b45bce2..1b08d87 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -39,11 +39,15 @@ static int connect_rings(struct backend_info *);
> static void connect(struct backend_info *);
> static void backend_create_xenvif(struct backend_info *be);
> static void unregister_hotplug_status_watch(struct backend_info *be);
> +static void set_backend_state(struct backend_info *be,
> + enum xenbus_state state);
>
> static int netback_remove(struct xenbus_device *dev)
> {
> struct backend_info *be = dev_get_drvdata(&dev->dev);
>
> + set_backend_state(be, XenbusStateClosed);
> +
> unregister_hotplug_status_watch(be);
> if (be->vif) {
> kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
> --
> 1.7.2.5
Looks good to me.
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
Paul
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv1 net] xen-netback: transition to CLOSED when removing a VIF
2013-10-07 12:55 [PATCHv1 net] xen-netback: transition to CLOSED when removing a VIF David Vrabel
2013-10-07 13:43 ` Wei Liu
2013-10-07 14:28 ` Paul Durrant
@ 2013-10-09 1:34 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-10-09 1:34 UTC (permalink / raw)
To: david.vrabel
Cc: xen-devel, konrad.wilk, boris.ostrovsky, netdev, ian.campbell,
wei.liu2, Paul.Durrant
From: David Vrabel <david.vrabel@citrix.com>
Date: Mon, 7 Oct 2013 13:55:19 +0100
> From: David Vrabel <david.vrabel@citrix.com>
>
> If a guest is destroyed without transitioning its frontend to CLOSED,
> the domain becomes a zombie as netback was not grant unmapping the
> shared rings.
>
> When removing a VIF, transition the backend to CLOSED so the VIF is
> disconnected if necessary (which will unmap the shared rings etc).
>
> This fixes a regression introduced by
> 279f438e36c0a70b23b86d2090aeec50155034a9 (xen-netback: Don't destroy
> the netdev until the vif is shut down).
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-09 1:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-07 12:55 [PATCHv1 net] xen-netback: transition to CLOSED when removing a VIF David Vrabel
2013-10-07 13:43 ` Wei Liu
2013-10-07 13:57 ` David Vrabel
2013-10-07 14:15 ` Wei Liu
2013-10-07 14:28 ` Paul Durrant
2013-10-09 1:34 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).