[PATCH] dm thin: fix table output when pool target disables discard passdown internally

[PATCH] dm thin: fix table output when pool target disables discard passdown internally

[Mike Snitzer]

When the thin pool target clears the discard_passdown parameter
internally, it incorrectly reports this to userspace on the table line.
This patch corrects this by no longer changing the table line to reflect
that discard passdown was disabled.

We can still tell when discard passdown is overridden by looking for the
message "Discard unsupported by data device: Disabling discard passdown."

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-thin.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
index 2fd87b5..d05fefd 100644
--- a/drivers/md/dm-thin.c
+++ b/drivers/md/dm-thin.c
@@ -497,6 +497,12 @@ struct pool_features {
 	unsigned zero_new_blocks:1;
 	unsigned discard_enabled:1;
 	unsigned discard_passdown:1;
+	/*
+	 * 'discard_passdown_original' preserves the value supplied to the
+	 * constructor that could be overridden internally if the data device
+	 * doesn't support discards.
+	 */
+	unsigned discard_passdown_original:1;
 };
 
 struct pool {
@@ -1650,6 +1656,7 @@ static void pool_features_init(struct pool_features *pf)
 	pf->zero_new_blocks = 1;
 	pf->discard_enabled = 1;
 	pf->discard_passdown = 1;
+	pf->discard_passdown_original = 1;
 }
 
 static void __pool_destroy(struct pool *pool)
@@ -1873,6 +1880,7 @@ static int parse_pool_features(struct dm_arg_set *as, struct pool_features *pf,
 			continue;
 		} else if (!strcasecmp(arg_name, "no_discard_passdown")) {
 			pf->discard_passdown = 0;
+			pf->discard_passdown_original = 0;
 			continue;
 		}
 
@@ -2385,7 +2393,7 @@ static int pool_status(struct dm_target *ti, status_type_t type,
 		       (unsigned long long)pt->low_water_blocks);
 
 		count = !pool->pf.zero_new_blocks + !pool->pf.discard_enabled +
-			!pool->pf.discard_passdown;
+			!pool->pf.discard_passdown_original;
 		DMEMIT("%u ", count);
 
 		if (!pool->pf.zero_new_blocks)
@@ -2394,7 +2402,7 @@ static int pool_status(struct dm_target *ti, status_type_t type,
 		if (!pool->pf.discard_enabled)
 			DMEMIT("ignore_discard ");
 
-		if (!pool->pf.discard_passdown)
+		if (!pool->pf.discard_passdown_original)
 			DMEMIT("no_discard_passdown ");
 
 		break;
-- 
1.7.4.4

[PATCH v2] dm thin: fix table output when pool target disables discard passdown internally

[Mike Snitzer]

When the thin pool target clears the discard_passdown parameter
internally, it incorrectly reports this to userspace on the table line.
This patch corrects this by no longer changing the table line to reflect
that discard passdown was disabled.

We can still tell when discard passdown is overridden by looking for the
message "Discard unsupported by data device (sdX): Disabling discard passdown."

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-thin.c |   32 +++++++++++++++++---------------
 1 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
index 2fd87b5..eb3d138 100644
--- a/drivers/md/dm-thin.c
+++ b/drivers/md/dm-thin.c
@@ -1632,6 +1632,21 @@ static int bind_control_target(struct pool *pool, struct dm_target *ti)
 	pool->low_water_blocks = pt->low_water_blocks;
 	pool->pf = pt->pf;
 
+	/*
+	 * If discard_passdown was enabled verify that the data device
+	 * supports discards.  Disable discard_passdown if not; otherwise
+	 * -EOPNOTSUPP will be returned.
+	 */
+	if (pt->pf.discard_passdown) {
+		struct request_queue *q = bdev_get_queue(pt->data_dev->bdev);
+		if (!q || !blk_queue_discard(q)) {
+			char buf[BDEVNAME_SIZE];
+			DMWARN("Discard unsupported by data device (%s): Disabling discard passdown.",
+			       bdevname(pt->data_dev->bdev, buf));
+			pool->pf.discard_passdown = 0;
+		}
+	}
+
 	return 0;
 }
 
@@ -1988,19 +2003,6 @@ static int pool_ctr(struct dm_target *ti, unsigned argc, char **argv)
 		goto out_flags_changed;
 	}
 
-	/*
-	 * If discard_passdown was enabled verify that the data device
-	 * supports discards.  Disable discard_passdown if not; otherwise
-	 * -EOPNOTSUPP will be returned.
-	 */
-	if (pf.discard_passdown) {
-		struct request_queue *q = bdev_get_queue(data_dev->bdev);
-		if (!q || !blk_queue_discard(q)) {
-			DMWARN("Discard unsupported by data device: Disabling discard passdown.");
-			pf.discard_passdown = 0;
-		}
-	}
-
 	pt->pool = pool;
 	pt->ti = ti;
 	pt->metadata_dev = metadata_dev;
@@ -2385,7 +2387,7 @@ static int pool_status(struct dm_target *ti, status_type_t type,
 		       (unsigned long long)pt->low_water_blocks);
 
 		count = !pool->pf.zero_new_blocks + !pool->pf.discard_enabled +
-			!pool->pf.discard_passdown;
+			!pt->pf.discard_passdown;
 		DMEMIT("%u ", count);
 
 		if (!pool->pf.zero_new_blocks)
@@ -2394,7 +2396,7 @@ static int pool_status(struct dm_target *ti, status_type_t type,
 		if (!pool->pf.discard_enabled)
 			DMEMIT("ignore_discard ");
 
-		if (!pool->pf.discard_passdown)
+		if (!pt->pf.discard_passdown)
 			DMEMIT("no_discard_passdown ");
 
 		break;

Re: [PATCH v2] dm thin: fix table output when pool target disables discard passdown internally

[Joe Thornber]

On Fri, May 18, 2012 at 11:04:01AM -0400, Mike Snitzer wrote:
> When the thin pool target clears the discard_passdown parameter
> internally, it incorrectly reports this to userspace on the table line.
> This patch corrects this by no longer changing the table line to reflect
> that discard passdown was disabled.
> 
> We can still tell when discard passdown is overridden by looking for the
> message "Discard unsupported by data device (sdX): Disabling discard passdown."
> 
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>

Ack