Skip to contentSkip to conversation

xen-blkback: refactor vbd remove/disconnect.

PATCH In review Joe Jin 2 participants 6 messages Aug 3, 2011 – Aug 4, 2011

Subsystems (2)
lore
Share & tools
lore

Joe Jin

author
#0 lore raw

cover xen-blkback: refactor vbd remove/disconnect.

This patchset is a backport and original patch author is Daniel Stodden:
http://xenbits.xen.org/hg/XCP/linux-2.6.32.pq.hg/file/tip/CA-7672-blkback-shutdown.patch

Initial issue:
When we do block device attach/detach test with below steps, umount hang in guest and the guest unable to shutdown:

1. start guest with the latest kernel.
2. attach new block device by xm block-attach in Dom0
3. mount new disk in guest
4. execute xm block-detach to detach the block device in dom0 until timeout
5. try to unmount the disk in guest, umount hung. at here, any IOs to the device will hang.

Root cause:
This caused by 'xm block-detach' in Dom0 set backend device's state to 'XenbusStateClosing', frontend received the notification and blkfront_closing() be called, at the moment, the disk still using by guest, so frontend refused to close. In the blkfront_closing(), frontend send a notification to backend said that the its state switched to 'Closing', when backend got the event, it will disconnect from real device, at here any IO request will be stuck, even tried to release the disk by umount.

So this may fix either frontend or backend, I have send a fix for frontend:
https://lkml.org/lkml/2011/7/8/159
Ian think we should fix it from backend and he pointed out Daniel Stodden have submitted a patch(see above link) for xen-blkback, I tried it and it works well.

Changes:
v2:
- Reformat code style.
- Per Knoard suggestions, change some int defines to bool.

 drivers/block/xen-blkback/blkback.c |   10 +--
 drivers/block/xen-blkback/common.h  |    5 +
 drivers/block/xen-blkback/xenbus.c  |  203 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 192 insertions(+), 26 deletions(-)
author
#1 lore raw

PATCH 1/3 xen-blkback: add remove_requested to xen_blkif and some declares

Add remove_requested to xen_blkif and some declares.

Cc Daniel Stodden <[email protected]>
Cc Jens Axboe <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Annie Li <[email protected]>
Cc Ian Campbell <[email protected]>
---
 drivers/block/xen-blkback/common.h |    5 +++++
 1 file changed, 5 insertions(+)
