[dm-devel] 答复: Re: [PATCH] libmultipath: fix multipath -q command logic

[tang . junhui]

Hello Hannes,
check_daemon() now is only used to determine whether to set/remove 
queue_if_no_path feature for
mapped devices, which you said maybe another issue, we are looking forward 
for your patch.

Thanks,
Tang



发件人: Hannes Reinecke 
收件人: tang.jun...@zte.com.cn, bmarz...@redhat.com, 
抄送:   Bart Van Assche , device-mapper 
development , zhang.ka...@zte.com.cn
日期:   2016/10/11 18:42
主题:   Re: [dm-devel] [PATCH] libmultipath: fix multipath -q command 
logic
发件人: dm-devel-boun...@redhat.com



On 10/11/2016 11:17 AM, tang.jun...@zte.com.cn wrote:
> Hello Hannes, Ben,
> Could you have a review for this patch, any comment will be highly
> appreciated.
>
> Thanks,
> Tang
>
>
>
>
> 发件人: Christophe Varoqui 
> 收件人: tang.jun...@zte.com.cn,
> 抄送:Bart Van Assche , device-mapper
> development , zhang.ka...@zte.com.cn
> 日期: 2016/10/11 14:59
> 主题:Re: [dm-devel] [PATCH] libmultipath: fix multipath -q
> command logic
> 发件人:dm-devel-boun...@redhat.com
> 
>
>
>
> Hannes, Ben,
>
> are you ok with the solution to these two issues.
> Seems sane to me.
>
This actually is only part of the story.

The whole idea of issuing 'multipath' is to check if a given path 
_should_ be multipathed (as this is typically called from an udev event).
But as it's called from an udev event we cannot rely on the multipath 
daemon to be started; we might just handle an event which came in before 
multipathd got started from systemd.
So checking for the PID file is not enough, we need to check if the 
daemon will be started eventually.
And in fact checking the PID file or calling mpath_connect() is 
equivalent, with the added advantage the mpath_connect() will start the 
daemon _if enabled by systemd_.
So this patch doesn't help much, as it doesn't solve the main problem of 
figuring out if multipathd _should_ be started.

I've done a patch for checking the '.wants' directories from systemd, 
but this obviously will only work if the OS is systemd-based.
And it's not really perfect, as there are corner-cases where just 
checking for the .wants directory is not enough.

Cheers,

Hannes
-- 
Dr. Hannes ReineckezSeries & Storage
h...@suse.de   +49 911 
74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 0/9] Additional checks for strace DM ioctl decoder test

[Eugene Syromyatnikov]

Hello.

Aside from additional checks themselves, this patchset also contains two
notable changes:
 * Fix for the previous patchset - misplaced comma printing ("dm: Fix comma
   printing for the case when dm_target_msg structure is inaccessible").
 * Update of printstr_ex call, which enables proper handling of
   QUOTE_0_TERMINATED user style (it prints cropped string without ellipsis
   otherwise).

Eugene Syromyatnikov (9):
  util: Add support for QUOTE_0_TERMINATED in user_style to
ptrintstr_ex
  tests: Add check for printing of overlength strings to ioctl_dm test
  tests: Add check for presence of HAVE_LINUX_DM_IOCTL_H macro
definition to ioctl_dm test
  tests/ioctl_dm: whitespace
  dm: Fix comma printing for the case when dm_target_msg structure is
inaccessible
  tests/ioctl_dm: overly long string printing checks
  tests: Some additional checks for ioctl_dm test
  tests: Add ioctl_dm to .gitignore
  tests: Add checks for abbreviated DM ioctl output

 dm.c  |4 +-
 tests/.gitignore  |2 +
 tests/Makefile.am |2 +
 tests/ioctl_dm-v.c|2 +
 tests/ioctl_dm-v.test |   12 +
 tests/ioctl_dm.c  |  653 +++--
 tests/ioctl_dm.test   |2 +-
 util.c|   18 +-
 8 files changed, 674 insertions(+), 21 deletions(-)
 create mode 100644 tests/ioctl_dm-v.c
 create mode 100755 tests/ioctl_dm-v.test

-- 
1.7.10.4

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 4/9] tests/ioctl_dm: whitespace

[Eugene Syromyatnikov]

---
 tests/ioctl_dm.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/ioctl_dm.c b/tests/ioctl_dm.c
index 5f2689c..261983c 100644
--- a/tests/ioctl_dm.c
+++ b/tests/ioctl_dm.c
@@ -24,7 +24,8 @@ static struct s {
} u;
 } s;
 
-static void init_s(struct dm_ioctl *s, size_t size, size_t offs)
+static void
+init_s(struct dm_ioctl *s, size_t size, size_t offs)
 {
memset(s, 0, size);
s->version[0] = DM_VERSION_MAJOR;
-- 
1.7.10.4

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] multipathd: fix issue in 'map $map getprstatus' reply

[peng . liang5]

Please have a review for this patch, hope for your comments.

Thanks


发件人: peng.lia...@zte.com.cn
收件人: christophe varoqui , 
抄送:   zhang.ka...@zte.com.cn, dm-devel@redhat.com, peng liang 

日期:   2016-08-04 15:31
主题:   [dm-devel] [PATCH] multipathd: fix issue in 'map $map getprstatus' 
reply
发件人: dm-devel-boun...@redhat.com



From: peng liang 

-add missing newline to 'map|multipath $map getprstatus' reply
-use asprintf instead of sprintf

Signed-off-by: peng liang 
---
 multipathd/cli_handlers.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 8ff4362..16445ea 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -1,6 +1,9 @@
 /*
  * Copyright (c) 2005 Christophe Varoqui
  */
+#define _GNU_SOURCE
+
+#include 
 #include "checkers.h"
 #include "memory.h"
 #include "vector.h"
@@ -1285,14 +1288,9 @@ cli_getprstatus (void * v, char ** reply, int * 
len, void * data)
 
 condlog(3, "%s: prflag = %u", param, (unsigned 
int)mpp->prflag);
 
