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

2016-10-11 Thread tang . junhui 
Hello Ben,

In multipath manual document, multipath -q command is described as:
"-q allow device tables with queue_if_no_path when multiathd is not 
running".
However the command does not take effect when we testing it.

About use-case, In my opinion, sometimes we need to stop multipath service 
temporarily (for example: multipath version upgrade), but we do not want 
IO interrupt during those times even path down existed, so we can execute 
“multiath -q” to keep IO queue until we starting service after version 
upgrade.

Cheers,
Tang





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



On Tue, Oct 11, 2016 at 12:33:43PM +0200, Hannes Reinecke wrote:
> 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.

I think you are dealing with a different issue.  the "-q" option for
multipath just overrides the default behaviour of not enabling
queue_if_no_path when creating a multipath device is multipathd isn't
running.  For this, we don't care if multipathd will start running in
the future, because if/when multipathd does start up, it will set the
queue_if_no_path flag itself. We only care about now, when we know it's
not running.

Really, I doubt that anyone ever uses the -q option. So your change in
how multipath checks if multipathd is running is fine by me. However,
you also changed what the "-q" option does.  Previously, it just kept
multipath from disabling "queue_if_no_path" on devices that were
configured to have it, when multipathd wasn't running.  With your code,
doesn't it now make all devices set queue_if_no_path if multipathd isn't
running, including devices that weren't configured to set it even when
multipathd is running? Is there a reason for this? In general, setting
"queue_if_no_path" when multiapthd isn't running is a bad idea, since
the paths will never come back, and nothing will ever disable the
queueing.  That's why I said that I doubt anybody uses the option.
Forcing all devices to set "queue_if_no_path", even if they weren't
configured to, seems even less useful.  Or is there actually a use-case
that I'm missing here?

-Ben

> 
> 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 mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] multipathd: fail path when path check timeout

2016-10-11 Thread Benjamin Marzinski 
On Tue, Oct 11, 2016 at 02:50:00PM +0800, tang.jun...@zte.com.cn wrote:
>Please have a review for this patch, any comment will be highly
>appreciated.

This is clearly correct. I suspect that there will be other places where
we need to also check for PATH_TIMEOUT, since it is basically the same
as PATH_DOWN, except with a different state name. For instance, we
only log error messages on repeated checks for (newstate == PATH_DOWN),
and we should probably do that for PATH_TIMEOUT as well. There are
probably more instances outside of check_path.

-Ben

> 
>��:         tang.jun...@zte.com.cn
>�ռ���:         christophe varoqui ,
>:        dm-devel@redhat.com, zhang.ka...@zte.com.cn, "tang.junhui"
>
>:         2016/08/10 16:11
>:        [PATCH] multipathd: fail path when path check timeout
> 
>--
> 
>From: "tang.junhui" 
> 
>path should be failed when path status is PATH_TIMEOUT after check,
>otherwise, the valid number of paths in the map would be increased when
>the path status is PATH_UP after the next turn check, which would cause
>the valid number of paths exceeding the total number of paths in the map.
> 
>Signed-off-by: tang.junhui 
>---
>multipathd/main.c | 2 +-
>1 file changed, 1 insertion(+), 1 deletion(-)
> 
>diff --git a/multipathd/main.c b/multipathd/main.c
>index f5e9a01..01f1e58 100644
>--- a/multipathd/main.c
>+++ b/multipathd/main.c
>@@ -1585,7 +1585,7 @@ check_path (struct vectors * vecs, struct path * pp,
>int ticks)
>                                  pp->checkint = conf->checkint;
>                                  put_multipath_config(conf);
> 
>-                                  if (newstate == PATH_DOWN || newstate
>== PATH_SHAKY) {
>+                                  if (newstate == PATH_DOWN || newstate
>== PATH_SHAKY || newstate == PATH_TIMEOUT) {
>                                                   /*
>                                                    * proactively fail
>path in the DM
>                                                    */
>--
>2.8.1.windows.1

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

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

2016-10-11 Thread Benjamin Marzinski 
On Tue, Oct 11, 2016 at 12:33:43PM +0200, Hannes Reinecke wrote:
> 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.

I think you are dealing with a different issue.  the "-q" option for
multipath just overrides the default behaviour of not enabling
queue_if_no_path when creating a multipath device is multipathd isn't
running.  For this, we don't care if multipathd will start running in
the future, because if/when multipathd does start up, it will set the
queue_if_no_path flag itself. We only care about now, when we know it's
not running.

Really, I doubt that anyone ever uses the -q option. So your change in
how multipath checks if multipathd is running is fine by me. However,
you also changed what the "-q" option does.  Previously, it just kept
multipath from disabling "queue_if_no_path" on devices that were
configured to have it, when multipathd wasn't running.  With your code,
doesn't it now make all devices set queue_if_no_path if multipathd isn't
running, including devices that weren't configured to set it even when
multipathd is running? Is there a reason for this? In general, setting
"queue_if_no_path" when multiapthd isn't running is a bad idea, since
the paths will never come back, and nothing will ever disable the
queueing.  That's why I said that I doubt anybody uses the option.
Forcing all devices to set "queue_if_no_path", even if they weren't
configured to, seems even less useful.  Or is there actually a use-case
that I'm missing here?

-Ben

> 
> 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

Re: [dm-devel] [PATCH] scsi: replace wrong device handler name for CLARiiON arrays

2016-10-11 Thread Martin K. Petersen 
> "Xose" == Xose Vazquez Perez  writes:

Xose> At drivers/scsi/device_handler/scsi_dh_emc.c it was defined as:
Xose> #define CLARIION_NAME "emc"

Applied to 4.9/scsi-fixes.

-- 
Martin K. Petersen  Oracle Linux Engineering

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

[dm-devel] [PATCH 3/5] rbd: check for exclusive lock enabled

2016-10-11 Thread Mike Christie 
Only attach the checker if the rbd image has the exclusive lock
enabled.

Signed-off-by: Mike Christie 
---
 libmultipath/checkers/rbd.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/libmultipath/checkers/rbd.c b/libmultipath/checkers/rbd.c
index f0497db..89d7525 100644
--- a/libmultipath/checkers/rbd.c
+++ b/libmultipath/checkers/rbd.c
@@ -34,6 +34,8 @@ typedef int (thread_fn)(struct rbd_checker_context *ct, char 
*msg);
 
 #define RBD_MSG(msg, fmt, args...) snprintf(msg, CHECKER_MSG_LEN, fmt, ##args);
 
+#define RBD_FEATURE_EXCLUSIVE_LOCK (1 << 2)
+
 struct rbd_checker_context {
int rbd_bus_id;
char *client_addr;
@@ -66,8 +68,9 @@ int libcheck_init(struct checker * c)
struct udev_device *bus_dev;
struct udev *udev;
struct stat sb;
-   const char *block_name, *addr, *config_info;
+   const char *block_name, *addr, *config_info, *features_str;
const char *image, *pool, *snap, *username;
+   uint64_t features = 0;
char sysfs_path[PATH_SIZE];
int ret;
 
@@ -120,6 +123,15 @@ int libcheck_init(struct checker * c)
if (!ct->client_addr)
goto free_dev;
 
+   features_str = udev_device_get_sysattr_value(bus_dev, "features");
+   if (!features_str)
+   goto free_addr;
+   features = strtoll(features_str, NULL, 16);
+   if (!(features & RBD_FEATURE_EXCLUSIVE_LOCK)) {
+   condlog(3, "Exclusive lock not set.");
+   goto free_addr;
+   }
+
config_info = udev_device_get_sysattr_value(bus_dev, "config_info");
if (!config_info)
goto free_addr;
-- 
2.7.2

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

[dm-devel] [PATCH 0/5] rbd checker fixes

2016-10-11 Thread Mike Christie 
The following pathces made over the multipath-tools tree today
fix some bugs in the rbd checker and sync up log messages.

The first 4 patches are a resend from here:
https://www.redhat.com/archives/dm-devel/2016-August/msg00429.html
I did not see any review comments (of course let me know if I missed
them), so the only changes are from being rebased on the current tree.

The last patch is new and adds a fix for when rbd's lock_on_read
feature is used.

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

[dm-devel] [PATCH 2/5] rbd: check for nonshared clients

2016-10-11 Thread Mike Christie 
The rbd checker only supports nonshared clients so add a check
during init time.

Signed-off-by: Mike Christie 
---
 libmultipath/checkers/rbd.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/libmultipath/checkers/rbd.c b/libmultipath/checkers/rbd.c
index 8f88154..f0497db 100644
--- a/libmultipath/checkers/rbd.c
+++ b/libmultipath/checkers/rbd.c
@@ -124,6 +124,11 @@ int libcheck_init(struct checker * c)
if (!config_info)
goto free_addr;
 
+   if (!strstr(config_info, "noshare")) {
+   condlog(3, "Only nonshared clients supported.");
+   goto free_addr;
+   }
+
ct->config_info = strdup(config_info);
if (!ct->config_info)
goto free_addr;
-- 
2.7.2

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

[dm-devel] [PATCH 5/5] rbd: use lock_on_read if set

2016-10-11 Thread Mike Christie 
If lock_on_read is set initially, pass it as a map option
when remapping the image.

Signed-off-by: Mike Christie 
---
 libmultipath/checkers/rbd.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/libmultipath/checkers/rbd.c b/libmultipath/checkers/rbd.c
index e516166..c920cbb 100644
--- a/libmultipath/checkers/rbd.c
+++ b/libmultipath/checkers/rbd.c
@@ -46,6 +46,7 @@ struct rbd_checker_context {
char *username;
int remapped;
int blacklisted;
+   int lock_on_read:1;
 
rados_t cluster;
 
@@ -142,6 +143,9 @@ int libcheck_init(struct checker * c)
goto free_addr;
}
 
+   if (strstr(config_info, "lock_on_read"))
+   ct->lock_on_read = 1;
+
ct->config_info = strdup(config_info);
if (!ct->config_info)
goto free_addr;
@@ -398,7 +402,10 @@ static int rbd_remap(struct rbd_checker_context *ct)
case 0:
argv[i++] = "rbd";
argv[i++] = "map";
-   argv[i++] = "-o noshare";
+   if (ct->lock_on_read)
+   argv[i++] = "-o noshare,lock_on_read";
+   else
+   argv[i++] = "-o noshare";
if (ct->username) {
argv[i++] = "--id";
argv[i++] = ct->username;
-- 
2.7.2

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

[dm-devel] [PATCH 4/5] rbd: fixup log messages

2016-10-11 Thread Mike Christie 
Add rbd device prefix to condlog messages that was missing it, and drop
it in RBD_MSG because it is already added by caller.

Signed-off-by: Mike Christie 
---
 libmultipath/checkers/rbd.c | 67 +++--
 1 file changed, 35 insertions(+), 32 deletions(-)

diff --git a/libmultipath/checkers/rbd.c b/libmultipath/checkers/rbd.c
index 89d7525..e516166 100644
--- a/libmultipath/checkers/rbd.c
+++ b/libmultipath/checkers/rbd.c
@@ -114,8 +114,8 @@ int libcheck_init(struct checker * c)
 
addr = udev_device_get_sysattr_value(bus_dev, "client_addr");
if (!addr) {
-   condlog(0, "Could not find client_addr in rbd sysfs. Try "
-   "updating kernel");
+   condlog(0, "rbd%d: Could not find client_addr in rbd sysfs. "
+   "Try updating kernel", ct->rbd_bus_id);
goto free_dev;
}
 
@@ -128,7 +128,7 @@ int libcheck_init(struct checker * c)
goto free_addr;
features = strtoll(features_str, NULL, 16);
if (!(features & RBD_FEATURE_EXCLUSIVE_LOCK)) {
-   condlog(3, "Exclusive lock not set.");
+   condlog(3, "rbd%d: Exclusive lock not set.", ct->rbd_bus_id);
goto free_addr;
}
 
@@ -137,7 +137,8 @@ int libcheck_init(struct checker * c)
goto free_addr;
 
if (!strstr(config_info, "noshare")) {
-   condlog(3, "Only nonshared clients supported.");
+   condlog(3, "rbd%d: Only nonshared clients supported.",
+   ct->rbd_bus_id);
goto free_addr;
}
 
@@ -190,18 +191,20 @@ int libcheck_init(struct checker * c)
}
 
if (rados_create(>cluster, NULL) < 0) {
-   condlog(0, "Could not create rados cluster");
+   condlog(0, "rbd%d: Could not create rados cluster",
+   ct->rbd_bus_id);
goto free_snap;
}
 
if (rados_conf_read_file(ct->cluster, NULL) < 0) {
-   condlog(0, "Could not read rados conf");
+   condlog(0, "rbd%d: Could not read rados conf", ct->rbd_bus_id);
goto shutdown_rados;
}
 
ret = rados_connect(ct->cluster);
if (ret < 0) {
-   condlog(0, "Could not connect to rados cluster");
+   condlog(0, "rbd%d: Could not connect to rados cluster",
+   ct->rbd_bus_id);
goto shutdown_rados;
}
 
@@ -292,8 +295,7 @@ static int rbd_is_blacklisted(struct rbd_checker_context 
*ct, char *msg)
ret = rados_mon_command(ct->cluster, (const char **)cmd, 1, "", 0,
, _len, , _len);
if (ret < 0) {
-   RBD_MSG(msg, "rbd checker failed: mon command failed %d",
-   ret);
+   RBD_MSG(msg, "checker failed: mon command failed %d", ret);
return ret;
}
 
@@ -314,16 +316,15 @@ static int rbd_is_blacklisted(struct rbd_checker_context 
*ct, char *msg)
 
end = strchr(addr_tok, ' ');
if (!end) {
-   RBD_MSG(msg, "rbd%d checker failed: invalid blacklist 
%s",
-ct->rbd_bus_id, addr_tok);
+   RBD_MSG(msg, "checker failed: invalid blacklist %s",
+addr_tok);
break;
}
*end = '\0';
 
if (!strcmp(addr_tok, ct->client_addr)) {
ct->blacklisted = 1;
-   RBD_MSG(msg, "rbd%d checker: %s is blacklisted",
-   ct->rbd_bus_id, ct->client_addr);
+   RBD_MSG(msg, "%s is blacklisted", ct->client_addr);
ret = 1;
break;
}
@@ -340,7 +341,7 @@ static int rbd_check(struct rbd_checker_context *ct, char 
*msg)
if (ct->blacklisted || rbd_is_blacklisted(ct, msg) == 1)
return PATH_DOWN;
 
-   RBD_MSG(msg, "rbd checker reports path is up");
+   RBD_MSG(msg, "checker reports path is up");
/*
 * Path may have issues, but the ceph cluster is at least
 * accepting IO, so we can attempt to do IO.
@@ -412,10 +413,12 @@ static int rbd_remap(struct rbd_checker_context *ct)
argv[i] = NULL;
 
ret = execvp(argv[0], argv);
-   condlog(0, "Error executing rbd: %s", strerror(errno));
+   condlog(0, "rbd%d: Error executing rbd: %s", ct->rbd_bus_id,
+   strerror(errno));
exit(-1);
case -1:
-   condlog(0, "fork failed: %s", strerror(errno));
+   condlog(0, "rbd%d: fork failed: %s", ct->rbd_bus_id,
+   strerror(errno));
return -1;
default:
ret = -1;
@@ -425,7 +428,8 @@

[dm-devel] [PATCH 1/5] rbd: fix sync repair support

2016-10-11 Thread Mike Christie 
If sync was set we were calling check instead
of function passed in.

Signed-off-by: Mike Christie 
---
 libmultipath/checkers/rbd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libmultipath/checkers/rbd.c b/libmultipath/checkers/rbd.c
index bfec2c9..8f88154 100644
--- a/libmultipath/checkers/rbd.c
+++ b/libmultipath/checkers/rbd.c
@@ -552,7 +552,7 @@ static int rbd_exec_fn(struct checker *c, thread_fn *fn)
int rbd_status, r;
 
if (c->sync)
-   return rbd_check(ct, c->message);
+   return fn(ct, c->message);
/*
 * Async mode
 */
-- 
2.7.2

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

Re: [dm-devel] [PATCH 1/1] dm raid: fix compat_features validation

2016-10-11 Thread Mike Snitzer 
On Tue, Oct 11 2016 at 11:44am -0400,
Heinz Mauelshagen  wrote:

> 
> 
> On 10/11/2016 05:38 PM, Andy Whitcroft wrote:
> >On Tue, Oct 11, 2016 at 05:04:34PM +0200, Heinz Mauelshagen wrote:
> >>Andy,
> >>
> >>good catch.
> >>
> >>We should rather check for  V190 support only in case any
> >>compat feature flags are actually set.
> >>
> >>{
> >>+   if (le32_to_cpu(sb->compat_features) &&
> >>+   le32_to_cpu(sb->compat_features) != FEATURE_FLAG_SUPPORTS_V190)
> >>{
> >> rs->ti->error = "Unable to assemble array: Unknown flag(s)
> >>in compatible feature flags";
> >> return -EINVAL;
> >> }
> >If the feature flags are single bit combinations then I believe the
> >below does check exactly that.  Checking for no 1s outside of the
> >expected features, caring not for the value of the valid bits:
> >
> >+ if (le32_to_cpu(sb->compat_features) & ~(FEATURE_FLAG_SUPPORTS_V190)) {
> >
> >with the possibilty to or in additional feature bits as they are added.
> 
> Thanks,
> I prefer this to be easier readable.

Readable or not, the code with the != is _not_ future-proof.  Whereas
Andy's solution is.  If/when a new compat feature comes along then
FEATURE_FLAG_SUPPORTS_V190 would be replaced to be a macro that ORs all
the new compat features together (e.g. FEATURE_FLAG_COMPAT).  E.g. how
dm-thin-metadata.c:__check_incompat_features() does.

We can go with the != code for now, since any future changes would
likely cause this test to be changed.  Or we could fix it now _for
real_.

Mike

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

Re: [dm-devel] [PATCH 1/1] dm raid: fix compat_features validation

2016-10-11 Thread Heinz Mauelshagen 



On 10/11/2016 05:38 PM, Andy Whitcroft wrote:

On Tue, Oct 11, 2016 at 05:04:34PM +0200, Heinz Mauelshagen wrote:

Andy,

good catch.

We should rather check for  V190 support only in case any
compat feature flags are actually set.

{
+   if (le32_to_cpu(sb->compat_features) &&
+   le32_to_cpu(sb->compat_features) != FEATURE_FLAG_SUPPORTS_V190)
{
 rs->ti->error = "Unable to assemble array: Unknown flag(s)
in compatible feature flags";
 return -EINVAL;
 }

If the feature flags are single bit combinations then I believe the
below does check exactly that.  Checking for no 1s outside of the
expected features, caring not for the value of the valid bits:

+ if (le32_to_cpu(sb->compat_features) & ~(FEATURE_FLAG_SUPPORTS_V190)) {

with the possibilty to or in additional feature bits as they are added.


Thanks,
I prefer this to be easier readable.



-apw

--
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 1/1] dm raid: fix compat_features validation

2016-10-11 Thread Andy Whitcroft 
On Tue, Oct 11, 2016 at 05:04:34PM +0200, Heinz Mauelshagen wrote:
> 
> Andy,
> 
> good catch.
> 
> We should rather check for  V190 support only in case any
> compat feature flags are actually set.
> 
> {
> +   if (le32_to_cpu(sb->compat_features) &&
> +   le32_to_cpu(sb->compat_features) != FEATURE_FLAG_SUPPORTS_V190)
> {
> rs->ti->error = "Unable to assemble array: Unknown flag(s)
> in compatible feature flags";
> return -EINVAL;
> }

If the feature flags are single bit combinations then I believe the
below does check exactly that.  Checking for no 1s outside of the
expected features, caring not for the value of the valid bits:

+ if (le32_to_cpu(sb->compat_features) & ~(FEATURE_FLAG_SUPPORTS_V190)) {

with the possibilty to or in additional feature bits as they are added.

-apw

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

Re: [dm-devel] [PATCH 1/1] dm raid: fix compat_features validation

2016-10-11 Thread Mike Snitzer 
On Tue, Oct 11 2016 at 10:28am -0400,
Andy Whitcroft  wrote:

> In commit ecbfb9f118bce4 ("dm raid: add raid level takeover support") a new
> compatible feature flag was added.  Validation for these compat_features
> was added but this only passes for new raid mappings with this feature
> flag.  This causes previously created raid mappings to be failed at import.
> 
> Check compat_features for any valid combinations.
> 
> Fixes: ecbfb9f118bce4 ("dm raid: add raid level takeover support")
> BugLink: http://bugs.launchpad.net/bugs/1631298
> Signed-off-by: Andy Whitcroft 
> ---
>  drivers/md/dm-raid.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> It very much looks like these are intended to be optional extended feature
> flags.  That we should be accepting any valid flag and rejecting any bit
> not in that set.  We should however not be ensuring that specific bits
> are actually set.  Certainly as things stand raid sets built on previous
> kernel versions cannot be assembled.

Right, your patch looks good to me.  But I'll wait for confirmation from
Heinz before I stage your fix.

Thanks,
Mike

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

[dm-devel] [PATCH 1/1] dm raid: fix compat_features validation

2016-10-11 Thread Andy Whitcroft 
In commit ecbfb9f118bce4 ("dm raid: add raid level takeover support") a new
compatible feature flag was added.  Validation for these compat_features
was added but this only passes for new raid mappings with this feature
flag.  This causes previously created raid mappings to be failed at import.

Check compat_features for any valid combinations.

Fixes: ecbfb9f118bce4 ("dm raid: add raid level takeover support")
BugLink: http://bugs.launchpad.net/bugs/1631298
Signed-off-by: Andy Whitcroft 
---
 drivers/md/dm-raid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

It very much looks like these are intended to be optional extended feature
flags.  That we should be accepting any valid flag and rejecting any bit
not in that set.  We should however not be ensuring that specific bits
are actually set.  Certainly as things stand raid sets built on previous
kernel versions cannot be assembled.

diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index 8abde6b..6ddea60 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -2258,7 +2258,7 @@ static int super_validate(struct raid_set *rs, struct 
md_rdev *rdev)
if (!mddev->events && super_init_validation(rs, rdev))
return -EINVAL;
 
-   if (le32_to_cpu(sb->compat_features) != FEATURE_FLAG_SUPPORTS_V190) {
+   if (le32_to_cpu(sb->compat_features) & ~(FEATURE_FLAG_SUPPORTS_V190)) {
rs->ti->error = "Unable to assemble array: Unknown flag(s) in 
compatible feature flags";
return -EINVAL;
}
-- 
2.9.3

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

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

2016-10-11 Thread Xose Vazquez Perez 
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
 
-- 
2.10.1

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

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

2016-10-11 Thread Hannes Reinecke 

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 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

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

2016-10-11 Thread tang . junhui 
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.

Thanks,
Christophe

On Tue, Oct 11, 2016 at 8:46 AM,  wrote:
Please have a review for this patch, any comment will be highly 
appreciated.




发件人: tang.jun...@zte.com.cn 
收件人: christophe varoqui , 
抄送:dm-devel@redhat.com, zhang.ka...@zte.com.cn, "tang.junhui" <
tang.jun...@zte.com.cn> 
日期: 2016/08/16 19:33 
主题:[PATCH] libmultipath: fix multipath -q command logic 




From: "tang.junhui" 

multipath judged whether multipathd service in running by check_daemon() 
when executing
mutipath commands, check_daemon() try to connect to the multipathd service 
and execute
"show dameon" command. The expected result is that the command will be 
failed when the
service is not running, however, mpath_connect() will activate the 
multipathd service when
the service is not running, so check_daemon() always return with true. 
Another problem is that
multipath command with -q parameter is not processed in coalesce_paths(). 
This patch fix for
those two problems.

Signed-off-by: tang.junhui 
---
libmultipath/configure.c | 85 
+++-
1 file changed, 48 insertions(+), 37 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 707e6be..d8a17a6 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -715,36 +715,36 @@ deadmap (struct multipath * mpp)
 return 1; /* dead */
}

-int check_daemon(void)
+static int
+check_daemon(void)
{
 int fd;
- char *reply;
- int ret = 0;
- unsigned int timeout;
- struct config *conf;
-
- fd = mpath_connect();
- if (fd == -1)
-  return 0;
+ struct flock lock;

- if (send_packet(fd, "show daemon") != 0)
-  goto out;
- conf = get_multipath_config();
- timeout = conf->uxsock_timeout;
- put_multipath_config(conf);
- if (recv_packet(fd, , timeout) != 0)
-  goto out;
-
- if (strstr(reply, "shutdown"))
-  goto out_free;
-
- ret = 1;
+ fd = open(DEFAULT_PIDFILE, O_RDONLY);
+ if (fd < 0) {
+  if (errno == ENOENT)
+   return 0;
+  if (errno == EMFILE)
+   condlog(0, "failed to 
open file, increase max_fds at %s", DEFAULT_CONFIGFILE);
+  else
+   condlog(0, "can not 
open pidfile %s: %s", DEFAULT_PIDFILE, strerror(errno));
+  return -1;
+ }

-out_free:
- FREE(reply);
-out:
- mpath_disconnect(fd);
- return ret;
+ lock.l_type = F_WRLCK;
+ lock.l_start = 0;
+ lock.l_whence = SEEK_SET;
+ lock.l_len = 0;
+ if (fcntl(fd, F_GETLK, ) < 0) {
+  condlog(0, "can not get file locker, 
%s: %s", DEFAULT_PIDFILE, strerror(errno));
+  close(fd);
+  return -1;
+ }
+ close(fd);
+ if (lock.l_type == F_UNLCK)
+  return 0;
+ return 1;
}

extern int
@@ -873,17 +873,28 @@ coalesce_paths (struct vectors * vecs, vector newmp, 
char * refwwid, int force_r
  if (r == DOMAP_DRY)
   continue;

-  conf = get_multipath_config();
-  allow_queueing = conf->allow_queueing;
-  put_multipath_config(conf);
-  if (!is_daemon && !allow_queueing && 
!check_daemon()) {
-   if (mpp->no_path_retry 
!= NO_PATH_RETRY_UNDEF &&
-   mpp->no_path_retry 
!=

Re: [dm-devel] [PATCH] multipathd: fail path when path check timeout

2016-10-11 Thread Christophe Varoqui 
Merged.
Thanks.

On Tue, Oct 11, 2016 at 8:50 AM,  wrote:

> Please have a review for this patch, any comment will be highly
> appreciated.
>
>
>
>
> 发件人: tang.jun...@zte.com.cn
> 收件人: christophe varoqui ,
> 抄送:dm-devel@redhat.com, zhang.ka...@zte.com.cn, "tang.junhui" <
> tang.jun...@zte.com.cn>
> 日期: 2016/08/10 16:11
> 主题:[PATCH] multipathd: fail path when path check timeout
> --
>
>
>
> From: "tang.junhui" 
>
> path should be failed when path status is PATH_TIMEOUT after check,
> otherwise, the valid number of paths in the map would be increased when
> the path status is PATH_UP after the next turn check, which would cause
> the valid number of paths exceeding the total number of paths in the map.
>
> Signed-off-by: tang.junhui 
> ---
> multipathd/main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/multipathd/main.c b/multipathd/main.c
> index f5e9a01..01f1e58 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -1585,7 +1585,7 @@ check_path (struct vectors * vecs, struct path * pp,
> int ticks)
>   pp->checkint = conf->checkint;
>   put_multipath_config(conf);
>
> -  if (newstate == PATH_DOWN || newstate
> == PATH_SHAKY) {
> +  if (newstate == PATH_DOWN || newstate
> == PATH_SHAKY || newstate == PATH_TIMEOUT) {
>/*
> * proactively fail
> path in the DM
> */
> --
> 2.8.1.windows.1
>
>
>
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

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

2016-10-11 Thread Christophe Varoqui 
Hannes, Ben,

are you ok with the solution to these two issues.
Seems sane to me.

Thanks,
Christophe

On Tue, Oct 11, 2016 at 8:46 AM,  wrote:

> Please have a review for this patch, any comment will be highly
> appreciated.
>
>
>
>
> 发件人: tang.jun...@zte.com.cn
> 收件人: christophe varoqui ,
> 抄送:dm-devel@redhat.com, zhang.ka...@zte.com.cn, "tang.junhui" <
> tang.jun...@zte.com.cn>
> 日期: 2016/08/16 19:33
> 主题:[PATCH] libmultipath: fix multipath -q command logic
> --
>
>
>
> From: "tang.junhui" 
>
> multipath judged whether multipathd service in running by check_daemon()
> when executing
> mutipath commands, check_daemon() try to connect to the multipathd service
> and execute
> "show dameon" command. The expected result is that the command will be
> failed when the
> service is not running, however, mpath_connect() will activate the
> multipathd service when
> the service is not running, so check_daemon() always return with true.
> Another problem is that
> multipath command with -q parameter is not processed in coalesce_paths().
> This patch fix for
> those two problems.
>
> Signed-off-by: tang.junhui 
> ---
> libmultipath/configure.c | 85 +++---
> --
> 1 file changed, 48 insertions(+), 37 deletions(-)
>
> diff --git a/libmultipath/configure.c b/libmultipath/configure.c
> index 707e6be..d8a17a6 100644
> --- a/libmultipath/configure.c
> +++ b/libmultipath/configure.c
> @@ -715,36 +715,36 @@ deadmap (struct multipath * mpp)
>  return 1; /* dead */
> }
>
> -int check_daemon(void)
> +static int
> +check_daemon(void)
> {
>  int fd;
> - char *reply;
> - int ret = 0;
> - unsigned int timeout;
> - struct config *conf;
> -
> - fd = mpath_connect();
> - if (fd == -1)
> -  return 0;
> + struct flock lock;
>
> - if (send_packet(fd, "show daemon") != 0)
> -  goto out;
> - conf = get_multipath_config();
> - timeout = conf->uxsock_timeout;
> - put_multipath_config(conf);
> - if (recv_packet(fd, , timeout) != 0)
> -  goto out;
> -
> - if (strstr(reply, "shutdown"))
> -  goto out_free;
> -
> - ret = 1;
> + fd = open(DEFAULT_PIDFILE, O_RDONLY);
> + if (fd < 0) {
> +  if (errno == ENOENT)
> +   return 0;
> +  if (errno == EMFILE)
> +   condlog(0, "failed to
> open file, increase max_fds at %s", DEFAULT_CONFIGFILE);
> +  else
> +   condlog(0, "can not
> open pidfile %s: %s", DEFAULT_PIDFILE, strerror(errno));
> +  return -1;
> + }
>
> -out_free:
> - FREE(reply);
> -out:
> - mpath_disconnect(fd);
> - return ret;
> + lock.l_type = F_WRLCK;
> + lock.l_start = 0;
> + lock.l_whence = SEEK_SET;
> + lock.l_len = 0;
> + if (fcntl(fd, F_GETLK, ) < 0) {
> +  condlog(0, "can not get file locker,
> %s: %s", DEFAULT_PIDFILE, strerror(errno));
> +  close(fd);
> +  return -1;
> + }
> + close(fd);
> + if (lock.l_type == F_UNLCK)
> +  return 0;
> + return 1;
> }
>
> extern int
> @@ -873,17 +873,28 @@ coalesce_paths (struct vectors * vecs, vector newmp,
> char * refwwid, int force_r
>   if (r == DOMAP_DRY)
>continue;
>
> -  conf = get_multipath_config();
> -  allow_queueing = conf->allow_queueing;
> -  put_multipath_config(conf);
> -  if (!is_daemon && !allow_queueing &&
> !check_daemon()) {
> -   if (mpp->no_path_retry
> != NO_PATH_RETRY_UNDEF &&
> -   mpp->no_path_retry
> != NO_PATH_RETRY_FAIL)
> -
>  condlog(3, "%s: multipathd not running, unset "
> -
> "queue_if_no_path feature", mpp->alias);
> -   if
> (!dm_queue_if_no_path(mpp->alias, 0))
> -
>

Re: [dm-devel] [PATCH] multipathd: uxsock_timeout should be assigned by conf->uxsock_timeout

2016-10-11 Thread Christophe Varoqui 
Good,
Merged.

On Tue, Oct 11, 2016 at 8:42 AM,  wrote:

> Please have a review for this patch, any comment will be highly
> appreciated.
>
>
>
>
> 发件人: tang.jun...@zte.com.cn
> 收件人: christophe varoqui ,
> 抄送:zhang.ka...@zte.com.cn, dm-devel@redhat.com, "tang.junhui" <
> tang.jun...@zte.com.cn>
> 日期: 2016/09/13 18:17
> 主题:[dm-devel] [PATCH] multipathd: uxsock_timeout should be
> assigned byconf->uxsock_timeout
> 发件人:dm-devel-boun...@redhat.com
> --
>
>
>
> From: "tang.junhui" 
>
> uxsock_timeout should be assigned by conf->uxsock_timeout
> before using it in uxclnt() as a CLI client timeout value,
> otherwise its default value is 0, and the CLI client timeout
> value is 0 + 100(ms), so the CLI client will be timeout very
> quickly.
>
> Signed-off-by: tang.junhui 
> ---
> multipathd/main.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 96ef01f..a08f1a5 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -2549,6 +2549,7 @@ main (int argc, char *argv[])
>
> exit(1);
>if (verbosity)
>
> conf->verbosity = verbosity;
> +   uxsock_timeout =
> conf->uxsock_timeout;
>uxclnt(optarg,
> uxsock_timeout + 100);
>exit(0);
>   case 'B':
> @@ -2573,6 +2574,7 @@ main (int argc, char *argv[])
>exit(1);
>   if (verbosity)
>conf->verbosity =
> verbosity;
> +  uxsock_timeout = conf->uxsock_timeout;
>   memset(cmd, 0x0, CMDSIZE);
>   while (optind < argc) {
>if
> (strchr(argv[optind], ' '))
> --
> 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

Re: [dm-devel] dm: free io_barrier after blk_cleanup_queue call

2016-10-11 Thread Tahsin Erdogan 
On Mon, Oct 10, 2016 at 6:25 AM, Mike Snitzer  wrote:
> I have to believe this was born out of code inspection rather than
> actual need (due to crash, etc)?

This got originated from several crashes I have seen with 4.3 kernel.
The crashes
were caused by null dereferencing of io_barrier->per_cpu_ref.

The issue may no longer be relevant after commit c91852ff0815
("dm: optimize dm_request_fn()") because conditions for accessing
io_barrier may not longer exist. But fix should be considered for
forked stable trees.

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

Re: [dm-devel] [PATCH] mpathpersist: memset length is wrong

2016-10-11 Thread Christophe Varoqui 
Seems right.
Merged, with thanks.

On Tue, Oct 11, 2016 at 8:37 AM,  wrote:

> Please have a review for this patch, any comment will be highly
> appreciated.
>
>
>
>
> 发件人: tang.jun...@zte.com.cn
> 收件人: christophe varoqui ,
> chau...@redhat.com, Vijay , Benjamin Marzinski <
> bmarz...@redhat.com>,
> 抄送:zhang.ka...@zte.com.cn, dm-devel@redhat.com, "tang.junhui" <
> tang.jun...@zte.com.cn>
> 日期: 2016/09/21 16:54
> 主题:[dm-devel] [PATCH] mpathpersist: memset length is wrong
> 发件人:dm-devel-boun...@redhat.com
> --
>
>
>
> From: "tang.junhui" 
>
> variable transportids is cleared by memset() with wrong length
> MPATH_MX_TIDS,
> the length should be MPATH_MX_TIDS*sizeof(struct transportid).
>
> Signed-off-by: tang.junhui 
> ---
> mpathpersist/main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mpathpersist/main.c b/mpathpersist/main.c
> index a55865f..8e8cc35 100644
> --- a/mpathpersist/main.c
> +++ b/mpathpersist/main.c
> @@ -105,7 +105,7 @@ int main (int argc, char * argv[])
>
>  udev = udev_new();
>  conf = mpath_lib_init(udev);
> - memset(transportids,0,MPATH_MX_TIDS);
> + memset(transportids, 0, MPATH_MX_TIDS * sizeof(struct
> transportid));
>  multipath_conf = conf;
>
>  while (1)
> --
> 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

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

2016-10-11 Thread Hannes Reinecke 
On 10/11/2016 08:02 AM, Christophe Varoqui wrote:
> I'm not comfortable with this patch that induces a less predictable
> behaviour.
> We have that retain hardware handler switch that users can activate to
> achieve the purpose.
> 
> Hannes, Ben, would you support that patch ?
> 
> Best Regards,
> Christophe
> 
Please don't apply this.
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.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

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

Re: [dm-devel] [PATCH] libmultipath/checkers/tur: Fix a recently introduced deadlock

2016-10-11 Thread Christophe Varoqui 
Merged.
Thanks.

On Tue, Oct 11, 2016 at 1:12 AM, Bart Van Assche  wrote:

> Avoid that the tur_devt() call from libcheck_check() hangs.
>
> Fixes: commit 873be9fef222 ("libmultipath/checkers/tur: Serialize
> tur_checker_context.devt accesses")
> Signed-off-by: Bart Van Assche 
> ---
>  libmultipath/checkers/tur.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c
> index a7a70f6..81206e4 100644
> --- a/libmultipath/checkers/tur.c
> +++ b/libmultipath/checkers/tur.c
> @@ -64,6 +64,7 @@ static const char *tur_devt(char *devt_buf, int size,
>  int libcheck_init (struct checker * c)
>  {
> struct tur_checker_context *ct;
> +   pthread_mutexattr_t attr;
>
> ct = malloc(sizeof(struct tur_checker_context));
> if (!ct)
> @@ -74,7 +75,10 @@ int libcheck_init (struct checker * c)
> ct->fd = -1;
> ct->holders = 1;
> pthread_cond_init_mono(>active);
> -   pthread_mutex_init(>lock, NULL);
> +   pthread_mutexattr_init();
> +   pthread_mutexattr_settype(, PTHREAD_MUTEX_RECURSIVE);
> +   pthread_mutex_init(>lock, );
> +   pthread_mutexattr_destroy();
> pthread_spin_init(>hldr_lock, PTHREAD_PROCESS_PRIVATE);
> c->context = ct;
>
> --
> 2.10.0
>
>
--
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

2016-10-11 Thread Christophe Varoqui 
I'm not comfortable with this patch that induces a less predictable
behaviour.
We have that retain hardware handler switch that users can activate to
achieve the purpose.

Hannes, Ben, would you support that patch ?

Best Regards,
Christophe

On Thu, Oct 6, 2016 at 8:50 PM, Xose Vazquez Perez 
wrote:

> Autodetected.
>
> Cc: Hannes Reinecke 
> Cc: Christophe Varoqui 
> Cc: device-mapper development 
> Signed-off-by: Xose Vazquez Perez 
> ---
>  libmultipath/hwtable.c | 7 ---
>  1 file changed, 7 deletions(-)
>
> diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
> index 367aa0a..1fc2f24 100644
> --- a/libmultipath/hwtable.c
> +++ b/libmultipath/hwtable.c
> @@ -46,7 +46,6 @@ static struct hwentry default_hw[] = {
> .product   = "VV",
> .pgpolicy  = GROUP_BY_PRIO,
> .pgfailback= -FAILBACK_IMMEDIATE,
> -   .hwhandler = "1 alua",
> .prio_name = PRIO_ALUA,
> .no_path_retry = 18,
> },
> @@ -124,7 +123,6 @@ static struct hwentry default_hw[] = {
> /* SAN Virtualization Services Platform */
> .vendor= "HP",
> .product   = "HSVX700",
> -   .hwhandler = "1 alua",
> .pgpolicy  = GROUP_BY_PRIO,
> .pgfailback= -FAILBACK_IMMEDIATE,
> .no_path_retry = 12,
> @@ -504,7 +502,6 @@ static struct hwentry default_hw[] = {
> .vendor= "IBM",
> .product   = "^IPR",
> .no_path_retry = NO_PATH_RETRY_QUEUE,
> -   .hwhandler = "1 alua",
> .pgpolicy  = GROUP_BY_PRIO,
> .pgfailback= -FAILBACK_IMMEDIATE,
> .prio_name = PRIO_ALUA,
> @@ -546,7 +543,6 @@ static struct hwentry default_hw[] = {
> {
> .vendor= "AIX",
> .product   = "NVDISK",
> -   .hwhandler = "1 alua",
> .pgpolicy  = GROUP_BY_PRIO,
> .pgfailback= -FAILBACK_IMMEDIATE,
> .no_path_retry = (300 / DEFAULT_CHECKINT),
> @@ -654,7 +650,6 @@ static struct hwentry default_hw[] = {
> /* M-Series */
> .vendor= "NEC",
> .product   = "DISK ARRAY",
> -   .hwhandler = "1 alua",
> .pgpolicy  = GROUP_BY_PRIO,
> .pgfailback= -FAILBACK_IMMEDIATE,
> .prio_name = PRIO_ALUA,
> @@ -780,7 +775,6 @@ static struct hwentry default_hw[] = {
> {
> .vendor= "(Intel|INTEL)",
> .product   = "Multi-Flex",
> -   .hwhandler = "1 alua",
> .pgpolicy  = GROUP_BY_PRIO,
> .pgfailback= -FAILBACK_IMMEDIATE,
> .no_path_retry = NO_PATH_RETRY_QUEUE,
> @@ -792,7 +786,6 @@ static struct hwentry default_hw[] = {
> {
> .vendor= "(LIO-ORG|SUSE)",
> .product   = "RBD",
> -   .hwhandler = "1 alua",
> .pgpolicy  = GROUP_BY_PRIO,
> .pgfailback= -FAILBACK_IMMEDIATE,
> .no_path_retry = 12,
> --
> 2.10.1
>
>
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel