[PATCH 0/6] xen: frontend devices should handle missed backend CLOSING

[PATCH 0/6] xen: frontend devices should handle missed backend CLOSING

[David Vrabel]

The series makes all the Xen frontend drivers handle the backend
transitioning to CLOSED without the frontend having previously seen
the backend in the CLOSING state.

Backends shouldn't do this but some do.  e.g., if the host is
XenServer and the toolstack decides to do a forced shutdown of a VBD,
then the blkfront may miss the CLOSING transition and the /dev/xvdX
device will not be destroyed which prevents it being reused.

I have seen systems that ended up in this state but it's not clear if
this was the actual cause.  However, I think in general it's a good
thing to thing to improve the handling of unexpected state
transitions.

Konrad, I've split this into a patch per frontend in case each patch
should go via a different maintainer.  But if you'd prefer, I can roll
this up into one patch to via your Xen tree.

David

[PATCH 1/6] xen-netfront: handle backend CLOSED without CLOSING

[David Vrabel]

From: David Vrabel <david.vrabel@citrix.com>

Backend drivers shouldn't transistion to CLOSED unless the frontend is
CLOSED.  If a backend does transition to CLOSED too soon then the
frontend may not see the CLOSING state and will not properly shutdown.

So, treat an unexpected backend CLOSED state the same as CLOSING.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: <netdev@vger.kernel.org>
---
 drivers/net/xen-netfront.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 3089990..843533a 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1719,7 +1719,6 @@ static void netback_changed(struct xenbus_device *dev,
 	case XenbusStateReconfiguring:
 	case XenbusStateReconfigured:
 	case XenbusStateUnknown:
-	case XenbusStateClosed:
 		break;
 
 	case XenbusStateInitWait:
@@ -1734,6 +1733,10 @@ static void netback_changed(struct xenbus_device *dev,
 		netif_notify_peers(netdev);
 		break;
 
+	case XenbusStateClosed:
+		if (dev->state == XenbusStateClosed)
+			break;
+		/* Missed the backend's CLOSING state -- fallthrough */
 	case XenbusStateClosing:
 		xenbus_frontend_closed(dev);
 		break;
-- 
1.7.2.5

[PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING

[David Vrabel]

From: David Vrabel <david.vrabel@citrix.com>

Backend drivers shouldn't transistion to CLOSED unless the frontend is
CLOSED.  If a backend does transition to CLOSED too soon then the
frontend may not see the CLOSING state and will not properly shutdown.

So, treat an unexpected backend CLOSED state the same as CLOSING.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 drivers/block/xen-blkfront.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 2c2d2e5..8da12a9 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1340,13 +1340,16 @@ static void blkback_changed(struct xenbus_device *dev,
 	case XenbusStateReconfiguring:
 	case XenbusStateReconfigured:
 	case XenbusStateUnknown:
-	case XenbusStateClosed:
 		break;
 
 	case XenbusStateConnected:
 		blkfront_connect(info);
 		break;
 
+	case XenbusStateClosed:
+		if (dev->state == XenbusStateClosed)
+			break;
+		/* Missed the backend's Closing state -- fallthrough */
 	case XenbusStateClosing:
 		blkfront_closing(info);
 		break;
-- 
1.7.2.5

[PATCH 3/6] xen-pcifront: handle backend CLOSED without CLOSING

[David Vrabel]

From: David Vrabel <david.vrabel@citrix.com>

Backend drivers shouldn't transistion to CLOSED unless the frontend is
CLOSED.  If a backend does transition to CLOSED too soon then the
frontend may not see the CLOSING state and will not properly shutdown.

So, treat an unexpected backend CLOSED state the same as CLOSING.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 drivers/pci/xen-pcifront.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index d6cc62c..b5692c3 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -1069,13 +1069,16 @@ static void __init_refok pcifront_backend_changed(struct xenbus_device *xdev,
 	case XenbusStateInitialising:
 	case XenbusStateInitWait:
 	case XenbusStateInitialised:
-	case XenbusStateClosed:
 		break;
 
 	case XenbusStateConnected:
 		pcifront_try_connect(pdev);
 		break;
 
+	case XenbusStateClosed:
+		if (xdev->state == XenbusStateClosed)
+			break;
+		/* Missed the backend's CLOSING state -- fallthrough */
 	case XenbusStateClosing:
 		dev_warn(&xdev->dev, "backend going away!\n");
 		pcifront_try_disconnect(pdev);
-- 
1.7.2.5

[PATCH 4/6] xen-fbfront: handle backend CLOSED without CLOSING

[David Vrabel]

From: David Vrabel <david.vrabel@citrix.com>

Backend drivers shouldn't transistion to CLOSED unless the frontend is
CLOSED.  If a backend does transition to CLOSED too soon then the
frontend may not see the CLOSING state and will not properly shutdown.

So, treat an unexpected backend CLOSED state the same as CLOSING.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Cc: <linux-fbdev@vger.kernel.org>
---
 drivers/video/xen-fbfront.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/video/xen-fbfront.c b/drivers/video/xen-fbfront.c
index b7f5173..917bb56 100644
--- a/drivers/video/xen-fbfront.c
+++ b/drivers/video/xen-fbfront.c
@@ -641,7 +641,6 @@ static void xenfb_backend_changed(struct xenbus_device *dev,
 	case XenbusStateReconfiguring:
 	case XenbusStateReconfigured:
 	case XenbusStateUnknown:
-	case XenbusStateClosed:
 		break;
 
 	case XenbusStateInitWait:
@@ -670,6 +669,10 @@ InitWait:
 		info->feature_resize = val;
 		break;
 
+	case XenbusStateClosed:
+		if (dev->state == XenbusStateClosed)
+			break;
+		/* Missed the backend's CLOSING state -- fallthrough */
 	case XenbusStateClosing:
 		xenbus_frontend_closed(dev);
 		break;
-- 
1.7.2.5

[PATCH 5/6] xen-kbdfront: handle backend CLOSED without CLOSING

[David Vrabel]

From: David Vrabel <david.vrabel@citrix.com>

Backend drivers shouldn't transistion to CLOSED unless the frontend is
CLOSED.  If a backend does transition to CLOSED too soon then the
frontend may not see the CLOSING state and will not properly shutdown.

So, treat an unexpected backend CLOSED state the same as CLOSING.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Cc: <linux-input@vger.kernel.org>
---
 drivers/input/misc/xen-kbdfront.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
index 02ca868..6f7d990 100644
--- a/drivers/input/misc/xen-kbdfront.c
+++ b/drivers/input/misc/xen-kbdfront.c
@@ -311,7 +311,6 @@ static void xenkbd_backend_changed(struct xenbus_device *dev,
 	case XenbusStateReconfiguring:
 	case XenbusStateReconfigured:
 	case XenbusStateUnknown:
-	case XenbusStateClosed:
 		break;
 
 	case XenbusStateInitWait:
@@ -350,6 +349,10 @@ InitWait:
 
 		break;
 
+	case XenbusStateClosed:
+		if (dev->state == XenbusStateClosed)
+			break;
+		/* Missed the backend's CLOSING state -- fallthrough */
 	case XenbusStateClosing:
 		xenbus_frontend_closed(dev);
 		break;
-- 
1.7.2.5

[PATCH 6/6] xen/hvc: handle backend CLOSED without CLOSING

[David Vrabel]

From: David Vrabel <david.vrabel@citrix.com>

Backend drivers shouldn't transistion to CLOSED unless the frontend is
CLOSED.  If a backend does transition to CLOSED too soon then the
frontend may not see the CLOSING state and will not properly shutdown.

So, treat an unexpected backend CLOSED state the same as CLOSING.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 drivers/tty/hvc/hvc_xen.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 1e456dc..82901fb 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -476,7 +476,6 @@ static void xencons_backend_changed(struct xenbus_device *dev,
 	case XenbusStateInitialising:
 	case XenbusStateInitialised:
 	case XenbusStateUnknown:
-	case XenbusStateClosed:
 		break;
 
 	case XenbusStateInitWait:
@@ -486,6 +485,10 @@ static void xencons_backend_changed(struct xenbus_device *dev,
 		xenbus_switch_state(dev, XenbusStateConnected);
 		break;
 
+	case XenbusStateClosed:
+		if (dev->state == XenbusStateClosed)
+			break;
+		/* Missed the backend's CLOSING state -- fallthrough */
 	case XenbusStateClosing:
 		xenbus_frontend_closed(dev);
 		break;
-- 
1.7.2.5

Re: [PATCH 0/6] xen: frontend devices should handle missed backend CLOSING

[Konrad Rzeszutek Wilk]

On Fri, Sep 21, 2012 at 05:04:18PM +0100, David Vrabel wrote:
> The series makes all the Xen frontend drivers handle the backend
> transitioning to CLOSED without the frontend having previously seen
> the backend in the CLOSING state.
> 
> Backends shouldn't do this but some do.  e.g., if the host is
> XenServer and the toolstack decides to do a forced shutdown of a VBD,
> then the blkfront may miss the CLOSING transition and the /dev/xvdX
> device will not be destroyed which prevents it being reused.
> 
> I have seen systems that ended up in this state but it's not clear if
> this was the actual cause.  However, I think in general it's a good
> thing to thing to improve the handling of unexpected state
> transitions.
> 
> Konrad, I've split this into a patch per frontend in case each patch
> should go via a different maintainer.  But if you'd prefer, I can roll
> this up into one patch to via your Xen tree.

I like the split-up. Can you repost them with the different maintainers
CC (scripts/get_maintainers.pl) and with Acked-by from me on them?

> 
> David

Re: [PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING

[David Vrabel]

On 21/09/12 17:04, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
> 
> Backend drivers shouldn't transistion to CLOSED unless the frontend is
> CLOSED.  If a backend does transition to CLOSED too soon then the
> frontend may not see the CLOSING state and will not properly shutdown.
> 
> So, treat an unexpected backend CLOSED state the same as CLOSING.

Didn't handle the frontend block device being mounted. Updated patch here.

8<---------------------------------
xen-blkfront: handle backend CLOSED without CLOSING

Backend drivers shouldn't transistion to CLOSED unless the frontend is
CLOSED.  If a backend does transition to CLOSED too soon then the
frontend may not see the CLOSING state and will not properly shutdown.

So, treat an unexpected backend CLOSED state the same as CLOSING.

If the backend is CLOSED then the frontend can't talk to it so go to
CLOSED immediately without waiting for the block device to be closed
or unmounted.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 drivers/block/xen-blkfront.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 2c2d2e5..c1f5f38 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1143,7 +1143,8 @@ static int blkfront_resume(struct xenbus_device *dev)
 }
 
 static void
-blkfront_closing(struct blkfront_info *info)
+blkfront_closing(struct blkfront_info *info,
+		 enum xenbus_state backend_state)
 {
 	struct xenbus_device *xbdev = info->xbdev;
 	struct block_device *bdev = NULL;
@@ -1167,7 +1168,8 @@ blkfront_closing(struct blkfront_info *info)
 
 	mutex_lock(&bdev->bd_mutex);
 
-	if (bdev->bd_openers) {
+	/* If the backend is already CLOSED, close now. */
+	if (bdev->bd_openers && backend_state != XenbusStateClosed) {
 		xenbus_dev_error(xbdev, -EBUSY,
 				 "Device in use; refusing to close");
 		xenbus_switch_state(xbdev, XenbusStateClosing);
@@ -1340,15 +1342,18 @@ static void blkback_changed(struct xenbus_device *dev,
 	case XenbusStateReconfiguring:
 	case XenbusStateReconfigured:
 	case XenbusStateUnknown:
-	case XenbusStateClosed:
 		break;
 
 	case XenbusStateConnected:
 		blkfront_connect(info);
 		break;
 
+	case XenbusStateClosed:
+		if (dev->state == XenbusStateClosed)
+			break;
+		/* Missed the backend's Closing state -- fallthrough */
 	case XenbusStateClosing:
-		blkfront_closing(info);
+		blkfront_closing(info, backend_state);
 		break;
 	}
 }
-- 
1.7.2.5

Re: [PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING

[David Vrabel]

On 25/09/12 18:53, David Vrabel wrote:
> On 21/09/12 17:04, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> Backend drivers shouldn't transistion to CLOSED unless the frontend is
>> CLOSED.  If a backend does transition to CLOSED too soon then the
>> frontend may not see the CLOSING state and will not properly shutdown.
>>
>> So, treat an unexpected backend CLOSED state the same as CLOSING.
> 
> Didn't handle the frontend block device being mounted. Updated patch here.

Konrad, can you ack this updated patch if you're happy with it.

Thanks.

David

> 8<---------------------------------
> xen-blkfront: handle backend CLOSED without CLOSING
> 
> Backend drivers shouldn't transistion to CLOSED unless the frontend is
> CLOSED.  If a backend does transition to CLOSED too soon then the
> frontend may not see the CLOSING state and will not properly shutdown.
> 
> So, treat an unexpected backend CLOSED state the same as CLOSING.
> 
> If the backend is CLOSED then the frontend can't talk to it so go to
> CLOSED immediately without waiting for the block device to be closed
> or unmounted.
> 
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
>  drivers/block/xen-blkfront.c |   13 +++++++++----
>  1 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 2c2d2e5..c1f5f38 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -1143,7 +1143,8 @@ static int blkfront_resume(struct xenbus_device *dev)
>  }
>  
>  static void
> -blkfront_closing(struct blkfront_info *info)
> +blkfront_closing(struct blkfront_info *info,
> +		 enum xenbus_state backend_state)
>  {
>  	struct xenbus_device *xbdev = info->xbdev;
>  	struct block_device *bdev = NULL;
> @@ -1167,7 +1168,8 @@ blkfront_closing(struct blkfront_info *info)
>  
>  	mutex_lock(&bdev->bd_mutex);
>  
> -	if (bdev->bd_openers) {
> +	/* If the backend is already CLOSED, close now. */
> +	if (bdev->bd_openers && backend_state != XenbusStateClosed) {
>  		xenbus_dev_error(xbdev, -EBUSY,
>  				 "Device in use; refusing to close");
>  		xenbus_switch_state(xbdev, XenbusStateClosing);
> @@ -1340,15 +1342,18 @@ static void blkback_changed(struct xenbus_device *dev,
>  	case XenbusStateReconfiguring:
>  	case XenbusStateReconfigured:
>  	case XenbusStateUnknown:
> -	case XenbusStateClosed:
>  		break;
>  
>  	case XenbusStateConnected:
>  		blkfront_connect(info);
>  		break;
>  
> +	case XenbusStateClosed:
> +		if (dev->state == XenbusStateClosed)
> +			break;
> +		/* Missed the backend's Closing state -- fallthrough */
>  	case XenbusStateClosing:
> -		blkfront_closing(info);
> +		blkfront_closing(info, backend_state);
>  		break;
>  	}
>  }

Re: [PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING

[Konrad Rzeszutek Wilk]

On Mon, Oct 01, 2012 at 06:19:19PM +0100, David Vrabel wrote:
> On 25/09/12 18:53, David Vrabel wrote:
> > On 21/09/12 17:04, David Vrabel wrote:
> >> From: David Vrabel <david.vrabel@citrix.com>
> >>
> >> Backend drivers shouldn't transistion to CLOSED unless the frontend is
> >> CLOSED.  If a backend does transition to CLOSED too soon then the
> >> frontend may not see the CLOSING state and will not properly shutdown.
> >>
> >> So, treat an unexpected backend CLOSED state the same as CLOSING.
> > 
> > Didn't handle the frontend block device being mounted. Updated patch here.
> 
> Konrad, can you ack this updated patch if you're happy with it.

Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Or should I just carry it in my for-jens-3.7 bug-fixes queue and ask
Jen to pull it once rc0 is out?
> 
> Thanks.
> 
> David
> 
> > 8<---------------------------------
> > xen-blkfront: handle backend CLOSED without CLOSING
> > 
> > Backend drivers shouldn't transistion to CLOSED unless the frontend is
> > CLOSED.  If a backend does transition to CLOSED too soon then the
> > frontend may not see the CLOSING state and will not properly shutdown.
> > 
> > So, treat an unexpected backend CLOSED state the same as CLOSING.
> > 
> > If the backend is CLOSED then the frontend can't talk to it so go to
> > CLOSED immediately without waiting for the block device to be closed
> > or unmounted.
> > 
> > Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> > ---
> >  drivers/block/xen-blkfront.c |   13 +++++++++----
> >  1 files changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> > index 2c2d2e5..c1f5f38 100644
> > --- a/drivers/block/xen-blkfront.c
> > +++ b/drivers/block/xen-blkfront.c
> > @@ -1143,7 +1143,8 @@ static int blkfront_resume(struct xenbus_device *dev)
> >  }
> >  
> >  static void
> > -blkfront_closing(struct blkfront_info *info)
> > +blkfront_closing(struct blkfront_info *info,
> > +		 enum xenbus_state backend_state)
> >  {
> >  	struct xenbus_device *xbdev = info->xbdev;
> >  	struct block_device *bdev = NULL;
> > @@ -1167,7 +1168,8 @@ blkfront_closing(struct blkfront_info *info)
> >  
> >  	mutex_lock(&bdev->bd_mutex);
> >  
> > -	if (bdev->bd_openers) {
> > +	/* If the backend is already CLOSED, close now. */
> > +	if (bdev->bd_openers && backend_state != XenbusStateClosed) {
> >  		xenbus_dev_error(xbdev, -EBUSY,
> >  				 "Device in use; refusing to close");
> >  		xenbus_switch_state(xbdev, XenbusStateClosing);
> > @@ -1340,15 +1342,18 @@ static void blkback_changed(struct xenbus_device *dev,
> >  	case XenbusStateReconfiguring:
> >  	case XenbusStateReconfigured:
> >  	case XenbusStateUnknown:
> > -	case XenbusStateClosed:
> >  		break;
> >  
> >  	case XenbusStateConnected:
> >  		blkfront_connect(info);
> >  		break;
> >  
> > +	case XenbusStateClosed:
> > +		if (dev->state == XenbusStateClosed)
> > +			break;
> > +		/* Missed the backend's Closing state -- fallthrough */
> >  	case XenbusStateClosing:
> > -		blkfront_closing(info);
> > +		blkfront_closing(info, backend_state);
> >  		break;
> >  	}
> >  }

Re: [Xen-devel] [PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING

[Jan Beulich]

>>> On 25.09.12 at 19:53, David Vrabel <david.vrabel@citrix.com> wrote:
> @@ -1167,7 +1168,8 @@ blkfront_closing(struct blkfront_info *info)
>  
>  	mutex_lock(&bdev->bd_mutex);
>  
> -	if (bdev->bd_openers) {
> +	/* If the backend is already CLOSED, close now. */
> +	if (bdev->bd_openers && backend_state != XenbusStateClosed) {
>  		xenbus_dev_error(xbdev, -EBUSY,
>  				 "Device in use; refusing to close");
>  		xenbus_switch_state(xbdev, XenbusStateClosing);

This looks wrong to me on a second glance: As long as there
are users of the device, I don't think we want to go into Closed
ourselves, irrespective of the backend state.

Jan

Re: [Xen-devel] [PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING

[David Vrabel]

On 04/10/12 11:14, Jan Beulich wrote:
>>>> On 25.09.12 at 19:53, David Vrabel <david.vrabel@citrix.com> wrote:
>> @@ -1167,7 +1168,8 @@ blkfront_closing(struct blkfront_info *info)
>>  
>>  	mutex_lock(&bdev->bd_mutex);
>>  
>> -	if (bdev->bd_openers) {
>> +	/* If the backend is already CLOSED, close now. */
>> +	if (bdev->bd_openers && backend_state != XenbusStateClosed) {
>>  		xenbus_dev_error(xbdev, -EBUSY,
>>  				 "Device in use; refusing to close");
>>  		xenbus_switch_state(xbdev, XenbusStateClosing);
> 
> This looks wrong to me on a second glance: As long as there
> are users of the device, I don't think we want to go into Closed
> ourselves, irrespective of the backend state.

Any users of the frontend device are screwed either way, as the backend
is gone.  It seems sensible to handle this case the same as (e.g.,) a
physical unplug of a USB storage device.  Removing the device and
forcing all outstanding I/O to fail immediately rather than lingering in
the rings, going nowhere.

David

Re: [PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING

[David Vrabel]

On 02/10/12 21:02, Konrad Rzeszutek Wilk wrote:
> On Mon, Oct 01, 2012 at 06:19:19PM +0100, David Vrabel wrote:
>> On 25/09/12 18:53, David Vrabel wrote:
>>> On 21/09/12 17:04, David Vrabel wrote:
>>>> From: David Vrabel <david.vrabel@citrix.com>
>>>>
>>>> Backend drivers shouldn't transistion to CLOSED unless the frontend is
>>>> CLOSED.  If a backend does transition to CLOSED too soon then the
>>>> frontend may not see the CLOSING state and will not properly shutdown.
>>>>
>>>> So, treat an unexpected backend CLOSED state the same as CLOSING.
>>>
>>> Didn't handle the frontend block device being mounted. Updated patch here.
>>
>> Konrad, can you ack this updated patch if you're happy with it.
> 
> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> Or should I just carry it in my for-jens-3.7 bug-fixes queue and ask
> Jen to pull it once rc0 is out?

This seems easiest, if Jan is happy with the patch.

Thanks.

David

Re: [PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING

[Jan Beulich]

>>> On 05.10.12 at 13:42, David Vrabel <david.vrabel@citrix.com> wrote:
> On 02/10/12 21:02, Konrad Rzeszutek Wilk wrote:
>> On Mon, Oct 01, 2012 at 06:19:19PM +0100, David Vrabel wrote:
>>> On 25/09/12 18:53, David Vrabel wrote:
>>>> On 21/09/12 17:04, David Vrabel wrote:
>>>>> From: David Vrabel <david.vrabel@citrix.com>
>>>>>
>>>>> Backend drivers shouldn't transistion to CLOSED unless the frontend is
>>>>> CLOSED.  If a backend does transition to CLOSED too soon then the
>>>>> frontend may not see the CLOSING state and will not properly shutdown.
>>>>>
>>>>> So, treat an unexpected backend CLOSED state the same as CLOSING.
>>>>
>>>> Didn't handle the frontend block device being mounted. Updated patch here.
>>>
>>> Konrad, can you ack this updated patch if you're happy with it.
>> 
>> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> 
>> Or should I just carry it in my for-jens-3.7 bug-fixes queue and ask
>> Jen to pull it once rc0 is out?
> 
> This seems easiest, if Jan is happy with the patch.

I see the point of your explanation yesterday, but am still not
convinced that the early cleanup in spite of active users of the
disk can't cause any problems (like dangling pointers or NULL
dereferences).

Jan

Re: [PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING

[David Vrabel]

On 05/10/12 12:55, Jan Beulich wrote:
>>>> On 05.10.12 at 13:42, David Vrabel <david.vrabel@citrix.com> wrote:
>> On 02/10/12 21:02, Konrad Rzeszutek Wilk wrote:
>>> On Mon, Oct 01, 2012 at 06:19:19PM +0100, David Vrabel wrote:
>>>> On 25/09/12 18:53, David Vrabel wrote:
>>>>> On 21/09/12 17:04, David Vrabel wrote:
>>>>>> From: David Vrabel <david.vrabel@citrix.com>
>>>>>>
>>>>>> Backend drivers shouldn't transistion to CLOSED unless the frontend is
>>>>>> CLOSED.  If a backend does transition to CLOSED too soon then the
>>>>>> frontend may not see the CLOSING state and will not properly shutdown.
>>>>>>
>>>>>> So, treat an unexpected backend CLOSED state the same as CLOSING.
>>>>>
>>>>> Didn't handle the frontend block device being mounted. Updated patch here.
>>>>
>>>> Konrad, can you ack this updated patch if you're happy with it.
>>>
>>> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>>
>>> Or should I just carry it in my for-jens-3.7 bug-fixes queue and ask
>>> Jen to pull it once rc0 is out?
>>
>> This seems easiest, if Jan is happy with the patch.
> 
> I see the point of your explanation yesterday, but am still not
> convinced that the early cleanup in spite of active users of the
> disk can't cause any problems (like dangling pointers or NULL
> dereferences).

I tested it some more and it is broken.  Please apply the first version
instead.

blkfront needs to cancel and complete with an error requests that are
are still on the ring after the backend has disconnected and it needs to
complete with an error all requests submitted while disconnected.
Otherwise, if there is outstanding requests or new requests then
del_gendisk() blocks waiting for the I/O to complete.

David

Re: [PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING

[Konrad Rzeszutek Wilk]

On Fri, Oct 05, 2012 at 04:57:36PM +0100, David Vrabel wrote:
> On 05/10/12 12:55, Jan Beulich wrote:
> >>>> On 05.10.12 at 13:42, David Vrabel <david.vrabel@citrix.com> wrote:
> >> On 02/10/12 21:02, Konrad Rzeszutek Wilk wrote:
> >>> On Mon, Oct 01, 2012 at 06:19:19PM +0100, David Vrabel wrote:
> >>>> On 25/09/12 18:53, David Vrabel wrote:
> >>>>> On 21/09/12 17:04, David Vrabel wrote:
> >>>>>> From: David Vrabel <david.vrabel@citrix.com>
> >>>>>>
> >>>>>> Backend drivers shouldn't transistion to CLOSED unless the frontend is
> >>>>>> CLOSED.  If a backend does transition to CLOSED too soon then the
> >>>>>> frontend may not see the CLOSING state and will not properly shutdown.
> >>>>>>
> >>>>>> So, treat an unexpected backend CLOSED state the same as CLOSING.
> >>>>>
> >>>>> Didn't handle the frontend block device being mounted. Updated patch here.
> >>>>
> >>>> Konrad, can you ack this updated patch if you're happy with it.
> >>>
> >>> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >>>
> >>> Or should I just carry it in my for-jens-3.7 bug-fixes queue and ask
> >>> Jen to pull it once rc0 is out?
> >>
> >> This seems easiest, if Jan is happy with the patch.
> > 
> > I see the point of your explanation yesterday, but am still not
> > convinced that the early cleanup in spite of active users of the
> > disk can't cause any problems (like dangling pointers or NULL
> > dereferences).
> 
> I tested it some more and it is broken.  Please apply the first version
> instead.

OK, can we do this once more - can you repost the patches, but CC the
invidiual maintainers? The FB and KBD need to go through Florian I think?

> 
> blkfront needs to cancel and complete with an error requests that are
> are still on the ring after the backend has disconnected and it needs to
> complete with an error all requests submitted while disconnected.
> Otherwise, if there is outstanding requests or new requests then
> del_gendisk() blocks waiting for the I/O to complete.
> 
> David