-*reply =(char *)malloc(2);
-*len = 2;
-memset(*reply,0,2);
-
-
-sprintf(*reply,"%d",mpp->prflag);
-(*reply)[1]='\0';
-
+*len = asprintf(reply, "%d\n", mpp->prflag);
+if (*len < 0)
+return 1;
 
 condlog(3, "%s: reply = %s", param, *reply);
 
-- 
2.8.1.windows.1

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] 答复: [RFC] unreleased lock after handler failure

[tang . junhui]

Hi, Michael Wang

This patch looks well for me, cleanup_lock() should be called without
affected by the return value of h->fn().

Regards,
Tang



发件人: Michael Wang 
收件人: christophe.varo...@opensvc.com, dm-devel@redhat.com, 
日期:   2016/10/12 16:11
主题:   [dm-devel] [RFC] unreleased lock after handler failure
发件人: dm-devel-boun...@redhat.com



Hi, folks

While we are testing with the latest multipathd, we encounter issue that 
once
'del map' failed, all the following operation will show 'error -1 
receiving packet'
whatever how long the timeout has been set (we have applied the latest fix 
which
make sure the poll will last for 10 seconds).

After some debugging we found that inside parse_cmd(), the 
pthread_cleanup_pop()
rely on '!r' as indicator for locked or not, while this will be 
overwritten if
the handler return failed (1 in our case), and then the unlock will be 
missed.

After applied below fix the problem got solved, although the del map 
failed, the
other operation can still works.

Could you please help to review whether this is really the problem to be 
fixed
like that?

Regards,
Michael Wang



diff --git a/multipathd/cli.c b/multipathd/cli.c
index e8a9384..50161be 100644
--- a/multipathd/cli.c
+++ b/multipathd/cli.c
@@ -481,6 +481,7 @@ parse_cmd (char * cmd, char ** reply, int * len, void 
* data, int timeout )
tmo.tv_sec = 0;
}
if (h->locked) {
+   int locked = 0;
struct vectors * vecs = (struct vectors *)data;
 
pthread_cleanup_push(cleanup_lock, >lock);
@@ -491,10 +492,11 @@ parse_cmd (char * cmd, char ** reply, int * len, 
void * data, int timeout )
r = 0;
}
if (r == 0) {
+   locked = 1;
pthread_testcancel();
r = h->fn(cmdvec, reply, len, data);
}
-   pthread_cleanup_pop(!r);
+   pthread_cleanup_pop(locked);
} else
r = h->fn(cmdvec, reply, len, data);
free_keys(cmdvec);

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 5/9] dm: Fix comma printing for the case when dm_target_msg structure is inaccessible

[Eugene Syromyatnikov]

---
 dm.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/dm.c b/dm.c
index d846233..b1d455c 100644
--- a/dm.c
+++ b/dm.c
@@ -342,10 +342,12 @@ dm_decode_dm_target_msg(struct tcb *tcp, unsigned long 
addr,
offset + target_msg_message_offs <= ioc->data_size) {
struct dm_target_msg s;
 
+   tprints(", ");
+
if (umove_or_printaddr(tcp, addr + offset, ))
return;
 
-   tprintf(", {sector=%" PRI__u64 ", message=", s.sector);
+   tprintf("{sector=%" PRI__u64 ", message=", s.sector);
printstr_ex(tcp, addr + offset + target_msg_message_offs,
ioc->data_size - offset - target_msg_message_offs,
QUOTE_0_TERMINATED);
-- 
1.7.10.4

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 9/9] tests: Add checks for abbreviated DM ioctl output

[Eugene Syromyatnikov]

---
 tests/.gitignore  |1 +
 tests/Makefile.am |2 +
 tests/ioctl_dm-v.c|2 +
 tests/ioctl_dm-v.test |   12 
 tests/ioctl_dm.c  |  178 +++--
 tests/ioctl_dm.test   |2 +-
 6 files changed, 160 insertions(+), 37 deletions(-)
 create mode 100644 tests/ioctl_dm-v.c
 create mode 100755 tests/ioctl_dm-v.test

diff --git a/tests/.gitignore b/tests/.gitignore
index 9045117..b2c5434 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -104,6 +104,7 @@ inet-cmsg
 ioctl
 ioctl_block
 ioctl_dm
+ioctl_dm-v
 ioctl_evdev
 ioctl_evdev-v
 ioctl_mtd
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 2405415..61b6db7 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -164,6 +164,7 @@ check_PROGRAMS = \
ioctl \
ioctl_block \
ioctl_dm \
+   ioctl_dm-v \
ioctl_evdev \
ioctl_evdev-v \
ioctl_mtd \
@@ -513,6 +514,7 @@ DECODER_TESTS = \
ioctl.test \
ioctl_block.test \
ioctl_dm.test \
+   ioctl_dm-v.test \
ioctl_evdev.test \
ioctl_evdev-v.test \
ioctl_mtd.test \
diff --git a/tests/ioctl_dm-v.c b/tests/ioctl_dm-v.c
new file mode 100644
index 000..d95058f
--- /dev/null
+++ b/tests/ioctl_dm-v.c
@@ -0,0 +1,2 @@
+#define VERBOSE 1
+#include "ioctl_dm.c"
diff --git a/tests/ioctl_dm-v.test b/tests/ioctl_dm-v.test
new file mode 100755
index 000..4f6d64c
--- /dev/null
+++ b/tests/ioctl_dm-v.test
@@ -0,0 +1,12 @@
+#!/bin/sh
+
+# Check abbreviated decoding of DM* ioctls.
+
+. "${srcdir=.}/init.sh"
+
+run_prog > /dev/null
+run_strace -a16 -s9 -veioctl $args > "$EXP"
+check_prog grep
+grep -v '^ioctl([012],' < "$LOG" > "$OUT"
+match_diff "$OUT" "$EXP"
+rm -f "$EXP" "$OUT"
diff --git a/tests/ioctl_dm.c b/tests/ioctl_dm.c
index 0b2c5a7..2fcd430 100644
--- a/tests/ioctl_dm.c
+++ b/tests/ioctl_dm.c
@@ -11,6 +11,10 @@
 # include 
 # include 
 
+# ifndef VERBOSE
+#  define VERBOSE 0
+# endif
+
 # define STR32 "AbCdEfGhIjKlMnOpQrStUvWxYz012345"
 
 static const char str129[] = STR32 STR32 STR32 STR32 "6";
@@ -102,6 +106,7 @@ init_dm_target_spec(struct dm_target_spec *ptr, uint32_t id)
ptr->target_type[id % (sizeof(ptr->target_type) + 1)] = '\0';
 }
 
+# if VERBOSE
 static void
 print_dm_target_spec(struct dm_target_spec *ptr, uint32_t id)
 {
@@ -112,6 +117,7 @@ print_dm_target_spec(struct dm_target_spec *ptr, uint32_t 
id)
   (int) (id % (sizeof(ptr->target_type) + 1)),
   str129 + id % (sizeof(str129) - sizeof(ptr->target_type)));
 }
+# endif /* VERBOSE */
 
 # define ARG_STR(_arg) (_arg), #_arg
 
@@ -303,9 +309,14 @@ main(void)
printf("ioctl(-1, DM_TABLE_LOAD, "
   "{version=4.1.2, data_size=%u, data_start=%u, "
   "dev=makedev(18, 52), name=\"nnn\", uuid=\"uuu\", "
-  "target_count=1, flags=0, {sector_start=16, "
-  "length=32, target_type=\"tgt\", string=\"tparams\"}}) = "
-  "-1 EBADF (%m)\n", s.ioc.data_size, s.ioc.data_start);
+  "target_count=1, flags=0, "
+# if VERBOSE
+  "{sector_start=16, length=32, target_type=\"tgt\", "
+  "string=\"tparams\"}"
+# else /* !VERBOSE */
+  "..."
+# endif /* VERBOSE */
+  "}) = -1 EBADF (%m)\n", s.ioc.data_size, s.ioc.data_start);
 
/* No targets */
init_s(dm_arg, sizeof(*dm_arg) - sizeof(dm_arg->data),
@@ -328,8 +339,12 @@ main(void)
   "{version=4.1.2, data_size=%zu, data_start=%u, "
   "dev=makedev(18, 52), name=\"nnn\", uuid=\"uuu\", "
   "target_count=1234, flags=0, "
-  "/* misplaced struct dm_target_spec */ ...}) = -1 EBADF (%m)\n",
-  sizeof(*dm_arg), 0xfff8);
+# if VERBOSE
+  "/* misplaced struct dm_target_spec */ ..."
+# else /* !VERBOSE */
+  "..."
+# endif /* VERBOSE */
+  "}) = -1 EBADF (%m)\n", sizeof(*dm_arg), 0xfff8);
 
/* Inaccessible pointer */
init_s(_arg_open1->ioc, offsetof(struct dm_table_open_test, target1),
@@ -340,11 +355,19 @@ main(void)
printf("ioctl(-1, DM_TABLE_LOAD, "
   "{version=4.1.2, data_size=%zu, data_start=%zu, "
   "dev=makedev(18, 52), name=\"nnn\", uuid=\"uuu\", "
-  "target_count=3735936673, flags=0, %p}) = -1 EBADF (%m)\n",
-  sizeof(*dm_arg_open1),
-  offsetof(struct dm_table_open_test, target1),
-  (char *) dm_arg_open1 +
-  offsetof(struct dm_table_open_test, target1));
+  "target_count=3735936673, flags=0, "
+# if VERBOSE
+  "%p"
+# else /* !VERBOSE */
+  "..."
+# endif /* VERBOSE */
+  "}) = -1 EBADF (%m)\n", sizeof(*dm_arg_open1),
+  offsetof(struct dm_table_open_test, target1)
+# if VERBOSE
+  , (char *) dm_arg_open1 +
+  offsetof(struct dm_table_open_test, target1)
+# endif /* VERBOSE */

[dm-devel] [PATCH 6/9] tests/ioctl_dm: overly long string printing checks

[Eugene Syromyatnikov]

---
 tests/ioctl_dm.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/ioctl_dm.c b/tests/ioctl_dm.c
index 261983c..24232b7 100644
--- a/tests/ioctl_dm.c
+++ b/tests/ioctl_dm.c
@@ -67,12 +67,12 @@ main(void)
init_s(, sizeof(s), offsetof(struct s, u));
s.u.tm.target_msg.sector = 0x1234;
strcpy(s.u.string + offsetof(struct dm_target_msg, message),
-   "tmsg");
+   "long target msg");
ioctl(-1, DM_TARGET_MSG, );
printf("ioctl(-1, DM_TARGET_MSG, "
   "{version=4.1.2, data_size=%u, data_start=%u, "
   "dev=makedev(18, 52), name=\"nnn\", uuid=\"uuu\", flags=0, "
-  "{sector=4660, message=\"tmsg\"}}) = -1 EBADF (%m)\n",
+  "{sector=4660, message=\"long targ\"...}}) = -1 EBADF (%m)\n",
   s.ioc.data_size, s.ioc.data_start);
 
init_s(, sizeof(s), offsetof(struct s, u));
@@ -85,12 +85,12 @@ main(void)
   s.ioc.data_size, s.ioc.data_start);
 
init_s(, sizeof(s), offsetof(struct s, u));
-   strcpy(s.u.string, "new-name");
+   strcpy(s.u.string, "new long name");
ioctl(-1, DM_DEV_RENAME, );
printf("ioctl(-1, DM_DEV_RENAME, "
   "{version=4.1.2, data_size=%u, data_start=%u, "
   "dev=makedev(18, 52), name=\"nnn\", uuid=\"uuu\", event_nr=0, "
-  "flags=0, string=\"new-name\"}) = -1 EBADF (%m)\n",
+  "flags=0, string=\"new long \"...}) = -1 EBADF (%m)\n",
   s.ioc.data_size, s.ioc.data_start);
 