drivers/block/xen-blkback/common.h +5 -0
index 9e40b28..acda757 100644
@@ -49,6 +49,7 @@
 	pr_debug(DRV_PFX "(%s:%d) " fmt ".\n",	\
 		 __func__, __LINE__, ##args)
 
+#define WPRINTK(fmt, args...) printk(KERN_WARNING "xen-blkback: " fmt, ##args)
 
 /* Not a real protocol.  Used to generate ring structs which contain
  * the elements common to all protocols only.  This way we get a
@@ -145,6 +146,7 @@ struct xen_blkif {
 	/* Back pointer to the backend_info. */
 	struct backend_info	*be;
 	/* Private fields. */
+	bool			remove_requested;
 	spinlock_t		blk_ring_lock;
 	atomic_t		refcnt;
 
@@ -198,6 +200,9 @@ int xen_blkbk_flush_diskcache(struct xenbus_transaction xbt,
 
 struct xenbus_device *xen_blkbk_xenbus(struct backend_info *be);
 
+void xen_vbd_sync(struct xen_vbd *vbd);
+void xen_blkback_close(struct xen_blkif *blkif);
+
 static inline void blkif_get_x86_32_req(struct blkif_request *dst,
 					struct blkif_x86_32_request *src)
 {
author
#2 lore raw

PATCH 2/3 xen-blkback: repleace check kthread_should_stop() to remove_requested in xen_blkif_schedule() loop.

In reply to Joe Jin

When backend state change to XenbusStateClosed, remove_requested will be set, so repleace check kthread_should_stop() to remove_requested in xen_blkif_schedule() loop.

Cc Daniel Stodden <[email protected]>
Cc Jens Axboe <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Annie Li <[email protected]>
Cc Ian Campbell <[email protected]>

--

 drivers/block/xen-blkback/blkback.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/block/xen-blkback/blkback.c +5 -5
index 2330a9a..3d64b52 100644
@@ -274,7 +274,7 @@ int xen_blkif_schedule(void *arg)
 
 	xen_blkif_get(blkif);
 
-	while (!kthread_should_stop()) {
+	while (!blkif->remove_requested) {
 		if (try_to_freeze())
 			continue;
 		if (unlikely(vbd->size != vbd_sz(vbd)))
@@ -282,11 +282,11 @@ int xen_blkif_schedule(void *arg)
 
 		wait_event_interruptible(
 			blkif->wq,
-			blkif->waiting_reqs || kthread_should_stop());
+			blkif->waiting_reqs || blkif->remove_requested);
 		wait_event_interruptible(
 			blkbk->pending_free_wq,
 			!list_empty(&blkbk->pending_free) ||
-			kthread_should_stop());
+			blkif->remove_requested);
 
 		blkif->waiting_reqs = 0;
 		smp_mb(); /* clear flag *before* checking for work */
@@ -301,8 +301,8 @@ int xen_blkif_schedule(void *arg)
 	if (log_stats)
 		print_stats(blkif);
 
-	blkif->xenblkd = NULL;
 	xen_blkif_put(blkif);
+	xen_blkback_close(blkif);
 
 	return 0;
 }
@@ -476,7 +476,7 @@ __do_block_io_op(struct xen_blkif *blkif)
 		if (RING_REQUEST_CONS_OVERFLOW(&blk_rings->common, rc))
 			break;
 
-		if (kthread_should_stop()) {
+		if (blkif->remove_requested) {
 			more_to_do = 1;
 			break;
 		}
author
#3 lore raw

PATCH 3/3 xen-blkback: refactor vbd remove/disconnect.

In reply to Joe Jin

This patch refactor vbd remove/disconnect.
1. Add blkback shutdown watch for the remove/disconnect.
2. Don't disconnect backend when frontend state is XenbusStateClosing until frontend state changed to XenbusStateClosed.

Cc Daniel Stodden <[email protected]>
Cc Jens Axboe <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Annie Li <[email protected]>
Cc Ian Campbell <[email protected]>
---
 drivers/block/xen-blkback/xenbus.c |  203 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 182 insertions(+), 21 deletions(-)
drivers/block/xen-blkback/xenbus.c +182 -21
index 3f129b4..32d4c3c 100644
@@ -25,16 +25,25 @@ struct backend_info {
 	struct xenbus_device	*dev;
 	struct xen_blkif	*blkif;
 	struct xenbus_watch	backend_watch;
+	struct xenbus_watch	shutdown_watch;
 	unsigned		major;
 	unsigned		minor;
 	char			*mode;
+	bool			group_added;
+	char			*nodename;
+	atomic_t		refcnt;
+	pid_t			kthread_pid;
+	bool			shutdown_signalled;
 };
 
+DEFINE_SEMAPHORE(blkback_dev_sem);
+
 static struct kmem_cache *xen_blkif_cachep;
 static void connect(struct backend_info *);
 static int connect_ring(struct backend_info *);
 static void backend_changed(struct xenbus_watch *, const char **,
 			    unsigned int);
+static void xen_vbd_free(struct xen_vbd *vbd);
 
 struct xenbus_device *xen_blkbk_xenbus(struct backend_info *be)
 {
@@ -99,6 +108,16 @@ static void xen_update_blkif_status(struct xen_blkif *blkif)
 		blkif->xenblkd = NULL;
 		xenbus_dev_error(blkif->be->dev, err, "start xenblkd");
 	}
+
+	blkif->be->kthread_pid = blkif->xenblkd->pid;
+	atomic_inc(&blkif->be->refcnt);
+
+	err = xenbus_printf(XBT_NIL, blkif->be->dev->nodename, "kthread-pid",
+			    "%d", blkif->xenblkd->pid);
+	if (err) {
+		xenbus_dev_error(blkif->be->dev, err, "write kthread-pid");
+		return;
+	}
 }
 
 static struct xen_blkif *xen_blkif_alloc(domid_t domid)
@@ -213,11 +232,6 @@ static int xen_blkif_map(struct xen_blkif *blkif, unsigned long shared_page,
 
 static void xen_blkif_disconnect(struct xen_blkif *blkif)
 {
-	if (blkif->xenblkd) {
-		kthread_stop(blkif->xenblkd);
-		blkif->xenblkd = NULL;
-	}
-
 	atomic_dec(&blkif->refcnt);
 	wait_event(blkif->waiting_to_free, atomic_read(&blkif->refcnt) == 0);
 	atomic_inc(&blkif->refcnt);
@@ -296,6 +310,7 @@ VBD_SHOW(mode, "%s\n", be->mode);
 int xenvbd_sysfs_addif(struct xenbus_device *dev)
 {
 	int error;
+	struct backend_info *be = dev_get_drvdata(&dev->dev);
 
 	error = device_create_file(&dev->dev, &dev_attr_physical_device);
 	if (error)
@@ -309,6 +324,8 @@ int xenvbd_sysfs_addif(struct xenbus_device *dev)
 	if (error)
 		goto fail3;
 
+	be->group_added = true;
+
 	return 0;
 
 fail3:	sysfs_remove_group(&dev->dev.kobj, &xen_vbdstat_group);
@@ -319,11 +336,73 @@ fail1:	device_remove_file(&dev->dev, &dev_attr_physical_device);
 
 void xenvbd_sysfs_delif(struct xenbus_device *dev)
 {
+	struct backend_info *be = dev_get_drvdata(&dev->dev);
+	if (!be->group_added)
+		return;
 	sysfs_remove_group(&dev->dev.kobj, &xen_vbdstat_group);
 	device_remove_file(&dev->dev, &dev_attr_mode);
 	device_remove_file(&dev->dev, &dev_attr_physical_device);
+	be->group_added = false;
+}
+
+static int xenvbd_kthread_remove(struct backend_info *be)
+{
+	struct xen_blkif *blkif = be->blkif;
+
+	if (!blkif || !blkif->xenblkd)
+		return 0;
+
+	blkif->remove_requested = true;
+	wake_up_process(blkif->xenblkd);
+
+	return -EBUSY;
+}
+
+static void xenvbd_signal_shutdown(struct backend_info *be)
+{
+	int err;
+
+	down(&blkback_dev_sem);
+
+	if (be->shutdown_signalled)
+		goto out;
+
+	err = xenbus_write(XBT_NIL, be->nodename, "shutdown-done", "");
+	if (err)
+		WPRINTK("Error writing shutdown-done for %s: %d\n",
+			be->nodename, err);
+
+	if (be->dev)
+		xenbus_switch_state(be->dev, XenbusStateClosed);
+
+	be->shutdown_signalled = true;
+
+ out:
+	up(&blkback_dev_sem);
 }
 
+static void backend_release(struct backend_info *be)
+{
+	struct xen_blkif *blkif = be->blkif;
+
+	if (current->pid == be->kthread_pid)
+		xenvbd_signal_shutdown(be);
+
+	if (!atomic_dec_and_test(&be->refcnt))
+		return;
+
+	xenvbd_signal_shutdown(be);
+
+	if (blkif) {
+		xen_blkif_disconnect(blkif);
+		xen_vbd_free(&blkif->vbd);
+		xen_blkif_free(blkif);
+		be->blkif = NULL;
+	}
+
+	kfree(be->nodename);
+	kfree(be);
+}
 
 static void xen_vbd_free(struct xen_vbd *vbd)
 {
@@ -332,6 +411,12 @@ static void xen_vbd_free(struct xen_vbd *vbd)
 	vbd->bdev = NULL;
 }
 
+void xen_vbd_sync(struct xen_vbd *vbd)
+{
+	if (vbd->bdev)
+		fsync_bdev(vbd->bdev);
+}
+
 static int xen_vbd_create(struct xen_blkif *blkif, blkif_vdev_t handle,
 			  unsigned major, unsigned minor, int readonly,
 			  int cdrom)
@@ -384,8 +469,9 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
 
 	DPRINTK("");
 
-	if (be->major || be->minor)
-		xenvbd_sysfs_delif(dev);
+	down(&blkback_dev_sem);
+	be->dev = NULL;
+	up(&blkback_dev_sem);
 
 	if (be->backend_watch.node) {
 		unregister_xenbus_watch(&be->backend_watch);
@@ -393,18 +479,73 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
 		be->backend_watch.node = NULL;
 	}
 
-	if (be->blkif) {
-		xen_blkif_disconnect(be->blkif);
-		xen_vbd_free(&be->blkif->vbd);
-		xen_blkif_free(be->blkif);
-		be->blkif = NULL;
+	if (be->shutdown_watch.node) {
+		unregister_xenbus_watch(&be->shutdown_watch);
+		kfree(be->shutdown_watch.node);
+		be->shutdown_watch.node = NULL;
 	}
 
-	kfree(be);
+	if (xenvbd_kthread_remove(be))
+		WPRINTK("BAD REMOVE REQUEST for %s\n", be->nodename);
+
+	xenvbd_sysfs_delif(dev);
+	backend_release(be);
+
 	dev_set_drvdata(&dev->dev, NULL);
+
 	return 0;
 }
 
+/*
+ * called by kthread when closing
+ */
+void xen_blkback_close(struct xen_blkif *blkif)
+{
+	xen_blkif_disconnect(blkif);
+	xen_vbd_sync(&blkif->vbd);
+	blkif->remove_requested = false;
+
+	down(&blkback_dev_sem);
+	if (blkif->be->dev)
+		xenvbd_sysfs_delif(blkif->be->dev);
+	up(&blkback_dev_sem);
+
+	backend_release(blkif->be);
+	blkif->xenblkd = NULL;
+}
+
+static void xenvbd_start_shutdown(struct xenbus_watch *watch,
+			   const char **vec, unsigned int length)
+{
+	int err;
+	char *type;
+	unsigned int len;
+	struct backend_info *be
+		= container_of(watch, struct backend_info, shutdown_watch);
+	struct xenbus_device *dev = be->dev;
+
+	if (be->shutdown_signalled)
+		return;
+
+	type = xenbus_read(XBT_NIL, dev->nodename, "shutdown-request", &len);
+	err  = (IS_ERR(type) ? PTR_ERR(type) : 0);
+
+	if (XENBUS_EXIST_ERR(err))
+		return;
+
+	if (err) {
+		xenbus_dev_fatal(dev, err, "reading shutdown-request");
+		return;
+	}
+
+	xenbus_switch_state(dev, XenbusStateClosing);
+
+	if (len == sizeof("force") - 1 && !memcmp(type, "force", len))
+		if (!xenvbd_kthread_remove(be))
+			xenvbd_signal_shutdown(be); /* shutdown immediately */
+
+	kfree(type);
+}
 int xen_blkbk_flush_diskcache(struct xenbus_transaction xbt,
 			      struct backend_info *be, int state)
 {
@@ -437,6 +578,15 @@ static int xen_blkbk_probe(struct xenbus_device *dev,
 	}
 	be->dev = dev;
 	dev_set_drvdata(&dev->dev, be);
+	atomic_set(&be->refcnt, 1);
+
+	be->nodename = kasprintf(GFP_KERNEL, "%s", dev->nodename);
+	if (!be->nodename) {
+		xenbus_dev_fatal(dev, -ENOMEM,
+				 "allocating backend structure");
+		kfree(be);
+		return -ENOMEM;
+	}
 
 	be->blkif = xen_blkif_alloc(dev->otherend_id);
 	if (IS_ERR(be->blkif)) {
@@ -454,6 +604,12 @@ static int xen_blkbk_probe(struct xenbus_device *dev,
 	if (err)
 		goto fail;
 
+	err = xenbus_watch_pathfmt(dev, &be->shutdown_watch,
+				   xenvbd_start_shutdown, "%s/%s",
+				   dev->nodename, "shutdown-request");
+	if (err)
+		goto fail;
+
 	err = xenbus_switch_state(dev, XenbusStateInitWait);
 	if (err)
 		goto fail;
@@ -567,13 +723,17 @@ static void frontend_changed(struct xenbus_device *dev,
 	struct backend_info *be = dev_get_drvdata(&dev->dev);
 	int err;
 
-	DPRINTK("%s", xenbus_strstate(frontend_state));
+	DPRINTK("%s: %s", dev->nodename, xenbus_strstate(frontend_state));
 
 	switch (frontend_state) {
 	case XenbusStateInitialising:
 		if (dev->state == XenbusStateClosed) {
 			pr_info(DRV_PFX "%s: prepare for reconnect\n",
 				dev->nodename);
+
+			xenbus_rm(XBT_NIL, dev->nodename, "shutdown-done");
+			be->shutdown_signalled = false;
+
 			xenbus_switch_state(dev, XenbusStateInitWait);
 		}
 		break;
@@ -590,7 +750,7 @@ static void frontend_changed(struct xenbus_device *dev,
 
 		/*
 		 * Enforce precondition before potential leak point.
-		 * blkif_disconnect() is idempotent.
+		 * xen_blkif_disconnect() is idempotent.
 		 */
 		xen_blkif_disconnect(be->blkif);
 
@@ -601,17 +761,16 @@ static void frontend_changed(struct xenbus_device *dev,
 		break;
 
 	case XenbusStateClosing:
-		xen_blkif_disconnect(be->blkif);
 		xenbus_switch_state(dev, XenbusStateClosing);
 		break;
 
 	case XenbusStateClosed:
-		xenbus_switch_state(dev, XenbusStateClosed);
-		if (xenbus_dev_is_online(dev))
-			break;
-		/* fall through if not online */
+		if (!xenvbd_kthread_remove(be))
+			xenvbd_signal_shutdown(be);
+		break;
+
 	case XenbusStateUnknown:
-		/* implies blkif_disconnect() via blkback_remove() */
+		/* implies xen_blkif_disconnect() via blkback_remove() */
 		device_unregister(&dev->dev);
 		break;
 
@@ -620,6 +779,8 @@ static void frontend_changed(struct xenbus_device *dev,
 				 frontend_state);
 		break;
 	}
+
+	DPRINTK("%s: %s", dev->nodename, xenbus_strstate(dev->state));
 }
 
 
#4 lore raw
In reply to Joe Jin
Quoted 6 lines · Joe Jin

This patchset is a backport and original patch author is Daniel Stodden:
http://xenbits.xen.org/hg/XCP/linux-2.6.32.pq.hg/file/tip/CA-7672-blkback-shutdown.patch

Initial issue:
When we do block device attach/detach test with below steps, umount hang in guest and the guest unable to shutdown:

So the patchset looks good and it fixes the guest hanging.. but

Quoted 3 lines

1. start guest with the latest kernel.
2. attach new block device by xm block-attach in Dom0

So I think your patch while it fixes this problem it introduces a bug:

I did this in Dom0:

18:10:23 # 5 :~/

Quoted 1 line

xm block-attach 1 phy:/dev/sda xvda w

and did _not_ attach the disk in the guest. Then I did

18:10:35 # 6 :~/

Quoted 1 line

xm block-list 1

Vdev BE handle state evt-ch ring-ref BE-path
51712 0 0 4 18 770 /local/domain/0/backend/vbd/1/51712

18:10:39 # 7 :~/

Quoted 1 line

xm block-detach 1 51712

18:10:46 # 8 :~/

Quoted 1 line

xm block-list 1

If I try the same sequence of events with your patch, I get this:

1:28:06 # 1 :~/

Quoted 1 line

xm list

Name                                        ID   Mem VCPUs      State   Time(s)
Domain-0                                     0  1500     4     r-----   1246.6
sda                                          2  2048     2     -b----   1034.7

sdb 6 2048 2 -b---- 3.4
21:28:09 # 2 :~/

Quoted 1 line

xm block-list 6

21:28:22 # 4 :~/

Quoted 1 line

xm block-attach 6 phy:/dev/sdb xvda w

[did not do anything in the guest]
21:28:33 # 5 :~/

Quoted 1 line

xm block-list 6

Vdev BE handle state evt-ch ring-ref BE-path
51712 0 0 4 18 770 /local/domain/0/backend/vbd/6/51712

21:28:37 # 6 :~/

Quoted 1 line

xm block-detach 6 51712

Error: Device 51712 (vbd) could not be disconnected.
Usage: xm block-detach <Domain> <DevId> [-f|--force]

Destroy a domain's virtual block device.

21:30:30 # 7 :~/

Any ideas?

Quoted 29 lines

3. mount new disk in guest
4. execute xm block-detach to detach the block device in dom0 until timeout
5. try to unmount the disk in guest, umount hung. at here, any IOs to the device will hang.

Root cause:
This caused by 'xm block-detach' in Dom0 set backend device's state to 'XenbusStateClosing', frontend received the notification and blkfront_closing() be called, at the moment, the disk still using by guest, so frontend refused to close. In the blkfront_closing(), frontend send a notification to backend said that the its state switched to 'Closing', when backend got the event, it will disconnect from real device, at here any IO request will be stuck, even tried to release the disk by umount.

So this may fix either frontend or backend, I have send a fix for frontend:
https://lkml.org/lkml/2011/7/8/159
Ian think we should fix it from backend and he pointed out Daniel Stodden have submitted a patch(see above link) for xen-blkback, I tried it and it works well.

Changes:
v2:
- Reformat code style.
- Per Knoard suggestions, change some int defines to bool.

 drivers/block/xen-blkback/blkback.c |   10 +--
 drivers/block/xen-blkback/common.h  |    5 +
 drivers/block/xen-blkback/xenbus.c  |  203 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 192 insertions(+), 26 deletions(-)
author
#5 lore raw
Quoted 58 lines · Konrad Rzeszutek Wilk
Quoted 6 lines · Joe Jin

This patchset is a backport and original patch author is Daniel Stodden:
http://xenbits.xen.org/hg/XCP/linux-2.6.32.pq.hg/file/tip/CA-7672-blkback-shutdown.patch

Initial issue:
When we do block device attach/detach test with below steps, umount hang in guest and the guest unable to shutdown:

So the patchset looks good and it fixes the guest hanging.. but

Quoted 3 lines

1. start guest with the latest kernel.
2. attach new block device by xm block-attach in Dom0

So I think your patch while it fixes this problem it introduces a bug:

I did this in Dom0:

18:10:23 # 5 :~/

Quoted 1 line

xm block-attach 1 phy:/dev/sda xvda w

and did _not_ attach the disk in the guest. Then I did

18:10:35 # 6 :~/

Quoted 1 line

xm block-list 1

Vdev BE handle state evt-ch ring-ref BE-path
51712 0 0 4 18 770 /local/domain/0/backend/vbd/1/51712

18:10:39 # 7 :~/

Quoted 1 line

xm block-detach 1 51712

18:10:46 # 8 :~/

Quoted 1 line

xm block-list 1

If I try the same sequence of events with your patch, I get this:

1:28:06 # 1 :~/

Quoted 1 line

xm list

Name                                        ID   Mem VCPUs      State   Time(s)
Domain-0                                     0  1500     4     r-----   1246.6
sda                                          2  2048     2     -b----   1034.7

sdb 6 2048 2 -b---- 3.4
21:28:09 # 2 :~/

Quoted 1 line

xm block-list 6

21:28:22 # 4 :~/

Quoted 1 line

xm block-attach 6 phy:/dev/sdb xvda w

[did not do anything in the guest]
21:28:33 # 5 :~/

Quoted 1 line

xm block-list 6

Vdev BE handle state evt-ch ring-ref BE-path
51712 0 0 4 18 770 /local/domain/0/backend/vbd/6/51712

21:28:37 # 6 :~/

Quoted 1 line

xm block-detach 6 51712

Quoted 8 lines

Error: Device 51712 (vbd) could not be disconnected.
Usage: xm block-detach <Domain> <DevId> [-f|--force]

Destroy a domain's virtual block device.

21:30:30 # 7 :~/

Any ideas?

Konrad,

Thanks for the finding.

Review the patch looked like it caused by below piece of codes in patch3:
case XenbusStateClosed:
- xenbus_switch_state(dev, XenbusStateClosed);
- if (xenbus_dev_is_online(dev))
- break;
- /* fall through if not online */
+ if (!xenvbd_kthread_remove(be))
+ xenvbd_signal_shutdown(be);
+ break;
+
case XenbusStateUnknown:
- /* implies blkif_disconnect() via blkback_remove() */
+ /* implies xen_blkif_disconnect() via blkback_remove() */

                device_unregister(&dev->dev);
                break;

When device's state switched to XenbusStateClosed, did not unregister the device. Will send new patches for this.

Regards,
Joe

Quoted 29 lines
Quoted 29 lines

3. mount new disk in guest
4. execute xm block-detach to detach the block device in dom0 until timeout
5. try to unmount the disk in guest, umount hung. at here, any IOs to the device will hang.

Root cause:
This caused by 'xm block-detach' in Dom0 set backend device's state to 'XenbusStateClosing', frontend received the notification and blkfront_closing() be called, at the moment, the disk still using by guest, so frontend refused to close. In the blkfront_closing(), frontend send a notification to backend said that the its state switched to 'Closing', when backend got the event, it will disconnect from real device, at here any IO request will be stuck, even tried to release the disk by umount.

So this may fix either frontend or backend, I have send a fix for frontend:
https://lkml.org/lkml/2011/7/8/159
Ian think we should fix it from backend and he pointed out Daniel Stodden have submitted a patch(see above link) for xen-blkback, I tried it and it works well.

Changes:
v2:
- Reformat code style.
- Per Knoard suggestions, change some int defines to bool.

 drivers/block/xen-blkback/blkback.c |   10 +--
 drivers/block/xen-blkback/common.h  |    5 +
 drivers/block/xen-blkback/xenbus.c  |  203 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 192 insertions(+), 26 deletions(-)
Signature · 4 lines
Oracle <http://www.oracle.com>
Joe Jin | Team Leader, Software Development | +8610.6106.5624
ORACLE | Linux and Virtualization
No. 24 Zhongguancun Software Park, Haidian District | 100193 Beijing