Re: [PATCH resubmit] xen-blkfront: Don't send closing notification to backend in blkfront_closing()

[Ian Campbell]

On Thu, 2011-07-14 at 09:55 +0100, Joe Jin wrote:
> On 07/14/11 16:13, Ian Campbell wrote:
> > On Wed, 2011-07-13 at 01:47 +0100, Joe Jin wrote:

> > 
> > I'm wondering if we might not be better off deferring the disconnect on
> > the backend side until the frontend enters XenbusStateClosed instead of
> > doing it in closing.
> 
> Yes this fix from backend side works too, also this looks reasonable than 
> fix in frontend.

I guess there is either a missing "more" or "less" in that sentence ;-)

The nice thing about a backend fix is that you don't need to go round
propagating the fix into distros etc and updating existing deployed
guests (not to mention non-Linux frontends).

I actually thought blkback had something like this already in older
"classic" Xen kernels but it seems like the patch never made it out of
the XCP patch queue :-(. See
http://xenbits.xen.org/hg/XCP/linux-2.6.32.pq.hg/file/tip/CA-7672-blkback-shutdown.patch
(which appears to also incorporate a toolstack directed graceful
shutdown of some sort as well as deferring the device close).

Daniel, do you know of any plans to extricate that stuff from the XCP
patch queue for upstream? Are there any other patches (e.g. incremental
fixes) in that pq which should go along with it?

Ian.

> 
> Konrad, any advice?
> 
> Thanks,
> Joe
> 
> > 
> > Ian
> > 
> >>
> >> Signed-off-by: Joe Jin <joe.jin@xxxxxxxxxx>
> >> Signed-off-by: Annie Li <annie.li@xxxxxxxxxx>
> >> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> >> Cc: Jens Axboe <jaxboe@xxxxxxxxxxxx>
> >> Cc: stable@xxxxxxxxxx
> >>
> >> ---
> >>  xen-blkfront.c |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> >> index b536a9c..f6d8ac2 100644
> >> --- a/drivers/block/xen-blkfront.c
> >> +++ b/drivers/block/xen-blkfront.c
> >> @@ -1088,7 +1088,7 @@ blkfront_closing(struct blkfront_info *info)
> >>  	if (bdev->bd_openers) {
> >>  		xenbus_dev_error(xbdev, -EBUSY,
> >>  				 "Device in use; refusing to close");
> >> -		xenbus_switch_state(xbdev, XenbusStateClosing);
> >> +		xbdev->state = XenbusStateClosing;
> >>  	} else {
> >>  		xlvbd_release_gendisk(info);
> >>  		xenbus_frontend_closed(xbdev);
> > 
> > 
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/