init_s(, sizeof(s), offsetof(struct s, u));
-- 
1.7.10.4

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [RFC] unreleased lock after handler failure

[Michael Wang]

Hi, folks

While we are testing with the latest multipathd, we encounter issue that once
'del map' failed, all the following operation will show 'error -1 receiving 
packet'
whatever how long the timeout has been set (we have applied the latest fix which
make sure the poll will last for 10 seconds).

After some debugging we found that inside parse_cmd(), the pthread_cleanup_pop()
rely on '!r' as indicator for locked or not, while this will be overwritten if
the handler return failed (1 in our case), and then the unlock will be missed.

After applied below fix the problem got solved, although the del map failed, the
other operation can still works.

Could you please help to review whether this is really the problem to be fixed
like that?

Regards,
Michael Wang



diff --git a/multipathd/cli.c b/multipathd/cli.c
index e8a9384..50161be 100644
--- a/multipathd/cli.c
+++ b/multipathd/cli.c
@@ -481,6 +481,7 @@ parse_cmd (char * cmd, char ** reply, int * len, void * 
data, int timeout )
tmo.tv_sec = 0;
}
if (h->locked) {
+   int locked = 0;
struct vectors * vecs = (struct vectors *)data;
 
pthread_cleanup_push(cleanup_lock, >lock);
@@ -491,10 +492,11 @@ parse_cmd (char * cmd, char ** reply, int * len, void * 
data, int timeout )
r = 0;
}
if (r == 0) {
+   locked = 1;
pthread_testcancel();
r = h->fn(cmdvec, reply, len, data);
}
-   pthread_cleanup_pop(!r);
+   pthread_cleanup_pop(locked);
} else
r = h->fn(cmdvec, reply, len, data);
free_keys(cmdvec);

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH] multipath-tools: release lock on handler failure

[Michael Wang]

Inside parse_cmd() the pthread_cleanup_pop() rely on '!r' as the
indicator of locked or not, while this will be overwritten if the
handler return failed, and the unlock will be missing.

This will lead into the situation that all the following operation
will trying to hold a lock which will never be released.

This patch using a separate flag to record the status of locking to
make sure the unlock and lock are in pairs.

Signed-off-by: Michael Wang 
---
 multipathd/cli.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/multipathd/cli.c b/multipathd/cli.c
index e8a9384..50161be 100644
--- a/multipathd/cli.c
+++ b/multipathd/cli.c
@@ -481,6 +481,7 @@ parse_cmd (char * cmd, char ** reply, int * len, void * 
data, int timeout )
tmo.tv_sec = 0;
}
if (h->locked) {
+   int locked = 0;
struct vectors * vecs = (struct vectors *)data;

pthread_cleanup_push(cleanup_lock, >lock);
@@ -491,10 +492,11 @@ parse_cmd (char * cmd, char ** reply, int * len, void * 
data, int timeout )
r = 0;
}
if (r == 0) {
+   locked = 1;
pthread_testcancel();
r = h->fn(cmdvec, reply, len, data);
}
-   pthread_cleanup_pop(!r);
+   pthread_cleanup_pop(locked);
} else
r = h->fn(cmdvec, reply, len, data);
free_keys(cmdvec);
-- 
2.5.0

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] multipath-tools: preload also scsi_dh_hp_sw before run multipathd

[Xose Vazquez Perez]

On 10/11/2016 03:41 PM, Hannes Reinecke wrote:

> On 10/11/2016 02:55 PM, Xose Vazquez Perez wrote:
>> Cc: Hannes Reinecke 
>> Cc: Christophe Varoqui 
>> Cc: device-mapper development 
>> Signed-off-by: Xose Vazquez Perez 
>> ---
>>  multipathd/multipathd.service | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/multipathd/multipathd.service b/multipathd/multipathd.service
>> index e3d6f91..45aca35 100644
>> --- a/multipathd/multipathd.service
>> +++ b/multipathd/multipathd.service
>> @@ -11,7 +11,7 @@ Conflicts=shutdown.target
>>  Type=notify
>>  NotifyAccess=main
>>  LimitCORE=infinity
>> -ExecStartPre=/sbin/modprobe -a scsi_dh_alua scsi_dh_emc scsi_dh_rdac 
>> dm-multipath
>> +ExecStartPre=/sbin/modprobe -a scsi_dh_alua scsi_dh_emc scsi_dh_hp_sw 
>> scsi_dh_rdac dm-multipath
>>  ExecStart=/sbin/multipathd -d -s
>>  ExecReload=/sbin/multipathd reconfigure
>>
> If you can point me to a single user of an MSA1500 I'm happy to include it. 
> If you don't
> know what an MSA1500 is nor what hp_sw does it's probably not a good idea to 
> have it.

Why?

scsi_dh_hp_sw is used also for dec/compaq ESA12000 and RA8000 arrays with HSG80 
controller.
And in HP MSA 1000/1500 and EVA 3000/5000, with old firmware.

Thank you.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 7/9] tests: Some additional checks for ioctl_dm test

[Eugene Syromyatnikov]

---
 tests/ioctl_dm.c |  505 ++
 1 file changed, 505 insertions(+)

diff --git a/tests/ioctl_dm.c b/tests/ioctl_dm.c
index 24232b7..0b2c5a7 100644
--- a/tests/ioctl_dm.c
+++ b/tests/ioctl_dm.c
@@ -2,13 +2,26 @@
 
 #ifdef HAVE_LINUX_DM_IOCTL_H
 
+# include 
 # include 
+# include 
 # include 
 # include 
 # include 
 # include 
 # include 
 
