[PATCH 00 of 10] blkfront pvops updates

[PATCH 00 of 10] blkfront pvops updates

[Daniel Stodden]

Hi.

Here are couple of bugfixes for blkfront.

 1. Bdev removal was slightly buggy. [Is nobody out there using
    block-detach??] Also it seems like we'be been leaking the gendisk
    structs for a long time.

 2. Fixes most common occasions where disk open/close races against
    backend state switches. This one didn't come for free:
	* xenbus_switch_state growing transactions again.
	* New mutex in blkfront_info
	* Bdev .open/.close rewritten accordingly.

Cheers,
Daniel

[PATCH 01 of 10] xenbus: Make xenbus_switch_state transactional (again)

[Daniel Stodden]

[-- Attachment #1: Type: text/plain, Size: 750 bytes --]

According to the comments, this was how it's been done years ago, but
apparently took an xbt pointer from elsewhere back then. The code was
removed because of consistency issues: cancellation wont't roll back
the saved xbdev->state.

Still, unsolicited writes to the state field remain an issue,
especially if device shutdown takes thread synchronization, and subtle
races cause accidental recreation of the device node.

Fixed by reintroducing the transaction. An internal one is sufficient,
so the xbdev->state value remains consistent.

Also fixes the original hack to prevent infinite recursion. Instead of
bailing out on the first attempt to switch to Closing, checks call
depth now.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>



[-- Attachment #2: xenbus-switch-transaction.diff --]
[-- Type: text/x-patch, Size: 4825 bytes --]

# HG changeset patch
# User Daniel Stodden <daniel.stodden@citrix.com>
# Date 1272506749 25200
# Node ID deecb3ec62a1818d368d94c36e4f32493da38e2d
# Parent  e6dbf213cd122e278cf04a9621363ff3ed46b423
xenbus: Make xenbus_switch_state transactional (again)

According to the comments, this was how it's been done years ago, but
apparently took an xbt pointer from elsewhere back then. The code was
removed because of consistency issues: cancellation wont't roll back
the saved xbdev->state.

Still, unsolicited writes to the state field remain an issue,
especially if device shutdown takes thread synchronization, and subtle
races cause accidental recreation of the device node.

Fixed by reintroducing the transaction. An internal one is sufficient,
so the xbdev->state value remains consistent.

Also fixes the original hack to prevent infinite recursion. Instead of
bailing out on the first attempt to switch to Closing, checks call
depth now.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>

diff -r e6dbf213cd12 -r deecb3ec62a1 drivers/xen/xenbus/xenbus_client.c
--- a/drivers/xen/xenbus/xenbus_client.c	Wed Apr 28 19:05:49 2010 -0700
+++ b/drivers/xen/xenbus/xenbus_client.c	Wed Apr 28 19:05:49 2010 -0700
@@ -134,6 +134,64 @@
 }
 EXPORT_SYMBOL_GPL(xenbus_watch_pathfmt);
 
+static void xenbus_switch_fatal(struct xenbus_device *, int, int,
+				const char *, ...);
+
+static int
+__xenbus_switch_state(struct xenbus_device *dev,
+		      enum xenbus_state state, int depth)
+{
+	/* We check whether the state is currently set to the given value, and
+	   if not, then the state is set.  We don't want to unconditionally
+	   write the given state, because we don't want to fire watches
+	   unnecessarily.  Furthermore, if the node has gone, we don't write
+	   to it, as the device will be tearing down, and we don't want to
+	   resurrect that directory.
+
+	   Note that, because of this cached value of our state, this
+	   function will not take a caller's Xenstore transaction
+	   (something it was trying to in the past) because dev->state
+	   would not get reset if the transaction was aborted.
+	 */
+
+	struct xenbus_transaction xbt;
+	int current_state;
+	int err, abort;
+
+	if (state == dev->state)
+		return 0;
+
+again:
+	abort = 1;
+
+	err = xenbus_transaction_start(&xbt);
+	if (err) {
+		xenbus_switch_fatal(dev, depth, err, "starting transaction");
+		return 0;
+	}
+
+	err = xenbus_scanf(xbt, dev->nodename, "state", "%d", &current_state);
+	if (err != 1)
+		goto abort;
+
+	err = xenbus_printf(xbt, dev->nodename, "state", "%d", state);
+	if (err) {
+		xenbus_switch_fatal(dev, depth, err, "writing new state");
+		goto abort;
+	}
+
+	abort = 0;
+abort:
+	err = xenbus_transaction_end(xbt, abort);
+	if (err) {
+		if (err == -EAGAIN && !abort)
+			goto again;
+		xenbus_switch_fatal(dev, depth, err, "ending transaction");
+	} else
+		dev->state = state;
+
+	return 0;
+}
 
 /**
  * xenbus_switch_state
@@ -146,42 +204,9 @@
  */
 int xenbus_switch_state(struct xenbus_device *dev, enum xenbus_state state)
 {
-	/* We check whether the state is currently set to the given value, and
-	   if not, then the state is set.  We don't want to unconditionally
-	   write the given state, because we don't want to fire watches
-	   unnecessarily.  Furthermore, if the node has gone, we don't write
-	   to it, as the device will be tearing down, and we don't want to
-	   resurrect that directory.
+	return __xenbus_switch_state(dev, state, 0);
+}
 
-	   Note that, because of this cached value of our state, this function
-	   will not work inside a Xenstore transaction (something it was
-	   trying to in the past) because dev->state would not get reset if
-	   the transaction was aborted.
-
-	 */
-
-	int current_state;
-	int err;
-
-	if (state == dev->state)
-		return 0;
-
-	err = xenbus_scanf(XBT_NIL, dev->nodename, "state", "%d",
-			   &current_state);
-	if (err != 1)
-		return 0;
-
-	err = xenbus_printf(XBT_NIL, dev->nodename, "state", "%d", state);
-	if (err) {
-		if (state != XenbusStateClosing) /* Avoid looping */
-			xenbus_dev_fatal(dev, err, "writing new state");
-		return err;
-	}
-
-	dev->state = state;
-
-	return 0;
-}
 EXPORT_SYMBOL_GPL(xenbus_switch_state);
 
 int xenbus_frontend_closed(struct xenbus_device *dev)
@@ -285,6 +310,23 @@
 EXPORT_SYMBOL_GPL(xenbus_dev_fatal);
 
 /**
+ * Equivalent to xenbus_dev_fatal(dev, err, fmt, args), but helps
+ * avoiding recursion within xenbus_switch_state.
+ */
+static void xenbus_switch_fatal(struct xenbus_device *dev, int depth, int err,
+				const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	xenbus_va_dev_error(dev, err, fmt, ap);
+	va_end(ap);
+
+	if (!depth)
+		__xenbus_switch_state(dev, XenbusStateClosing, 1);
+}
+
+/**
  * xenbus_grant_ring
  * @dev: xenbus device
  * @ring_mfn: mfn of ring to grant

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 02 of 10] blkfront: Fix backtrace in del_gendisk

[Daniel Stodden]

[-- Attachment #1: Type: text/plain, Size: 218 bytes --]

The call to del_gendisk follows an non-refcounted gd->queue
pointer. We release the last ref in blk_cleanup_queue. Fixed by
reordering releases accordingly.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>



[-- Attachment #2: blkfront-cleanup-queue.diff --]
[-- Type: text/x-patch, Size: 1077 bytes --]

# HG changeset patch
# User Daniel Stodden <daniel.stodden@citrix.com>
# Date 1272506749 25200
# Node ID 7a3a94d3c7ce264f885ef54ae7a4040aafbf99a1
# Parent  deecb3ec62a1818d368d94c36e4f32493da38e2d
blkfront: Fix backtrace in del_gendisk.

The call to del_gendisk follows an non-refcounted gd->queue
pointer. We release the last ref in blk_cleanup_queue. Fixed by
reordering releases accordingly.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>

diff -r deecb3ec62a1 -r 7a3a94d3c7ce drivers/block/xen-blkfront.c
--- a/drivers/block/xen-blkfront.c	Wed Apr 28 19:05:49 2010 -0700
+++ b/drivers/block/xen-blkfront.c	Wed Apr 28 19:05:49 2010 -0700
@@ -1001,14 +1001,14 @@
 	/* Flush gnttab callback work. Must be done with no locks held. */
 	flush_scheduled_work();
 
-	blk_cleanup_queue(info->rq);
-	info->rq = NULL;
-
 	minor = info->gd->first_minor;
 	nr_minors = info->gd->minors;
 	del_gendisk(info->gd);
 	xlbd_release_minors(minor, nr_minors);
 
+	blk_cleanup_queue(info->rq);
+	info->rq = NULL;
+
  out:
 	if (info->xbdev)
 		xenbus_frontend_closed(info->xbdev);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 03 of 10] blkfront: Fix gendisk leak

