* [PATCH 01/42] libmultipath: Invalid check for mpp->wwid in dm_addmap()
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 02/42] Remove newline from condlog() Hannes Reinecke
` (40 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
mpp->wwid is an array, and so a check against NULL
is wrong; we need to use strlen here.
Found by coverity.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/devmapper.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index de7d446..5132399 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -281,7 +281,7 @@ dm_addmap (int task, const char *target, struct multipath *mpp, char * params,
if (ro)
dm_task_set_ro(dmt);
- if (use_uuid && mpp->wwid){
+ if (use_uuid && strlen(mpp->wwid) > 0){
prefixed_uuid = MALLOC(UUID_PREFIX_LEN + strlen(mpp->wwid) + 1);
if (!prefixed_uuid) {
condlog(0, "cannot create prefixed uuid : %s\n",
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 02/42] Remove newline from condlog()
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
2013-01-08 13:53 ` [PATCH 01/42] libmultipath: Invalid check for mpp->wwid in dm_addmap() Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 03/42] Fixup pathgroup allocation in disassemble_map() Hannes Reinecke
` (39 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
condlog() already adds a newline to each message.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/devmapper.c | 8 ++++----
libmultipath/dmparser.c | 4 ++--
libmultipath/structs_vec.c | 2 +-
libmultipath/sysfs.c | 2 +-
libmultipath/uevent.c | 4 ++--
libmultipath/util.c | 12 ++++++------
multipath/main.c | 2 +-
multipathd/main.c | 27 ++++++++++++++-------------
8 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 5132399..051ecb2 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -284,7 +284,7 @@ dm_addmap (int task, const char *target, struct multipath *mpp, char * params,
if (use_uuid && strlen(mpp->wwid) > 0){
prefixed_uuid = MALLOC(UUID_PREFIX_LEN + strlen(mpp->wwid) + 1);
if (!prefixed_uuid) {
- condlog(0, "cannot create prefixed uuid : %s\n",
+ condlog(0, "cannot create prefixed uuid : %s",
strerror(errno));
goto addout;
}
@@ -302,7 +302,7 @@ dm_addmap (int task, const char *target, struct multipath *mpp, char * params,
if (mpp->attribute_flags & (1 << ATTR_GID) &&
!dm_task_set_gid(dmt, mpp->gid))
goto freeout;
- condlog(4, "%s: addmap [0 %llu %s %s]\n", mpp->alias, mpp->size,
+ condlog(4, "%s: addmap [0 %llu %s %s]", mpp->alias, mpp->size,
target, params);
dm_task_no_open_count(dmt);
@@ -931,7 +931,7 @@ dm_get_name(char *uuid)
prefixed_uuid = MALLOC(UUID_PREFIX_LEN + strlen(uuid) + 1);
if (!prefixed_uuid) {
- condlog(0, "cannot create prefixed uuid : %s\n",
+ condlog(0, "cannot create prefixed uuid : %s",
strerror(errno));
goto freeout;
}
@@ -1366,7 +1366,7 @@ int dm_reassign(const char *mapname)
int r = 0, i;
if (dm_dev_t(mapname, &dev_t[0], 32)) {
- condlog(3, "%s: failed to get device number\n", mapname);
+ condlog(3, "%s: failed to get device number", mapname);
return 1;
}
diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
index 5848ec5..b3c52fc 100644
--- a/libmultipath/dmparser.c
+++ b/libmultipath/dmparser.c
@@ -100,7 +100,7 @@ assemble_map (struct multipath * mp, char * params, int len)
shift = snprintf(p, freechar, " %s %i 1", mp->selector,
VECTOR_SIZE(pgp->paths));
if (shift >= freechar) {
- condlog(0, "%s: params too small\n", mp->alias);
+ condlog(0, "%s: params too small", mp->alias);
return 1;
}
p += shift;
@@ -113,7 +113,7 @@ assemble_map (struct multipath * mp, char * params, int len)
&& pp->priority > 0)
tmp_minio = minio * pp->priority;
if (!strlen(pp->dev_t) ) {
- condlog(0, "dev_t not set for '%s'\n", pp->dev);
+ condlog(0, "dev_t not set for '%s'", pp->dev);
return 1;
}
shift = snprintf(p, freechar, " %s %d",
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 6e70d63..f998708 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -483,7 +483,7 @@ int update_multipath (struct vectors *vecs, char *mapname, int reset)
mpp = find_mp_by_alias(vecs->mpvec, mapname);
if (!mpp) {
- condlog(3, "%s: multipath map not found\n", mapname);
+ condlog(3, "%s: multipath map not found", mapname);
return 2;
}
diff --git a/libmultipath/sysfs.c b/libmultipath/sysfs.c
index 8e986e8..9c554dd 100644
--- a/libmultipath/sysfs.c
+++ b/libmultipath/sysfs.c
@@ -77,7 +77,7 @@ ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
condlog(4, "write to %s failed: %s", devpath, strerror(errno));
size = 0;
} else if (size < value_len) {
- condlog(4, "tried to write %ld to %s. Wrote %ld\n",
+ condlog(4, "tried to write %ld to %s. Wrote %ld",
(long)value_len, devpath, (long)size);
size = 0;
}
diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 0b7eb7a..b74fb08 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -422,10 +422,10 @@ int uevent_listen(void)
err = udev_monitor_filter_add_match_subsystem_devtype(monitor, "block",
NULL);
if (err)
- condlog(2, "failed to create filter : %s\n", strerror(-err));
+ condlog(2, "failed to create filter : %s", strerror(-err));
err = udev_monitor_enable_receiving(monitor);
if (err) {
- condlog(2, "failed to enable receiving : %s\n", strerror(-err));
+ condlog(2, "failed to enable receiving : %s", strerror(-err));
goto out;
}
while (1) {
diff --git a/libmultipath/util.c b/libmultipath/util.c
index 70735e6..7cdfd28 100644
--- a/libmultipath/util.c
+++ b/libmultipath/util.c
@@ -86,7 +86,7 @@ get_word (char * sentence, char ** word)
*word = MALLOC(len + 1);
if (!*word) {
- condlog(0, "get_word : oom\n");
+ condlog(0, "get_word : oom");
return 0;
}
strncpy(*word, sentence, len);
@@ -178,7 +178,7 @@ devt2devname (char *devname, int devname_len, char *devt)
char *p = strrchr(dev, '/');
if (!p) {
- condlog(0, "No sysfs entry for %s\n",
+ condlog(0, "No sysfs entry for %s",
block_path);
return 1;
}
@@ -208,7 +208,7 @@ devt2devname (char *devname, int devname_len, char *devt)
if ((major == tmpmaj) && (minor == tmpmin)) {
if (snprintf(block_path, sizeof(block_path),
"/sys/block/%s", dev) >= sizeof(block_path)) {
- condlog(0, "device name %s is too long\n", dev);
+ condlog(0, "device name %s is too long", dev);
fclose(fd);
return 1;
}
@@ -218,17 +218,17 @@ devt2devname (char *devname, int devname_len, char *devt)
fclose(fd);
skip_proc:
if (strncmp(block_path,"/sys/block", 10)) {
- condlog(3, "No device found for %u:%u\n", major, minor);
+ condlog(3, "No device found for %u:%u", major, minor);
return 1;
}
if (stat(block_path, &statbuf) < 0) {
- condlog(0, "No sysfs entry for %s\n", block_path);
+ condlog(0, "No sysfs entry for %s", block_path);
return 1;
}
if (S_ISDIR(statbuf.st_mode) == 0) {
- condlog(0, "sysfs entry %s is not a directory\n", block_path);
+ condlog(0, "sysfs entry %s is not a directory", block_path);
return 1;
}
basenamecpy((const char *)block_path, devname, devname_len);
diff --git a/multipath/main.c b/multipath/main.c
index 92e852b..6208995 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -535,7 +535,7 @@ main (int argc, char *argv[])
fd_limit.rlim_cur = conf->max_fds;
fd_limit.rlim_max = conf->max_fds;
if (setrlimit(RLIMIT_NOFILE, &fd_limit) < 0)
- condlog(0, "can't set open fds limit to %d : %s\n",
+ condlog(0, "can't set open fds limit to %d : %s",
conf->max_fds, strerror(errno));
}
diff --git a/multipathd/main.c b/multipathd/main.c
index 64c1a0c..84574b2 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1154,7 +1154,7 @@ check_path (struct vectors * vecs, struct path * pp)
* Synchronize with kernel state
*/
if (update_multipath_strings(pp->mpp, vecs->pathvec)) {
- condlog(1, "%s: Could not synchronize with kernel state\n",
+ condlog(1, "%s: Could not synchronize with kernel state",
pp->dev);
pp->dmstate = PSTATE_UNDEF;
}
@@ -1611,7 +1611,7 @@ child (void * param)
struct rlimit fd_limit;
if (getrlimit(RLIMIT_NOFILE, &fd_limit) < 0) {
- condlog(0, "can't get open fds limit: %s\n",
+ condlog(0, "can't get open fds limit: %s",
strerror(errno));
fd_limit.rlim_cur = 0;
fd_limit.rlim_max = 0;
@@ -1622,11 +1622,11 @@ child (void * param)
fd_limit.rlim_max = conf->max_fds;
if (setrlimit(RLIMIT_NOFILE, &fd_limit) < 0) {
condlog(0, "can't set open fds limit to "
- "%lu/%lu : %s\n",
+ "%lu/%lu : %s",
fd_limit.rlim_cur, fd_limit.rlim_max,
strerror(errno));
} else {
- condlog(3, "set open fds limit to %lu/%lu\n",
+ condlog(3, "set open fds limit to %lu/%lu",
fd_limit.rlim_cur, fd_limit.rlim_max);
}
}
@@ -1713,7 +1713,8 @@ child (void * param)
/* Now all the waitevent threads will start rushing in. */
while (vecs->lock.depth > 0) {
sleep (1); /* This is weak. */
- condlog(3,"Have %d wait event checkers threads to de-alloc, waiting..\n", vecs->lock.depth);
+ condlog(3, "Have %d wait event checkers threads to de-alloc,"
+ " waiting...", vecs->lock.depth);
}
pthread_mutex_destroy(vecs->lock.mutex);
FREE(vecs->lock.mutex);
@@ -1900,18 +1901,18 @@ void * mpath_pr_event_handler_fn (void * pathp )
resp = mpath_alloc_prin_response(MPATH_PRIN_RKEY_SA);
if (!resp){
- condlog(0,"%s Alloc failed for prin response \n", pp->dev);
+ condlog(0,"%s Alloc failed for prin response", pp->dev);
return NULL;
}
ret = prin_do_scsi_ioctl(pp->dev, MPATH_PRIN_RKEY_SA, resp, 0);
if (ret != MPATH_PR_SUCCESS )
{
- condlog(0,"%s : pr in read keys service action failed. Error=%d\n", pp->dev, ret);
+ condlog(0,"%s : pr in read keys service action failed. Error=%d", pp->dev, ret);
goto out;
}
- condlog(3, " event pr=%d addlen=%d\n",resp->prin_descriptor.prin_readkeys.prgeneration,
+ condlog(3, " event pr=%d addlen=%d",resp->prin_descriptor.prin_readkeys.prgeneration,
resp->prin_descriptor.prin_readkeys.additional_length );
if (resp->prin_descriptor.prin_readkeys.additional_length == 0 )
@@ -1933,7 +1934,7 @@ void * mpath_pr_event_handler_fn (void * pathp )
isFound =0;
for (i = 0; i < resp->prin_descriptor.prin_readkeys.additional_length/8; i++ )
{
- condlog(2, "PR IN READKEYS[%d] reservation key:\n",i);
+ condlog(2, "PR IN READKEYS[%d] reservation key:",i);
dumpHex((char *)&resp->prin_descriptor.prin_readkeys.key_list[i*8], 8 , -1);
if (!memcmp(mpp->reservation_key, &resp->prin_descriptor.prin_readkeys.key_list[i*8], 8))
{
@@ -1945,7 +1946,7 @@ void * mpath_pr_event_handler_fn (void * pathp )
if (!isFound)
{
condlog(0, "%s: Either device not registered or ", pp->dev);
- condlog(0, "host is not authorised for registration. Skip path\n");
+ condlog(0, "host is not authorised for registration. Skip path");
ret = MPATH_PR_OTHER;
goto out;
}
@@ -1959,12 +1960,12 @@ void * mpath_pr_event_handler_fn (void * pathp )
}
param->num_transportid = 0;
- condlog(3, "device %s:%s \n", pp->dev, pp->mpp->wwid);
+ condlog(3, "device %s:%s", pp->dev, pp->mpp->wwid);
ret = prout_do_scsi_ioctl(pp->dev, MPATH_PROUT_REG_IGN_SA, 0, 0, param, 0);
if (ret != MPATH_PR_SUCCESS )
{
- condlog(0,"%s: Reservation registration failed. Error: %d\n", pp->dev, ret);
+ condlog(0,"%s: Reservation registration failed. Error: %d", pp->dev, ret);
}
mpp->prflag = 1;
@@ -1991,7 +1992,7 @@ int mpath_pr_event_handle(struct path *pp)
rc = pthread_create(&thread, NULL , mpath_pr_event_handler_fn, pp);
if (rc) {
- condlog(0, "%s: ERROR; return code from pthread_create() is %d\n", pp->dev, rc);
+ condlog(0, "%s: ERROR; return code from pthread_create() is %d", pp->dev, rc);
return -1;
}
pthread_attr_destroy(&attr);
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 03/42] Fixup pathgroup allocation in disassemble_map()
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
2013-01-08 13:53 ` [PATCH 01/42] libmultipath: Invalid check for mpp->wwid in dm_addmap() Hannes Reinecke
2013-01-08 13:53 ` [PATCH 02/42] Remove newline from condlog() Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 04/42] libmultipath: resource leak in read_value_block() Hannes Reinecke
` (38 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
The check for empty path groups in disassemble_map() is not quite
correct; we might end up removing the pathgroup vector even though
there are some entries in it.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/dmparser.c | 15 +++++++++------
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
index b3c52fc..a45854d 100644
--- a/libmultipath/dmparser.c
+++ b/libmultipath/dmparser.c
@@ -232,13 +232,16 @@ disassemble_map (vector pathvec, char * params, struct multipath * mpp)
num_pg = atoi(word);
FREE(word);
- if (num_pg > 0 && !mpp->pg) {
- mpp->pg = vector_alloc();
-
- if (!mpp->pg)
- return 1;
- } else
+ if (num_pg > 0) {
+ if (!mpp->pg) {
+ mpp->pg = vector_alloc();
+ if (!mpp->pg)
+ return 1;
+ }
+ } else {
+ free_pgvec(mpp->pg, KEEP_PATHS);
mpp->pg = NULL;
+ }
/*
* first pg to try
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 04/42] libmultipath: resource leak in read_value_block()
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (2 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 03/42] Fixup pathgroup allocation in disassemble_map() Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 05/42] prio: fix merging of prioritizers with different args Hannes Reinecke
` (37 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
read_value_block() allocates the vector 'elements', but
doesn't free it on error.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/parser.c | 32 +++++++++++++++++---------------
1 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/libmultipath/parser.c b/libmultipath/parser.c
index 79c2d22..526c45b 100644
--- a/libmultipath/parser.c
+++ b/libmultipath/parser.c
@@ -311,8 +311,10 @@ read_value_block(void)
buf = (char *) MALLOC(MAXBUF);
- if (!buf)
+ if (!buf) {
+ vector_free(elements);
return NULL;
+ }
while (read_line(buf, MAXBUF)) {
vec = alloc_strvec(buf);
@@ -323,21 +325,20 @@ read_value_block(void)
break;
}
- if (VECTOR_SIZE(vec))
- for (i = 0; i < VECTOR_SIZE(vec); i++) {
- str = VECTOR_SLOT(vec, i);
- dup = (char *) MALLOC(strlen(str) + 1);
- if (!dup)
- goto out;
- memcpy(dup, str, strlen(str));
-
- if (!vector_alloc_slot(elements)) {
- free_strvec(vec);
- goto out1;
- }
+ for (i = 0; i < VECTOR_SIZE(vec); i++) {
+ str = VECTOR_SLOT(vec, i);
+ dup = (char *) MALLOC(strlen(str) + 1);
+ if (!dup)
+ goto out;
+ memcpy(dup, str, strlen(str));
- vector_set_slot(elements, dup);
+ if (!vector_alloc_slot(elements)) {
+ free_strvec(vec);
+ goto out1;
}
+
+ vector_set_slot(elements, dup);
+ }
free_strvec(vec);
}
memset(buf, 0, MAXBUF);
@@ -348,6 +349,7 @@ out1:
FREE(dup);
out:
FREE(buf);
+ vector_free(elements);
return NULL;
}
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 05/42] prio: fix merging of prioritizers with different args
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (3 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 04/42] libmultipath: resource leak in read_value_block() Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 06/42] Accept several whitespaces in bindings file Hannes Reinecke
` (36 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Petr Uzel
From: Petr Uzel <petr.uzel@suse.cz>
Signed-off-by: Petr Uzel <petr.uzel@suse.cz>
---
libmultipath/discovery.c | 12 ++++---
libmultipath/prio.c | 49 +++++++++++++++++++++++++++++-
libmultipath/prio.h | 9 +++++-
libmultipath/prioritizers/alua.c | 1 +
libmultipath/prioritizers/datacore.c | 1 +
libmultipath/prioritizers/emc.c | 1 +
libmultipath/prioritizers/hds.c | 1 +
libmultipath/prioritizers/hp_sw.c | 1 +
libmultipath/prioritizers/ontap.c | 1 +
libmultipath/prioritizers/rdac.c | 1 +
libmultipath/prioritizers/weightedpath.c | 6 +++-
libmultipath/propsel.c | 15 ++++-----
libmultipath/structs.c | 3 ++
libmultipath/structs.h | 6 +++-
libmultipath/structs_vec.c | 2 +-
15 files changed, 90 insertions(+), 19 deletions(-)
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 33e44b6..6f5470f 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -777,21 +777,23 @@ get_prio (struct path * pp)
if (!pp)
return 0;
- if (!pp->prio) {
+ struct prio * p = &pp->prio;
+
+ if (!prio_selected(p)) {
select_prio(pp);
- if (!pp->prio) {
+ if (!prio_selected(p)) {
condlog(3, "%s: no prio selected", pp->dev);
return 1;
}
}
- pp->priority = prio_getprio(pp->prio, pp);
+ pp->priority = prio_getprio(p, pp);
if (pp->priority < 0) {
- condlog(3, "%s: %s prio error", pp->dev, prio_name(pp->prio));
+ condlog(3, "%s: %s prio error", pp->dev, prio_name(p));
pp->priority = PRIO_UNDEF;
return 1;
}
condlog(3, "%s: %s prio = %u",
- pp->dev, prio_name(pp->prio), pp->priority);
+ pp->dev, prio_name(p), pp->priority);
return 0;
}
diff --git a/libmultipath/prio.c b/libmultipath/prio.c
index 61c19b7..cf97fad 100644
--- a/libmultipath/prio.c
+++ b/libmultipath/prio.c
@@ -22,13 +22,23 @@ static struct prio * alloc_prio (void)
struct prio *p;
p = MALLOC(sizeof(struct prio));
- if (p)
+ if (p) {
INIT_LIST_HEAD(&p->node);
+ p->refcount = 1;
+ }
return p;
}
void free_prio (struct prio * p)
{
+ if (!p)
+ return;
+ p->refcount--;
+ if (p->refcount) {
+ condlog(3, "%s prioritizer refcount %d",
+ p->name, p->refcount);
+ return;
+ }
condlog(3, "unloading %s prioritizer", p->name);
list_del(&p->node);
if (p->handle) {
@@ -110,6 +120,13 @@ int prio_getprio (struct prio * p, struct path * pp)
return p->getprio(pp, p->args);
}
+int prio_selected (struct prio * p)
+{
+ if (!p || !p->getprio)
+ return 0;
+ return (p->getprio) ? 1 : 0;
+}
+
char * prio_name (struct prio * p)
{
return p->name;
@@ -119,3 +136,33 @@ char * prio_args (struct prio * p)
{
return p->args;
}
+
+void prio_get (struct prio * dst, char * name, char * args)
+{
+ struct prio * src = prio_lookup(name);
+
+ if (!src) {
+ dst->getprio = NULL;
+ return;
+ }
+
+ strncpy(dst->name, src->name, PRIO_NAME_LEN);
+ if (args)
+ strncpy(dst->args, args, PRIO_ARGS_LEN);
+ dst->getprio = src->getprio;
+ dst->handle = NULL;
+
+ src->refcount++;
+}
+
+void prio_put (struct prio * dst)
+{
+ struct prio * src;
+
+ if (!dst)
+ return;
+
+ src = prio_lookup(dst->name);
+ memset(dst, 0x0, sizeof(struct prio));
+ free_prio(src);
+}
diff --git a/libmultipath/prio.h b/libmultipath/prio.h
index 36929fb..4eeb216 100644
--- a/libmultipath/prio.h
+++ b/libmultipath/prio.h
@@ -6,7 +6,10 @@
*/
#include "checkers.h"
#include "vector.h"
-#include "structs.h"
+
+/* forward declaration to avoid circular dependency */
+struct path;
+
#include "list.h"
#include "memory.h"
@@ -41,6 +44,7 @@
struct prio {
void *handle;
+ int refcount;
struct list_head node;
char name[PRIO_NAME_LEN];
char args[PRIO_ARGS_LEN];
@@ -52,6 +56,9 @@ void cleanup_prio (void);
struct prio * add_prio (char *);
struct prio * prio_lookup (char *);
int prio_getprio (struct prio *, struct path *);
+void prio_get (struct prio *, char *, char *);
+void prio_put (struct prio *);
+int prio_selected (struct prio *);
char * prio_name (struct prio *);
char * prio_args (struct prio *);
int prio_set_args (struct prio *, char *);
diff --git a/libmultipath/prioritizers/alua.c b/libmultipath/prioritizers/alua.c
index 4da3ee7..b9493a4 100644
--- a/libmultipath/prioritizers/alua.c
+++ b/libmultipath/prioritizers/alua.c
@@ -16,6 +16,7 @@
#include <debug.h>
#include <prio.h>
+#include <structs.h>
#include "alua.h"
diff --git a/libmultipath/prioritizers/datacore.c b/libmultipath/prioritizers/datacore.c
index 2c16c6c..e3e6a51 100644
--- a/libmultipath/prioritizers/datacore.c
+++ b/libmultipath/prioritizers/datacore.c
@@ -24,6 +24,7 @@
#include <sg_include.h>
#include <debug.h>
#include <prio.h>
+#include <structs.h>
#define INQ_REPLY_LEN 255
#define INQ_CMD_CODE 0x12
diff --git a/libmultipath/prioritizers/emc.c b/libmultipath/prioritizers/emc.c
index 20d727e..87e9a8d 100644
--- a/libmultipath/prioritizers/emc.c
+++ b/libmultipath/prioritizers/emc.c
@@ -5,6 +5,7 @@
#include <sg_include.h>
#include <debug.h>
#include <prio.h>
+#include <structs.h>
#define INQUIRY_CMD 0x12
#define INQUIRY_CMDLEN 6
diff --git a/libmultipath/prioritizers/hds.c b/libmultipath/prioritizers/hds.c
index 4789340..b22e1df 100644
--- a/libmultipath/prioritizers/hds.c
+++ b/libmultipath/prioritizers/hds.c
@@ -75,6 +75,7 @@
#include <sg_include.h>
#include <debug.h>
#include <prio.h>
+#include <structs.h>
#define INQ_REPLY_LEN 255
#define INQ_CMD_CODE 0x12
diff --git a/libmultipath/prioritizers/hp_sw.c b/libmultipath/prioritizers/hp_sw.c
index 2de460f..c24baad 100644
--- a/libmultipath/prioritizers/hp_sw.c
+++ b/libmultipath/prioritizers/hp_sw.c
@@ -15,6 +15,7 @@
#include <sg_include.h>
#include <debug.h>
#include <prio.h>
+#include <structs.h>
#define TUR_CMD_LEN 6
#define SCSI_CHECK_CONDITION 0x2
diff --git a/libmultipath/prioritizers/ontap.c b/libmultipath/prioritizers/ontap.c
index 6e6e3d3..0d34092 100644
--- a/libmultipath/prioritizers/ontap.c
+++ b/libmultipath/prioritizers/ontap.c
@@ -22,6 +22,7 @@
#include <sg_include.h>
#include <debug.h>
#include <prio.h>
+#include <structs.h>
#define INQUIRY_CMD 0x12
#define INQUIRY_CMDLEN 6
diff --git a/libmultipath/prioritizers/rdac.c b/libmultipath/prioritizers/rdac.c
index 41ea887..8667790 100644
--- a/libmultipath/prioritizers/rdac.c
+++ b/libmultipath/prioritizers/rdac.c
@@ -5,6 +5,7 @@
#include <sg_include.h>
#include <debug.h>
#include <prio.h>
+#include <structs.h>
#define INQUIRY_CMD 0x12
#define INQUIRY_CMDLEN 6
diff --git a/libmultipath/prioritizers/weightedpath.c b/libmultipath/prioritizers/weightedpath.c
index d6c81f0..54c9039 100644
--- a/libmultipath/prioritizers/weightedpath.c
+++ b/libmultipath/prioritizers/weightedpath.c
@@ -60,6 +60,10 @@ int prio_path_weight(struct path *pp, char *prio_args)
regex = get_next_string(&temp, split_char);
+ /* Return default priority if the argument is not parseable */
+ if (!regex)
+ return priority;
+
if (!strcmp(regex, HBTL)) {
sprintf(path, "%d:%d:%d:%d", pp->sg_id.host_no,
pp->sg_id.channel, pp->sg_id.scsi_id, pp->sg_id.lun);
@@ -67,7 +71,7 @@ int prio_path_weight(struct path *pp, char *prio_args)
strcpy(path, pp->dev);
} else {
condlog(0, "%s: %s - Invalid arguments", pp->dev,
- pp->prio->name);
+ pp->prio.name);
return priority;
}
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index 6ac4caa..17bd893 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -383,20 +383,19 @@ extern int
select_prio (struct path * pp)
{
struct mpentry * mpe;
+ struct prio * p = &pp->prio;
if ((mpe = find_mpe(pp->wwid))) {
if (mpe->prio_name) {
- pp->prio = prio_lookup(mpe->prio_name);
- prio_set_args(pp->prio, mpe->prio_args);
+ prio_get(p, mpe->prio_name, mpe->prio_args);
condlog(3, "%s: prio = %s (LUN setting)",
- pp->dev, pp->prio->name);
+ pp->dev, prio_name(p));
return 0;
}
}
if (pp->hwe && pp->hwe->prio_name) {
- pp->prio = prio_lookup(pp->hwe->prio_name);
- prio_set_args(pp->prio, pp->hwe->prio_args);
+ prio_get(p, pp->hwe->prio_name, pp->hwe->prio_name);
condlog(3, "%s: prio = %s (controller setting)",
pp->dev, pp->hwe->prio_name);
condlog(3, "%s: prio args = %s (controller setting)",
@@ -404,16 +403,14 @@ select_prio (struct path * pp)
return 0;
}
if (conf->prio_name) {
- pp->prio = prio_lookup(conf->prio_name);
- prio_set_args(pp->prio, conf->prio_args);
+ prio_get(p, conf->prio_name, conf->prio_args);
condlog(3, "%s: prio = %s (config file default)",
pp->dev, conf->prio_name);
condlog(3, "%s: prio args = %s (config file default)",
pp->dev, conf->prio_args);
return 0;
}
- pp->prio = prio_lookup(DEFAULT_PRIO);
- prio_set_args(pp->prio, DEFAULT_PRIO_ARGS);
+ prio_get(p, DEFAULT_PRIO, DEFAULT_PRIO_ARGS);
condlog(3, "%s: prio = %s (internal default)",
pp->dev, DEFAULT_PRIO);
condlog(3, "%s: prio = %s (internal default)",
diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index 3c0fe90..ab57559 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -45,6 +45,9 @@ free_path (struct path * pp)
if (checker_selected(&pp->checker))
checker_put(&pp->checker);
+ if (prio_selected(&pp->prio))
+ prio_put(&pp->prio);
+
if (pp->fd >= 0)
close(pp->fd);
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 991ea6e..312014b 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -3,6 +3,8 @@
#include <sys/types.h>
+#include "prio.h"
+
#define WWID_SIZE 128
#define SERIAL_SIZE 65
#define NODE_NAME_SIZE 224
@@ -132,6 +134,7 @@ struct hd_geometry {
unsigned long start;
};
#endif
+
struct path {
char dev[FILE_NAME_SIZE];
char dev_t[BLK_DEV_SIZE];
@@ -157,7 +160,8 @@ struct path {
int priority;
int pgindex;
char * uid_attribute;
- struct prio * prio;
+ struct prio prio;
+ char * prio_args;
struct checker checker;
struct multipath * mpp;
int fd;
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index f998708..d914435 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -82,7 +82,7 @@ orphan_path (struct path * pp)
pp->mpp = NULL;
pp->dmstate = PSTATE_UNDEF;
pp->uid_attribute = NULL;
- pp->prio = NULL;
+ prio_put(&pp->prio);
checker_put(&pp->checker);
if (pp->fd >= 0)
close(pp->fd);
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 06/42] Accept several whitespaces in bindings file
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (4 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 05/42] prio: fix merging of prioritizers with different args Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 07/42] Add TAGS makefile target Hannes Reinecke
` (35 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
Prior versions of multipathd would accept several whitespaces
in the bindings file.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/alias.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/libmultipath/alias.c b/libmultipath/alias.c
index e1f3073..2b42606 100644
--- a/libmultipath/alias.c
+++ b/libmultipath/alias.c
@@ -99,7 +99,7 @@ lookup_binding(FILE *f, char *map_wwid, char **map_alias, char *prefix)
curr_id = scan_devname(alias, prefix);
if (curr_id >= id)
id = curr_id + 1;
- wwid = strtok(NULL, "");
+ wwid = strtok(NULL, " \t");
if (!wwid){
condlog(3,
"Ignoring malformed line %u in bindings file",
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 07/42] Add TAGS makefile target
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (5 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 06/42] Accept several whitespaces in bindings file Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 08/42] libmultipath: Fix typo in mp_prio_handler() Hannes Reinecke
` (34 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
Makefile | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index 5b0c61a..baf7753 100644
--- a/Makefile
+++ b/Makefile
@@ -66,6 +66,13 @@ install: recurse_install
uninstall: recurse_uninstall
+.PHONY: TAGS
+TAGS:
+ etags -a libmultipath/*.c
+ etags -a libmultipath/*.h
+ etags -a multipathd/*.c
+ etags -a multipathd/*.h
+
release:
sed -e "s/__VERSION__/${VERSION}/" \
multipath-tools.spec.in > multipath-tools.spec
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 08/42] libmultipath: Fix typo in mp_prio_handler()
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (6 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 07/42] Add TAGS makefile target Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 09/42] Do not trigger a map reload on priority updates Hannes Reinecke
` (33 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
The mpentry is found in conf->mptable, not conf->hwtable.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/dict.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index ebc49a3..fe1362e 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -1622,7 +1622,7 @@ mp_flush_on_last_del_handler(vector strvec)
static int
mp_prio_handler(vector strvec)
{
- struct mpentry * mpe = VECTOR_LAST_SLOT(conf->hwtable);
+ struct mpentry * mpe = VECTOR_LAST_SLOT(conf->mptable);
if (!mpe)
return 1;
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 09/42] Do not trigger a map reload on priority updates
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (7 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 08/42] libmultipath: Fix typo in mp_prio_handler() Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 10/42] Introduce MP_FAST_IO_FAIL_UNSET Hannes Reinecke
` (32 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
update_path_groups() is just there to update the priority groups,
so it should trigger a table reload only if the priority has
indeed changed.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/configure.c | 13 ++++++++-----
libmultipath/configure.h | 2 +-
multipathd/cli_handlers.c | 2 +-
multipathd/main.c | 27 ++++++---------------------
4 files changed, 16 insertions(+), 28 deletions(-)
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 1bb45a3..0f1fe43 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -765,14 +765,17 @@ out:
return NULL;
}
-extern int reload_map(struct vectors *vecs, struct multipath *mpp)
+extern int reload_map(struct vectors *vecs, struct multipath *mpp, int refresh)
{
- char params[PARAMS_SIZE];
- int r;
+ char params[PARAMS_SIZE] = {0};
+ struct path *pp;
+ int i, r;
update_mpp_paths(mpp, vecs->pathvec);
-
- params[0] = '\0';
+ if (refresh) {
+ vector_foreach_slot (mpp->paths, pp, i)
+ pathinfo(pp, conf->hwtable, DI_PRIO);
+ }
if (setup_map(mpp, params, PARAMS_SIZE)) {
condlog(0, "%s: failed to setup map", mpp->alias);
return 1;
diff --git a/libmultipath/configure.h b/libmultipath/configure.h
index 6c1c493..d13c0ac 100644
--- a/libmultipath/configure.h
+++ b/libmultipath/configure.h
@@ -28,5 +28,5 @@ int domap (struct multipath * mpp, char * params);
int reinstate_paths (struct multipath *mpp);
int coalesce_paths (struct vectors *vecs, vector curmp, char * refwwid, int force_reload);
char * get_refwwid (char * dev, enum devtypes dev_type, vector pathvec);
-int reload_map(struct vectors *vecs, struct multipath *mpp);
+int reload_map(struct vectors *vecs, struct multipath *mpp, int refresh);
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 544cbfb..6053b5a 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -561,7 +561,7 @@ cli_reload(void *v, char **reply, int *len, void *data)
return 1;
}
- return reload_map(vecs, mpp);
+ return reload_map(vecs, mpp, 0);
}
int resize_map(struct multipath *mpp, unsigned long long size,
diff --git a/multipathd/main.c b/multipathd/main.c
index 84574b2..7fe9c5b 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -674,11 +674,12 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
uev->kernel);
return 1;
}
- if (pp->mpp)
- retval = reload_map(vecs, pp->mpp);
+ if (pp->mpp) {
+ retval = reload_map(vecs, pp->mpp, 0);
- condlog(2, "%s: map %s reloaded (retval %d)",
- uev->kernel, pp->mpp->alias, retval);
+ condlog(2, "%s: map %s reloaded (retval %d)",
+ uev->kernel, pp->mpp->alias, retval);
+ }
}
@@ -1086,25 +1087,9 @@ int update_prio(struct path *pp, int refresh_all)
int update_path_groups(struct multipath *mpp, struct vectors *vecs, int refresh)
{
- int i;
- struct path * pp;
- char params[PARAMS_SIZE];
-
- update_mpp_paths(mpp, vecs->pathvec);
- if (refresh) {
- vector_foreach_slot (mpp->paths, pp, i)
- pathinfo(pp, conf->hwtable, DI_PRIO);
- }
- params[0] = '\0';
- if (setup_map(mpp, params, PARAMS_SIZE))
+ if (reload_map(vecs, mpp, refresh))
return 1;
- mpp->action = ACT_RELOAD;
- if (domap(mpp, params) <= 0) {
- condlog(0, "%s: failed to update map : %s", mpp->alias,
- strerror(errno));
- return 1;
- }
dm_lib_release();
if (setup_multipath(vecs, mpp) != 0)
return 1;
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 10/42] Introduce MP_FAST_IO_FAIL_UNSET
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (8 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 09/42] Do not trigger a map reload on priority updates Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 11/42] Checker name is not displayed on failure Hannes Reinecke
` (31 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
For completeness; all other special values are encoded with
defines, so 'unset' should be, too.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/config.h | 1 +
libmultipath/dict.c | 8 ++++----
libmultipath/discovery.c | 6 +++---
libmultipath/propsel.c | 6 +++---
libmultipath/structs.c | 1 +
5 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/libmultipath/config.h b/libmultipath/config.h
index 16f530f..51fc492 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -11,6 +11,7 @@
* In kernel, fast_io_fail == 0 means immediate failure on rport delete.
* OTOH '0' means not-configured in various places in multipath-tools.
*/
+#define MP_FAST_IO_FAIL_UNSET (0)
#define MP_FAST_IO_FAIL_OFF (-1)
#define MP_FAST_IO_FAIL_ZERO (-2)
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index fe1362e..ab9fdeb 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -47,7 +47,7 @@ def_fast_io_fail_handler(vector strvec)
conf->fast_io_fail = MP_FAST_IO_FAIL_OFF;
else if (sscanf(buff, "%d", &conf->fast_io_fail) != 1 ||
conf->fast_io_fail < MP_FAST_IO_FAIL_ZERO)
- conf->fast_io_fail = 0;
+ conf->fast_io_fail = MP_FAST_IO_FAIL_UNSET;
else if (conf->fast_io_fail == 0)
conf->fast_io_fail = MP_FAST_IO_FAIL_ZERO;
@@ -882,7 +882,7 @@ hw_fast_io_fail_handler(vector strvec)
hwe->fast_io_fail = MP_FAST_IO_FAIL_OFF;
else if (sscanf(buff, "%d", &hwe->fast_io_fail) != 1 ||
hwe->fast_io_fail < MP_FAST_IO_FAIL_ZERO)
- hwe->fast_io_fail = 0;
+ hwe->fast_io_fail = MP_FAST_IO_FAIL_UNSET;
else if (hwe->fast_io_fail == 0)
hwe->fast_io_fail = MP_FAST_IO_FAIL_ZERO;
@@ -1973,7 +1973,7 @@ static int
snprint_hw_fast_io_fail(char * buff, int len, void * data)
{
struct hwentry * hwe = (struct hwentry *)data;
- if (!hwe->fast_io_fail)
+ if (hwe->fast_io_fail == MP_FAST_IO_FAIL_UNSET)
return 0;
if (hwe->fast_io_fail == conf->fast_io_fail)
return 0;
@@ -2282,7 +2282,7 @@ snprint_def_polling_interval (char * buff, int len, void * data)
static int
snprint_def_fast_io_fail(char * buff, int len, void * data)
{
- if (!conf->fast_io_fail)
+ if (conf->fast_io_fail == MP_FAST_IO_FAIL_UNSET)
return 0;
if (conf->fast_io_fail == MP_FAST_IO_FAIL_OFF)
return snprintf(buff, len, "off");
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 6f5470f..e328332 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -283,7 +283,7 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
snprintf(value, 11, "%u", mpp->dev_loss);
if (mpp->dev_loss &&
sysfs_attr_set_value(rport_dev, "dev_loss_tmo", value, 11) <= 0) {
- if ((!mpp->fast_io_fail ||
+ if ((mpp->fast_io_fail == MP_FAST_IO_FAIL_UNSET ||
mpp->fast_io_fail == MP_FAST_IO_FAIL_OFF)
&& mpp->dev_loss > 600) {
condlog(3, "%s: limiting dev_loss_tmo to 600, since "
@@ -296,7 +296,7 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
goto out;
}
}
- if (mpp->fast_io_fail){
+ if (mpp->fast_io_fail != MP_FAST_IO_FAIL_UNSET){
if (mpp->fast_io_fail == MP_FAST_IO_FAIL_OFF)
sprintf(value, "off");
else if (mpp->fast_io_fail == MP_FAST_IO_FAIL_ZERO)
@@ -340,7 +340,7 @@ sysfs_set_scsi_tmo (struct multipath *mpp)
mpp->alias, mpp->fast_io_fail);
mpp->fast_io_fail = MP_FAST_IO_FAIL_OFF;
}
- if (!mpp->dev_loss && !mpp->fast_io_fail)
+ if (!mpp->dev_loss && mpp->fast_io_fail == MP_FAST_IO_FAIL_UNSET)
return 0;
vector_foreach_slot(mpp->paths, pp, i) {
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index 17bd893..756aced 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -558,7 +558,7 @@ select_pg_timeout(struct multipath *mp)
extern int
select_fast_io_fail(struct multipath *mp)
{
- if (mp->hwe && mp->hwe->fast_io_fail) {
+ if (mp->hwe && mp->hwe->fast_io_fail != MP_FAST_IO_FAIL_UNSET) {
mp->fast_io_fail = mp->hwe->fast_io_fail;
if (mp->fast_io_fail == MP_FAST_IO_FAIL_OFF)
condlog(3, "%s: fast_io_fail_tmo = off (controller default)", mp->alias);
@@ -567,7 +567,7 @@ select_fast_io_fail(struct multipath *mp)
mp->fast_io_fail == MP_FAST_IO_FAIL_ZERO ? 0 : mp->fast_io_fail);
return 0;
}
- if (conf->fast_io_fail) {
+ if (conf->fast_io_fail != MP_FAST_IO_FAIL_UNSET) {
mp->fast_io_fail = conf->fast_io_fail;
if (mp->fast_io_fail == MP_FAST_IO_FAIL_OFF)
condlog(3, "%s: fast_io_fail_tmo = off (config file default)", mp->alias);
@@ -576,7 +576,7 @@ select_fast_io_fail(struct multipath *mp)
mp->fast_io_fail == MP_FAST_IO_FAIL_ZERO ? 0 : mp->fast_io_fail);
return 0;
}
- mp->fast_io_fail = 0;
+ mp->fast_io_fail = MP_FAST_IO_FAIL_UNSET;
return 0;
}
diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index ab57559..88e8706 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -131,6 +131,7 @@ alloc_multipath (void)
mpp->bestpg = 1;
mpp->mpcontext = NULL;
mpp->no_path_retry = NO_PATH_RETRY_UNDEF;
+ mpp->fast_io_fail = MP_FAST_IO_FAIL_UNSET;
}
return mpp;
}
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 11/42] Checker name is not displayed on failure
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (9 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 10/42] Introduce MP_FAST_IO_FAIL_UNSET Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 12/42] Valgrind fixes for prioritizer Hannes Reinecke
` (30 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
If add_checker() isn't able to locate the checker
it won't display the name in free_checker().
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/prio.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/libmultipath/prio.c b/libmultipath/prio.c
index cf97fad..8e6b93e 100644
--- a/libmultipath/prio.c
+++ b/libmultipath/prio.c
@@ -86,6 +86,7 @@ struct prio * add_prio (char * name)
p = alloc_prio();
if (!p)
return NULL;
+ snprintf(p->name, PRIO_NAME_LEN, "%s", name);
snprintf(libname, LIB_PRIO_NAMELEN, "%s/libprio%s.so",
conf->multipath_dir, name);
if (stat(libname,&stbuf) < 0) {
@@ -107,7 +108,6 @@ struct prio * add_prio (char * name)
condlog(0, "A dynamic linking error occurred: (%s)", errstr);
if (!p->getprio)
goto out;
- snprintf(p->name, PRIO_NAME_LEN, "%s", name);
list_add(&p->node, &prioritizers);
return p;
out:
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 12/42] Valgrind fixes for prioritizer
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (10 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 11/42] Checker name is not displayed on failure Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 13/42] Incorrect inquiry vendor length in hds prioritizer Hannes Reinecke
` (29 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
Declaring an array does not zero out its contents. So we might
be reading random garbage here.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/prioritizers/hds.c | 1 +
libmultipath/prioritizers/ontap.c | 1 +
libmultipath/prioritizers/rdac.c | 1 +
3 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/libmultipath/prioritizers/hds.c b/libmultipath/prioritizers/hds.c
index b22e1df..5d75f84 100644
--- a/libmultipath/prioritizers/hds.c
+++ b/libmultipath/prioritizers/hds.c
@@ -105,6 +105,7 @@ int hds_modular_prio (const char *dev, int fd)
}
memset (&io_hdr, 0, sizeof (sg_io_hdr_t));
+ memset (inqBuff, 0, INQ_REPLY_LEN);
io_hdr.interface_id = 'S';
io_hdr.cmd_len = sizeof (inqCmdBlk);
io_hdr.mx_sb_len = sizeof (sense_buffer);
diff --git a/libmultipath/prioritizers/ontap.c b/libmultipath/prioritizers/ontap.c
index 0d34092..026d45d 100644
--- a/libmultipath/prioritizers/ontap.c
+++ b/libmultipath/prioritizers/ontap.c
@@ -80,6 +80,7 @@ static int send_gva(const char *dev, int fd, unsigned char pg,
int ret = -1;
memset(&io_hdr, 0, sizeof (struct sg_io_hdr));
+ memset(results, 0, *results_size);
io_hdr.interface_id = 'S';
io_hdr.cmd_len = sizeof (cdb);
io_hdr.mx_sb_len = sizeof (sb);
diff --git a/libmultipath/prioritizers/rdac.c b/libmultipath/prioritizers/rdac.c
index 8667790..441b3b0 100644
--- a/libmultipath/prioritizers/rdac.c
+++ b/libmultipath/prioritizers/rdac.c
@@ -22,6 +22,7 @@ int rdac_prio(const char *dev, int fd)
int ret = 0;
memset(&io_hdr, 0, sizeof (struct sg_io_hdr));
+ memset(sense_buffer, 0, 256);
io_hdr.interface_id = 'S';
io_hdr.cmd_len = sizeof (inqCmdBlk);
io_hdr.mx_sb_len = sizeof (sb);
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 13/42] Incorrect inquiry vendor length in hds prioritizer
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (11 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 12/42] Valgrind fixes for prioritizer Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 14/42] Print out multipath alias for flush_on_last_del messages Hannes Reinecke
` (28 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
The inquiry vendor length is 8 bytes, but snprintf writes
the given number of bytes _including_ the NULL byte. So
we need to supply a 9 byte buffer here.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/prioritizers/hds.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libmultipath/prioritizers/hds.c b/libmultipath/prioritizers/hds.c
index 5d75f84..f748707 100644
--- a/libmultipath/prioritizers/hds.c
+++ b/libmultipath/prioritizers/hds.c
@@ -87,7 +87,7 @@
int hds_modular_prio (const char *dev, int fd)
{
int k;
- char vendor[8];
+ char vendor[9];
char product[32];
char serial[32];
char ldev[32];
@@ -125,7 +125,7 @@ int hds_modular_prio (const char *dev, int fd)
return -1;
}
- snprintf (vendor, 8, "%.8s", inqBuffp + 8);
+ snprintf (vendor, 9, "%.8s", inqBuffp + 8);
snprintf (product, 17, "%.16s", inqBuffp + 16);
snprintf (serial, 5, "%.4s", inqBuffp + 40);
snprintf (ldev, 5, "%.4s", inqBuffp + 44);
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 14/42] Print out multipath alias for flush_on_last_del messages
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (12 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 13/42] Incorrect inquiry vendor length in hds prioritizer Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 15/42] Clarify setting origin in propsel.c Hannes Reinecke
` (27 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
Added for consistency.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/propsel.c | 15 ++++++++-------
1 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index 756aced..a02a644 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -606,24 +606,25 @@ select_flush_on_last_del(struct multipath *mp)
return 0;
if (mp->mpe && mp->mpe->flush_on_last_del != FLUSH_UNDEF) {
mp->flush_on_last_del = mp->mpe->flush_on_last_del;
- condlog(3, "flush_on_last_del = %i (multipath setting)",
- mp->flush_on_last_del);
+ condlog(3, "%s: flush_on_last_del = %i (multipath setting)",
+ mp->alias, mp->flush_on_last_del);
return 0;
}
if (mp->hwe && mp->hwe->flush_on_last_del != FLUSH_UNDEF) {
mp->flush_on_last_del = mp->hwe->flush_on_last_del;
- condlog(3, "flush_on_last_del = %i (controler setting)",
- mp->flush_on_last_del);
+ condlog(3, "%s: flush_on_last_del = %i (controler setting)",
+ mp->alias, mp->flush_on_last_del);
return 0;
}
if (conf->flush_on_last_del != FLUSH_UNDEF) {
mp->flush_on_last_del = conf->flush_on_last_del;
- condlog(3, "flush_on_last_del = %i (config file default)",
- mp->flush_on_last_del);
+ condlog(3, "%s: flush_on_last_del = %i (config file default)",
+ mp->alias, mp->flush_on_last_del);
return 0;
}
mp->flush_on_last_del = FLUSH_UNDEF;
- condlog(3, "flush_on_last_del = DISABLED (internal default)");
+ condlog(3, "%s: flush_on_last_del = DISABLED (internal default)",
+ mp->alias);
return 0;
}
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 15/42] Clarify setting origin in propsel.c
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (13 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 14/42] Print out multipath alias for flush_on_last_del messages Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 16/42] libmultipath: error checking in remove_features() Hannes Reinecke
` (26 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
We should differentiate between config file and internal default.
And the controller settings are not defaults.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/propsel.c | 17 ++++++++++++-----
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index a02a644..b8c6b08 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -282,8 +282,11 @@ select_features (struct multipath * mp)
} else if (mp->hwe && mp->hwe->features) {
mp->features = STRDUP(mp->hwe->features);
origin = "controller setting";
- } else {
+ } else if (conf->features) {
mp->features = STRDUP(conf->features);
+ origin = "config file default";
+ } else {
+ mp->features = set_default(DEFAULT_FEATURES);
origin = "internal default";
}
condlog(3, "%s: features = %s (%s)",
@@ -561,18 +564,22 @@ select_fast_io_fail(struct multipath *mp)
if (mp->hwe && mp->hwe->fast_io_fail != MP_FAST_IO_FAIL_UNSET) {
mp->fast_io_fail = mp->hwe->fast_io_fail;
if (mp->fast_io_fail == MP_FAST_IO_FAIL_OFF)
- condlog(3, "%s: fast_io_fail_tmo = off (controller default)", mp->alias);
+ condlog(3, "%s: fast_io_fail_tmo = off "
+ "(controller setting)", mp->alias);
else
- condlog(3, "%s: fast_io_fail_tmo = %d (controller default)", mp->alias,
+ condlog(3, "%s: fast_io_fail_tmo = %d "
+ "(controller setting)", mp->alias,
mp->fast_io_fail == MP_FAST_IO_FAIL_ZERO ? 0 : mp->fast_io_fail);
return 0;
}
if (conf->fast_io_fail != MP_FAST_IO_FAIL_UNSET) {
mp->fast_io_fail = conf->fast_io_fail;
if (mp->fast_io_fail == MP_FAST_IO_FAIL_OFF)
- condlog(3, "%s: fast_io_fail_tmo = off (config file default)", mp->alias);
+ condlog(3, "%s: fast_io_fail_tmo = off "
+ "(config file default)", mp->alias);
else
- condlog(3, "%s: fast_io_fail_tmo = %d (config file default)", mp->alias,
+ condlog(3, "%s: fast_io_fail_tmo = %d "
+ "(config file default)", mp->alias,
mp->fast_io_fail == MP_FAST_IO_FAIL_ZERO ? 0 : mp->fast_io_fail);
return 0;
}
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 16/42] libmultipath: error checking in remove_features()
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (14 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 15/42] Clarify setting origin in propsel.c Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 17/42] Increase parameter buffer Hannes Reinecke
` (25 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
An error check was missing in remove_features().
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/structs.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index 88e8706..049f17d 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -559,6 +559,9 @@ remove_feature(char **f, char *o)
* about to be removed
*/
p = strchr(*f, ' ');
+ if (!p)
+ /* Internal error, feature string inconsistent */
+ return 1;
while (*p == ' ')
p++;
p--;
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 17/42] Increase parameter buffer
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (15 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 16/42] libmultipath: error checking in remove_features() Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 18/42] Check return code from pathinfo() Hannes Reinecke
` (24 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
Multipath is using an internal static buffer for assembling
device-mapper tables, which might be too small for large setups.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/structs.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 312014b..16e940b 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -9,7 +9,7 @@
#define SERIAL_SIZE 65
#define NODE_NAME_SIZE 224
#define PATH_STR_SIZE 16
-#define PARAMS_SIZE 1024
+#define PARAMS_SIZE 4096
#define FILE_NAME_SIZE 256
#define CALLOUT_MAX_SIZE 256
#define BLK_DEV_SIZE 33
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 18/42] Check return code from pathinfo()
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (16 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 17/42] Increase parameter buffer Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 19/42] Inconsistent string quoting Hannes Reinecke
` (23 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
Pathinfo might fail, which indicates that the path is not
available anymore. So check the return value and take
appropriate action.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/discovery.c | 2 +-
libmultipath/structs_vec.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index e328332..b5df8e3 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -680,7 +680,7 @@ path_offline (struct path * pp)
return PATH_DOWN;
}
-extern int
+int
sysfs_pathinfo(struct path * pp)
{
if (common_sysfs_pathinfo(pp))
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index d914435..6d36d58 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -68,9 +68,9 @@ adopt_paths (vector pathvec, struct multipath * mpp, int get_info)
if (!find_path_by_dev(mpp->paths, pp->dev) &&
store_path(mpp->paths, pp))
return 1;
- if (get_info)
- pathinfo(pp, conf->hwtable,
- DI_PRIO | DI_CHECKER);
+ if (get_info && pathinfo(pp, conf->hwtable,
+ DI_PRIO | DI_CHECKER))
+ return 1;
}
}
return 0;
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 19/42] Inconsistent string quoting
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (17 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 18/42] Check return code from pathinfo() Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 20/42] Switch off 'queue_if_no_path' before removing maps Hannes Reinecke
` (22 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
When printing the hardware table strings are quoted twice, and
even numerical values have single quotes. This patch removes
the double quotes and retains single quotes only for string
values.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/dict.c | 98 +++++++++++++++++++++++++-------------------------
1 files changed, 49 insertions(+), 49 deletions(-)
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index ab9fdeb..6dae335 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -1062,11 +1062,11 @@ hw_failback_handler(vector strvec)
buff = set_value(strvec);
- if (strlen(buff) == 6 && !strcmp(buff, "manual"))
+ if (strlen(buff) == 6 && !strcmp(buff, "\"manual\""))
hwe->pgfailback = -FAILBACK_MANUAL;
- else if (strlen(buff) == 9 && !strcmp(buff, "immediate"))
+ else if (strlen(buff) == 9 && !strcmp(buff, "\"immediate\""))
hwe->pgfailback = -FAILBACK_IMMEDIATE;
- else if (strlen(buff) == 10 && !strcmp(buff, "followover"))
+ else if (strlen(buff) == 10 && !strcmp(buff, "\"followover\""))
hwe->pgfailback = -FAILBACK_FOLLOWOVER;
else
hwe->pgfailback = atoi(buff);
@@ -1757,7 +1757,7 @@ snprint_mp_path_grouping_policy (char * buff, int len, void * data)
return 0;
get_pgpolicy_name(str, POLICY_NAME_SIZE, mpe->pgpolicy);
- return snprintf(buff, len, "%s", str);
+ return snprintf(buff, len, "\"%s\"", str);
}
static int
@@ -1832,9 +1832,9 @@ snprint_mp_rr_weight (char * buff, int len, void * data)
if (!mpe->rr_weight)
return 0;
if (mpe->rr_weight == RR_WEIGHT_PRIO)
- return snprintf(buff, len, "priorities");
+ return snprintf(buff, len, "\"priorities\"");
if (mpe->rr_weight == RR_WEIGHT_NONE)
- return snprintf(buff, len, "uniform");
+ return snprintf(buff, len, "\"uniform\"");
return 0;
}
@@ -1851,9 +1851,9 @@ snprint_mp_no_path_retry (char * buff, int len, void * data)
case NO_PATH_RETRY_UNDEF:
break;
case NO_PATH_RETRY_FAIL:
- return snprintf(buff, len, "fail");
+ return snprintf(buff, len, "\"fail\"");
case NO_PATH_RETRY_QUEUE:
- return snprintf(buff, len, "queue");
+ return snprintf(buff, len, "\"queue\"");
default:
return snprintf(buff, len, "%i",
mpe->no_path_retry);
@@ -1892,7 +1892,7 @@ snprint_mp_pg_timeout (char * buff, int len, void * data)
case PGTIMEOUT_UNDEF:
break;
case -PGTIMEOUT_NONE:
- return snprintf(buff, len, "none");
+ return snprintf(buff, len, "\"none\"");
default:
return snprintf(buff, len, "%i", mpe->pg_timeout);
}
@@ -1910,7 +1910,7 @@ snprint_mp_features (char * buff, int len, void * data)
!strcmp(mpe->features, conf->features))
return 0;
- return snprintf(buff, len, "%s", mpe->features);
+ return snprintf(buff, len, "\"%s\"", mpe->features);
}
static int
@@ -1920,9 +1920,9 @@ snprint_mp_flush_on_last_del (char * buff, int len, void * data)
switch (mpe->flush_on_last_del) {
case FLUSH_DISABLED:
- return snprintf(buff, len, "no");
+ return snprintf(buff, len, "\"no\"");
case FLUSH_ENABLED:
- return snprintf(buff, len, "yes");
+ return snprintf(buff, len, "\"yes\"");
}
return 0;
}
@@ -1935,7 +1935,7 @@ snprint_mp_prio(char * buff, int len, void * data)
if (!mpe->prio_name)
return 0;
- return snprintf(buff, len, "%s", mpe->prio_name);
+ return snprintf(buff, len, "\"%s\"", mpe->prio_name);
}
static int
@@ -1946,7 +1946,7 @@ snprint_mp_prio_args(char * buff, int len, void * data)
if (!mpe->prio_args)
return 0;
- return snprintf(buff, len, "%s", mpe->prio_args);
+ return snprintf(buff, len, "\"%s\"", mpe->prio_args);
}
static int
@@ -1978,7 +1978,7 @@ snprint_hw_fast_io_fail(char * buff, int len, void * data)
if (hwe->fast_io_fail == conf->fast_io_fail)
return 0;
if (hwe->fast_io_fail == MP_FAST_IO_FAIL_OFF)
- return snprintf(buff, len, "off");
+ return snprintf(buff, len, "\"off\"");
if (hwe->fast_io_fail == MP_FAST_IO_FAIL_ZERO)
return snprintf(buff, len, "0");
return snprintf(buff, len, "%d", hwe->fast_io_fail);
@@ -1993,7 +1993,7 @@ snprint_hw_dev_loss(char * buff, int len, void * data)
if (hwe->dev_loss == conf->dev_loss)
return 0;
if (hwe->dev_loss >= MAX_DEV_LOSS_TMO)
- return snprintf(buff, len, "infinity");
+ return snprintf(buff, len, "\"infinity\"");
return snprintf(buff, len, "%u", hwe->dev_loss);
}
@@ -2061,7 +2061,7 @@ snprint_hw_prio (char * buff, int len, void * data)
if (!hwe->prio_name)
return 0;
- return snprintf(buff, len, "%s", hwe->prio_name);
+ return snprintf(buff, len, "\"%s\"", hwe->prio_name);
}
static int
@@ -2131,7 +2131,7 @@ snprint_hw_path_grouping_policy (char * buff, int len, void * data)
get_pgpolicy_name(str, POLICY_NAME_SIZE, hwe->pgpolicy);
- return snprintf(buff, len, "%s", str);
+ return snprintf(buff, len, "\"%s\"", str);
}
static int
@@ -2165,9 +2165,9 @@ snprint_hw_rr_weight (char * buff, int len, void * data)
if (!hwe->rr_weight)
return 0;
if (hwe->rr_weight == RR_WEIGHT_PRIO)
- return snprintf(buff, len, "priorities");
+ return snprintf(buff, len, "\"priorities\"");
if (hwe->rr_weight == RR_WEIGHT_NONE)
- return snprintf(buff, len, "uniform");
+ return snprintf(buff, len, "\"uniform\"");
return 0;
}
@@ -2184,9 +2184,9 @@ snprint_hw_no_path_retry (char * buff, int len, void * data)
case NO_PATH_RETRY_UNDEF:
break;
case NO_PATH_RETRY_FAIL:
- return snprintf(buff, len, "fail");
+ return snprintf(buff, len, "\"fail\"");
case NO_PATH_RETRY_QUEUE:
- return snprintf(buff, len, "queue");
+ return snprintf(buff, len, "\"queue\"");
default:
return snprintf(buff, len, "%i",
hwe->no_path_retry);
@@ -2228,7 +2228,7 @@ snprint_hw_pg_timeout (char * buff, int len, void * data)
case PGTIMEOUT_UNDEF:
break;
case -PGTIMEOUT_NONE:
- return snprintf(buff, len, "none");
+ return snprintf(buff, len, "\"none\"");
default:
return snprintf(buff, len, "%i", hwe->pg_timeout);
}
@@ -2242,9 +2242,9 @@ snprint_hw_flush_on_last_del (char * buff, int len, void * data)
switch (hwe->flush_on_last_del) {
case FLUSH_DISABLED:
- return snprintf(buff, len, "no");
+ return snprintf(buff, len, "\"no\"");
case FLUSH_ENABLED:
- return snprintf(buff, len, "yes");
+ return snprintf(buff, len, "\"yes\"");
}
return 0;
}
@@ -2257,7 +2257,7 @@ snprint_hw_path_checker (char * buff, int len, void * data)
if (!hwe->checker_name)
return 0;
- return snprintf(buff, len, "%s", hwe->checker_name);
+ return snprintf(buff, len, "\"%s\"", hwe->checker_name);
}
static int
@@ -2285,7 +2285,7 @@ snprint_def_fast_io_fail(char * buff, int len, void * data)
if (conf->fast_io_fail == MP_FAST_IO_FAIL_UNSET)
return 0;
if (conf->fast_io_fail == MP_FAST_IO_FAIL_OFF)
- return snprintf(buff, len, "off");
+ return snprintf(buff, len, "\"off\"");
if (conf->fast_io_fail == MP_FAST_IO_FAIL_ZERO)
return snprintf(buff, len, "0");
return snprintf(buff, len, "%d", conf->fast_io_fail);
@@ -2297,7 +2297,7 @@ snprint_def_dev_loss(char * buff, int len, void * data)
if (!conf->dev_loss)
return 0;
if (conf->dev_loss >= MAX_DEV_LOSS_TMO)
- return snprintf(buff, len, "infinity");
+ return snprintf(buff, len, "\"infinity\"");
return snprintf(buff, len, "%u", conf->dev_loss);
}
@@ -2320,7 +2320,7 @@ snprint_reassign_maps (char * buff, int len, void * data)
{
if (conf->reassign_maps == DEFAULT_REASSIGN_MAPS)
return 0;
- return snprintf(buff, len, "%s",
+ return snprintf(buff, len, "\"%s\"",
conf->reassign_maps?"yes":"no");
}
@@ -2353,7 +2353,7 @@ snprint_def_path_grouping_policy (char * buff, int len, void * data)
get_pgpolicy_name(str, POLICY_NAME_SIZE, pgpolicy);
- return snprintf(buff, len, "%s", str);
+ return snprintf(buff, len, "\"%s\"", str);
}
static int
@@ -2369,9 +2369,9 @@ static int
snprint_def_prio (char * buff, int len, void * data)
{
if (!conf->prio_name)
- return snprintf(buff, len, "%s", DEFAULT_PRIO);
+ return snprintf(buff, len, "\"%s\"", DEFAULT_PRIO);
- return snprintf(buff, len, "%s", conf->prio_name);
+ return snprintf(buff, len, "\"%s\"", conf->prio_name);
}
static int
@@ -2396,9 +2396,9 @@ static int
snprint_def_path_checker (char * buff, int len, void * data)
{
if (!conf->checker_name)
- return snprintf(buff, len, "%s", DEFAULT_CHECKER);
+ return snprintf(buff, len, "\"%s\"", DEFAULT_CHECKER);
- return snprintf(buff, len, "%s", conf->checker_name);
+ return snprintf(buff, len, "\"%s\"", conf->checker_name);
}
static int
@@ -2412,11 +2412,11 @@ snprint_def_failback (char * buff, int len, void * data)
case FAILBACK_UNDEF:
break;
case -FAILBACK_MANUAL:
- return snprintf(buff, len, "manual");
+ return snprintf(buff, len, "\"manual\"");
case -FAILBACK_IMMEDIATE:
- return snprintf(buff, len, "immediate");
+ return snprintf(buff, len, "\"immediate\"");
case -FAILBACK_FOLLOWOVER:
- return snprintf(buff, len, "followover");
+ return snprintf(buff, len, "\"followover\"");
default:
return snprintf(buff, len, "%i", conf->pgfailback);
}
@@ -2478,9 +2478,9 @@ static int
snprint_def_rr_weight (char * buff, int len, void * data)
{
if (!conf->rr_weight || conf->rr_weight == RR_WEIGHT_NONE)
- return snprintf(buff, len, "uniform");
+ return snprintf(buff, len, "\"uniform\"");
if (conf->rr_weight == RR_WEIGHT_PRIO)
- return snprintf(buff, len, "priorities");
+ return snprintf(buff, len, "\"priorities\"");
return 0;
}
@@ -2492,9 +2492,9 @@ snprint_def_no_path_retry (char * buff, int len, void * data)
case NO_PATH_RETRY_UNDEF:
break;
case NO_PATH_RETRY_FAIL:
- return snprintf(buff, len, "fail");
+ return snprintf(buff, len, "\"fail\"");
case NO_PATH_RETRY_QUEUE:
- return snprintf(buff, len, "queue");
+ return snprintf(buff, len, "\"queue\"");
default:
return snprintf(buff, len, "%i",
conf->no_path_retry);
@@ -2507,10 +2507,10 @@ snprint_def_queue_without_daemon (char * buff, int len, void * data)
{
switch (conf->queue_without_daemon) {
case QUE_NO_DAEMON_OFF:
- return snprintf(buff, len, "no");
+ return snprintf(buff, len, "\"no\"");
case QUE_NO_DAEMON_ON:
case QUE_NO_DAEMON_UNDEF:
- return snprintf(buff, len, "yes");
+ return snprintf(buff, len, "\"yes\"");
}
return 0;
}
@@ -2530,7 +2530,7 @@ snprint_def_pg_timeout (char * buff, int len, void * data)
switch (conf->pg_timeout) {
case PGTIMEOUT_UNDEF:
case -PGTIMEOUT_NONE:
- return snprintf(buff, len, "none");
+ return snprintf(buff, len, "\"none\"");
default:
return snprintf(buff, len, "%i", conf->pg_timeout);
}
@@ -2543,10 +2543,10 @@ snprint_def_flush_on_last_del (char * buff, int len, void * data)
switch (conf->flush_on_last_del) {
case FLUSH_UNDEF:
case FLUSH_DISABLED:
- return snprintf(buff, len, "no");
+ return snprintf(buff, len, "\"no\"");
case FLUSH_ENABLED:
case FLUSH_IN_PROGRESS:
- return snprintf(buff, len, "yes");
+ return snprintf(buff, len, "\"yes\"");
}
return 0;
}
@@ -2563,9 +2563,9 @@ static int
snprint_def_user_friendly_names (char * buff, int len, void * data)
{
if (conf->user_friendly_names == USER_FRIENDLY_NAMES_ON)
- return snprintf(buff, len, "yes");
+ return snprintf(buff, len, "\"yes\"");
else
- return snprintf(buff, len, "no");
+ return snprintf(buff, len, "\"no\"");
}
static int
@@ -2581,7 +2581,7 @@ snprint_def_bindings_file (char * buff, int len, void * data)
{
if (conf->bindings_file == NULL)
return 0;
- return snprintf(buff, len, "%s", conf->bindings_file);
+ return snprintf(buff, len, "\"%s\"", conf->bindings_file);
}
static int
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 20/42] Switch off 'queue_if_no_path' before removing maps
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (18 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 19/42] Inconsistent string quoting Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:53 ` [PATCH 21/42] Double free in disassemble_map() Hannes Reinecke
` (21 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
Before we try to flush a map we have to switch off the
'queue_if_no_path' setting to flush any outstanding I/O.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/devmapper.c | 26 +++++++++++++++++++++++++-
libmultipath/devmapper.h | 1 +
multipath/main.c | 6 +++---
3 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 051ecb2..a6e7150 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -707,6 +707,30 @@ _dm_flush_map (const char * mapname, int need_sync)
}
extern int
+dm_suspend_and_flush_map (const char * mapname)
+{
+ int s;
+
+ if (!dm_map_present(mapname))
+ return 0;
+
+ if (dm_type(mapname, TGT_MPATH) <= 0)
+ return 0; /* nothing to do */
+
+ s = dm_queue_if_no_path((char *)mapname, 0);
+ if (!s)
+ s = dm_simplecmd_flush(DM_DEVICE_SUSPEND, mapname, 0);
+
+ if (!dm_flush_map(mapname)) {
+ condlog(4, "multipath map %s removed", mapname);
+ return 0;
+ }
+ condlog(2, "failed to remove multipath map %s", mapname);
+ dm_simplecmd_noflush(DM_DEVICE_RESUME, mapname);
+ return 1;
+}
+
+extern int
dm_flush_maps (void)
{
int r = 0;
@@ -729,7 +753,7 @@ dm_flush_maps (void)
goto out;
do {
- r |= dm_flush_map(names->name);
+ r |= dm_suspend_and_flush_map(names->name);
next = names->next;
names = (void *) names + next;
} while (next);
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index 0c2e03f..b08aa31 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -22,6 +22,7 @@ int dm_type(const char *, char *);
int _dm_flush_map (const char *, int);
#define dm_flush_map(mapname) _dm_flush_map(mapname, 1)
#define dm_flush_map_nosync(mapname) _dm_flush_map(mapname, 0)
+int dm_suspend_and_flush_map(const char * mapname);
int dm_flush_maps (void);
int dm_fail_path(char * mapname, char * path);
int dm_reinstate_path(char * mapname, char * path);
diff --git a/multipath/main.c b/multipath/main.c
index 6208995..396c4b9 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -555,9 +555,9 @@ main (int argc, char *argv[])
goto out;
}
if (conf->remove == FLUSH_ONE) {
- if (conf->dev_type == DEV_DEVMAP)
- r = dm_flush_map(conf->dev);
- else
+ if (conf->dev_type == DEV_DEVMAP) {
+ r = dm_suspend_and_flush_map(conf->dev);
+ } else
condlog(0, "must provide a map name to remove");
goto out;
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 21/42] Double free in disassemble_map()
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (19 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 20/42] Switch off 'queue_if_no_path' before removing maps Hannes Reinecke
@ 2013-01-08 13:53 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 22/42] libmultipath: prio keyword ignored for multipath config Hannes Reinecke
` (20 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:53 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
Label 'out1' already frees 'word'; no need to do it here.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/dmparser.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
index a45854d..5184c41 100644
--- a/libmultipath/dmparser.c
+++ b/libmultipath/dmparser.c
@@ -276,7 +276,6 @@ disassemble_map (vector pathvec, char * params, struct multipath * mpp)
num_pg_args = atoi(word);
if (merge_words(&mpp->selector, word, 1)) {
- FREE(word);
goto out1;
}
FREE(word);
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 22/42] libmultipath: prio keyword ignored for multipath config
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (20 preceding siblings ...)
2013-01-08 13:53 ` [PATCH 21/42] Double free in disassemble_map() Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 23/42] Path checker should return PATH_DOWN when no path is found Hannes Reinecke
` (19 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
When specifying the 'prio' keyword in the multipath section
of the configuration file the value is ignored.
Problem is that the 'wwid' value is set only after the call
to select_prio(), so the correct definition couldn't been
found in the config file.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/discovery.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index b5df8e3..6848b70 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -891,6 +891,9 @@ pathinfo (struct path *pp, vector hwtable, int mask)
}
}
+ if (path_state == PATH_UP && (mask & DI_WWID) && !strlen(pp->wwid))
+ get_uid(pp);
+
/*
* Retrieve path priority, even for PATH_DOWN paths if it has never
* been successfully obtained before.
@@ -905,9 +908,6 @@ pathinfo (struct path *pp, vector hwtable, int mask)
}
}
- if (path_state == PATH_UP && (mask & DI_WWID) && !strlen(pp->wwid))
- get_uid(pp);
-
return 0;
blank:
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 23/42] Path checker should return PATH_DOWN when no path is found
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (21 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 22/42] libmultipath: prio keyword ignored for multipath config Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 24/42] Do not call sysfs_get_timeout for non-SCSI devices Hannes Reinecke
` (18 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
If the path checker fails to lookup the path in sysfs it's
already gone, so we should rather return 'PATH_DOWN' here.
Otherwise the path will never marked failed and no failover
will happen.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/discovery.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 6848b70..7ed8556 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -659,11 +659,11 @@ path_offline (struct path * pp)
if (!parent) {
condlog(1, "%s: failed to get sysfs information", pp->dev);
- return PATH_WILD;
+ return PATH_DOWN;
}
if (sysfs_get_state(parent, buff, SCSI_STATE_SIZE))
- return PATH_WILD;
+ return PATH_DOWN;
condlog(3, "%s: path state = %s", pp->dev, buff);
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 24/42] Do not call sysfs_get_timeout for non-SCSI devices
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (22 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 23/42] Path checker should return PATH_DOWN when no path is found Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 25/42] Make log_pthread more robust Hannes Reinecke
` (17 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
Only SCSI devices have a timeout, so there is no point in
trying to set a timeout for other types.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/discovery.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 7ed8556..3ffdc62 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -761,11 +761,13 @@ get_state (struct path * pp, int daemon)
checker_clear_message(c);
if (daemon)
checker_set_async(c);
- if (!conf->checker_timeout)
- sysfs_get_timeout(pp, &(c->timeout));
+ if (!conf->checker_timeout &&
+ (pp->bus != SYSFS_BUS_SCSI ||
+ sysfs_get_timeout(pp, &(c->timeout))))
+ c->timeout = DEF_TIMEOUT;
state = checker_check(c);
condlog(3, "%s: state = %s", pp->dev, checker_state_name(state));
- if (state == PATH_DOWN && strlen(checker_message(c)))
+ if (state != PATH_UP && strlen(checker_message(c)))
condlog(3, "%s: checker msg is \"%s\"",
pp->dev, checker_message(c));
return state;
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 25/42] Make log_pthread more robust
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (23 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 24/42] Do not call sysfs_get_timeout for non-SCSI devices Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-09 0:16 ` Christophe Varoqui
2013-01-08 13:54 ` [PATCH 26/42] Print log messages when updating tables failed Hannes Reinecke
` (16 subsequent siblings)
41 siblings, 1 reply; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
We don't need to allocate memory for mutexes, we can just
be using static variables. And valgrind complained about
logqueue flush from shutdown, so don't do this.
The normal shutdown process should be flushing the log
queue anyway.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/log_pthread.c | 101 +++++++++++++++++++++++++++++--------------
libmultipath/log_pthread.h | 10 +++--
2 files changed, 74 insertions(+), 37 deletions(-)
diff --git a/libmultipath/log_pthread.c b/libmultipath/log_pthread.c
index d701ba1..9e92a70 100644
--- a/libmultipath/log_pthread.c
+++ b/libmultipath/log_pthread.c
@@ -13,20 +13,41 @@
#include "log.h"
#include "lock.h"
+pthread_t log_thr;
+
+pthread_mutex_t logq_lock;
+pthread_mutex_t logev_lock;
+pthread_cond_t logev_cond;
+
+int logq_running;
+
+static void
+sigusr1 (int sig)
+{
+ pthread_mutex_lock(&logq_lock);
+ log_reset("multipathd");
+ pthread_mutex_unlock(&logq_lock);
+}
+
void log_safe (int prio, const char * fmt, va_list ap)
{
sigset_t old;
+ if (log_thr == (pthread_t)0) {
+ syslog(prio, fmt, ap);
+ return;
+ }
+
block_signal(SIGUSR1, &old);
block_signal(SIGHUP, NULL);
- pthread_mutex_lock(logq_lock);
+ pthread_mutex_lock(&logq_lock);
log_enqueue(prio, fmt, ap);
- pthread_mutex_unlock(logq_lock);
+ pthread_mutex_unlock(&logq_lock);
- pthread_mutex_lock(logev_lock);
- pthread_cond_signal(logev_cond);
- pthread_mutex_unlock(logev_lock);
+ pthread_mutex_lock(&logev_lock);
+ pthread_cond_signal(&logev_cond);
+ pthread_mutex_unlock(&logev_lock);
pthread_sigmask(SIG_SETMASK, &old, NULL);
}
@@ -36,9 +57,9 @@ static void flush_logqueue (void)
int empty;
do {
- pthread_mutex_lock(logq_lock);
+ pthread_mutex_lock(&logq_lock);
empty = log_dequeue(la->buff);
- pthread_mutex_unlock(logq_lock);
+ pthread_mutex_unlock(&logq_lock);
if (!empty)
log_syslog(la->buff);
} while (empty == 0);
@@ -46,15 +67,30 @@ static void flush_logqueue (void)
static void * log_thread (void * et)
{
+ struct sigaction sig;
+ int running;
+
+ sig.sa_handler = sigusr1;
+ sigemptyset(&sig.sa_mask);
+ sig.sa_flags = 0;
+ if (sigaction(SIGUSR1, &sig, NULL) < 0)
+ logdbg(stderr, "Cannot set signal handler");
+
+ pthread_mutex_lock(&logev_lock);
+ logq_running = 1;
+ pthread_mutex_unlock(&logev_lock);
+
mlockall(MCL_CURRENT | MCL_FUTURE);
logdbg(stderr,"enter log_thread\n");
while (1) {
- pthread_mutex_lock(logev_lock);
- pthread_cond_wait(logev_cond, logev_lock);
- pthread_mutex_unlock(logev_lock);
-
- flush_logqueue();
+ pthread_mutex_lock(&logev_lock);
+ pthread_cond_wait(&logev_cond, &logev_lock);
+ running = logq_running;
+ pthread_mutex_unlock(&logev_lock);
+ if (!running)
+ break;
+ log_thread_flush();
}
return NULL;
}
@@ -63,19 +99,18 @@ void log_thread_start (pthread_attr_t *attr)
{
logdbg(stderr,"enter log_thread_start\n");
- logq_lock = (pthread_mutex_t *) malloc(sizeof(pthread_mutex_t));
- logev_lock = (pthread_mutex_t *) malloc(sizeof(pthread_mutex_t));
- logev_cond = (pthread_cond_t *) malloc(sizeof(pthread_cond_t));
-
- pthread_mutex_init(logq_lock, NULL);
- pthread_mutex_init(logev_lock, NULL);
- pthread_cond_init(logev_cond, NULL);
+ pthread_mutex_init(&logq_lock, NULL);
+ pthread_mutex_init(&logev_lock, NULL);
+ pthread_cond_init(&logev_cond, NULL);
if (log_init("multipathd", 0)) {
fprintf(stderr,"can't initialize log buffer\n");
exit(1);
}
- pthread_create(&log_thr, attr, log_thread, NULL);
+ if (pthread_create(&log_thr, attr, log_thread, NULL)) {
+ fprintf(stderr,"can't start log thread\n");
+ exit(1);
+ }
return;
}
@@ -84,22 +119,22 @@ void log_thread_stop (void)
{
logdbg(stderr,"enter log_thread_stop\n");
- pthread_mutex_lock(logq_lock);
+ pthread_mutex_lock(&logev_lock);
+ logq_running = 0;
+ pthread_cond_signal(&logev_cond);
+ pthread_mutex_unlock(&logev_lock);
+
+ pthread_mutex_lock(&logq_lock);
pthread_cancel(log_thr);
- pthread_mutex_unlock(logq_lock);
+ pthread_mutex_unlock(&logq_lock);
pthread_join(log_thr, NULL);
+ log_thr = (pthread_t)0;
flush_logqueue();
- pthread_mutex_destroy(logq_lock);
- pthread_mutex_destroy(logev_lock);
- pthread_cond_destroy(logev_cond);
-
- free(logq_lock);
- logq_lock = NULL;
- free(logev_lock);
- logev_lock = NULL;
- free(logev_cond);
- logev_cond = NULL;
- free_logarea();
+ pthread_mutex_destroy(&logq_lock);
+ pthread_mutex_destroy(&logev_lock);
+ pthread_cond_destroy(&logev_cond);
+
+ log_close();
}
diff --git a/libmultipath/log_pthread.h b/libmultipath/log_pthread.h
index 77780d8..f3b70ea 100644
--- a/libmultipath/log_pthread.h
+++ b/libmultipath/log_pthread.h
@@ -3,11 +3,13 @@
#include <pthread.h>
-pthread_t log_thr;
+extern pthread_t log_thr;
-pthread_mutex_t *logq_lock;
-pthread_mutex_t *logev_lock;
-pthread_cond_t *logev_cond;
+extern pthread_mutex_t logq_lock;
+extern pthread_mutex_t logev_lock;
+extern pthread_cond_t logev_cond;
+
+extern int logq_running;
void log_safe(int prio, const char * fmt, va_list ap);
void log_thread_start(pthread_attr_t *attr);
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH 25/42] Make log_pthread more robust
2013-01-08 13:54 ` [PATCH 25/42] Make log_pthread more robust Hannes Reinecke
@ 2013-01-09 0:16 ` Christophe Varoqui
2013-01-09 19:15 ` Xose Vazquez Perez
0 siblings, 1 reply; 48+ messages in thread
From: Christophe Varoqui @ 2013-01-09 0:16 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: dm-devel
On mar., 2013-01-08 at 14:54 +0100, Hannes Reinecke wrote:
> We don't need to allocate memory for mutexes, we can just
> be using static variables. And valgrind complained about
> logqueue flush from shutdown, so don't do this.
> The normal shutdown process should be flushing the log
> queue anyway.
>
It seems the log_thread_flush() function is missing from the patchset.
Care to send the incremental patch ?
Regards,
Christophe Varoqui
www.opensvc.com
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> libmultipath/log_pthread.c | 101 +++++++++++++++++++++++++++++--------------
> libmultipath/log_pthread.h | 10 +++--
> 2 files changed, 74 insertions(+), 37 deletions(-)
>
> diff --git a/libmultipath/log_pthread.c b/libmultipath/log_pthread.c
> index d701ba1..9e92a70 100644
> --- a/libmultipath/log_pthread.c
> +++ b/libmultipath/log_pthread.c
> @@ -13,20 +13,41 @@
> #include "log.h"
> #include "lock.h"
>
> +pthread_t log_thr;
> +
> +pthread_mutex_t logq_lock;
> +pthread_mutex_t logev_lock;
> +pthread_cond_t logev_cond;
> +
> +int logq_running;
> +
> +static void
> +sigusr1 (int sig)
> +{
> + pthread_mutex_lock(&logq_lock);
> + log_reset("multipathd");
> + pthread_mutex_unlock(&logq_lock);
> +}
> +
> void log_safe (int prio, const char * fmt, va_list ap)
> {
> sigset_t old;
>
> + if (log_thr == (pthread_t)0) {
> + syslog(prio, fmt, ap);
> + return;
> + }
> +
> block_signal(SIGUSR1, &old);
> block_signal(SIGHUP, NULL);
>
> - pthread_mutex_lock(logq_lock);
> + pthread_mutex_lock(&logq_lock);
> log_enqueue(prio, fmt, ap);
> - pthread_mutex_unlock(logq_lock);
> + pthread_mutex_unlock(&logq_lock);
>
> - pthread_mutex_lock(logev_lock);
> - pthread_cond_signal(logev_cond);
> - pthread_mutex_unlock(logev_lock);
> + pthread_mutex_lock(&logev_lock);
> + pthread_cond_signal(&logev_cond);
> + pthread_mutex_unlock(&logev_lock);
>
> pthread_sigmask(SIG_SETMASK, &old, NULL);
> }
> @@ -36,9 +57,9 @@ static void flush_logqueue (void)
> int empty;
>
> do {
> - pthread_mutex_lock(logq_lock);
> + pthread_mutex_lock(&logq_lock);
> empty = log_dequeue(la->buff);
> - pthread_mutex_unlock(logq_lock);
> + pthread_mutex_unlock(&logq_lock);
> if (!empty)
> log_syslog(la->buff);
> } while (empty == 0);
> @@ -46,15 +67,30 @@ static void flush_logqueue (void)
>
> static void * log_thread (void * et)
> {
> + struct sigaction sig;
> + int running;
> +
> + sig.sa_handler = sigusr1;
> + sigemptyset(&sig.sa_mask);
> + sig.sa_flags = 0;
> + if (sigaction(SIGUSR1, &sig, NULL) < 0)
> + logdbg(stderr, "Cannot set signal handler");
> +
> + pthread_mutex_lock(&logev_lock);
> + logq_running = 1;
> + pthread_mutex_unlock(&logev_lock);
> +
> mlockall(MCL_CURRENT | MCL_FUTURE);
> logdbg(stderr,"enter log_thread\n");
>
> while (1) {
> - pthread_mutex_lock(logev_lock);
> - pthread_cond_wait(logev_cond, logev_lock);
> - pthread_mutex_unlock(logev_lock);
> -
> - flush_logqueue();
> + pthread_mutex_lock(&logev_lock);
> + pthread_cond_wait(&logev_cond, &logev_lock);
> + running = logq_running;
> + pthread_mutex_unlock(&logev_lock);
> + if (!running)
> + break;
> + log_thread_flush();
> }
> return NULL;
> }
> @@ -63,19 +99,18 @@ void log_thread_start (pthread_attr_t *attr)
> {
> logdbg(stderr,"enter log_thread_start\n");
>
> - logq_lock = (pthread_mutex_t *) malloc(sizeof(pthread_mutex_t));
> - logev_lock = (pthread_mutex_t *) malloc(sizeof(pthread_mutex_t));
> - logev_cond = (pthread_cond_t *) malloc(sizeof(pthread_cond_t));
> -
> - pthread_mutex_init(logq_lock, NULL);
> - pthread_mutex_init(logev_lock, NULL);
> - pthread_cond_init(logev_cond, NULL);
> + pthread_mutex_init(&logq_lock, NULL);
> + pthread_mutex_init(&logev_lock, NULL);
> + pthread_cond_init(&logev_cond, NULL);
>
> if (log_init("multipathd", 0)) {
> fprintf(stderr,"can't initialize log buffer\n");
> exit(1);
> }
> - pthread_create(&log_thr, attr, log_thread, NULL);
> + if (pthread_create(&log_thr, attr, log_thread, NULL)) {
> + fprintf(stderr,"can't start log thread\n");
> + exit(1);
> + }
>
> return;
> }
> @@ -84,22 +119,22 @@ void log_thread_stop (void)
> {
> logdbg(stderr,"enter log_thread_stop\n");
>
> - pthread_mutex_lock(logq_lock);
> + pthread_mutex_lock(&logev_lock);
> + logq_running = 0;
> + pthread_cond_signal(&logev_cond);
> + pthread_mutex_unlock(&logev_lock);
> +
> + pthread_mutex_lock(&logq_lock);
> pthread_cancel(log_thr);
> - pthread_mutex_unlock(logq_lock);
> + pthread_mutex_unlock(&logq_lock);
> pthread_join(log_thr, NULL);
> + log_thr = (pthread_t)0;
>
> flush_logqueue();
>
> - pthread_mutex_destroy(logq_lock);
> - pthread_mutex_destroy(logev_lock);
> - pthread_cond_destroy(logev_cond);
> -
> - free(logq_lock);
> - logq_lock = NULL;
> - free(logev_lock);
> - logev_lock = NULL;
> - free(logev_cond);
> - logev_cond = NULL;
> - free_logarea();
> + pthread_mutex_destroy(&logq_lock);
> + pthread_mutex_destroy(&logev_lock);
> + pthread_cond_destroy(&logev_cond);
> +
> + log_close();
> }
> diff --git a/libmultipath/log_pthread.h b/libmultipath/log_pthread.h
> index 77780d8..f3b70ea 100644
> --- a/libmultipath/log_pthread.h
> +++ b/libmultipath/log_pthread.h
> @@ -3,11 +3,13 @@
>
> #include <pthread.h>
>
> -pthread_t log_thr;
> +extern pthread_t log_thr;
>
> -pthread_mutex_t *logq_lock;
> -pthread_mutex_t *logev_lock;
> -pthread_cond_t *logev_cond;
> +extern pthread_mutex_t logq_lock;
> +extern pthread_mutex_t logev_lock;
> +extern pthread_cond_t logev_cond;
> +
> +extern int logq_running;
>
> void log_safe(int prio, const char * fmt, va_list ap);
> void log_thread_start(pthread_attr_t *attr);
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 25/42] Make log_pthread more robust
2013-01-09 0:16 ` Christophe Varoqui
@ 2013-01-09 19:15 ` Xose Vazquez Perez
0 siblings, 0 replies; 48+ messages in thread
From: Xose Vazquez Perez @ 2013-01-09 19:15 UTC (permalink / raw)
To: christophe.varoqui, device-mapper development
On 01/09/2013 01:16 AM, Christophe Varoqui wrote:
> On mar., 2013-01-08 at 14:54 +0100, Hannes Reinecke wrote:
>> We don't need to allocate memory for mutexes, we can just
>> be using static variables. And valgrind complained about
>> logqueue flush from shutdown, so don't do this.
>> The normal shutdown process should be flushing the log
>> queue anyway.
>>
> It seems the log_thread_flush() function is missing from the patchset.
> Care to send the incremental patch ?
and also log_reset()
chuchi:~/tmp/multipath-tools $ make -i
make[1]: Entering directory `/home/xose/tmp/multipath-tools/libmultipath'
make[1]: Nothing to be done for `all'.
make[1]: Leaving directory `/home/xose/tmp/multipath-tools/libmultipath'
make[1]: Entering directory `/home/xose/tmp/multipath-tools/libmultipath/prioritizers'
make[1]: Nothing to be done for `all'.
make[1]: Leaving directory `/home/xose/tmp/multipath-tools/libmultipath/prioritizers'
make[1]: Entering directory `/home/xose/tmp/multipath-tools/libmultipath/checkers'
make[1]: Nothing to be done for `all'.
make[1]: Leaving directory `/home/xose/tmp/multipath-tools/libmultipath/checkers'
make[1]: Entering directory `/home/xose/tmp/multipath-tools/libmpathpersist'
make[1]: Nothing to be done for `all'.
make[1]: Leaving directory `/home/xose/tmp/multipath-tools/libmpathpersist'
make[1]: Entering directory `/home/xose/tmp/multipath-tools/multipath'
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -Wunused -Wstrict-prototypes -fPIC -DLIB_STRING=\"lib64\" -I../libmultipath main.o -o multipath -lpthread -ldevmapper -ldl -L../libmultipath -lmultipath
../libmultipath/libmultipath.so: undefined reference to `log_thread_flush'
../libmultipath/libmultipath.so: undefined reference to `log_reset'
collect2: error: ld returned 1 exit status
make[1]: [multipath] Error 1 (ignored)
/bin/gzip -9 -c multipath.8 > multipath.8.gz
/bin/gzip -9 -c multipath.conf.5 > multipath.conf.5.gz
make[1]: Leaving directory `/home/xose/tmp/multipath-tools/multipath'
make[1]: Entering directory `/home/xose/tmp/multipath-tools/multipathd'
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -Wunused -Wstrict-prototypes -fPIC -DLIB_STRING=\"lib64\" -I../libmultipath -I../libmpathpersist main.o pidfile.o uxlsnr.o uxclnt.o cli.o cli_handlers.o -lpthread -ldevmapper -lreadline -lncurses -ludev -ldl -L../libmultipath -lmultipath -L../libmpathpersist -lmpathpersist -o multipathd
../libmultipath/libmultipath.so: undefined reference to `log_thread_flush'
../libmultipath/libmultipath.so: undefined reference to `log_reset'
collect2: error: ld returned 1 exit status
make[1]: [multipathd] Error 1 (ignored)
/bin/gzip -9 -c multipathd.8 > multipathd.8.gz
make[1]: Leaving directory `/home/xose/tmp/multipath-tools/multipathd'
make[1]: Entering directory `/home/xose/tmp/multipath-tools/mpathpersist'
cc -g main.o -o mpathpersist -lpthread -ldevmapper -L../libmpathpersist -lmpathpersist -L../libmultipath -lmultipath -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -Wunused -Wstrict-prototypes -fPIC -DLIB_STRING=\"lib64\" -I../libmultipath -I../libmpathpersist ../libmultipath/libmultipath.so: undefined reference to `log_thread_flush'
../libmultipath/libmultipath.so: undefined reference to `log_reset'
collect2: error: ld returned 1 exit status
make[1]: [mpathpersist] Error 1 (ignored)
/bin/gzip -9 -c mpathpersist.8 > mpathpersist.8.gz
make[1]: Leaving directory `/home/xose/tmp/multipath-tools/mpathpersist'
make[1]: Entering directory `/home/xose/tmp/multipath-tools/kpartx'
make[1]: Nothing to be done for `all'.
make[1]: Leaving directory `/home/xose/tmp/multipath-tools/kpartx'
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 26/42] Print log messages when updating tables failed
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (24 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 25/42] Make log_pthread more robust Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 27/42] Update 'no_path_retry' correctly for failed paths Hannes Reinecke
` (15 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
Add some logging messages to identify the case of failure.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/structs_vec.c | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 6d36d58..384afb7 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -246,11 +246,15 @@ update_multipath_table (struct multipath *mpp, vector pathvec)
if (!mpp)
return 1;
- if (dm_get_map(mpp->alias, &mpp->size, params))
+ if (dm_get_map(mpp->alias, &mpp->size, params)) {
+ condlog(3, "%s: cannot get map", mpp->alias);
return 1;
+ }
- if (disassemble_map(pathvec, params, mpp))
+ if (disassemble_map(pathvec, params, mpp)) {
+ condlog(3, "%s: cannot disassemble map", mpp->alias);
return 1;
+ }
return 0;
}
@@ -263,11 +267,15 @@ update_multipath_status (struct multipath *mpp)
if (!mpp)
return 1;
- if(dm_get_status(mpp->alias, status))
+ if (dm_get_status(mpp->alias, status)) {
+ condlog(3, "%s: cannot get status", mpp->alias);
return 1;
+ }
- if (disassemble_status(status, mpp))
+ if (disassemble_status(status, mpp)) {
+ condlog(3, "%s: cannot disassemble status", mpp->alias);
return 1;
+ }
return 0;
}
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 27/42] Update 'no_path_retry' correctly for failed paths
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (25 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 26/42] Print log messages when updating tables failed Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 28/42] Clean up uevent queue on shutdown Hannes Reinecke
` (14 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: Martin Wilck, dm-devel
The bug is triggered if path failed event is received by multipathd after all
paths have been already marked as failed. Surprisingly enough, it seems to
happen quite often; colleague of mine who tested this hit this bug every time.
Here is event sequence that explains this bug. I left some messages for
clarity; full log is available on request. We have completed initialization and
set feature queue_if_no_path for map CX_201 by virtue of using no_path_retry >
0.
Aug 31 10:49:09 | CX_201: devmap event #18
Aug 31 10:49:09 | CX_201: discover
Aug 31 10:49:09 | CX_201: rr_weight = 1 (internal default)
Aug 31 10:49:09 | CX_201: pgfailback = -2 (controller setting)
Aug 31 10:49:09 | CX_201: no_path_retry = 2 (controller setting)
Aug 31 10:49:09 | pg_timeout = NONE (internal default)
Aug 31 10:49:09 | 65:192: mark as failed
Aug 31 10:49:09 | CX_201: remaining active paths: 3
Aug 31 10:49:09 | 8:192: mark as failed
Aug 31 10:49:09 | CX_201: remaining active paths: 2
Aug 31 10:49:09 | CX_201: devmap event #19
Aug 31 10:49:09 | CX_201: discover
Aug 31 10:49:09 | CX_201: rr_weight = 1 (internal default)
Aug 31 10:49:09 | CX_201: pgfailback = -2 (controller setting)
Aug 31 10:49:09 | CX_201: no_path_retry = 2 (controller setting)
Aug 31 10:49:09 | pg_timeout = NONE (internal default)
Two paths failed by driver, multipahd marked them as failed.
Aug 31 10:49:09 | checker failed path 66:0 in map CX_201
Aug 31 10:49:09 | CX_201: remaining active paths: 1
Checker failed third path
Aug 31 10:49:09 | checker failed path 8:96 in map CX_201
Aug 31 10:49:09 | CX_201: Entering recovery mode: max_retries=2
Aug 31 10:49:09 | CX_201: remaining active paths: 0
Checker failed last path; multipathd entered retry loop.
Aug 31 10:49:10 | CX_201: devmap event #20
We got late event about failed path
Aug 31 10:49:10 | CX_201: discover
Start discovery. Call update_multipath -> setup_multipath ->
update_multipath_strings -> update_multipath_tablle -> disassemble_map.
Now disassemble_map tries to set no_path_retry value from kernel. This
obviously is not going to work as kernel is able remembering only Boolean
(queue/fail), while no_path_retry is arbitrary integer. So no_path_retry is set
to NO_PATH_RETRY_QUEUE from kernel.
Aug 31 10:49:10 | CX_201: rr_weight = 1 (internal default)
Aug 31 10:49:10 | CX_201: pgfailback = -2 (controller setting)
At this point we call set_no_path_retry:
set_no_path_retry(struct multipath *mpp)
{
mpp->retry_tick = 0;
mpp->nr_active = pathcount(mpp, PATH_UP) + pathcount(mpp, PATH_GHOST);
if (mpp->nr_active > 0)
select_no_path_retry(mpp);
So
1) retry_tick is reset
2) nr_active = 0 (no active path)
3) we do not set no_path_retry from config file because nr_active == 0 => left
with NO_PATH_RETRY_QUEUE.
Aug 31 10:49:10 | pg_timeout = NONE (internal default)
From now on there is no state changes, so map is hung forever.
Signed-off-by: Martin Wilck <martin.wilck@ts.fujitsu.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/structs_vec.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 384afb7..7073915 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -306,8 +306,7 @@ set_no_path_retry(struct multipath *mpp)
{
mpp->retry_tick = 0;
mpp->nr_active = pathcount(mpp, PATH_UP) + pathcount(mpp, PATH_GHOST);
- if (mpp->nr_active > 0)
- select_no_path_retry(mpp);
+ select_no_path_retry(mpp);
switch (mpp->no_path_retry) {
case NO_PATH_RETRY_UNDEF:
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 28/42] Clean up uevent queue on shutdown
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (26 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 27/42] Update 'no_path_retry' correctly for failed paths Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 29/42] libmultipath: Print out uevent sequence number Hannes Reinecke
` (13 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
During shutdown there might be some unprocessed events
in the queue. So clear them up.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/uevent.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index b74fb08..5b0b258 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -132,6 +132,17 @@ static void uevq_stop(void *arg)
pthread_mutex_unlock(uevq_lockp);
}
+void
+uevq_cleanup(struct list_head *tmpq)
+{
+ struct uevent *uev, *tmp;
+
+ list_for_each_entry_safe(uev, tmp, tmpq, node) {
+ list_del_init(&uev->node);
+ FREE(uev);
+ }
+}
+
/*
* Service the uevent queue.
*/
@@ -163,6 +174,7 @@ int uevent_dispatch(int (*uev_trigger)(struct uevent *, void * trigger_data),
service_uevq(&uevq_tmp);
}
condlog(3, "Terminating uev service queue");
+ uevq_cleanup(&uevq);
return 0;
}
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 29/42] libmultipath: Print out uevent sequence number
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (27 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 28/42] Clean up uevent queue on shutdown Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 30/42] Fix race condition in stop_waiter_thread() Hannes Reinecke
` (12 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
For debugging we should be printing out the sequence number.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/uevent.c | 11 ++++++++++-
libmultipath/uevent.h | 1 +
2 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 5b0b258..bc74d38 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -361,11 +361,20 @@ int failback_listen(void)
key = &buffer[bufpos];
keylen = strlen(key);
uev->envp[i] = key;
+ /* Filter out sequence number */
+ if (strncmp(key, "SEQNUM=", 7) == 0) {
+ char *eptr;
+
+ uev->seqnum = strtoul(key + 7, &eptr, 10);
+ if (eptr == key + 7)
+ uev->seqnum = -1;
+ }
bufpos += keylen + 1;
}
uev->envp[i] = NULL;
- condlog(3, "uevent '%s' from '%s'", uev->action, uev->devpath);
+ condlog(3, "uevent %ld '%s' from '%s'", uev->seqnum,
+ uev->action, uev->devpath);
uev->kernel = strrchr(uev->devpath, '/');
if (uev->kernel)
uev->kernel++;
diff --git a/libmultipath/uevent.h b/libmultipath/uevent.h
index 64f00b7..762595a 100644
--- a/libmultipath/uevent.h
+++ b/libmultipath/uevent.h
@@ -20,6 +20,7 @@ struct uevent {
char *devpath;
char *action;
char *kernel;
+ unsigned long seqnum;
char *envp[HOTPLUG_NUM_ENVP];
};
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 30/42] Fix race condition in stop_waiter_thread()
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (28 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 29/42] libmultipath: Print out uevent sequence number Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 31/42] Use VECTOR_SIZE() defines Hannes Reinecke
` (11 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
The signal handler might run before we had a chance to
set the 'waiter' context to '0', so better do it previously.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/waiter.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/libmultipath/waiter.c b/libmultipath/waiter.c
index 076539c..da71543 100644
--- a/libmultipath/waiter.c
+++ b/libmultipath/waiter.c
@@ -45,6 +45,8 @@ void free_waiter (void *data)
void stop_waiter_thread (struct multipath *mpp, struct vectors *vecs)
{
+ pthread_t thread;
+
if (mpp->waiter == (pthread_t)0) {
condlog(3, "%s: event checker thread already stopped",
mpp->alias);
@@ -52,8 +54,9 @@ void stop_waiter_thread (struct multipath *mpp, struct vectors *vecs)
}
condlog(2, "%s: stop event checker thread (%lu)", mpp->alias,
mpp->waiter);
- pthread_cancel(mpp->waiter);
+ thread = mpp->waiter;
mpp->waiter = (pthread_t)0;
+ pthread_cancel(thread);
}
/*
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 31/42] Use VECTOR_SIZE() defines
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (29 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 30/42] Fix race condition in stop_waiter_thread() Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 32/42] Make 'allocated' an integer in vector.h Hannes Reinecke
` (10 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
The size of a vector slot might be larger than one, so we should
be using the VECTOR_SIZE() define everywhere.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/vector.c | 8 ++++----
libmultipath/vector.h | 10 +++++-----
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/libmultipath/vector.c b/libmultipath/vector.c
index 652f118..74079a4 100644
--- a/libmultipath/vector.c
+++ b/libmultipath/vector.c
@@ -81,7 +81,7 @@ vector_insert_slot(vector v, int slot, void *value)
if (!vector_alloc_slot(v))
return NULL;
- for (i = (v->allocated /VECTOR_DEFAULT_SIZE) - 2; i >= slot; i--)
+ for (i = VECTOR_SIZE(v) - 2; i >= slot; i--)
v->slot[i + 1] = v->slot[i];
v->slot[slot] = value;
@@ -94,7 +94,7 @@ find_slot(vector v, void * addr)
{
int i;
- for (i = 0; i < (v->allocated / VECTOR_DEFAULT_SIZE); i++)
+ for (i = 0; i < VECTOR_SIZE(v); i++)
if (v->slot[i] == addr)
return i;
@@ -109,7 +109,7 @@ vector_del_slot(vector v, int slot)
if (!v || !v->allocated || slot < 0 || slot > VECTOR_SIZE(v))
return;
- for (i = slot + 1; i < (v->allocated / VECTOR_DEFAULT_SIZE); i++)
+ for (i = slot + 1; i < VECTOR_SIZE(v); i++)
v->slot[i-1] = v->slot[i];
v->allocated -= VECTOR_DEFAULT_SIZE;
@@ -137,7 +137,7 @@ vector_repack(vector v)
if (!v || !v->allocated)
return;
- for (i = 0; i < (v->allocated / VECTOR_DEFAULT_SIZE); i++)
+ for (i = 0; i < VECTOR_SIZE(v); i++)
if (i > 0 && v->slot[i] == NULL)
vector_del_slot(v, i--);
}
diff --git a/libmultipath/vector.h b/libmultipath/vector.h
index ca42be1..6779186 100644
--- a/libmultipath/vector.h
+++ b/libmultipath/vector.h
@@ -31,14 +31,14 @@ struct _vector {
typedef struct _vector *vector;
#define VECTOR_DEFAULT_SIZE 1
-#define VECTOR_SLOT(V,E) (((V) && (E) < (V)->allocated) ? (V)->slot[(E)] : NULL)
-#define VECTOR_SIZE(V) ((V) ? (V)->allocated : 0)
-#define VECTOR_LAST_SLOT(V) (((V) && (V)->allocated) ? (V)->slot[((V)->allocated - 1)] : NULL)
+#define VECTOR_SIZE(V) ((V) ? ((V)->allocated) / VECTOR_DEFAULT_SIZE : 0)
+#define VECTOR_SLOT(V,E) (((V) && (E) < VECTOR_SIZE(V)) ? (V)->slot[(E)] : NULL)
+#define VECTOR_LAST_SLOT(V) (((V) && VECTOR_SIZE(V) > 0) ? (V)->slot[(VECTOR_SIZE(V) - 1)] : NULL)
#define vector_foreach_slot(v,p,i) \
- for (i = 0; (v) && i < (v)->allocated && ((p) = (v)->slot[i]); i++)
+ for (i = 0; (v) && i < VECTOR_SIZE(v) && ((p) = (v)->slot[i]); i++)
#define vector_foreach_slot_after(v,p,i) \
- for (; (v) && i < (v)->allocated && ((p) = (v)->slot[i]); i++)
+ for (; (v) && i < VECTOR_SIZE(v) && ((p) = (v)->slot[i]); i++)
#define vector_foreach_slot_backwards(v,p,i) \
for (i = VECTOR_SIZE(v); i > 0 && ((p) = (v)->slot[i-1]); i--)
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 32/42] Make 'allocated' an integer in vector.h
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (30 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 31/42] Use VECTOR_SIZE() defines Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 33/42] Syntax error in /etc/init.d/boot.multipath Hannes Reinecke
` (9 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
I don't trust the programmers here, as we're unconditionally
decreasing the 'allocated' setting on vector_free().
So better make that an integer to catch underflows.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/vector.c | 2 +-
libmultipath/vector.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libmultipath/vector.c b/libmultipath/vector.c
index 74079a4..dcf69bf 100644
--- a/libmultipath/vector.c
+++ b/libmultipath/vector.c
@@ -114,7 +114,7 @@ vector_del_slot(vector v, int slot)
v->allocated -= VECTOR_DEFAULT_SIZE;
- if (!v->allocated) {
+ if (v->allocated <= 0) {
FREE(v->slot);
v->slot = NULL;
v->allocated = 0;
diff --git a/libmultipath/vector.h b/libmultipath/vector.h
index 6779186..7612b4c 100644
--- a/libmultipath/vector.h
+++ b/libmultipath/vector.h
@@ -25,7 +25,7 @@
/* vector definition */
struct _vector {
- unsigned int allocated;
+ int allocated;
void **slot;
};
typedef struct _vector *vector;
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 33/42] Syntax error in /etc/init.d/boot.multipath
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (31 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 32/42] Make 'allocated' an integer in vector.h Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 34/42] multipath.init.suse: Update usage message Hannes Reinecke
` (8 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
The multipath init script has a syntax error in line 113.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
multipath/multipath.init.suse | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/multipath/multipath.init.suse b/multipath/multipath.init.suse
index cc7c04b..c7cd5c7 100644
--- a/multipath/multipath.init.suse
+++ b/multipath/multipath.init.suse
@@ -110,7 +110,7 @@ case "$1" in
partlist=$(/sbin/kpartx -l -p _part /dev/disk/by-id/dm-name-$map | sed 's/\([^ ]*\) :.*/\1/p')
for part in $partlist; do
[ -e /dev/disk/by-id/dm-name-$part ] && continue
- num$((num + 1))
+ num=$((num + 1))
done
done
[ $num -eq 0 ] && break
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 34/42] multipath.init.suse: Update usage message
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (32 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 33/42] Syntax error in /etc/init.d/boot.multipath Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 35/42] multipath.conf.5: Clarify dev_loss_tmo settings Hannes Reinecke
` (7 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
The usage message in multipath.init.suse doesn't list 'reload'.
And 'reload' is a misnomer, as it's actually a restart.
So rename 'reload' to 'restart' and add to the usage message.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
multipath/multipath.init.suse | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/multipath/multipath.init.suse b/multipath/multipath.init.suse
index c7cd5c7..de1cc1f 100644
--- a/multipath/multipath.init.suse
+++ b/multipath/multipath.init.suse
@@ -146,12 +146,12 @@ case "$1" in
fi
rc_status -v
;;
- reload)
+ restart)
$0 stop
$0 start
;;
*)
- echo "Usage: $0 {start|stop|status}"
+ echo "Usage: $0 {start|stop|status|restart}"
exit 1
;;
esac
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 35/42] multipath.conf.5: Clarify dev_loss_tmo settings
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (33 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 34/42] multipath.init.suse: Update usage message Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 36/42] Clarify dev_loss_tmo capping in multipath.conf.5 Hannes Reinecke
` (6 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
We need to document that dev_loss_tmo is in fact modified
by no_path_retry.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
multipath/multipath.conf.5 | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index 9d6266c..247cdc0 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -334,9 +334,14 @@ will disable the timeout.
.TP
.B dev_loss_tmo
Specify the number of seconds the scsi layer will wait after a problem has
+<<<<<<< HEAD
been detected on a FC remote port before removing it from the system. This
can be set to "infinity" which sets it to the max value of 2147483647
-seconds, or 68 years.
+seconds, or 68 years. It will be automatically adjusted to the overall
+retry interval
+\fIno_path_retry\fR * \fIpolling_interval\fR
+if a number of retries is given with \fIno_path_retry\fR and the
+overall retry interval is longer than the specified \fIdev_loss_tmo\fR value.
.TP
.B queue_without_daemon
If set to
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 36/42] Clarify dev_loss_tmo capping in multipath.conf.5
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (34 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 35/42] multipath.conf.5: Clarify dev_loss_tmo settings Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 38/42] multipathd: Ignore errors when creating pidfile Hannes Reinecke
` (5 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
The linux kernel will not allow any dev_loss_tmo setting larger
than 300 if fast_io_fail is not set. So we should document this
in the manpage.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
multipath/multipath.conf.5 | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index 247cdc0..35021f6 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -342,6 +342,8 @@ retry interval
\fIno_path_retry\fR * \fIpolling_interval\fR
if a number of retries is given with \fIno_path_retry\fR and the
overall retry interval is longer than the specified \fIdev_loss_tmo\fR value.
+The linux kernel will cap this value to \fI300\fR if \fBfast_io_fail_tmo\fR
+is not set.
.TP
.B queue_without_daemon
If set to
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 38/42] multipathd: Ignore errors when creating pidfile
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (35 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 36/42] Clarify dev_loss_tmo capping in multipath.conf.5 Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 39/42] multipathd deadlocks during restart Hannes Reinecke
` (4 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
We can use CLI commands to communicate with the daemon,
so we don't need the pidfile for correct operation.
Hence any errors from creating the pidfile can be safely
ignored.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
multipathd/main.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/multipathd/main.c b/multipathd/main.c
index 7fe9c5b..6c5e243 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1560,7 +1560,7 @@ child (void * param)
struct vectors * vecs;
struct multipath * mpp;
int i;
- int rc;
+ int rc, pid_rc;
mlockall(MCL_CURRENT | MCL_FUTURE);
@@ -1667,9 +1667,8 @@ child (void * param)
pthread_mutex_lock(&exit_mutex);
/* Startup complete, create logfile */
- if (pidfile_create(DEFAULT_PIDFILE, daemon_pid))
- /* Ignore errors, we can live without */
- condlog(1, "failed to create pidfile");
+ pid_rc = pidfile_create(DEFAULT_PIDFILE, daemon_pid);
+ /* Ignore errors, we can live without */
running_state = DAEMON_RUNNING;
pthread_cond_wait(&exit_cond, &exit_mutex);
@@ -1715,8 +1714,10 @@ child (void * param)
dm_lib_exit();
/* We're done here */
- condlog(3, "unlink pidfile");
- unlink(DEFAULT_PIDFILE);
+ if (!pid_rc) {
+ condlog(3, "unlink pidfile");
+ unlink(DEFAULT_PIDFILE);
+ }
condlog(2, "--------shut down-------");
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 39/42] multipathd deadlocks during restart
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (36 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 38/42] multipathd: Ignore errors when creating pidfile Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 40/42] multipathd: sighandlers might use uninitialized gvecs Hannes Reinecke
` (3 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
During restart multipathd might deadlock as the uevent handler
is missing a cleanup handler. Thus the thread might be terminated
while it still holds the vector lock.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
multipathd/main.c | 19 ++++++++++++-------
1 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/multipathd/main.c b/multipathd/main.c
index 6c5e243..3a6e88f 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -895,13 +895,11 @@ exit_daemon (int status)
if (status != 0)
fprintf(stderr, "bad exit status. see daemon.log\n");
- condlog(3, "unlink pidfile");
- unlink(DEFAULT_PIDFILE);
-
- pthread_mutex_lock(&exit_mutex);
- pthread_cond_signal(&exit_cond);
- pthread_mutex_unlock(&exit_mutex);
-
+ if (running_state != DAEMON_SHUTDOWN) {
+ pthread_mutex_lock(&exit_mutex);
+ pthread_cond_signal(&exit_cond);
+ pthread_mutex_unlock(&exit_mutex);
+ }
return status;
}
@@ -1560,6 +1558,7 @@ child (void * param)
struct vectors * vecs;
struct multipath * mpp;
int i;
+ sigset_t set;
int rc, pid_rc;
mlockall(MCL_CURRENT | MCL_FUTURE);
@@ -1672,11 +1671,17 @@ child (void * param)
running_state = DAEMON_RUNNING;
pthread_cond_wait(&exit_cond, &exit_mutex);
+ /* Need to block these to avoid deadlocking */
+ sigemptyset(&set);
+ sigaddset(&set, SIGTERM);
+ sigaddset(&set, SIGINT);
+ pthread_sigmask(SIG_BLOCK, &set, NULL);
/*
* exit path
*/
running_state = DAEMON_SHUTDOWN;
+ pthread_sigmask(SIG_UNBLOCK, &set, NULL);
block_signal(SIGHUP, NULL);
lock(vecs->lock);
if (conf->queue_without_daemon == QUE_NO_DAEMON_OFF)
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 40/42] multipathd: sighandlers might use uninitialized gvecs
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (37 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 39/42] multipathd deadlocks during restart Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 41/42] multipathd: crash in reconfigure CLI command Hannes Reinecke
` (2 subsequent siblings)
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
gvecs are initialized after signal handlers, which in turn
might access the vectors.
So the signal handlers might access uninitialized variables.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
multipathd/main.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/multipathd/main.c b/multipathd/main.c
index 3a6e88f..f491a24 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1617,14 +1617,14 @@ child (void * param)
}
- signal_init();
- setscheduler();
- set_oom_adj();
vecs = gvecs = init_vecs();
-
if (!vecs)
exit(1);
+ signal_init();
+ setscheduler();
+ set_oom_adj();
+
conf->daemon = 1;
udev_set_sync_support(0);
/*
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 41/42] multipathd: crash in reconfigure CLI command
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (38 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 40/42] multipathd: sighandlers might use uninitialized gvecs Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 13:54 ` [PATCH 42/42] multipathd: lock vectors during initial configuration Hannes Reinecke
2013-01-08 23:53 ` [PATCH 00/42] SLES resync Christophe Varoqui
41 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
The 'reconfigure' CLI command doesn't take the vector lock,
so if multipathd is processing a table / udev event at the
same time it'll crash.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
multipathd/main.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/multipathd/main.c b/multipathd/main.c
index f491a24..395307e 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1389,6 +1389,7 @@ reconfigure (struct vectors * vecs)
struct config * old = conf;
int retval = 1;
+ lock(vecs->lock);
/*
* free old map and path vectors ... they use old conf state
*/
@@ -1409,6 +1410,7 @@ reconfigure (struct vectors * vecs)
retval = 0;
}
+ unlock(vecs->lock);
return retval;
}
@@ -1466,9 +1468,7 @@ sighup (int sig)
if (running_state != DAEMON_RUNNING)
return;
- lock(gvecs->lock);
reconfigure(gvecs);
- unlock(gvecs->lock);
#ifdef _DEBUG_
dbg_free_final(NULL);
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 42/42] multipathd: lock vectors during initial configuration
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (39 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 41/42] multipathd: crash in reconfigure CLI command Hannes Reinecke
@ 2013-01-08 13:54 ` Hannes Reinecke
2013-01-08 23:37 ` Christophe Varoqui
2013-01-08 23:53 ` [PATCH 00/42] SLES resync Christophe Varoqui
41 siblings, 1 reply; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-08 13:54 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel
During initial configuration the CLI thread is already running,
so we need to lock the vectors here to not race with the
'reconfigure' CLI command.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
multipathd/main.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/multipathd/main.c b/multipathd/main.c
index 395307e..8917499 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1389,7 +1389,6 @@ reconfigure (struct vectors * vecs)
struct config * old = conf;
int retval = 1;
- lock(vecs->lock);
/*
* free old map and path vectors ... they use old conf state
*/
@@ -1410,7 +1409,6 @@ reconfigure (struct vectors * vecs)
retval = 0;
}
- unlock(vecs->lock);
return retval;
}
@@ -1641,9 +1639,9 @@ child (void * param)
/*
* fetch and configure both paths and multipaths
*/
- lock(vecs->lock);
running_state = DAEMON_CONFIGURE;
+ lock(vecs->lock);
if (configure(vecs, 1)) {
unlock(vecs->lock);
condlog(0, "failure during configuration");
--
1.7.4.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH 42/42] multipathd: lock vectors during initial configuration
2013-01-08 13:54 ` [PATCH 42/42] multipathd: lock vectors during initial configuration Hannes Reinecke
@ 2013-01-08 23:37 ` Christophe Varoqui
2013-01-09 7:01 ` Hannes Reinecke
0 siblings, 1 reply; 48+ messages in thread
From: Christophe Varoqui @ 2013-01-08 23:37 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: dm-devel
On mar., 2013-01-08 at 14:54 +0100, Hannes Reinecke wrote:
> During initial configuration the CLI thread is already running,
> so we need to lock the vectors here to not race with the
> 'reconfigure' CLI command.
>
Are you sure of the 41 - 42 patches ?
- 41 moves the vecs locking from reconfigure caller to reconfigure
- 42 removes the locking from reconfigure
Regards,
Christophe Varoqui
www.opensvc.com
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> multipathd/main.c | 4 +---
> 1 files changed, 1 insertions(+), 3 deletions(-)
>
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 395307e..8917499 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -1389,7 +1389,6 @@ reconfigure (struct vectors * vecs)
> struct config * old = conf;
> int retval = 1;
>
> - lock(vecs->lock);
> /*
> * free old map and path vectors ... they use old conf state
> */
> @@ -1410,7 +1409,6 @@ reconfigure (struct vectors * vecs)
> retval = 0;
> }
>
> - unlock(vecs->lock);
> return retval;
> }
>
> @@ -1641,9 +1639,9 @@ child (void * param)
> /*
> * fetch and configure both paths and multipaths
> */
> - lock(vecs->lock);
> running_state = DAEMON_CONFIGURE;
>
> + lock(vecs->lock);
> if (configure(vecs, 1)) {
> unlock(vecs->lock);
> condlog(0, "failure during configuration");
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 42/42] multipathd: lock vectors during initial configuration
2013-01-08 23:37 ` Christophe Varoqui
@ 2013-01-09 7:01 ` Hannes Reinecke
0 siblings, 0 replies; 48+ messages in thread
From: Hannes Reinecke @ 2013-01-09 7:01 UTC (permalink / raw)
To: christophe.varoqui; +Cc: dm-devel, Christophe Varoqui
On 01/09/2013 12:37 AM, Christophe Varoqui wrote:
> On mar., 2013-01-08 at 14:54 +0100, Hannes Reinecke wrote:
>> During initial configuration the CLI thread is already running,
>> so we need to lock the vectors here to not race with the
>> 'reconfigure' CLI command.
>>
> Are you sure of the 41 - 42 patches ?
>
> - 41 moves the vecs locking from reconfigure caller to reconfigure
> - 42 removes the locking from reconfigure
>
> Regards,
> Christophe Varoqui
> www.opensvc.com
>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>> multipathd/main.c | 4 +---
>> 1 files changed, 1 insertions(+), 3 deletions(-)
>>
>> diff --git a/multipathd/main.c b/multipathd/main.c
>> index 395307e..8917499 100644
>> --- a/multipathd/main.c
>> +++ b/multipathd/main.c
>> @@ -1389,7 +1389,6 @@ reconfigure (struct vectors * vecs)
>> struct config * old = conf;
>> int retval = 1;
>>
>> - lock(vecs->lock);
>> /*
>> * free old map and path vectors ... they use old conf state
>> */
>> @@ -1410,7 +1409,6 @@ reconfigure (struct vectors * vecs)
>> retval = 0;
>> }
>>
>> - unlock(vecs->lock);
>> return retval;
>> }
>>
>> @@ -1641,9 +1639,9 @@ child (void * param)
>> /*
>> * fetch and configure both paths and multipaths
>> */
>> - lock(vecs->lock);
>> running_state = DAEMON_CONFIGURE;
>>
>> + lock(vecs->lock);
>> if (configure(vecs, 1)) {
>> unlock(vecs->lock);
>> condlog(0, "failure during configuration");
>
>
>
Yes.
Problem is we're already taking the lock prior to running any CLI
commands in multipathd/main.c:uxsock_trigger().
So we cannot take the lock in reconfigure().
We also should not take the lock from the interrupt handler,
as we might be in god knows which state.
If you're bothered by this I'd rather remove the SIGHUP signal
handler; we can achieve very much the same with the 'reconfigure'
CLI command.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@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
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 00/42] SLES resync
2013-01-08 13:53 [PATCH 00/42] SLES resync Hannes Reinecke
` (40 preceding siblings ...)
2013-01-08 13:54 ` [PATCH 42/42] multipathd: lock vectors during initial configuration Hannes Reinecke
@ 2013-01-08 23:53 ` Christophe Varoqui
2013-01-10 5:36 ` Benjamin Marzinski
41 siblings, 1 reply; 48+ messages in thread
From: Christophe Varoqui @ 2013-01-08 23:53 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: dm-devel, Christophe Varoqui
On mar., 2013-01-08 at 14:53 +0100, Hannes Reinecke wrote:
> Hi Christophe,
>
> here are some patches and bugfixes with have accumulated on
> my SLES11 branch. Please apply.
>
> (Patch 37 is blank due to merging, so it'll be missing from
> the patch set).
>
Applied the full set before Ben's set ... this one big larger.
Ruined my evening, but definitely worth it.
I only add to remove a merge resolution conflict label from
"Clarify dev_loss_tmo settings" and resolve a conflict with latest
vector.c::find_slot() patch from Mike.
Benjamin, please let me know if you will rebase your set. If not I'll be
able to work on rebasing myself next week.
Thanks,
Christophe Varoqui
www.opensvc.com
> Hannes Reinecke (41):
> libmultipath: Invalid check for mpp->wwid in dm_addmap()
> Remove newline from condlog()
> Fixup pathgroup allocation in disassemble_map()
> libmultipath: resource leak in read_value_block()
> Accept several whitespaces in bindings file
> Add TAGS makefile target
> libmultipath: Fix typo in mp_prio_handler()
> Do not trigger a map reload on priority updates
> Introduce MP_FAST_IO_FAIL_UNSET
> Checker name is not displayed on failure
> Valgrind fixes for prioritizer
> Incorrect inquiry vendor length in hds prioritizer
> Print out multipath alias for flush_on_last_del messages
> Clarify setting origin in propsel.c
> libmultipath: error checking in remove_features()
> Increase parameter buffer
> Check return code from pathinfo()
> Inconsistent string quoting
> Switch off 'queue_if_no_path' before removing maps
> Double free in disassemble_map()
> libmultipath: prio keyword ignored for multipath config
> Path checker should return PATH_DOWN when no path is found
> Do not call sysfs_get_timeout for non-SCSI devices
> Make log_pthread more robust
> Print log messages when updating tables failed
> Update 'no_path_retry' correctly for failed paths
> Clean up uevent queue on shutdown
> libmultipath: Print out uevent sequence number
> Fix race condition in stop_waiter_thread()
> Use VECTOR_SIZE() defines
> Make 'allocated' an integer in vector.h
> Syntax error in /etc/init.d/boot.multipath
> multipath.init.suse: Update usage message
> multipath.conf.5: Clarify dev_loss_tmo settings
> Clarify dev_loss_tmo capping in multipath.conf.5
> multipathd: Ignore errors when creating pidfile
> multipathd deadlocks during restart
> multipathd: sighandlers might use uninitialized gvecs
> multipathd: crash in reconfigure CLI command
> multipathd: lock vectors during initial configuration
>
> Petr Uzel (1):
> prio: fix merging of prioritizers with different args
>
> Makefile | 7 ++
> libmultipath/alias.c | 2 +-
> libmultipath/config.h | 1 +
> libmultipath/configure.c | 13 ++--
> libmultipath/configure.h | 2 +-
> libmultipath/devmapper.c | 36 ++++++++--
> libmultipath/devmapper.h | 1 +
> libmultipath/dict.c | 108 +++++++++++++++---------------
> libmultipath/discovery.c | 38 ++++++-----
> libmultipath/dmparser.c | 20 +++---
> libmultipath/log_pthread.c | 101 +++++++++++++++++++---------
> libmultipath/log_pthread.h | 10 ++-
> libmultipath/parser.c | 32 +++++----
> libmultipath/prio.c | 51 ++++++++++++++-
> libmultipath/prio.h | 9 ++-
> libmultipath/prioritizers/alua.c | 1 +
> libmultipath/prioritizers/datacore.c | 1 +
> libmultipath/prioritizers/emc.c | 1 +
> libmultipath/prioritizers/hds.c | 6 +-
> libmultipath/prioritizers/hp_sw.c | 1 +
> libmultipath/prioritizers/ontap.c | 2 +
> libmultipath/prioritizers/rdac.c | 2 +
> libmultipath/prioritizers/weightedpath.c | 6 ++-
> libmultipath/propsel.c | 53 ++++++++-------
> libmultipath/structs.c | 7 ++
> libmultipath/structs.h | 8 ++-
> libmultipath/structs_vec.c | 29 +++++---
> libmultipath/sysfs.c | 2 +-
> libmultipath/uevent.c | 27 +++++++-
> libmultipath/uevent.h | 1 +
> libmultipath/util.c | 12 ++--
> libmultipath/vector.c | 10 ++--
> libmultipath/vector.h | 12 ++--
> libmultipath/waiter.c | 5 +-
> multipath.conf.annotated | 1 +
> multipath/main.c | 8 +-
> multipath/multipath.conf.5 | 9 ++-
> multipath/multipath.init.suse | 6 +-
> multipathd/cli_handlers.c | 2 +-
> multipathd/main.c | 98 ++++++++++++---------------
> 40 files changed, 468 insertions(+), 273 deletions(-)
>
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 00/42] SLES resync
2013-01-08 23:53 ` [PATCH 00/42] SLES resync Christophe Varoqui
@ 2013-01-10 5:36 ` Benjamin Marzinski
0 siblings, 0 replies; 48+ messages in thread
From: Benjamin Marzinski @ 2013-01-10 5:36 UTC (permalink / raw)
To: christophe.varoqui, device-mapper development
On Wed, Jan 09, 2013 at 12:53:11AM +0100, Christophe Varoqui wrote:
> On mar., 2013-01-08 at 14:53 +0100, Hannes Reinecke wrote:
> > Hi Christophe,
> >
> > here are some patches and bugfixes with have accumulated on
> > my SLES11 branch. Please apply.
> >
> > (Patch 37 is blank due to merging, so it'll be missing from
> > the patch set).
> >
> Applied the full set before Ben's set ... this one big larger.
> Ruined my evening, but definitely worth it.
>
> I only add to remove a merge resolution conflict label from
> "Clarify dev_loss_tmo settings" and resolve a conflict with latest
> vector.c::find_slot() patch from Mike.
>
> Benjamin, please let me know if you will rebase your set. If not I'll be
> able to work on rebasing myself next week.
Yeah. I should be able to get to it tomorrow.
-Ben
>
> Thanks,
> Christophe Varoqui
> www.opensvc.com
>
>
> > Hannes Reinecke (41):
> > libmultipath: Invalid check for mpp->wwid in dm_addmap()
> > Remove newline from condlog()
> > Fixup pathgroup allocation in disassemble_map()
> > libmultipath: resource leak in read_value_block()
> > Accept several whitespaces in bindings file
> > Add TAGS makefile target
> > libmultipath: Fix typo in mp_prio_handler()
> > Do not trigger a map reload on priority updates
> > Introduce MP_FAST_IO_FAIL_UNSET
> > Checker name is not displayed on failure
> > Valgrind fixes for prioritizer
> > Incorrect inquiry vendor length in hds prioritizer
> > Print out multipath alias for flush_on_last_del messages
> > Clarify setting origin in propsel.c
> > libmultipath: error checking in remove_features()
> > Increase parameter buffer
> > Check return code from pathinfo()
> > Inconsistent string quoting
> > Switch off 'queue_if_no_path' before removing maps
> > Double free in disassemble_map()
> > libmultipath: prio keyword ignored for multipath config
> > Path checker should return PATH_DOWN when no path is found
> > Do not call sysfs_get_timeout for non-SCSI devices
> > Make log_pthread more robust
> > Print log messages when updating tables failed
> > Update 'no_path_retry' correctly for failed paths
> > Clean up uevent queue on shutdown
> > libmultipath: Print out uevent sequence number
> > Fix race condition in stop_waiter_thread()
> > Use VECTOR_SIZE() defines
> > Make 'allocated' an integer in vector.h
> > Syntax error in /etc/init.d/boot.multipath
> > multipath.init.suse: Update usage message
> > multipath.conf.5: Clarify dev_loss_tmo settings
> > Clarify dev_loss_tmo capping in multipath.conf.5
> > multipathd: Ignore errors when creating pidfile
> > multipathd deadlocks during restart
> > multipathd: sighandlers might use uninitialized gvecs
> > multipathd: crash in reconfigure CLI command
> > multipathd: lock vectors during initial configuration
> >
> > Petr Uzel (1):
> > prio: fix merging of prioritizers with different args
> >
> > Makefile | 7 ++
> > libmultipath/alias.c | 2 +-
> > libmultipath/config.h | 1 +
> > libmultipath/configure.c | 13 ++--
> > libmultipath/configure.h | 2 +-
> > libmultipath/devmapper.c | 36 ++++++++--
> > libmultipath/devmapper.h | 1 +
> > libmultipath/dict.c | 108 +++++++++++++++---------------
> > libmultipath/discovery.c | 38 ++++++-----
> > libmultipath/dmparser.c | 20 +++---
> > libmultipath/log_pthread.c | 101 +++++++++++++++++++---------
> > libmultipath/log_pthread.h | 10 ++-
> > libmultipath/parser.c | 32 +++++----
> > libmultipath/prio.c | 51 ++++++++++++++-
> > libmultipath/prio.h | 9 ++-
> > libmultipath/prioritizers/alua.c | 1 +
> > libmultipath/prioritizers/datacore.c | 1 +
> > libmultipath/prioritizers/emc.c | 1 +
> > libmultipath/prioritizers/hds.c | 6 +-
> > libmultipath/prioritizers/hp_sw.c | 1 +
> > libmultipath/prioritizers/ontap.c | 2 +
> > libmultipath/prioritizers/rdac.c | 2 +
> > libmultipath/prioritizers/weightedpath.c | 6 ++-
> > libmultipath/propsel.c | 53 ++++++++-------
> > libmultipath/structs.c | 7 ++
> > libmultipath/structs.h | 8 ++-
> > libmultipath/structs_vec.c | 29 +++++---
> > libmultipath/sysfs.c | 2 +-
> > libmultipath/uevent.c | 27 +++++++-
> > libmultipath/uevent.h | 1 +
> > libmultipath/util.c | 12 ++--
> > libmultipath/vector.c | 10 ++--
> > libmultipath/vector.h | 12 ++--
> > libmultipath/waiter.c | 5 +-
> > multipath.conf.annotated | 1 +
> > multipath/main.c | 8 +-
> > multipath/multipath.conf.5 | 9 ++-
> > multipath/multipath.init.suse | 6 +-
> > multipathd/cli_handlers.c | 2 +-
> > multipathd/main.c | 98 ++++++++++++---------------
> > 40 files changed, 468 insertions(+), 273 deletions(-)
> >
>
>
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 48+ messages in thread