+# define STR32 "AbCdEfGhIjKlMnOpQrStUvWxYz012345"
+
+static const char str129[] = STR32 STR32 STR32 STR32 "6";
+
+static const __u64 dts_sector_base = (__u64) 0xdeadca75facef157ULL;
+static const __u64 dts_sector_step = (__u64) 0x10001ULL;
+static const __u64 dts_length_base = (__u64) 0xbadc0dedda7a1057ULL;
+static const __u64 dts_length_step = (__u64) 0x70007ULL;
+static const __s32 dts_status_base = (__s32) 3141592653U;
+static const __s32 dts_status_step = 0x1234;
+
 static struct s {
struct dm_ioctl ioc;
union {
@@ -24,6 +37,43 @@ static struct s {
} u;
 } s;
 
+struct dm_table_open_test {
+   struct dm_ioctl ioc;
+   struct dm_target_spec target0;
+   char param0[1];
+   struct dm_target_spec target1;
+   char param1[2];
+   struct dm_target_spec target2;
+   char param2[3];
+   struct dm_target_spec target3;
+   char param3[4];
+   struct dm_target_spec target4;
+   char param4[5];
+   struct dm_target_spec target5;
+   char param5[6];
+   struct dm_target_spec target6;
+   char param6[7];
+   struct dm_target_spec target7;
+   char param7[8];
+   struct dm_target_spec target8;
+   char param8[9];
+   struct dm_target_spec target9;
+   char param9[10];
+};
+
+struct dm_target_msg_test {
+   struct dm_ioctl ioc;
+   struct dm_target_msg msg;
+};
+
+struct args {
+   unsigned int arg;
+   const char *str;
+   bool has_params;
+   bool has_event_nr;
+};
+
+
 static void
 init_s(struct dm_ioctl *s, size_t size, size_t offs)
 {
@@ -38,9 +88,147 @@ init_s(struct dm_ioctl *s, size_t size, size_t offs)
strcpy(s->uuid, "uuu");
 }
 
+static void
+init_dm_target_spec(struct dm_target_spec *ptr, uint32_t id)
+{
+   ptr->sector_start = dts_sector_base + dts_sector_step * id;
+   ptr->length   = dts_length_base + dts_length_step * id;
+   ptr->status   = dts_status_base + dts_status_step * id;
+
+   strncpy(ptr->target_type, str129 +
+   id % (sizeof(str129) - sizeof(ptr->target_type)),
+   id % (sizeof(ptr->target_type) + 1));
+   if (id % (sizeof(ptr->target_type) + 1) < sizeof(ptr->target_type))
+   ptr->target_type[id % (sizeof(ptr->target_type) + 1)] = '\0';
+}
+
+static void
+print_dm_target_spec(struct dm_target_spec *ptr, uint32_t id)
+{
+   printf("{sector_start=%" PRI__u64 ", length=%" PRI__u64 ", "
+  "target_type=\"%.*s\", string=",
+  dts_sector_base + dts_sector_step * id,
+  dts_length_base + dts_length_step * id,
+  (int) (id % (sizeof(ptr->target_type) + 1)),
+  str129 + id % (sizeof(str129) - sizeof(ptr->target_type)));
+}
+
+# define ARG_STR(_arg) (_arg), #_arg
+
 int
 main(void)
 {
+   /* We can't check these properly for now */
+   static struct args dummy_check_cmds_nodev[] = {
+   { ARG_STR(DM_REMOVE_ALL),false },
+   { ARG_STR(DM_LIST_DEVICES),  true  },
+   { ARG_STR(DM_LIST_VERSIONS), true  },
+   };
+   static struct args dummy_check_cmds[] = {
+   { ARG_STR(DM_DEV_CREATE),false },
+   { ARG_STR(DM_DEV_REMOVE),false, true },
+   { ARG_STR(DM_DEV_STATUS),false },
+   { ARG_STR(DM_DEV_WAIT),  true,  true },
+   { ARG_STR(DM_TABLE_CLEAR),   false },
+   { ARG_STR(DM_TABLE_DEPS),true  },
+   { ARG_STR(DM_TABLE_STATUS),  true  },
+   };
+
+   struct dm_ioctl *dm_arg =
+   tail_alloc(sizeof(*dm_arg) - sizeof(dm_arg->data));
+   struct dm_table_open_test *dm_arg_open1 =
+   tail_alloc(offsetof(struct dm_table_open_test, target1));
+   struct dm_table_open_test *dm_arg_open2 =
+   tail_alloc(offsetof(struct dm_table_open_test, param1));
+   struct dm_table_open_test *dm_arg_open3 =
+   tail_alloc(offsetof(struct dm_table_open_test, target9));
+   struct dm_target_msg_test *dm_arg_msg =
+   tail_alloc(sizeof(*dm_arg_msg));
+
+   int saved_errno;
+   unsigned int i;
+
+
+   /* Incorrect operation */
+   ioctl(-1, _IOW(DM_IOCTL, 0xde, int), dm_arg);
+   printf("ioctl(-1, _IOC(_IOC_WRITE, %#04x, 0xde, %#04zx), %p) = "
+   "-1 EBADF (%m)\n",
+   DM_IOCTL, sizeof(int), dm_arg);
+
+
+   /* DM_VERSION */
+   /* Incorrect pointer */
+   ioctl(-1, DM_VERSION, dm_arg + 1);
+   printf("ioctl(-1, DM_VERSION, %p) = -1 EBADF (%m)\n", dm_arg + 1);
+
+   

[dm-devel] [PATCH 2/9] tests: Add check for printing of overlength strings to ioctl_dm test

[Eugene Syromyatnikov]

---
 tests/ioctl_dm.c|2 +-
 tests/ioctl_dm.test |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/ioctl_dm.c b/tests/ioctl_dm.c
index 6ad4ea9..0a3bbf4 100644
--- a/tests/ioctl_dm.c
+++ b/tests/ioctl_dm.c
@@ -77,7 +77,7 @@ main(void)
printf("ioctl(-1, DM_DEV_SET_GEOMETRY, "
   "{version=4.1.2, data_size=%u, data_start=%u, "
   "dev=makedev(18, 52), name=\"nnn\", uuid=\"uuu\", flags=0, "
-  "string=\"10 20 30 40\"}) = -1 EBADF (%m)\n",
+  "string=\"10 20 30 \"...}) = -1 EBADF (%m)\n",
   s.ioc.data_size, s.ioc.data_start);
 
init_s(, sizeof(s), offsetof(struct s, u));
diff --git a/tests/ioctl_dm.test b/tests/ioctl_dm.test
index db77616..78866d3 100755
--- a/tests/ioctl_dm.test
+++ b/tests/ioctl_dm.test
@@ -5,7 +5,7 @@
 . "${srcdir=.}/init.sh"
 
 run_prog > /dev/null
-run_strace -a16 -veioctl $args > "$EXP"
+run_strace -a16 -s9 -veioctl $args > "$EXP"
 check_prog grep
 grep -v '^ioctl([012],' < "$LOG" > "$OUT"
 match_diff "$OUT" "$EXP"
-- 
1.7.10.4

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 8/9] tests: Add ioctl_dm to .gitignore

[Eugene Syromyatnikov]

---
 tests/.gitignore |1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/.gitignore b/tests/.gitignore
index a6d014d..9045117 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -103,6 +103,7 @@ getxxid
 inet-cmsg
 ioctl
 ioctl_block
+ioctl_dm
 ioctl_evdev
 ioctl_evdev-v
 ioctl_mtd
-- 
1.7.10.4

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] multipath-tools: remove hwhandler when it's alua

[Xose Vazquez Perez]

On 10/11/2016 08:22 AM, Hannes Reinecke wrote:

> Autodetection will only work if the hardware handler is loaded.
> If the handler is _not_ loaded autodetection won't work, either.
> And if we add this patch multipath will never load the modules, neither.
> So we need this functionality as a fallback if autodetect does not work.

I'm pretty sure all works flawlessly with this patch. Because I use similar
options for EMC VNX7500(CLARiiON) configured as ALUA "Failover Mode 4 
(Asymmetric Active/Active)",
with SLES-11/12 and RHEL-6 booting from SAN.

For SLES it was added to DGC config:
retain_attached_hw_handler yes
detect_prio yes

In RHEL it's included by default:
http://pkgs.fedoraproject.org/cgit/rpms/device-mapper-multipath.git/plain/0117-RHBZ-1198424-autodetect-clariion-alua.patch

And the modules are preloaded by multipathd/multipathd.service:
ExecStartPre=/sbin/modprobe -a scsi_dh_alua scsi_dh_emc scsi_dh_rdac 
dm-multipath


RDAC devices also depend *exclusively* on "retain_attached_hw_handler" and 
"detect_prio"
to run in ALUA mode.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] 答复: [RFC] unreleased lock after handler failure

[Michael Wang]

Hi, Tang

On 10/12/2016 10:33 AM, tang.jun...@zte.com.cn wrote:
> Hi, Michael Wang
> 
> This patch looks well for me, cleanup_lock() should be called without
> affectedby the return value of h->fn().

Thanks for the quick respond, I'll submit a formal patch then :-)

Regards,
Michael Wang

> 
> Regards,
> Tang
> 
> 
> 
> 发件人: Michael Wang 
> 收件人: christophe.varo...@opensvc.com, dm-devel@redhat.com,
> 日期: 2016/10/12 16:11
> 主题:[dm-devel] [RFC] unreleased lock after handler failure
> 发件人:dm-devel-boun...@redhat.com
> --
> 
> 
> 
> Hi, folks
> 
> While we are testing with the latest multipathd, we encounter issue that once
> 'del map' failed, all the following operation will show 'error -1 receiving 
> packet'
> whatever how long the timeout has been set (we have applied the latest fix 
> which
> make sure the poll will last for 10 seconds).
> 
> After some debugging we found that inside parse_cmd(), the 
> pthread_cleanup_pop()
> rely on '!r' as indicator for locked or not, while this will be overwritten if
> the handler return failed (1 in our case), and then the unlock will be missed.
> 
> After applied below fix the problem got solved, although the del map failed, 
> the
> other operation can still works.
> 
> Could you please help to review whether this is really the problem to be fixed
> like that?
> 
> Regards,
> Michael Wang
> 
> 
> 
> diff --git a/multipathd/cli.c b/multipathd/cli.c
> index e8a9384..50161be 100644
> --- a/multipathd/cli.c
> +++ b/multipathd/cli.c
> @@ -481,6 +481,7 @@ parse_cmd (char * cmd, char ** reply, int * len, void * 
> data, int timeout )
>tmo.tv_sec = 0;
>}
>if (h->locked) {
> +   int locked = 0;
>struct vectors * vecs = (struct vectors *)data;
> 
>pthread_cleanup_push(cleanup_lock, >lock);
> @@ -491,10 +492,11 @@ parse_cmd (char * cmd, char ** reply, int * len, void * 
> data, int timeout )
>r = 0;
>}
>if (r == 0) {
> +   locked = 1;
>pthread_testcancel();
>r = h->fn(cmdvec, reply, len, data);
>}
> -   pthread_cleanup_pop(!r);
> +   pthread_cleanup_pop(locked);
>} else
>r = h->fn(cmdvec, reply, len, data);
>free_keys(cmdvec);
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
> 

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] multipathd: fix issue in 'map $map getprstatus' reply

[Bart Van Assche]

On 10/11/16 20:03, peng.lia...@zte.com.cn wrote:
> From: peng liang 
> 
> -add missing newline to 'map|multipath $map getprstatus' reply
> -use asprintf instead of sprintf
> 
> Signed-off-by: peng liang 
> ---
> multipathd/cli_handlers.c | 14 ++
> 1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
> index 8ff4362..16445ea 100644
> --- a/multipathd/cli_handlers.c
> +++ b/multipathd/cli_handlers.c
> @@ -1,6 +1,9 @@
> /*
>  * Copyright (c) 2005 Christophe Varoqui
>  */
> +#define _GNU_SOURCE
> +
> +#include 
> #include "checkers.h"
> #include "memory.h"
> #include "vector.h"
> @@ -1285,14 +1288,9 @@ cli_getprstatus (void * v, char ** reply, int *
> len, void * data)
> 
>  condlog(3, "%s: prflag = %u", param, (unsigned 
> int)mpp->prflag);
> 
> - *reply =(char *)malloc(2);
> - *len = 2;
> - memset(*reply,0,2);
> -
> -
> - sprintf(*reply,"%d",mpp->prflag);
> - (*reply)[1]='\0';
> -
> + *len = asprintf(reply, "%d\n", mpp->prflag);
> + if (*len < 0)
> +  return 1;
> 
>  condlog(3, "%s: reply = %s", param, *reply);

Hello Peng,

Sorry but returning 1 looks somewhat inconsistent to me. This function
is called indirectly by uxsock_trigger() and that function expects that
cli_getprstatus() either returns a positive error code (E...) or a
negative error code. Please change this patch such that ENOMEM is
returned instead of 1 if asprintf() fails.

Thanks,

Bart.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] multipath-tools: remove hwhandler when it's alua

[Xose Vazquez Perez]

On 10/11/2016 08:43 AM, Christophe Varoqui wrote:

> so Xosé, can you confirm the last introduced hwtable entries don't need the 
> alua hwhandler :

Yes, defined:
> - tegile
> - nimble

No needed/defined:
> - nexsan
> - nfinidat
- NexGen|Pivot3

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] multipath-tools: preload also scsi_dh_hp_sw before run multipathd

[Xose Vazquez Perez]

On 10/12/2016 02:43 PM, Hannes Reinecke wrote:
> On 10/12/2016 01:37 PM, Xose Vazquez Perez wrote:

>> scsi_dh_hp_sw is used also for dec/compaq ESA12000 and RA8000 arrays with 
>> HSG80 controller.
>> And in HP MSA 1000/1500 and EVA 3000/5000, with old firmware.
>>
> EVA? I sincerely doubt that.
> Do you really have an example for that?

{
/* MSA 1000/1500 and EVA 3000/5000, with old firmware */
.vendor= "(COMPAQ|HP)",
.product   = "(MSA|HSV)1[01]0",
.hwhandler = "1 hp_sw",
.pgpolicy  = GROUP_BY_PRIO,
.no_path_retry = 12,
.checker_name  = HP_SW,
.prio_name = PRIO_HP_SW,
},

HSV100 is HP StorageWorks Enterprise Virtual Array 3000:
 
http://h18000.www1.hp.com/products/quickspecs/archives_Division/11619_div_v9/11619_div.PDF
HSV110 is HP StorageWorks Enterprise Virtual Array 5000:
 https://www.hpe.com/h20195/v2/GetPDF.aspx/c04282828.pdf

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] multipath-tools: add IBM/FlashSystem to hwtable

[Xose Vazquez Perez]

On 10/07/2016 04:24 PM, Steffen Maier wrote:

> You'll notice that we from Linux on s390x have a bit different recommendations
> for some of the values (no_path_retry actually depends on whether you have a
> cluster software/filesystem that can cope with EIO on last-path-loss. 
> Currently,
> I see the default without that, i.e. non-clustered from which we would like 
> to hide any path issues).

This has to be modified manually.

> Is there some way to have "dependent" settings?

No. multipath.conf is mainly static.
Only two options are dynamically reconfigured, ALUA-prio and the hardware 
handler
with "detect_prio" and "retain_attached_hw_handler".

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] multipath-tools: preload also scsi_dh_hp_sw before run multipathd

[Hannes Reinecke]

On 10/12/2016 01:37 PM, Xose Vazquez Perez wrote:

On 10/11/2016 03:41 PM, Hannes Reinecke wrote:


On 10/11/2016 02:55 PM, Xose Vazquez Perez wrote:

Cc: Hannes Reinecke 
Cc: Christophe Varoqui 
Cc: device-mapper development 
Signed-off-by: Xose Vazquez Perez 
---
 multipathd/multipathd.service | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/multipathd/multipathd.service b/multipathd/multipathd.service
index e3d6f91..45aca35 100644
--- a/multipathd/multipathd.service
+++ b/multipathd/multipathd.service
@@ -11,7 +11,7 @@ Conflicts=shutdown.target
 Type=notify
 NotifyAccess=main
 LimitCORE=infinity
-ExecStartPre=/sbin/modprobe -a scsi_dh_alua scsi_dh_emc scsi_dh_rdac 
dm-multipath
+ExecStartPre=/sbin/modprobe -a scsi_dh_alua scsi_dh_emc scsi_dh_hp_sw 
scsi_dh_rdac dm-multipath
 ExecStart=/sbin/multipathd -d -s
 ExecReload=/sbin/multipathd reconfigure


If you can point me to a single user of an MSA1500 I'm happy to include it. If 
you don't
know what an MSA1500 is nor what hp_sw does it's probably not a good idea to 
have it.


Why?

scsi_dh_hp_sw is used also for dec/compaq ESA12000 and RA8000 arrays with HSG80 
controller.
And in HP MSA 1000/1500 and EVA 3000/5000, with old firmware.


EVA? I sincerely doubt that.
Do you really have an example for that?

Anyway.
Point here is, these are _really_ old SCSI parallel arrays. And the 
failover command is _really_ stupid (just send a START STOP UNIT to the 
passive path).
As this has a tendency to interfere with normal operation I prefer to 
_NOT_ have it loaded per default; in fact, I'm on the verge of removing 
it altogether.


Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH] multipath-tools: add Xiotech ISE arrays to hwtable

[Xose Vazquez Perez]

Based on documentation provided by the manufacturer:
https://drive.google.com/file/d/0B_B6YmEmO7cDc201a3ZlcmFvUmM

Cc: Christophe Varoqui 
Cc: device-mapper development 
Signed-off-by: Xose Vazquez Perez 
---
 libmultipath/hwtable.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