[Daniel Stodden]

[-- Attachment #1: Type: text/plain, Size: 60 bytes --]

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>



[-- Attachment #2: blkfront-del-gendisk.diff --]
[-- Type: text/x-patch, Size: 666 bytes --]

# HG changeset patch
# User Daniel Stodden <daniel.stodden@citrix.com>
# Date 1272506749 25200
# Node ID daa6ed550e0cccc3c755ed8057e954fdba7ac734
# Parent  7a3a94d3c7ce264f885ef54ae7a4040aafbf99a1
blkfront: Fix gendisk leak.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>

diff -r 7a3a94d3c7ce -r daa6ed550e0c drivers/block/xen-blkfront.c
--- a/drivers/block/xen-blkfront.c	Wed Apr 28 19:05:49 2010 -0700
+++ b/drivers/block/xen-blkfront.c	Wed Apr 28 19:05:49 2010 -0700
@@ -1009,6 +1009,9 @@
 	blk_cleanup_queue(info->rq);
 	info->rq = NULL;
 
+	put_disk(info->gd);
+	info->gd = NULL;
+
  out:
 	if (info->xbdev)
 		xenbus_frontend_closed(info->xbdev);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 04 of 10] blkfront: Clean up vbd release

[Daniel Stodden]

[-- Attachment #1: Type: text/plain, Size: 402 bytes --]

* Current blkfront_closing is rather a xlvbd_release_gendisk.
   Renamed in preparation of later patches (need the name again).

 * Removed the misleading comment -- this only applied to the backend
   switch handler, and the queue is already flushed btw.

 * Break out the xenbus call, callers know better when to switch
   frontend state.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>



[-- Attachment #2: blkfront-vbd-release.diff --]
[-- Type: text/x-patch, Size: 3655 bytes --]

# HG changeset patch
# User Daniel Stodden <daniel.stodden@citrix.com>
# Date 1272506749 25200
# Node ID 046e1c8ed0dd519c2a5b2f8178906394b008fb7c
# Parent  daa6ed550e0cccc3c755ed8057e954fdba7ac734
blkfront: Clean up vbd release.

 * Current blkfront_closing is rather a xlvbd_release_gendisk.
   Renamed in preparation of later patches (need the name again).

 * Removed the misleading comment -- this only applied to the backend
   switch handler, and the queue is already flushed btw.

 * Break out the xenbus call, callers know better when to switch
   frontend state.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>

diff -r daa6ed550e0c -r 046e1c8ed0dd drivers/block/xen-blkfront.c
--- a/drivers/block/xen-blkfront.c	Wed Apr 28 19:05:49 2010 -0700
+++ b/drivers/block/xen-blkfront.c	Wed Apr 28 19:05:49 2010 -0700
@@ -533,6 +533,39 @@
 	return err;
 }
 
+static void xlvbd_release_gendisk(struct blkfront_info *info)
+{
+	unsigned int minor, nr_minors;
+	unsigned long flags;
+
+	if (info->rq == NULL)
+		return;
+
+	spin_lock_irqsave(&blkif_io_lock, flags);
+
+	/* No more blkif_request(). */
+	blk_stop_queue(info->rq);
+
+	/* No more gnttab callback work. */
+	gnttab_cancel_free_callback(&info->callback);
+	spin_unlock_irqrestore(&blkif_io_lock, flags);
+
+	/* Flush gnttab callback work. Must be done with no locks held. */
+	flush_scheduled_work();
+
+	del_gendisk(info->gd);
+
+	minor = info->gd->first_minor;
+	nr_minors = info->gd->minors;
+	xlbd_release_minors(minor, nr_minors);
+
+	blk_cleanup_queue(info->rq);
+	info->rq = NULL;
+
+	put_disk(info->gd);
+	info->gd = NULL;
+}
+
 static void kick_pending_request_queues(struct blkfront_info *info)
 {
 	if (!RING_FULL(&info->ring)) {
@@ -975,49 +1008,6 @@
 }
 
 /**
- * Handle the change of state of the backend to Closing.  We must delete our
- * device-layer structures now, to ensure that writes are flushed through to
- * the backend.  Once is this done, we can switch to Closed in
- * acknowledgement.
- */
-static void blkfront_closing(struct blkfront_info *info)
-{
-	unsigned int minor, nr_minors;
-	unsigned long flags;
-
-
-	if (info->rq == NULL)
-		goto out;
-
-	spin_lock_irqsave(&blkif_io_lock, flags);
-
-	/* No more blkif_request(). */
-	blk_stop_queue(info->rq);
-
-	/* No more gnttab callback work. */
-	gnttab_cancel_free_callback(&info->callback);
-	spin_unlock_irqrestore(&blkif_io_lock, flags);
-
-	/* Flush gnttab callback work. Must be done with no locks held. */
-	flush_scheduled_work();
-
-	minor = info->gd->first_minor;
-	nr_minors = info->gd->minors;
-	del_gendisk(info->gd);
-	xlbd_release_minors(minor, nr_minors);
-
-	blk_cleanup_queue(info->rq);
-	info->rq = NULL;
-
-	put_disk(info->gd);
-	info->gd = NULL;
-
- out:
-	if (info->xbdev)
-		xenbus_frontend_closed(info->xbdev);
-}
-
-/**
  * Callback received when the backend's state changes.
  */
 static void blkback_changed(struct xenbus_device *dev,
@@ -1055,8 +1045,11 @@
 		if (info->users > 0)
 			xenbus_dev_error(dev, -EBUSY,
 					 "Device in use; refusing to close");
-		else
-			blkfront_closing(info);
+		else {
+			xlvbd_release_gendisk(info);
+			xenbus_frontend_closed(info->xbdev);
+		}
+
 		mutex_unlock(&bd->bd_mutex);
 		bdput(bd);
 		break;
@@ -1107,11 +1100,13 @@
 		struct xenbus_device *dev = info->xbdev;
 
 		if (!dev) {
-			blkfront_closing(info);
+			xlvbd_release_gendisk(info);
 			kfree(info);
 		} else if (xenbus_read_driver_state(dev->otherend)
-			   == XenbusStateClosing && info->is_ready)
-			blkfront_closing(info);
+			   == XenbusStateClosing && info->is_ready) {
+			xlvbd_release_gendisk(info);
+			xenbus_frontend_closed(dev);
+		}
 	}
 	return 0;
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 05 of 10] blkfront: Lock blkfront_info when closing

[Daniel Stodden]

[-- Attachment #1: Type: text/plain, Size: 461 bytes --]

The bdev .open/.release fops race against backend switches to Closing,
handled by the XenBus thread.

The original code attempted to serialize block device holders and
xenbus only via bd_mutex. This is insufficient, the info->bd pointer
may already be stale (or null) while xenbus tries to bump up the
refcount. Worst case scenario is a null ptr deref.

Protect blkfront_info with a dedicated mutex.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>



[-- Attachment #2: blkfront-xenbus-closing.diff --]
[-- Type: text/x-patch, Size: 3049 bytes --]

# HG changeset patch
# User Daniel Stodden <daniel.stodden@citrix.com>
# Date 1272506749 25200
# Node ID 6a52e76a65073a521d531d875d141b1e89a38954
# Parent  046e1c8ed0dd519c2a5b2f8178906394b008fb7c
blkfront: Lock blkfront_info when closing.

The bdev .open/.release fops race against backend switches to Closing,
handled by the XenBus thread.

The original code attempted to serialize block device holders and
xenbus only via bd_mutex. This is insufficient, the info->bd pointer
may already be stale (or null) while xenbus tries to bump up the
refcount. Worst case scenario is a null ptr deref.

Protect blkfront_info with a dedicated mutex.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>

diff -r 046e1c8ed0dd -r 6a52e76a6507 drivers/block/xen-blkfront.c
--- a/drivers/block/xen-blkfront.c	Wed Apr 28 19:05:49 2010 -0700
+++ b/drivers/block/xen-blkfront.c	Wed Apr 28 19:05:49 2010 -0700
@@ -76,6 +76,7 @@
  */
 struct blkfront_info
 {
+	struct mutex mutex;
 	struct xenbus_device *xbdev;
 	struct gendisk *gd;
 	int vdevice;
@@ -803,7 +804,6 @@
 	return err;
 }
 
-
 /**
  * Entry point to this code when a new device is created.  Allocate the basic
  * structures and the ring buffer for communication with the backend, and
@@ -835,6 +835,7 @@
 		return -ENOMEM;
 	}
 
+	mutex_init(&info->mutex);
 	info->xbdev = dev;
 	info->vdevice = vdevice;
 	info->connected = BLKIF_STATE_DISCONNECTED;
@@ -950,6 +951,43 @@
 	return err;
 }
 
+static void
+blkfront_closing(struct blkfront_info *info)
+{
+	struct xenbus_device *xbdev = info->xbdev;
+	struct block_device *bdev = NULL;
+
+	mutex_lock(&info->mutex);
+
+	if (xbdev->state == XenbusStateClosing) {
+		mutex_unlock(&info->mutex);
+		return;
+	}
+
+	if (info->gd)
+		bdev = bdget_disk(info->gd, 0);
+
+	mutex_unlock(&info->mutex);
+
+	if (!bdev) {
+		xenbus_frontend_closed(xbdev);
+		return;
+	}
+
+	mutex_lock(&bdev->bd_mutex);
+
+	if (info->users) {
+		xenbus_dev_error(xbdev, -EBUSY,
+				 "Device in use; refusing to close");
+		xenbus_switch_state(xbdev, XenbusStateClosing);
+	} else {
+		xlvbd_release_gendisk(info);
+		xenbus_frontend_closed(xbdev);
+	}
+
+	mutex_unlock(&bdev->bd_mutex);
+	bdput(bdev);
+}
 
 /*
  * Invoked when the backend is finally 'ready' (and has told produced
@@ -1014,7 +1052,6 @@
 			    enum xenbus_state backend_state)
 {
 	struct blkfront_info *info = dev_get_drvdata(&dev->dev);
-	struct block_device *bd;
 
 	dev_dbg(&dev->dev, "blkfront:blkback_changed to state %d.\n", backend_state);
 
@@ -1033,25 +1070,7 @@
 		break;
 
 	case XenbusStateClosing:
-		if (info->gd == NULL) {
-			xenbus_frontend_closed(dev);
-			break;
-		}
-		bd = bdget_disk(info->gd, 0);
-		if (bd == NULL)
-			xenbus_dev_fatal(dev, -ENODEV, "bdget failed");
-
-		mutex_lock(&bd->bd_mutex);
-		if (info->users > 0)
-			xenbus_dev_error(dev, -EBUSY,
-					 "Device in use; refusing to close");
-		else {
-			xlvbd_release_gendisk(info);
-			xenbus_frontend_closed(info->xbdev);
-		}
-
-		mutex_unlock(&bd->bd_mutex);
-		bdput(bd);
+		blkfront_closing(info);
 		break;
 	}
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 06 of 10] blkfront: Fix blkfront backend switch race (bdev open)

[Daniel Stodden]

[-- Attachment #1: Type: text/plain, Size: 328 bytes --]

We need not mind if users grab a late handle on a closing disk. We
probably even should not. But we have to make sure it's not a dead
one already

Let the bdev deal with a gendisk deleted under its feet. Takes the
info mutex to decide a race against backend closing.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>



[-- Attachment #2: blkfront-bdev-open.diff --]
[-- Type: text/x-patch, Size: 1395 bytes --]

# HG changeset patch
# User Daniel Stodden <daniel.stodden@citrix.com>
# Date 1272506749 25200
# Node ID 5e73c8d7aaf5bff5caa7d431184ffed8e837997a
# Parent  6a52e76a65073a521d531d875d141b1e89a38954
blkfront: Fix blkfront backend switch race (bdev open)

We need not mind if users grab a late handle on a closing disk. We
probably even should not. But we have to make sure it's not a dead
one already

Let the bdev deal with a gendisk deleted under its feet. Takes the
info mutex to decide a race against backend closing.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>

diff -r 6a52e76a6507 -r 5e73c8d7aaf5 drivers/block/xen-blkfront.c
--- a/drivers/block/xen-blkfront.c	Wed Apr 28 19:05:49 2010 -0700
+++ b/drivers/block/xen-blkfront.c	Wed Apr 28 19:05:49 2010 -0700
@@ -1100,12 +1100,23 @@
 
 static int blkif_open(struct block_device *bdev, fmode_t mode)
 {
-	struct blkfront_info *info = bdev->bd_disk->private_data;
+	struct gendisk *disk = bdev->bd_disk;
+	struct blkfront_info *info = disk->private_data;
+	int err = 0;
 
-	if (!info->xbdev)
-		return -ENODEV;
-	info->users++;
-	return 0;
+	mutex_lock(&info->mutex);
+	/*
+	 * Let the bdev deal with a gendisk deleted under its feet.
+	 */
+	if (!info->gd)
+		err = -ERESTARTSYS;
+
+	mutex_unlock(&info->mutex);
+
+	if (!err)
+		++info->users;
+
+	return err;
 }
 
 static int blkif_release(struct gendisk *disk, fmode_t mode)

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 07 of 10] blkfront: Fix blkfront backend switch race (bdev release)

[Daniel Stodden]

[-- Attachment #1: Type: text/plain, Size: 407 bytes --]

We cannot read backend state within bdev operations, because it risks
grabbing the state change before xenbus gets to do it.

Fixed by tracking deferral with a frontend switch to Closing. State
exposure isn't strictly necessary, but the backends won't mind.

For a 'clean' deferral this seems actually a more decent protocol than
raising errors.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>



[-- Attachment #2: blkfront-bdev-release.diff --]
[-- Type: text/x-patch, Size: 2129 bytes --]

# HG changeset patch
# User Daniel Stodden <daniel.stodden@citrix.com>
# Date 1272506749 25200
# Node ID b324514f8f83ef0d8c7991e2c995400937c540ac
# Parent  5e73c8d7aaf5bff5caa7d431184ffed8e837997a
blkfront: Fix blkfront backend switch race (bdev release)

We cannot read backend state within bdev operations, because it risks
grabbing the state change before xenbus gets to do it.

Fixed by tracking deferral with a frontend switch to Closing. State
exposure isn't strictly necessary, but the backends won't mind.

For a 'clean' deferral this seems actually a more decent protocol than
raising errors.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>

diff -r 5e73c8d7aaf5 -r b324514f8f83 drivers/block/xen-blkfront.c
--- a/drivers/block/xen-blkfront.c	Wed Apr 28 19:05:49 2010 -0700
+++ b/drivers/block/xen-blkfront.c	Wed Apr 28 19:05:49 2010 -0700
@@ -1122,22 +1122,37 @@
 static int blkif_release(struct gendisk *disk, fmode_t mode)
 {
 	struct blkfront_info *info = disk->private_data;
-	info->users--;
-	if (info->users == 0) {
-		/* Check whether we have been instructed to close.  We will
-		   have ignored this request initially, as the device was
-		   still mounted. */
-		struct xenbus_device *dev = info->xbdev;
+	struct block_device *bdev;
+	struct xenbus_device *xbdev;
 
-		if (!dev) {
-			xlvbd_release_gendisk(info);
-			kfree(info);
-		} else if (xenbus_read_driver_state(dev->otherend)
-			   == XenbusStateClosing && info->is_ready) {
-			xlvbd_release_gendisk(info);
-			xenbus_frontend_closed(dev);
-		}
+	if (--info->users)
+		return 0;
+
+	bdev = bdget_disk(disk, 0);
+	bdput(bdev);
+
+	/*
+	 * Check if we have been instructed to close. We will have
+	 * deferred this request, because the bdev was still open.
+	 */
+
+	mutex_lock(&info->mutex);
+	xbdev = info->xbdev;
+
+	if (xbdev && xbdev->state == XenbusStateClosing) {
+		/* pending switch to state closed */
+		xlvbd_release_gendisk(info);
+		xenbus_frontend_closed(info->xbdev);
 	}
+
+	mutex_unlock(&info->mutex);
+
+	if (!xbdev) {
+		/* sudden device removal */
+		xlvbd_release_gendisk(info);
+		kfree(info);
+	}
+
 	return 0;
 }
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 08 of 10] blkfront: Lock blockfront_info during xbdev removal

[Daniel Stodden]

[-- Attachment #1: Type: text/plain, Size: 236 bytes --]

Same approach as blkfront_closing:
 * Grab the bdev safely, holding the info mutex.
 * Zap xbdev safely, holding the info mutex.
 * Try bdev removal safely, holding bd_mutex.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>



[-- Attachment #2: blkfront-xenbus-remove.diff --]
[-- Type: text/x-patch, Size: 1722 bytes --]

# HG changeset patch
# User Daniel Stodden <daniel.stodden@citrix.com>
# Date 1272506749 25200
# Node ID a781d46d987f8f8a9308412ba50bad4654a3d97c
# Parent  b324514f8f83ef0d8c7991e2c995400937c540ac
blkfront: Lock blockfront_info during xbdev removal.

Same approach as blkfront_closing:
 * Grab the bdev safely, holding the info mutex.
 * Zap xbdev safely, holding the info mutex.
 * Try bdev removal safely, holding bd_mutex.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>

diff -r b324514f8f83 -r a781d46d987f drivers/block/xen-blkfront.c
--- a/drivers/block/xen-blkfront.c	Wed Apr 28 19:05:49 2010 -0700
+++ b/drivers/block/xen-blkfront.c	Wed Apr 28 19:05:49 2010 -0700
@@ -1075,18 +1075,41 @@
 	}
 }
 
-static int blkfront_remove(struct xenbus_device *dev)
+static int blkfront_remove(struct xenbus_device *xbdev)
 {
-	struct blkfront_info *info = dev_get_drvdata(&dev->dev);
+	struct blkfront_info *info = dev_get_drvdata(&xbdev->dev);
+	struct block_device *bdev = NULL;
 
-	dev_dbg(&dev->dev, "blkfront_remove: %s removed\n", dev->nodename);
+	dev_dbg(&xbdev->dev, "%s removed", xbdev->nodename);
 
 	blkif_free(info, 0);
 
-	if(info->users == 0)
+	mutex_lock(&info->mutex);
+
+	if (info->gd)
+		bdev = bdget_disk(info->gd, 0);
+
+	info->xbdev = NULL;
+	mutex_unlock(&info->mutex);
+
+	if (!bdev)
+		return 0;
+
+	/*
+	 * The xbdev was removed before we reached the Closed
+	 * state. See if it's safe to remove the disk. If the bdev
+	 * isn't closed yet, then release will take care of it.
+	 */
+
+	mutex_lock(&bdev->bd_mutex);
+
+	if (!info->users) {
+		xlvbd_release_gendisk(info);
 		kfree(info);
-	else
-		info->xbdev = NULL;
+	}
+
+	mutex_unlock(&bdev->bd_mutex);
+	bdput(bdev);
 
 	return 0;
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 09 of 10] blkfront: Remove obsolete info->users

[Daniel Stodden]

[-- Attachment #1: Type: text/plain, Size: 113 bytes --]

This is just bd_openers, protected by the bd_mutex.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>



[-- Attachment #2: blkfront-bd-openers.diff --]
[-- Type: text/x-patch, Size: 1642 bytes --]

# HG changeset patch
# User Daniel Stodden <daniel.stodden@citrix.com>
# Date 1272506749 25200
# Node ID fc49ba48fc5dde4869704cc05cc3737682ca650a
# Parent  a781d46d987f8f8a9308412ba50bad4654a3d97c
blkfront: Remove obsolete info->users.

This is just bd_openers, protected by the bd_mutex.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>

diff -r a781d46d987f -r fc49ba48fc5d drivers/block/xen-blkfront.c
--- a/drivers/block/xen-blkfront.c	Wed Apr 28 19:05:49 2010 -0700
+++ b/drivers/block/xen-blkfront.c	Wed Apr 28 19:05:49 2010 -0700
@@ -93,12 +93,6 @@
 	unsigned long shadow_free;
 	int feature_barrier;
 	int is_ready;
-
-	/**
-	 * The number of people holding this device open.  We won't allow a
-	 * hot-unplug unless this is 0.
-	 */
-	int users;
 };
 
 static DEFINE_SPINLOCK(blkif_io_lock);
@@ -976,7 +970,7 @@
 
 	mutex_lock(&bdev->bd_mutex);
 
-	if (info->users) {
+	if (bdev->bd_openers) {
 		xenbus_dev_error(xbdev, -EBUSY,
 				 "Device in use; refusing to close");
 		xenbus_switch_state(xbdev, XenbusStateClosing);
@@ -1103,7 +1097,7 @@
 
 	mutex_lock(&bdev->bd_mutex);
 
-	if (!info->users) {
+	if (!bdev->bd_openers) {
 		xlvbd_release_gendisk(info);
 		kfree(info);
 	}
@@ -1136,9 +1130,6 @@
 
 	mutex_unlock(&info->mutex);
 
-	if (!err)
-		++info->users;
-
 	return err;
 }
 
@@ -1148,12 +1139,12 @@
 	struct block_device *bdev;
 	struct xenbus_device *xbdev;
 
-	if (--info->users)
-		return 0;
-
 	bdev = bdget_disk(disk, 0);
 	bdput(bdev);
 
+	if (bdev->bd_openers)
+		return 0;
+
 	/*
 	 * Check if we have been instructed to close. We will have
 	 * deferred this request, because the bdev was still open.

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 10 of 10] blkfront: Klog the unclean release path

[Daniel Stodden]

[-- Attachment #1: Type: text/plain, Size: 60 bytes --]

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>



[-- Attachment #2: blkfront-vbd-release-log.diff --]
[-- Type: text/x-patch, Size: 1188 bytes --]

# HG changeset patch
# User Daniel Stodden <daniel.stodden@citrix.com>
# Date 1272506749 25200
# Node ID 47771bd9c9a1ee54efa3bf9d176c72f1dcb87b89
# Parent  fc49ba48fc5dde4869704cc05cc3737682ca650a
blkfront: Klog the unclean release path.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>

diff -r fc49ba48fc5d -r 47771bd9c9a1 drivers/block/xen-blkfront.c
--- a/drivers/block/xen-blkfront.c	Wed Apr 28 19:05:49 2010 -0700
+++ b/drivers/block/xen-blkfront.c	Wed Apr 28 19:05:49 2010 -0700
@@ -1097,6 +1097,10 @@
 
 	mutex_lock(&bdev->bd_mutex);
 
+	dev_warn(disk_to_dev(bdev->bd_disk),
+		 "%s was hot-unplugged with %d handles",
+		 xbdev->nodename, bdev->bd_openers);
+
 	if (!bdev->bd_openers) {
 		xlvbd_release_gendisk(info);
 		kfree(info);
@@ -1155,6 +1159,7 @@
 
 	if (xbdev && xbdev->state == XenbusStateClosing) {
 		/* pending switch to state closed */
+		dev_info(disk_to_dev(bdev->bd_disk), "releasing disk");
 		xlvbd_release_gendisk(info);
 		xenbus_frontend_closed(info->xbdev);
 	}
@@ -1163,6 +1168,7 @@
 
 	if (!xbdev) {
 		/* sudden device removal */
+		dev_info(disk_to_dev(bdev->bd_disk), "releasing disk");
 		xlvbd_release_gendisk(info);
 		kfree(info);
 	}

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 00 of 10] blkfront pvops updates

[Jeremy Fitzhardinge]

On 04/28/2010 07:19 PM, Daniel Stodden wrote:
> Hi.
>
> Here are couple of bugfixes for blkfront.
>
>  1. Bdev removal was slightly buggy. [Is nobody out there using
>     block-detach??] Also it seems like we'be been leaking the gendisk
>     structs for a long time.
>
>  2. Fixes most common occasions where disk open/close races against
>     backend state switches. This one didn't come for free:
> 	* xenbus_switch_state growing transactions again.
> 	* New mutex in blkfront_info
> 	* Bdev .open/.close rewritten accordingly.
>   

Thanks for this.  However, when I went to apply it, it looks like I've
made a mess of the various blkfront branches. 

xen/frontend has some changes from Jan and Ky which are merged into
xen/next and the various stable branches.

I also have xen/blkfront, which has your last set of changes, which I
seem to have forgotten to merge into anything else, and now conflicts
non-trivially with xen/blkfront.  I'd like to add this new set of
patches into xen/blkfront, but it doesn't apply cleanly.

Could you look at this and advise me what I should do?

Thanks,
    J
> Cheers,
> Daniel
>
>   

Re: [PATCH 00 of 10] blkfront pvops updates

[Daniel Stodden]

On Thu, 2010-04-29 at 15:50 -0400, Jeremy Fitzhardinge wrote:
> On 04/28/2010 07:19 PM, Daniel Stodden wrote:
> > Hi.
> >
> > Here are couple of bugfixes for blkfront.
> >
> >  1. Bdev removal was slightly buggy. [Is nobody out there using
> >     block-detach??] Also it seems like we'be been leaking the gendisk
> >     structs for a long time.
> >
> >  2. Fixes most common occasions where disk open/close races against
> >     backend state switches. This one didn't come for free:
> > 	* xenbus_switch_state growing transactions again.
> > 	* New mutex in blkfront_info
> > 	* Bdev .open/.close rewritten accordingly.
> >   
> 
> Thanks for this.  However, when I went to apply it, it looks like I've
> made a mess of the various blkfront branches. 
> 
> xen/frontend has some changes from Jan and Ky which are merged into
> xen/next and the various stable branches.
> 
> I also have xen/blkfront, which has your last set of changes, which I
> seem to have forgotten to merge into anything else, and now conflicts
> non-trivially with xen/blkfront.  I'd like to add this new set of
> patches into xen/blkfront, but it doesn't apply cleanly.
> 
> Could you look at this and advise me what I should do?

Oh, NP. I'll try and see if I can gather those and what it takes to
merge.

Daniel

Re: [PATCH 00 of 10] blkfront pvops updates

[Daniel Stodden]

On Thu, 2010-04-29 at 16:11 -0400, Daniel Stodden wrote:
> On Thu, 2010-04-29 at 15:50 -0400, Jeremy Fitzhardinge wrote:
> > On 04/28/2010 07:19 PM, Daniel Stodden wrote:
> > > Hi.
> > >
> > > Here are couple of bugfixes for blkfront.
> > >
> > >  1. Bdev removal was slightly buggy. [Is nobody out there using
> > >     block-detach??] Also it seems like we'be been leaking the gendisk
> > >     structs for a long time.
> > >
> > >  2. Fixes most common occasions where disk open/close races against
> > >     backend state switches. This one didn't come for free:
> > > 	* xenbus_switch_state growing transactions again.
> > > 	* New mutex in blkfront_info
> > > 	* Bdev .open/.close rewritten accordingly.
> > >   
> > 
> > Thanks for this.  However, when I went to apply it, it looks like I've
> > made a mess of the various blkfront branches. 
> > 
> > xen/frontend has some changes from Jan and Ky which are merged into
> > xen/next and the various stable branches.
> > 
> > I also have xen/blkfront, which has your last set of changes, which I
> > seem to have forgotten to merge into anything else, and now conflicts
> > non-trivially with xen/blkfront.  I'd like to add this new set of
> > patches into xen/blkfront, but it doesn't apply cleanly.
> > 
> > Could you look at this and advise me what I should do?
> 
> Oh, NP. I'll try and see if I can gather those and what it takes to
> merge.

Ah, that's where the previous stuff ended up. I thought those probably
just got dropped.

Let's just scratch those. It's been months, so if they didn't make it
back then, they won't need to complicate stuff now either.

So I should probably merge with xen/frontend. If those matter, they must
be on some branch which matters. Which one is it? I thought the one
which matters is xen/master? Is it xen/next?

Daniel

Re: [PATCH 00 of 10] blkfront pvops updates

[Jeremy Fitzhardinge]

On 04/29/2010 01:58 PM, Daniel Stodden wrote:
> Ah, that's where the previous stuff ended up. I thought those probably
> just got dropped.
>   

I thought the changes in these patches looked somewhat familiar.

> Let's just scratch those. It's been months, so if they didn't make it
> back then, they won't need to complicate stuff now either.
>
> So I should probably merge with xen/frontend. If those matter, they must
> be on some branch which matters. Which one is it? I thought the one
> which matters is xen/master? Is it xen/next?
>   

The flow of changes is (ideally):

    <topic branch> -> xen/next -> xen/stable*

where "->" is merged into.

I'm going to clean things up a bit by re-creating xen/blkfront with the
changes from xen/frontend.  If you rebase your patches onto xen/frontend
then I should be able to easily incorporate them into that cleanup.

Sorry for the mixup.

Thanks,
    J

Re: [PATCH 00 of 10] blkfront pvops updates

[Daniel Stodden]

On Thu, 2010-04-29 at 17:10 -0400, Jeremy Fitzhardinge wrote:
> On 04/29/2010 01:58 PM, Daniel Stodden wrote:
> > Ah, that's where the previous stuff ended up. I thought those probably
> > just got dropped.
> >   
> 
> I thought the changes in these patches looked somewhat familiar.
> 
> > Let's just scratch those. It's been months, so if they didn't make it
> > back then, they won't need to complicate stuff now either.
> >
> > So I should probably merge with xen/frontend. If those matter, they must
> > be on some branch which matters. Which one is it? I thought the one
> > which matters is xen/master? Is it xen/next?
> >   
> 
> The flow of changes is (ideally):
> 
>     <topic branch> -> xen/next -> xen/stable*
> 
> where "->" is merged into.

So I'm probably building the wrong tree by default.

I used to make linux-2.6-pvops when starting to work.
Which checks out xen/master.

How does this relate?

Daniel

> I'm going to clean things up a bit by re-creating xen/blkfront with the
> changes from xen/frontend.  If you rebase your patches onto xen/frontend
> then I should be able to easily incorporate them into that cleanup.
> 
> Sorry for the mixup.
> 
> Thanks,
>     J

Re: [PATCH 00 of 10] blkfront pvops updates

[Jeremy Fitzhardinge]

On 04/29/2010 02:28 PM, Daniel Stodden wrote:
> So I'm probably building the wrong tree by default.
>
> I used to make linux-2.6-pvops when starting to work.
> Which checks out xen/master.
>
> How does this relate?
>   

xen/master is an alias for xen/stable-2.6.31.x at the moment.  I just
submitted a patch to make this switchover explicit, and will move it to
2.6.32 soon.

    J

Re: [PATCH 00 of 10] blkfront pvops updates

[Daniel Stodden]

On Thu, 2010-04-29 at 17:36 -0400, Jeremy Fitzhardinge wrote:
> On 04/29/2010 02:28 PM, Daniel Stodden wrote:
> > So I'm probably building the wrong tree by default.
> >
> > I used to make linux-2.6-pvops when starting to work.
> > Which checks out xen/master.
> >
> > How does this relate?
> >   
> 
> xen/master is an alias for xen/stable-2.6.31.x at the moment.  I just
> submitted a patch to make this switchover explicit, and will move it to
> 2.6.32 soon.

Okay, so xen/master rather defaults to some stable tree.

If xen/frontend is the topic branch, people ideally prepare patches per
topic, and trunk is rather xen/next, then the thing I still don't follow
is than xen/frontend hasn't been merged since

* commit 8800a1de3df00fa994f72ad0d7c1eda9b5a0b514
| Author: Paolo Bonzini <pbonzini@redhat.com>
| Date:   Wed Jul 8 12:27:37 2009 +0200

Why would I want to build a kernel derived from last July's state? Or,
put differently: Is it okay to merge these topic branches before
anything else?

Daniel

Re: [PATCH 00 of 10] blkfront pvops updates

[Jeremy Fitzhardinge]

On 04/29/2010 04:05 PM, Daniel Stodden wrote:
> On Thu, 2010-04-29 at 17:36 -0400, Jeremy Fitzhardinge wrote:
>   
>> On 04/29/2010 02:28 PM, Daniel Stodden wrote:
>>     
>>> So I'm probably building the wrong tree by default.
>>>
>>> I used to make linux-2.6-pvops when starting to work.
>>> Which checks out xen/master.
>>>
>>> How does this relate?
>>>   
>>>       
>> xen/master is an alias for xen/stable-2.6.31.x at the moment.  I just
>> submitted a patch to make this switchover explicit, and will move it to
>> 2.6.32 soon.
>>     
> Okay, so xen/master rather defaults to some stable tree.
>
> If xen/frontend is the topic branch, people ideally prepare patches per
> topic, and trunk is rather xen/next, then the thing I still don't follow
> is than xen/frontend hasn't been merged since
>
> * commit 8800a1de3df00fa994f72ad0d7c1eda9b5a0b514
> | Author: Paolo Bonzini <pbonzini@redhat.com>
> | Date:   Wed Jul 8 12:27:37 2009 +0200
>
> Why would I want to build a kernel derived from last July's state? Or,
> put differently: Is it okay to merge these topic branches before
> anything else?
>   

My usual workflow is:

   1. Develop some patches on xen/next, until they work
   2. Rebase those patches onto a topic branch
   3. Merge that topic branch onto xen/next

This is necessary because an individual topic branch isn't necessarily
compilable into a working kernel (though they should always compile).

The topic branches are often based on very old kernel versions, and I
avoid rebasing or merging new kernels into them for as long as
possible.  This is so that they will share a common base version with
any other branch I want to merge them into (including mainline Linux),
so that 3-way merging is most likely to work.

It gets awkward if xen/next has other changes which conflict with the
topic branch either because some other piece of necessary infrastructure
is missing, or just plain merge conflicts.  If they're minor then I
resolve them in the merge to xen/next (git rerere is your friend for
remembering these kinds of conflict resolutions).   But if there's some
major subsystem change, then I'll update the topic branch by merging in
an appropriate branch to update the branch (ideally always from the
mainline Linux branch, or from another Xen topic branch).

    J

Re: Re: [PATCH 00 of 10] blkfront pvops updates

[Jeremy Fitzhardinge]

On 04/29/2010 04:05 PM, Daniel Stodden wrote:
> On Thu, 2010-04-29 at 17:36 -0400, Jeremy Fitzhardinge wrote:
>   
>> On 04/29/2010 02:28 PM, Daniel Stodden wrote:
>>     
>>> So I'm probably building the wrong tree by default.
>>>
>>> I used to make linux-2.6-pvops when starting to work.
>>> Which checks out xen/master.
>>>
>>> How does this relate?
>>>   
>>>       
>> xen/master is an alias for xen/stable-2.6.31.x at the moment.  I just
>> submitted a patch to make this switchover explicit, and will move it to
>> 2.6.32 soon.
>>     
> Okay, so xen/master rather defaults to some stable tree.
>
> If xen/frontend is the topic branch, people ideally prepare patches per
> topic, and trunk is rather xen/next, then the thing I still don't follow
> is than xen/frontend hasn't been merged since
>
> * commit 8800a1de3df00fa994f72ad0d7c1eda9b5a0b514
> | Author: Paolo Bonzini <pbonzini@redhat.com>
> | Date:   Wed Jul 8 12:27:37 2009 +0200
>   

BTW, it looks like you haven't updated your tree in a while.  That
branch has a few more changes on it since that one:

ad9ecc8ebfae8cab30ea2467b987f9b18b3172cc xen/blkfront: revalidate after setting capacity
7a634554e8ba2be858448bcf9617527c34ce326b xen/blkfront: avoid compiler warning from missing cases
065157270688777b934770fa4dcfcd68800560c3 xen/front: Propagate changed size of VBDs
1ebf91c24ab17ee62afeb1640278dd02da781fd7 blkfront: don't access freed struct xenbus_device
eba64a07039d46222ca9d157c8c0549f4177f53d xen: fbdev frontend needs xenbus frontend
eebc80638fe365a0386ed5ee57c7f0c6d5e56fb1 blkfront: fixes for 'xm block-detach ... --force'
71133087313f15db44ffb6ea802e5bdb2479a600 xen: use less generic names in blkfront driver.
9c9c87f53f87d0368ef04207cce4c92884f4ae3d xen: use less generic names in netfront driver.
11b84f69d36b518ff862a1bcd61ace7f8dac76b5 xen: wait up to 5 minutes for device connetion
ed9ef1e71122628b50b0a526607509511b0d9135 xen: improvement to wait_for_devices()

	J

Re: Re: [PATCH 00 of 10] blkfront pvops updates

[Daniel Stodden]

On Fri, 2010-04-30 at 14:13 -0400, Jeremy Fitzhardinge wrote:
> On 04/29/2010 04:05 PM, Daniel Stodden wrote:
> > On Thu, 2010-04-29 at 17:36 -0400, Jeremy Fitzhardinge wrote:
> >   
> >> On 04/29/2010 02:28 PM, Daniel Stodden wrote:
> >>     
> >>> So I'm probably building the wrong tree by default.
> >>>
> >>> I used to make linux-2.6-pvops when starting to work.
> >>> Which checks out xen/master.
> >>>
> >>> How does this relate?
> >>>   
> >>>       
> >> xen/master is an alias for xen/stable-2.6.31.x at the moment.  I just
> >> submitted a patch to make this switchover explicit, and will move it to
> >> 2.6.32 soon.
> >>     
> > Okay, so xen/master rather defaults to some stable tree.
> >
> > If xen/frontend is the topic branch, people ideally prepare patches per
> > topic, and trunk is rather xen/next, then the thing I still don't follow
> > is than xen/frontend hasn't been merged since
> >
> > * commit 8800a1de3df00fa994f72ad0d7c1eda9b5a0b514
> > | Author: Paolo Bonzini <pbonzini@redhat.com>
> > | Date:   Wed Jul 8 12:27:37 2009 +0200
> >   
> 
> BTW, it looks like you haven't updated your tree in a while.  That
> branch has a few more changes on it since that one:
> 
> ad9ecc8ebfae8cab30ea2467b987f9b18b3172cc xen/blkfront: revalidate after setting capacity
> 7a634554e8ba2be858448bcf9617527c34ce326b xen/blkfront: avoid compiler warning from missing cases
> 065157270688777b934770fa4dcfcd68800560c3 xen/front: Propagate changed size of VBDs
> 1ebf91c24ab17ee62afeb1640278dd02da781fd7 blkfront: don't access freed struct xenbus_device
> eba64a07039d46222ca9d157c8c0549f4177f53d xen: fbdev frontend needs xenbus frontend
> eebc80638fe365a0386ed5ee57c7f0c6d5e56fb1 blkfront: fixes for 'xm block-detach ... --force'
> 71133087313f15db44ffb6ea802e5bdb2479a600 xen: use less generic names in blkfront driver.
> 9c9c87f53f87d0368ef04207cce4c92884f4ae3d xen: use less generic names in netfront driver.
> 11b84f69d36b518ff862a1bcd61ace7f8dac76b5 xen: wait up to 5 minutes for device connetion
> ed9ef1e71122628b50b0a526607509511b0d9135 xen: improvement to wait_for_devices()

No, I certainly got those too. I was just surprised about where that
branch was rooted. Okay, so sort of a revision- vs. bare patch-
management thing then. Thanks for the clarification. 143 branches just
started to make sort of sense. :o)

Daniel

Re: [PATCH 00 of 10] blkfront pvops updates

[Daniel Stodden]

On Thu, 2010-04-29 at 16:11 -0400, Daniel Stodden wrote:
> On Thu, 2010-04-29 at 15:50 -0400, Jeremy Fitzhardinge wrote:
> > On 04/28/2010 07:19 PM, Daniel Stodden wrote:
> > > Hi.
> > >
> > > Here are couple of bugfixes for blkfront.
> > >
> > >  1. Bdev removal was slightly buggy. [Is nobody out there using
> > >     block-detach??] Also it seems like we'be been leaking the gendisk
> > >     structs for a long time.
> > >
> > >  2. Fixes most common occasions where disk open/close races against
> > >     backend state switches. This one didn't come for free:
> > > 	* xenbus_switch_state growing transactions again.
> > > 	* New mutex in blkfront_info
> > > 	* Bdev .open/.close rewritten accordingly.
> > >   
> > 
> > Thanks for this.  However, when I went to apply it, it looks like I've
> > made a mess of the various blkfront branches. 
> > 
> > xen/frontend has some changes from Jan and Ky which are merged into
> > xen/next and the various stable branches.
> > 
> > I also have xen/blkfront, which has your last set of changes, which I
> > seem to have forgotten to merge into anything else, and now conflicts
> > non-trivially with xen/blkfront.  I'd like to add this new set of
> > patches into xen/blkfront, but it doesn't apply cleanly.
> > 
> > Could you look at this and advise me what I should do?
> 
> Oh, NP. I'll try and see if I can gather those and what it takes to
> merge.

I moved on to xen/next. That still seems to apply okay.

But as usual when looking at that kind of thing a day later, I promptly
found 2 more loopholes I didn't see before. %}

These should be closed now too, but I'll rather rerun this stuff
tomorrow again before resubmitting.

Thanks,
Daniel