Re: [dm-devel] [PATCH v2] dm ioctl: allow change device target type to error

[Joe Jin <joe.jin@oracle.com>]

Mikulas, thanks for you suggestions, I create new patch, can you please help
review?

Subject: dm: add map_rq define for error

commit a5664da "dm ioctl: make bio or request based device type immutable"
prevented "dmsetup wape_table" change the target type to "error" for there
is not map_rq for error target type.

Signed-off-by: Joe Jin <joe.jin@oracle.com>
---
 drivers/md/dm-target.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/md/dm-target.c b/drivers/md/dm-target.c
index 37ba5db..b690910 100644
--- a/drivers/md/dm-target.c
+++ b/drivers/md/dm-target.c
@@ -131,12 +131,19 @@ static int io_err_map(struct dm_target *tt, struct bio *bio)
 	return -EIO;
 }
 
+static int io_err_map_rq(struct dm_target *ti, struct request *clone,
+			 union map_info *map_context)
+{
+	return -EIO;
+}
+
 static struct target_type error_target = {
 	.name = "error",
 	.version = {1, 1, 0},
 	.ctr  = io_err_ctr,
 	.dtr  = io_err_dtr,
 	.map  = io_err_map,
+	.map_rq = io_err_map_rq,
 };
 
 int __init dm_target_init(void)
-- 
1.8.3.1

On 08/21/13 22:48, Mikulas Patocka wrote:
> 
> 
> On Wed, 21 Aug 2013, Joe Jin wrote:
> 
>> commit a5664da "dm ioctl: make bio or request based device type immutable"
>> prevented "dmsetup wape_table" change the target type to "error".
> 
> That commit a5664da is there for a reason (it is not possible to change 
> bio-based device to request-based and vice versa) and I don't really see 
> how this patch is supposed to work.
> 
> If there are bios that are in flight and that already passed through 
> blk_queue_bio, and you change the device from request-based to bio-based, 
> what are you going to do with them? - The patch doesn't do anything about 
> it.
> 
> A better approach would be to create a new request-based target "error-rq" 
> and change the multipath target to "error-rq" target. That way, you don't 
> have to change device type from request based to bio based.
> 
> Mikulas
> 
>> -v2: setup md->queue even target type is "error".
>>
>> Signed-off-by: Joe Jin <joe.jin@oracle.com>
>> ---
>>  drivers/md/dm-ioctl.c |  4 ++++
>>  drivers/md/dm-table.c | 12 ++++++++++++
>>  drivers/md/dm.h       |  1 +
>>  3 files changed, 17 insertions(+)
>>
>> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
>> index f1b7586..2a9b63d 100644
>> --- a/drivers/md/dm-ioctl.c
>> +++ b/drivers/md/dm-ioctl.c
>> @@ -1280,6 +1280,9 @@ static int table_load(struct dm_ioctl *param, size_t param_size)
>>  		goto out;
>>  	}
>>  
>> +	if (dm_is_error_target(t))
>> +		goto error_target;
>> +
>>  	/* Protect md->type and md->queue against concurrent table loads. */
>>  	dm_lock_md_type(md);
>>  	if (dm_get_md_type(md) == DM_TYPE_NONE)
>> @@ -1293,6 +1296,7 @@ static int table_load(struct dm_ioctl *param, size_t param_size)
>>  		goto out;
>>  	}
>>  
>> +error_target:
>>  	/* setup md->queue to reflect md's type (may block) */
>>  	r = dm_setup_md_queue(md);
>>  	if (r) {
>> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
>> index f221812..27be46a 100644
>> --- a/drivers/md/dm-table.c
>> +++ b/drivers/md/dm-table.c
>> @@ -184,6 +184,18 @@ static int alloc_targets(struct dm_table *t, unsigned int num)
>>  	return 0;
>>  }
>>  
>> +bool dm_is_error_target(struct dm_table *t)
>> +{
>> +	unsigned i;
>> +
>> +	for (i = 0; i < t->num_targets; i++) {
>> +		struct dm_target *tgt = t->targets + i;
>> +		if (strcmp(tgt->type->name, "error") == 0)
>> +			return true;
>> +	}
>> +	return false;
>> +}
>> +
>>  int dm_table_create(struct dm_table **result, fmode_t mode,
>>  		    unsigned num_targets, struct mapped_device *md)
>>  {
>> diff --git a/drivers/md/dm.h b/drivers/md/dm.h
>> index 45b97da..c7bceeb 100644
>> --- a/drivers/md/dm.h
>> +++ b/drivers/md/dm.h
>> @@ -69,6 +69,7 @@ unsigned dm_table_get_type(struct dm_table *t);
>>  struct target_type *dm_table_get_immutable_target_type(struct dm_table *t);
>>  bool dm_table_request_based(struct dm_table *t);
>>  bool dm_table_supports_discards(struct dm_table *t);
>> +bool dm_is_error_target(struct dm_table *t);
>>  int dm_table_alloc_md_mempools(struct dm_table *t);
>>  void dm_table_free_md_mempools(struct dm_table *t);
>>  struct dm_md_mempools *dm_table_get_md_mempools(struct dm_table *t);
>> -- 
>> 1.8.3.1
>>
>> --
>> dm-devel mailing list
>> dm-devel@redhat.com
>> https://www.redhat.com/mailman/listinfo/dm-devel
>>