commit a5664da "dm ioctl: make bio or request based device type immutable"
prevented "dmsetup wape_table" change the target type to "error".
-v2: setup md->queue even target type is "error".
Signed-off-by: Joe Jin <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
> [email protected]
> https://www.redhat.com/mailman/listinfo/dm-devel
>
On Wed, Aug 21 2013 at 10:48am -0400,
Mikulas Patocka <[email protected]> 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.
My thoughts _exactly_. This patch is very confused.
Joe, what are you looking to be able to do? Switch a dm-multipath
device to error? Or allowing switching a target that has
DM_TARGET_IMMUTABLE flag set to be switched to error target?
The latter restriction was introduced with commit 36a0456fb ("dm table:
add immutable feature").
Mike
On 08/21/13 23:06, Mike Snitzer wrote:
> On Wed, Aug 21 2013 at 10:48am -0400,
> Mikulas Patocka <[email protected]> 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.
>
> My thoughts _exactly_. This patch is very confused.
>
> Joe, what are you looking to be able to do? Switch a dm-multipath
> device to error? Or allowing switching a target that has
> DM_TARGET_IMMUTABLE flag set to be switched to error target?
>
> The latter restriction was introduced with commit 36a0456fb ("dm table:
> add immutable feature").
Hi Mike,
So far dmsetup support wipe_table:
https://bugzilla.redhat.com/show_bug.cgi?id=742607
As description in the bug Doc Text, "This could be useful, for example,
if a long-running process keeps a device open after it has finished using
it and you need to release the underlying devices before that process exits."
After apply the commit, wipe_table no long works.
Thanks,
Joe
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 <[email protected]>
---
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 <[email protected]>
>> ---
>> 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
>> [email protected]
>> https://www.redhat.com/mailman/listinfo/dm-devel
>>
On Wed, Aug 21 2013 at 10:40pm -0400,
Joe Jin <[email protected]> wrote:
> On 08/21/13 23:06, Mike Snitzer wrote:
> > On Wed, Aug 21 2013 at 10:48am -0400,
> > Mikulas Patocka <[email protected]> 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.
> >
> > My thoughts _exactly_. This patch is very confused.
> >
> > Joe, what are you looking to be able to do? Switch a dm-multipath
> > device to error? Or allowing switching a target that has
> > DM_TARGET_IMMUTABLE flag set to be switched to error target?
> >
> > The latter restriction was introduced with commit 36a0456fb ("dm table:
> > add immutable feature").
>
> Hi Mike,
>
> So far dmsetup support wipe_table:
> https://bugzilla.redhat.com/show_bug.cgi?id=742607
> As description in the bug Doc Text, "This could be useful, for example,
> if a long-running process keeps a device open after it has finished using
> it and you need to release the underlying devices before that process exits."
>
> After apply the commit, wipe_table no long works.
Well, it never _really_ worked even before that commit because it was
switching from request-based to bio-based. Some queued requests
could've easily slipped through the cracks.
But I now understand what it is you want to be able to do.
On Wed, Aug 21 2013 at 10:50pm -0400,
Joe Jin <[email protected]> wrote:
> 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 <[email protected]>
> ---
> 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
Hi Joe,
Unfortunately this isn't going to work properly. Mikulas suggested a
new "error-rq" target.
I do like the idea of a single error target that is hybrid (supports
both bio-based and request-based) but the DM core would need to be
updated to support this.
Specifically, we'd need to check if the device (and active table) is
already bio-based or request-based and select the appropriate type. If
it is a new device, default to selecting bio-based.
There are some wrappers and other logic thoughout DM core that will need
auditing too.
Mike
On Thu, Aug 22 2013 at 4:19pm -0400,
Mike Snitzer <[email protected]> wrote:
> Hi Joe,
>
> Unfortunately this isn't going to work properly. Mikulas suggested a
> new "error-rq" target.
>
> I do like the idea of a single error target that is hybrid (supports
> both bio-based and request-based) but the DM core would need to be
> updated to support this.
>
> Specifically, we'd need to check if the device (and active table) is
> already bio-based or request-based and select the appropriate type. If
> it is a new device, default to selecting bio-based.
>
> There are some wrappers and other logic thoughout DM core that will need
> auditing too.
Here is a patch that should work for your needs (I tested it to work
with 'dmsetup wipe_table' on both request-based and bio-based devices):
From: Mike Snitzer <[email protected]>
Date: Thu, 22 Aug 2013 18:21:38 -0400
Subject: [PATCH] dm: allow error target to replace either bio-based and request-based targets
In may be useful to switch a request-based table to the "error" target.
Enhance the DM core to allow a single hybrid target to be capable of
handling either bios or requests.
Add a request-based (.map_rq) member to the error target_type and train
dm_table_set_type() to prefer the md's established type (request-based
or bio-based). If the md doesn't have an established type default to
making the hybrid target bio-based.
Cc: Joe Jin <[email protected]>
Cc: Mikulas Patocka <[email protected]>
Signed-off-by: Mike Snitzer <[email protected]>
---
drivers/md/dm-table.c | 18 +++++++++++++++++-
drivers/md/dm-target.c | 9 ++++++++-
drivers/md/dm.h | 11 +++++++++++
3 files changed, 36 insertions(+), 2 deletions(-)
Index: linux/drivers/md/dm-table.c
===================================================================
--- linux.orig/drivers/md/dm-table.c
+++ linux/drivers/md/dm-table.c
@@ -864,10 +864,26 @@ static int dm_table_set_type(struct dm_t
struct dm_target *tgt;
struct dm_dev_internal *dd;
struct list_head *devices;
+ unsigned live_md_type;
+
+ dm_lock_md_type(t->md);
+ live_md_type = dm_get_md_type(t->md);
+ dm_unlock_md_type(t->md);
for (i = 0; i < t->num_targets; i++) {
tgt = t->targets + i;
- if (dm_target_request_based(tgt))
+ if (dm_target_hybrid(tgt)) {
+ switch (live_md_type) {
+ case DM_TYPE_NONE:
+ case DM_TYPE_BIO_BASED:
+ bio_based = 1;
+ break;
+ case DM_TYPE_REQUEST_BASED:
+ request_based = 1;
+ break;
+ }
+ }
+ else if (dm_target_request_based(tgt))
request_based = 1;
else
bio_based = 1;
Index: linux/drivers/md/dm-target.c
===================================================================
--- linux.orig/drivers/md/dm-target.c
+++ linux/drivers/md/dm-target.c
@@ -131,12 +131,19 @@ static int io_err_map(struct dm_target *
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},
+ .version = {1, 2, 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)
Index: linux/drivers/md/dm.h
===================================================================
--- linux.orig/drivers/md/dm.h
+++ linux/drivers/md/dm.h
@@ -91,10 +91,21 @@ int dm_setup_md_queue(struct mapped_devi
#define dm_target_is_valid(t) ((t)->table)
/*
+ * To check whether the target type is bio-based or not (request-based).
+ */
+#define dm_target_bio_based(t) ((t)->type->map != NULL)
+
+/*
* To check whether the target type is request-based or not (bio-based).
*/
#define dm_target_request_based(t) ((t)->type->map_rq != NULL)
+/*
+ * To check whether the target type is a hybrid (capable of being
+ * either request-based or bio-based).
+ */
+#define dm_target_hybrid(t) (dm_target_bio_based(t) && dm_target_request_based(t))
+
/*-----------------------------------------------------------------
* A registry of target types.
*---------------------------------------------------------------*/
On 08/23/13 08:17, Mike Snitzer wrote:
> Here is a patch that should work for your needs (I tested it to work
> with 'dmsetup wipe_table' on both request-based and bio-based devices):
This really what I looking for, thanks!
>
> From: Mike Snitzer <[email protected]>
> Date: Thu, 22 Aug 2013 18:21:38 -0400
> Subject: [PATCH] dm: allow error target to replace either bio-based and request-based targets
>
> In may be useful to switch a request-based table to the "error" target.
> Enhance the DM core to allow a single hybrid target to be capable of
> handling either bios or requests.
>
> Add a request-based (.map_rq) member to the error target_type and train
> dm_table_set_type() to prefer the md's established type (request-based
> or bio-based). If the md doesn't have an established type default to
> making the hybrid target bio-based.
Signed-off-by: Joe Jin <[email protected]>
Thanks,
Joe
Hello Mike,
On 08/23/13 09:17, Mike Snitzer wrote:
>> I do like the idea of a single error target that is hybrid (supports
>> both bio-based and request-based) but the DM core would need to be
>> updated to support this.
>>
>> Specifically, we'd need to check if the device (and active table) is
>> already bio-based or request-based and select the appropriate type. If
>> it is a new device, default to selecting bio-based.
>>
>> There are some wrappers and other logic thoughout DM core that will need
>> auditing too.
>
> Here is a patch that should work for your needs (I tested it to work
> with 'dmsetup wipe_table' on both request-based and bio-based devices):
How about moving the default handling in dm_table_set_type() outside of
the for-each-target loop, like the modified patch below?
For example, if a table has 2 targets, hybrid and request_based,
and live_md_type is DM_TYPE_NONE, the table should be considered as
request_based, not inconsistent.
Though the end result is same as such a table is rejected by other
constraint anyway, I think it's good to keep the semantics clean
and error messages consistent.
I.e. for the above case, the error message should be
"Request-based dm doesn't support multiple targets yet",
not "Inconsistent table: different target types can't be mixed up".
---
Jun'ichi Nomura, NEC Corporation
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index f221812..6e683c8 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -860,14 +860,16 @@ EXPORT_SYMBOL(dm_consume_args);
static int dm_table_set_type(struct dm_table *t)
{
unsigned i;
- unsigned bio_based = 0, request_based = 0;
+ unsigned bio_based = 0, request_based = 0, hybrid = 0;
struct dm_target *tgt;
struct dm_dev_internal *dd;
struct list_head *devices;
for (i = 0; i < t->num_targets; i++) {
tgt = t->targets + i;
- if (dm_target_request_based(tgt))
+ if (dm_target_hybrid(tgt))
+ hybrid = 1;
+ else if (dm_target_request_based(tgt))
request_based = 1;
else
bio_based = 1;
@@ -879,6 +881,25 @@ static int dm_table_set_type(struct dm_table *t)
}
}
+ if (hybrid && !bio_based && !request_based) {
+ /*
+ * The targets can work either way.
+ * Determine the type from the live device.
+ */
+ unsigned live_md_type;
+ dm_lock_md_type(t->md);
+ live_md_type = dm_get_md_type(t->md);
+ dm_unlock_md_type(t->md);
+ switch (live_md_type) {
+ case DM_TYPE_REQUEST_BASED:
+ request_based = 1;
+ break;
+ default:
+ bio_based = 1;
+ break;
+ }
+ }
+
if (bio_based) {
/* We must use this table as bio-based */
t->type = DM_TYPE_BIO_BASED;
diff --git a/drivers/md/dm-target.c b/drivers/md/dm-target.c
index 37ba5db..242e3ce 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},
+ .version = {1, 2, 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)
diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index 45b97da..8b4c075 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -89,10 +89,21 @@ int dm_setup_md_queue(struct mapped_device *md);
#define dm_target_is_valid(t) ((t)->table)
/*
+ * To check whether the target type is bio-based or not (request-based).
+ */
+#define dm_target_bio_based(t) ((t)->type->map != NULL)
+
+/*
* To check whether the target type is request-based or not (bio-based).
*/
#define dm_target_request_based(t) ((t)->type->map_rq != NULL)
+/*
+ * To check whether the target type is a hybrid (capable of being
+ * either request-based or bio-based).
+ */
+#define dm_target_hybrid(t) (dm_target_bio_based(t) && dm_target_request_based(t))
+
/*-----------------------------------------------------------------
* A registry of target types.
*---------------------------------------------------------------*/
On Thu, Aug 22 2013 at 11:14pm -0400,
Jun'ichi Nomura <[email protected]> wrote:
> Hello Mike,
>
> On 08/23/13 09:17, Mike Snitzer wrote:
> >> I do like the idea of a single error target that is hybrid (supports
> >> both bio-based and request-based) but the DM core would need to be
> >> updated to support this.
> >>
> >> Specifically, we'd need to check if the device (and active table) is
> >> already bio-based or request-based and select the appropriate type. If
> >> it is a new device, default to selecting bio-based.
> >>
> >> There are some wrappers and other logic thoughout DM core that will need
> >> auditing too.
> >
> > Here is a patch that should work for your needs (I tested it to work
> > with 'dmsetup wipe_table' on both request-based and bio-based devices):
>
> How about moving the default handling in dm_table_set_type() outside of
> the for-each-target loop, like the modified patch below?
>
> For example, if a table has 2 targets, hybrid and request_based,
> and live_md_type is DM_TYPE_NONE, the table should be considered as
> request_based, not inconsistent.
> Though the end result is same as such a table is rejected by other
> constraint anyway, I think it's good to keep the semantics clean
> and error messages consistent.
>
> I.e. for the above case, the error message should be
> "Request-based dm doesn't support multiple targets yet",
> not "Inconsistent table: different target types can't be mixed up".
Hi,
Looks good, I'll fold your changes in and add your Signed-off-by.
Thanks,
Mike