index 4fffeb7..dc3010a 100644
--- a/libmultipath/hwtable.c
+++ b/libmultipath/hwtable.c
@@ -933,6 +933,15 @@ static struct hwentry default_hw[] = {
.prio_name = PRIO_ALUA,
.no_path_retry = 15,
},
+   /*
+* Xiotech
+*/
+   {
+   .vendor= "(XIOTECH|XIOtech)",
+   .product   = "ISE",
+   .pgpolicy  = MULTIBUS,
+   .no_path_retry = 12,
+   },
 #if 0
/*
 * Copy this TEMPLATE to add new hardware.
-- 
2.10.1

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] multipath-tools: remove hwhandler when it's alua

[Benjamin Marzinski]

On Wed, Oct 12, 2016 at 01:06:10PM +0200, Xose Vazquez Perez wrote:

> On 10/11/2016 08:22 AM, Hannes Reinecke wrote:
> 
> > Autodetection will only work if the hardware handler is loaded.
> > If the handler is _not_ loaded autodetection won't work, either.
> > And if we add this patch multipath will never load the modules, neither.
> > So we need this functionality as a fallback if autodetect does not work.
> 
> I'm pretty sure all works flawlessly with this patch. Because I use similar
> options for EMC VNX7500(CLARiiON) configured as ALUA "Failover Mode 4 
> (Asymmetric Active/Active)",
> with SLES-11/12 and RHEL-6 booting from SAN.
> 
> For SLES it was added to DGC config:
> retain_attached_hw_handler yes
> detect_prio yes
> 
> In RHEL it's included by default:
> http://pkgs.fedoraproject.org/cgit/rpms/device-mapper-multipath.git/plain/0117-RHBZ-1198424-autodetect-clariion-alua.patch
> 
> And the modules are preloaded by multipathd/multipathd.service:
> ExecStartPre=/sbin/modprobe -a scsi_dh_alua scsi_dh_emc scsi_dh_rdac 
> dm-multipath
> 
> 
> RDAC devices also depend *exclusively* on "retain_attached_hw_handler" and 
> "detect_prio"
> to run in ALUA mode.

The purpose of the the "retain_attached_hw_handler" and "detect_prio"
options were to allow devices that support ALUA and non-ALUA modes, to
detect which mode the device was in, so that the builtin configuration
would work correctly for both modes.

If a device is ALUA only, I don't see any benefit to not hardcoding
that.  Simply from a ease-of-use persepective, having ALUA in the
configuration make it obvious how the device is supposed to be
configured.  But more importantly, as has already been metioned, it's
more robust to hardcode the ALUA handler for the devices that need it in
case the handler was not attached before multipath started working on
the device.  Also, if I recall correctly, for the device handler to get
attached correctly, we don't just need the device handler module
isntalled before multipathd starts. We need it installed when the path
device is initially discovered.

The more I think about this, the more I think we might want to revisit
some to the configuration simplifications that have been made.  In some
cases, I think they went too far. For instance, most devices will work
correctly regardless of the values for things like "rr_minio_rq" and
"path_selector", so I have no objection to these values being removed
from the device configuartion, if they are the same as the default
values.  On the other hand, we have things like "hardware_handler" and
"path_checker", where the device simply will not function correctly with
certain values.  For these attributes, it makes sense to leave them in
the device configuration, even if they are the same as the default
values.  The goal should be that you can't break any device
configurations by changing the default values.

-Ben

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2] dm raid1: "mirror" target doesn't use all available legs on multiple failures

[Heinz Mauelshagen]

In case legs of a mirror target fail, any read will cause a new,
operational default leg to be selected and the read be resubmitted
to it. If that new default leg fails the read too, no other still
accessible legs are used to resubmit the read again thus failing
the io.

Fix by allowing the read to get resubmitted until there's no
operational legs any more.

Removes any details.bi_dev use as a flag.

Resolves: rhbz1383444

Signed-off-by: Heinz Mauelshagen 
---
 drivers/md/dm-raid1.c | 16 
 1 file changed, 16 deletions(-)

diff --git a/drivers/md/dm-raid1.c b/drivers/md/dm-raid1.c
index 7a6254d..a7fe697 100644
--- a/drivers/md/dm-raid1.c
+++ b/drivers/md/dm-raid1.c
@@ -145,7 +145,6 @@ static void dispatch_bios(void *context, struct bio_list 
*bio_list)
 
 struct dm_raid1_bio_record {
struct mirror *m;
-   /* if details->bi_bdev == NULL, details were not saved */
struct dm_bio_details details;
region_t write_region;
 };
@@ -1200,8 +1199,6 @@ static int mirror_map(struct dm_target *ti, struct bio 
*bio)
struct dm_raid1_bio_record *bio_record =
  dm_per_bio_data(bio, sizeof(struct dm_raid1_bio_record));
 
-   bio_record->details.bi_bdev = NULL;
-
if (rw == WRITE) {
/* Save region for mirror_end_io() handler */
bio_record->write_region = dm_rh_bio_to_region(ms->rh, bio);
@@ -1266,16 +1263,6 @@ static int mirror_end_io(struct dm_target *ti, struct 
bio *bio, int error)
goto out;
 
if (unlikely(error)) {
-   if (!bio_record->details.bi_bdev) {
-   /*
-* There wasn't enough memory to record necessary
-* information for a retry or there was no other
-* mirror in-sync.
-*/
-   DMERR_LIMIT("Mirror read failed.");
-   return -EIO;
-   }
-
m = bio_record->m;
 
DMERR("Mirror read failed from %s. Trying alternative device.",
@@ -1291,7 +1278,6 @@ static int mirror_end_io(struct dm_target *ti, struct bio 
*bio, int error)
bd = _record->details;
 
dm_bio_restore(bd, bio);
-   bio_record->details.bi_bdev = NULL;
bio->bi_error = 0;
 
queue_bio(ms, bio, rw);
@@ -1301,8 +1287,6 @@ static int mirror_end_io(struct dm_target *ti, struct bio 
*bio, int error)
}
 
 out:
-   bio_record->details.bi_bdev = NULL;
-
return error;
 }
 
-- 
2.7.4

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel