* [PATCH 01/10] libmultipath: Add prioritizer context data
2015-05-15 23:14 [PATCH 00/10] Multipath path resync Benjamin Marzinski
@ 2015-05-15 23:14 ` Benjamin Marzinski
2015-05-15 23:14 ` [PATCH 02/10] fix memory leaks on realloc failures Benjamin Marzinski
` (8 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Benjamin Marzinski @ 2015-05-15 23:14 UTC (permalink / raw)
To: device-mapper development; +Cc: Christophe Varoqui
Currently, running the alua prioritizer on a path causes 5 ioctls on many
devices. get_target_port_group_support() returns whether alua is
supported. get_target_port_group() gets the TPG id. This often takes two
ioctls because 128 bytes is not a large enough buffer size on many
devices. Finally, get_asymmetric_access_state() also often takes two
ioctls because of the buffer size. This can get to be problematic when
there are thousands of paths. The goal of this patch to to cut this down
to one call in the usual case.
In order to do this, I've added a context pointer to the prio structure,
similar to what exists for the checker structure, and initprio() and
freeprio() functions to the prioritizers. The only one that currently uses
these is the alua prioritizer. It caches the type of alua support, the TPG
id, and the necessary buffer size. The only thing I'm worried about with
this patch is whether the first two values could change. In order to deal
with that possibility, whenever a path gets a change event, or becomes
valid again after a failure, it resets the context structure values, which
forces all of them to get checked the next time the prioritizer is called.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/prio.c | 34 ++++++++++++++++++-
libmultipath/prio.h | 7 ++++
libmultipath/prioritizers/alua.c | 58 +++++++++++++++++++++++++-------
libmultipath/prioritizers/alua_rtpg.c | 22 +++++++++---
libmultipath/prioritizers/alua_rtpg.h | 4 +--
libmultipath/prioritizers/const.c | 4 +++
libmultipath/prioritizers/datacore.c | 3 ++
libmultipath/prioritizers/def_func.h | 11 ++++++
libmultipath/prioritizers/emc.c | 4 +++
libmultipath/prioritizers/hds.c | 4 +++
libmultipath/prioritizers/hp_sw.c | 4 +++
libmultipath/prioritizers/iet.c | 4 +++
libmultipath/prioritizers/ontap.c | 4 +++
libmultipath/prioritizers/random.c | 4 +++
libmultipath/prioritizers/rdac.c | 4 +++
libmultipath/prioritizers/weightedpath.c | 3 ++
libmultipath/propsel.c | 4 +--
multipathd/main.c | 24 ++++++++-----
18 files changed, 171 insertions(+), 31 deletions(-)
create mode 100644 libmultipath/prioritizers/def_func.h
diff --git a/libmultipath/prio.c b/libmultipath/prio.c
index ab8eca9..c8fe252 100644
--- a/libmultipath/prio.c
+++ b/libmultipath/prio.c
@@ -115,9 +115,24 @@ struct prio * add_prio (char * name)
p->getprio = (int (*)(struct path *, char *)) dlsym(p->handle, "getprio");
errstr = dlerror();
if (errstr != NULL)
- condlog(0, "A dynamic linking error occurred: (%s)", errstr);
+ condlog(0, "A dynamic linking error occurred with getprio: (%s)", errstr);
if (!p->getprio)
goto out;
+
+ p->initprio = (int (*)(struct prio *)) dlsym(p->handle, "initprio");
+ errstr = dlerror();
+ if (errstr != NULL)
+ condlog(0, "A dynamic linking error occurred with initprio: (%s)", errstr);
+ if (!p->initprio)
+ goto out;
+
+ p->freeprio = (int (*)(struct prio *)) dlsym(p->handle, "freeprio");
+ errstr = dlerror();
+ if (errstr != NULL)
+ condlog(0, "A dynamic linking error occurred with freeprio: (%s)", errstr);
+ if (!p->freeprio)
+ goto out;
+
list_add(&p->node, &prioritizers);
return p;
out:
@@ -125,6 +140,13 @@ out:
return NULL;
}
+int prio_init (struct prio * p)
+{
+ if (!p || !p->initprio)
+ return 1;
+ return p->initprio(p);
+}
+
int prio_getprio (struct prio * p, struct path * pp)
{
return p->getprio(pp, p->args);
@@ -159,8 +181,16 @@ void prio_get (struct prio * dst, char * name, char * args)
strncpy(dst->name, src->name, PRIO_NAME_LEN);
if (args)
strncpy(dst->args, args, PRIO_ARGS_LEN);
+ dst->initprio = src->initprio;
dst->getprio = src->getprio;
+ dst->freeprio = src->freeprio;
dst->handle = NULL;
+ dst->context = NULL;
+
+ if (dst->initprio(dst) != 0){
+ memset(dst, 0x0, sizeof(struct prio));
+ return;
+ }
src->refcount++;
}
@@ -173,6 +203,8 @@ void prio_put (struct prio * dst)
return;
src = prio_lookup(dst->name);
+ if (dst->freeprio)
+ dst->freeprio(dst);
memset(dst, 0x0, sizeof(struct prio));
free_prio(src);
}
diff --git a/libmultipath/prio.h b/libmultipath/prio.h
index 495688f..6d57ecd 100644
--- a/libmultipath/prio.h
+++ b/libmultipath/prio.h
@@ -46,9 +46,15 @@ struct prio {
void *handle;
int refcount;
struct list_head node;
+ void * context;
char name[PRIO_NAME_LEN];
char args[PRIO_ARGS_LEN];
+ int (*initprio)(struct prio * p);
+ /* You are allowed to call initprio multiple times without calling
+ * freeprio. Doing so will reinitialize it (possibly skipping
+ * allocations) */
int (*getprio)(struct path *, char *);
+ int (*freeprio)(struct prio * p);
};
unsigned int get_prio_timeout(unsigned int default_timeout);
@@ -57,6 +63,7 @@ void cleanup_prio (void);
struct prio * add_prio (char *);
struct prio * prio_lookup (char *);
int prio_getprio (struct prio *, struct path *);
+int prio_init (struct prio *);
void prio_get (struct prio *, char *, char *);
void prio_put (struct prio *);
int prio_selected (struct prio *);
diff --git a/libmultipath/prioritizers/alua.c b/libmultipath/prioritizers/alua.c
index 39ed1c8..7691156 100644
--- a/libmultipath/prioritizers/alua.c
+++ b/libmultipath/prioritizers/alua.c
@@ -37,6 +37,12 @@ static const char * aas_string[] = {
[AAS_TRANSITIONING] = "transitioning between states",
};
+struct alua_context {
+ int tpg_support;
+ int tpg;
+ int buflen;
+};
+
static const char *aas_print_string(int rc)
{
rc &= 0x7f;
@@ -51,24 +57,25 @@ static const char *aas_print_string(int rc)
}
int
-get_alua_info(int fd)
+get_alua_info(int fd, struct alua_context *ct)
{
int rc;
- int tpg;
- rc = get_target_port_group_support(fd);
- if (rc < 0)
- return -ALUA_PRIO_TPGS_FAILED;
+ if (ct->tpg_support <= 0 || ct->tpg < 0) {
+ ct->tpg_support = get_target_port_group_support(fd);
+ if (ct->tpg_support < 0)
+ return -ALUA_PRIO_TPGS_FAILED;
- if (rc == TPGS_NONE)
- return -ALUA_PRIO_NOT_SUPPORTED;
+ if (ct->tpg_support == TPGS_NONE)
+ return -ALUA_PRIO_NOT_SUPPORTED;
- tpg = get_target_port_group(fd);
- if (tpg < 0)
- return -ALUA_PRIO_RTPG_FAILED;
+ ct->tpg = get_target_port_group(fd, &ct->buflen);
+ if (ct->tpg < 0)
+ return -ALUA_PRIO_RTPG_FAILED;
+ }
- condlog(3, "reported target port group is %i", tpg);
- rc = get_asymmetric_access_state(fd, tpg);
+ condlog(3, "reported target port group is %i", ct->tpg);
+ rc = get_asymmetric_access_state(fd, ct->tpg, &ct->buflen);
if (rc < 0)
return -ALUA_PRIO_GETAAS_FAILED;
@@ -86,7 +93,7 @@ int getprio (struct path * pp, char * args)
if (pp->fd < 0)
return -ALUA_PRIO_NO_INFORMATION;
- rc = get_alua_info(pp->fd);
+ rc = get_alua_info(pp->fd, pp->prio.context);
if (rc >= 0) {
aas = (rc & 0x0f);
priopath = (rc & 0x80);
@@ -126,3 +133,28 @@ int getprio (struct path * pp, char * args)
}
return rc;
}
+
+int initprio(struct prio *p)
+{
+ if (!p->context) {
+ struct alua_context *ct;
+
+ ct = malloc(sizeof(struct alua_context));
+ if (!ct)
+ return 1;
+ p->context = ct;
+ }
+ memset(p->context, 0, sizeof(struct alua_context));
+ return 0;
+}
+
+
+int freeprio(struct prio *p)
+{
+ if (p->context) {
+ free(p->context);
+ p->context = NULL;
+ }
+ return 0;
+}
+
diff --git a/libmultipath/prioritizers/alua_rtpg.c b/libmultipath/prioritizers/alua_rtpg.c
index 6d04fc1..7ef2ef5 100644
--- a/libmultipath/prioritizers/alua_rtpg.c
+++ b/libmultipath/prioritizers/alua_rtpg.c
@@ -171,7 +171,7 @@ get_target_port_group_support(int fd)
}
int
-get_target_port_group(int fd)
+get_target_port_group(int fd, int *buflen_ptr)
{
unsigned char *buf;
struct vpd83_data * vpd83;
@@ -179,7 +179,12 @@ get_target_port_group(int fd)
int rc;
int buflen, scsi_buflen;
- buflen = 128; /* Lets start from 128 */
+ if (!buflen_ptr || *buflen_ptr == 0) {
+ buflen = 128; /* Lets start from 128 */
+ if (buflen_ptr)
+ *buflen_ptr = 128;
+ } else
+ buflen = *buflen_ptr;
buf = (unsigned char *)malloc(buflen);
if (!buf) {
PRINT_DEBUG("malloc failed: could not allocate"
@@ -202,6 +207,8 @@ get_target_port_group(int fd)
return -RTPG_RTPG_FAILED;
}
buflen = scsi_buflen;
+ if (buflen_ptr)
+ *buflen_ptr = buflen;
memset(buf, 0, buflen);
rc = do_inquiry(fd, 1, 0x83, buf, buflen);
if (rc < 0)
@@ -269,7 +276,7 @@ do_rtpg(int fd, void* resp, long resplen)
}
int
-get_asymmetric_access_state(int fd, unsigned int tpg)
+get_asymmetric_access_state(int fd, unsigned int tpg, int *buflen_ptr)
{
unsigned char *buf;
struct rtpg_data * tpgd;
@@ -278,7 +285,12 @@ get_asymmetric_access_state(int fd, unsigned int tpg)
int buflen;
uint32_t scsi_buflen;
- buflen = 128; /* Initial value from old code */
+ if (!buflen_ptr || *buflen_ptr == 0) {
+ buflen = 128; /* Initial value from old code */
+ if (buflen_ptr)
+ *buflen_ptr = 128;
+ } else
+ buflen = *buflen_ptr;
buf = (unsigned char *)malloc(buflen);
if (!buf) {
PRINT_DEBUG ("malloc failed: could not allocate"
@@ -299,6 +311,8 @@ get_asymmetric_access_state(int fd, unsigned int tpg)
return -RTPG_RTPG_FAILED;
}
buflen = scsi_buflen;
+ if (buflen_ptr)
+ *buflen_ptr = buflen;
memset(buf, 0, buflen);
rc = do_rtpg(fd, buf, buflen);
if (rc < 0)
diff --git a/libmultipath/prioritizers/alua_rtpg.h b/libmultipath/prioritizers/alua_rtpg.h
index c43e0a9..cade64d 100644
--- a/libmultipath/prioritizers/alua_rtpg.h
+++ b/libmultipath/prioritizers/alua_rtpg.h
@@ -23,8 +23,8 @@
#define RTPG_TPG_NOT_FOUND 4
int get_target_port_group_support(int fd);
-int get_target_port_group(int fd);
-int get_asymmetric_access_state(int fd, unsigned int tpg);
+int get_target_port_group(int fd, int *buflen_ptr);
+int get_asymmetric_access_state(int fd, unsigned int tpg, int *buflen_ptr);
#endif /* __RTPG_H__ */
diff --git a/libmultipath/prioritizers/const.c b/libmultipath/prioritizers/const.c
index bf689cd..362d4e1 100644
--- a/libmultipath/prioritizers/const.c
+++ b/libmultipath/prioritizers/const.c
@@ -1,8 +1,12 @@
#include <stdio.h>
#include <prio.h>
+#include "def_func.h"
int getprio (struct path * pp, char * args)
{
return 1;
}
+
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/prioritizers/datacore.c b/libmultipath/prioritizers/datacore.c
index e3e6a51..c2f0d0b 100644
--- a/libmultipath/prioritizers/datacore.c
+++ b/libmultipath/prioritizers/datacore.c
@@ -25,6 +25,7 @@
#include <debug.h>
#include <prio.h>
#include <structs.h>
+#include "def_func.h"
#define INQ_REPLY_LEN 255
#define INQ_CMD_CODE 0x12
@@ -111,3 +112,5 @@ int getprio (struct path * pp, char * args)
return datacore_prio(pp->dev, pp->fd, args);
}
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/prioritizers/def_func.h b/libmultipath/prioritizers/def_func.h
new file mode 100644
index 0000000..8976b3f
--- /dev/null
+++ b/libmultipath/prioritizers/def_func.h
@@ -0,0 +1,11 @@
+#ifndef _DEF_FUNC_H
+#define _DEF_FUNC_H
+
+#include "prio.h"
+
+#define declare_nop_prio(name) \
+int name (struct prio *p) \
+{ \
+ return 0; \
+}
+#endif /* _DEF_FUNC_H */
diff --git a/libmultipath/prioritizers/emc.c b/libmultipath/prioritizers/emc.c
index e49809c..448528f 100644
--- a/libmultipath/prioritizers/emc.c
+++ b/libmultipath/prioritizers/emc.c
@@ -6,6 +6,7 @@
#include <debug.h>
#include <prio.h>
#include <structs.h>
+#include "def_func.h"
#define INQUIRY_CMD 0x12
#define INQUIRY_CMDLEN 6
@@ -85,3 +86,6 @@ int getprio (struct path * pp, char * args)
{
return emc_clariion_prio(pp->dev, pp->fd);
}
+
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/prioritizers/hds.c b/libmultipath/prioritizers/hds.c
index 8043b5b..292fe1c 100644
--- a/libmultipath/prioritizers/hds.c
+++ b/libmultipath/prioritizers/hds.c
@@ -76,6 +76,7 @@
#include <debug.h>
#include <prio.h>
#include <structs.h>
+#include "def_func.h"
#define INQ_REPLY_LEN 255
#define INQ_CMD_CODE 0x12
@@ -170,3 +171,6 @@ int getprio (struct path * pp, char * args)
{
return hds_modular_prio(pp->dev, pp->fd);
}
+
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/prioritizers/hp_sw.c b/libmultipath/prioritizers/hp_sw.c
index 4950cf7..371d766 100644
--- a/libmultipath/prioritizers/hp_sw.c
+++ b/libmultipath/prioritizers/hp_sw.c
@@ -16,6 +16,7 @@
#include <debug.h>
#include <prio.h>
#include <structs.h>
+#include "def_func.h"
#define TUR_CMD_LEN 6
#define SCSI_CHECK_CONDITION 0x2
@@ -99,3 +100,6 @@ int getprio (struct path * pp, char * args)
{
return hp_sw_prio(pp->dev, pp->fd);
}
+
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/prioritizers/iet.c b/libmultipath/prioritizers/iet.c
index 0bcc48b..a6b73d5 100644
--- a/libmultipath/prioritizers/iet.c
+++ b/libmultipath/prioritizers/iet.c
@@ -9,6 +9,7 @@
#include <debug.h>
#include <unistd.h>
#include <structs.h>
+#include "def_func.h"
//
// This prioritizer suits iSCSI needs, makes it possible to prefer one path.
@@ -141,3 +142,6 @@ int getprio(struct path * pp, char * args)
{
return iet_prio(pp->dev, args);
}
+
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/prioritizers/ontap.c b/libmultipath/prioritizers/ontap.c
index 5e82a17..c556e3b 100644
--- a/libmultipath/prioritizers/ontap.c
+++ b/libmultipath/prioritizers/ontap.c
@@ -23,6 +23,7 @@
#include <debug.h>
#include <prio.h>
#include <structs.h>
+#include "def_func.h"
#define INQUIRY_CMD 0x12
#define INQUIRY_CMDLEN 6
@@ -245,3 +246,6 @@ int getprio (struct path * pp, char * args)
{
return ontap_prio(pp->dev, pp->fd);
}
+
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/prioritizers/random.c b/libmultipath/prioritizers/random.c
index 281a0d1..c38b0f6 100644
--- a/libmultipath/prioritizers/random.c
+++ b/libmultipath/prioritizers/random.c
@@ -4,6 +4,7 @@
#include <time.h>
#include <prio.h>
+#include "def_func.h"
int getprio (struct path * pp, char * args)
{
@@ -13,3 +14,6 @@ int getprio (struct path * pp, char * args)
srand((unsigned int)tv.tv_usec);
return 1+(int) (10.0*rand()/(RAND_MAX+1.0));
}
+
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/prioritizers/rdac.c b/libmultipath/prioritizers/rdac.c
index a210055..1fc302f 100644
--- a/libmultipath/prioritizers/rdac.c
+++ b/libmultipath/prioritizers/rdac.c
@@ -6,6 +6,7 @@
#include <debug.h>
#include <prio.h>
#include <structs.h>
+#include "def_func.h"
#define INQUIRY_CMD 0x12
#define INQUIRY_CMDLEN 6
@@ -95,3 +96,6 @@ int getprio (struct path * pp, char * args)
{
return rdac_prio(pp->dev, pp->fd);
}
+
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/prioritizers/weightedpath.c b/libmultipath/prioritizers/weightedpath.c
index 13bc52f..1c3f821 100644
--- a/libmultipath/prioritizers/weightedpath.c
+++ b/libmultipath/prioritizers/weightedpath.c
@@ -32,6 +32,7 @@
#include <memory.h>
#include <debug.h>
#include <regex.h>
+#include "def_func.h"
char *get_next_string(char **temp, char *split_char)
{
@@ -104,3 +105,5 @@ int getprio(struct path *pp, char *args)
return prio_path_weight(pp, args);
}
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index f64d5e4..347cc66 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -376,10 +376,10 @@ detect_prio(struct path * pp)
if (get_target_port_group_support(pp->fd) <= 0)
return;
- ret = get_target_port_group(pp->fd);
+ ret = get_target_port_group(pp->fd, NULL);
if (ret < 0)
return;
- if (get_asymmetric_access_state(pp->fd, ret) < 0)
+ if (get_asymmetric_access_state(pp->fd, ret, NULL) < 0)
return;
prio_get(p, PRIO_ALUA, DEFAULT_PRIO_ARGS);
}
diff --git a/multipathd/main.c b/multipathd/main.c
index f876258..360a584 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -721,20 +721,23 @@ static int
uev_update_path (struct uevent *uev, struct vectors * vecs)
{
int ro, retval = 0;
+ struct path * pp;
+
+ pp = find_path_by_dev(vecs->pathvec, uev->kernel);
+ if (!pp) {
+ condlog(0, "%s: spurious uevent, path not found",
+ uev->kernel);
+ return 1;
+ }
+ /* reinit the prio values on change event, in case something is
+ * different */
+ prio_init(&pp->prio);
ro = uevent_get_disk_ro(uev);
if (ro >= 0) {
- struct path * pp;
-
condlog(2, "%s: update path write_protect to '%d' (uevent)",
uev->kernel, ro);
- pp = find_path_by_dev(vecs->pathvec, uev->kernel);
- if (!pp) {
- condlog(0, "%s: spurious uevent, path not found",
- uev->kernel);
- return 1;
- }
if (pp->mpp) {
retval = reload_map(vecs, pp->mpp, 0);
@@ -1272,6 +1275,7 @@ check_path (struct vectors * vecs, struct path * pp)
}
if(newstate == PATH_UP || newstate == PATH_GHOST){
+ prio_init(&pp->prio);
if ( pp->mpp && pp->mpp->prflag ){
/*
* Check Persistent Reservation.
@@ -1308,7 +1312,9 @@ check_path (struct vectors * vecs, struct path * pp)
/*
* if at least one path is up in a group, and
- * the group is disabled, re-enable it
+ * the group is disabled, re-enable it.
+ * Reinitialize the prioritizer, in case something
+ * changed.
*/
if (newstate == PATH_UP)
enable_group(pp);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 02/10] fix memory leaks on realloc failures
2015-05-15 23:14 [PATCH 00/10] Multipath path resync Benjamin Marzinski
2015-05-15 23:14 ` [PATCH 01/10] libmultipath: Add prioritizer context data Benjamin Marzinski
@ 2015-05-15 23:14 ` Benjamin Marzinski
2015-05-15 23:14 ` [PATCH 03/10] multipathd: use /run instead of /var/run Benjamin Marzinski
` (7 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Benjamin Marzinski @ 2015-05-15 23:14 UTC (permalink / raw)
To: device-mapper development; +Cc: Christophe Varoqui
When realloc fails, it doesn't free the existing memory. In many places,
multipath was just forgetting about that that memory. It needs to free
up the existing memory, or to reset back to using it.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/dmparser.c | 6 ++++--
multipath/main.c | 9 ++++++---
multipathd/cli_handlers.c | 50 +++++++++++++----------------------------------
multipathd/uxlsnr.c | 19 +++++++++++++++++-
4 files changed, 42 insertions(+), 42 deletions(-)
diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
index 2562ba1..2209b2d 100644
--- a/libmultipath/dmparser.c
+++ b/libmultipath/dmparser.c
@@ -20,14 +20,16 @@
static int
merge_words (char ** dst, char * word, int space)
{
- char * p;
+ char * p = *dst;
int len;
len = strlen(*dst) + strlen(word) + space;
*dst = REALLOC(*dst, len + 1);
- if (!*dst)
+ if (!*dst) {
+ free(p);
return 1;
+ }
p = *dst;
diff --git a/multipath/main.c b/multipath/main.c
index f62dcb1..d761621 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -395,7 +395,7 @@ out:
static int
dump_config (void)
{
- char * c;
+ char * c, * tmp = NULL;
char * reply;
unsigned int maxlen = 256;
int again = 1;
@@ -403,9 +403,12 @@ dump_config (void)
reply = MALLOC(maxlen);
while (again) {
- if (!reply)
+ if (!reply) {
+ if (tmp)
+ free(tmp);
return 1;
- c = reply;
+ }
+ c = tmp = reply;
c += snprint_defaults(c, reply + maxlen - c);
again = ((c - reply) == maxlen);
if (again) {
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index dc96c45..305cd7f 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -26,11 +26,14 @@
#define REALLOC_REPLY(r, a, m) \
do { \
if ((a)) { \
+ char *tmp = (r); \
(r) = REALLOC((r), (m) * 2); \
if ((r)) { \
memset((r) + (m), 0, (m)); \
(m) *= 2; \
} \
+ else \
+ free(tmp); \
} \
} while (0)
@@ -173,7 +176,7 @@ show_config (char ** r, int * len)
unsigned int maxlen = INITIAL_REPLY_LEN;
int again = 1;
- reply = MALLOC(maxlen);
+ c = reply = MALLOC(maxlen);
while (again) {
if (!reply)
@@ -181,54 +184,29 @@ show_config (char ** r, int * len)
c = reply;
c += snprint_defaults(c, reply + maxlen - c);
again = ((c - reply) == maxlen);
- if (again) {
- reply = REALLOC(reply, maxlen * 2);
- if (!reply)
- return 1;
- memset(reply + maxlen, 0, maxlen);
- maxlen *= 2;
+ REALLOC_REPLY(reply, again, maxlen);
+ if (again)
continue;
- }
c += snprint_blacklist(c, reply + maxlen - c);
again = ((c - reply) == maxlen);
- if (again) {
- reply = REALLOC(reply, maxlen * 2);
- if (!reply)
- return 1;
- memset(reply + maxlen, 0, maxlen);
- maxlen *= 2;
+ REALLOC_REPLY(reply, again, maxlen);
+ if (again)
continue;
- }
c += snprint_blacklist_except(c, reply + maxlen - c);
again = ((c - reply) == maxlen);
- if (again) {
- reply = REALLOC(reply, maxlen * 2);
- if (!reply)
- return 1;
- memset(reply + maxlen, 0, maxlen);
- maxlen *= 2;
+ REALLOC_REPLY(reply, again, maxlen);
+ if (again)
continue;
- }
c += snprint_hwtable(c, reply + maxlen - c, conf->hwtable);
again = ((c - reply) == maxlen);
- if (again) {
- reply = REALLOC(reply, maxlen * 2);
- if (!reply)
- return 1;
- memset(reply + maxlen, 0, maxlen);
- maxlen *= 2;
+ REALLOC_REPLY(reply, again, maxlen);
+ if (again)
continue;
- }
c += snprint_overrides(c, reply + maxlen - c, conf->overrides);
again = ((c - reply) == maxlen);
- if (again) {
- reply = REALLOC(reply, maxlen * 2);
- if (!reply)
- return 1;
- memset(reply + maxlen, 0, maxlen);
- maxlen *= 2;
+ REALLOC_REPLY(reply, again, maxlen);
+ if (again)
continue;
- }
if (VECTOR_SIZE(conf->mptable) > 0) {
c += snprint_mptable(c, reply + maxlen - c,
conf->mptable);
diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
index 61ba49a..64bd35c 100644
--- a/multipathd/uxlsnr.c
+++ b/multipathd/uxlsnr.c
@@ -65,6 +65,10 @@ static void new_client(int ux_sock)
return;
c = (struct client *)MALLOC(sizeof(*c));
+ if (!c) {
+ close(fd);
+ return;
+ }
memset(c, 0, sizeof(*c));
INIT_LIST_HEAD(&c->node);
c->fd = fd;
@@ -151,6 +155,7 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, void * trigger_data)
sigdelset(&mask, SIGHUP);
sigdelset(&mask, SIGUSR1);
while (1) {
+ struct pollfd *new;
struct client *c, *tmp;
int i, poll_count, num_clients;
@@ -168,7 +173,19 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, void * trigger_data)
list_for_each_entry(c, &clients, node) {
num_clients++;
}
- polls = REALLOC(polls, (1+num_clients) * sizeof(*polls));
+ new = REALLOC(polls, (1+num_clients) * sizeof(*polls));
+ /* If we can't allocate poliing space for the new client,
+ * close it */
+ if (!new) {
+ if (!num_clients) {
+ condlog(1, "can't listen for new clients");
+ return NULL;
+ }
+ dead_client(list_entry(clients.prev,
+ typeof(struct client), node));
+ }
+ else
+ polls = new;
polls[0].fd = ux_sock;
polls[0].events = POLLIN;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 03/10] multipathd: use /run instead of /var/run
2015-05-15 23:14 [PATCH 00/10] Multipath path resync Benjamin Marzinski
2015-05-15 23:14 ` [PATCH 01/10] libmultipath: Add prioritizer context data Benjamin Marzinski
2015-05-15 23:14 ` [PATCH 02/10] fix memory leaks on realloc failures Benjamin Marzinski
@ 2015-05-15 23:14 ` Benjamin Marzinski
2015-06-03 21:38 ` Sebastian Herbszt
2015-05-15 23:14 ` [PATCH 04/10] retrigger uevents to try and get the uid through udev Benjamin Marzinski
` (6 subsequent siblings)
9 siblings, 1 reply; 12+ messages in thread
From: Benjamin Marzinski @ 2015-05-15 23:14 UTC (permalink / raw)
To: device-mapper development; +Cc: Christophe Varoqui
/var/run is usually a symlink to /run. If /var is on a separate
filesytem, when multipathd starts up, it might end up writing to
/var/run before the /var filesytem is mounted and thus not have
its pidfile accessible at /var/run afterwards. /run is a tmpfs and
should always be available before multipathd is started, so
multipath should just write there directly, instead of through the
symlink.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/defaults.h | 2 +-
multipathd/multipathd.init.suse | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
index 8902f40..79d6b91 100644
--- a/libmultipath/defaults.h
+++ b/libmultipath/defaults.h
@@ -26,7 +26,7 @@
#define MAX_CHECKINT(a) (a << 2)
#define MAX_DEV_LOSS_TMO 0x7FFFFFFF
-#define DEFAULT_PIDFILE "/var/run/multipathd.pid"
+#define DEFAULT_PIDFILE "/run/multipathd.pid"
#define DEFAULT_SOCKET "/org/kernel/linux/storage/multipathd"
#define DEFAULT_CONFIGFILE "/etc/multipath.conf"
#define DEFAULT_BINDINGS_FILE "/etc/multipath/bindings"
diff --git a/multipathd/multipathd.init.suse b/multipathd/multipathd.init.suse
index d1319b1..ed699fa 100644
--- a/multipathd/multipathd.init.suse
+++ b/multipathd/multipathd.init.suse
@@ -17,7 +17,7 @@
PATH=/bin:/usr/bin:/sbin:/usr/sbin
DAEMON=/sbin/multipathd
-PIDFILE=/var/run/multipathd.pid
+PIDFILE=/run/multipathd.pid
MPATH_INIT_TIMEOUT=10
ARGS=""
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 03/10] multipathd: use /run instead of /var/run
2015-05-15 23:14 ` [PATCH 03/10] multipathd: use /run instead of /var/run Benjamin Marzinski
@ 2015-06-03 21:38 ` Sebastian Herbszt
0 siblings, 0 replies; 12+ messages in thread
From: Sebastian Herbszt @ 2015-06-03 21:38 UTC (permalink / raw)
To: device-mapper development; +Cc: Sebastian Herbszt, Christophe Varoqui
Benjamin Marzinski wrote:
> /var/run is usually a symlink to /run. If /var is on a separate
> filesytem, when multipathd starts up, it might end up writing to
> /var/run before the /var filesytem is mounted and thus not have
> its pidfile accessible at /var/run afterwards. /run is a tmpfs and
> should always be available before multipathd is started, so
> multipath should just write there directly, instead of through the
> symlink.
My older openSUSE doesn't have a /run directory.
Should this maybe depend on systemd or something else?
Sebastian
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 04/10] retrigger uevents to try and get the uid through udev
2015-05-15 23:14 [PATCH 00/10] Multipath path resync Benjamin Marzinski
` (2 preceding siblings ...)
2015-05-15 23:14 ` [PATCH 03/10] multipathd: use /run instead of /var/run Benjamin Marzinski
@ 2015-05-15 23:14 ` Benjamin Marzinski
2015-05-15 23:14 ` [PATCH 05/10] update multipath rules to deal with partition devices Benjamin Marzinski
` (5 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Benjamin Marzinski @ 2015-05-15 23:14 UTC (permalink / raw)
To: device-mapper development; +Cc: Christophe Varoqui
Ideally, udev will be able to grab the wwid when a path device is
discovered, but sometimes this isn't possible. In these cases, the
best thing that could happen would be for udev to actually get the
information, and add it to its database. This patch makes multipath
retrigger uevents a limited number of times before giving up and
trying to get the information itself.
There are two configurables that control how it does this,
"retrigger_tries" and "retrigger_delay". The first sets the number of
times it will try to retrigger a uevent to get the wwid, the second
sets the amount of time to wait between retriggers.
This patch currently only tries reinitializing the path on change events
after multipathd has triggered a change event, and it only tries once
per triggered change event. Now, its possible that other change events
could occur on the device without multipathd tirggering them. As the
patch stands now, it won't try to initialize the device on those. It will,
however still try in the checkerloop, but only after it has finished
retriggering the uevents. We could be much more aggressive here, and assume
that devices that simply won't have a WWID should already be taken care of
by the blacklists, so it would be always a good idea to recheck devices on
change events. What would be ideal is if udev would let us know when it had
problems or timed out when processing a uevent, so we would know if
retiggering the uevent would be useful.
Cc: Hannes Reinecke <hare@suse.de>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/config.c | 2 ++
libmultipath/config.h | 2 ++
libmultipath/defaults.h | 2 ++
libmultipath/dict.c | 8 ++++++++
libmultipath/discovery.c | 28 ++++++++++++++++------------
libmultipath/structs.h | 8 ++++++++
multipathd/main.c | 25 ++++++++++++++++++-------
7 files changed, 56 insertions(+), 19 deletions(-)
diff --git a/libmultipath/config.c b/libmultipath/config.c
index c788cf6..cfcc685 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -619,6 +619,8 @@ load_config (char * file, struct udev *udev)
conf->find_multipaths = DEFAULT_FIND_MULTIPATHS;
conf->uxsock_timeout = DEFAULT_UXSOCK_TIMEOUT;
conf->uid_attribute = set_default(DEFAULT_UID_ATTRIBUTE);
+ conf->retrigger_tries = DEFAULT_RETRIGGER_TRIES;
+ conf->retrigger_delay = DEFAULT_RETRIGGER_DELAY;
/*
* preload default hwtable
diff --git a/libmultipath/config.h b/libmultipath/config.h
index 0183969..372eace 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -135,6 +135,8 @@ struct config {
int delay_watch_checks;
int delay_wait_checks;
int uxsock_timeout;
+ int retrigger_tries;
+ int retrigger_delay;
unsigned int version[3];
char * dev;
diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
index 79d6b91..025a016 100644
--- a/libmultipath/defaults.h
+++ b/libmultipath/defaults.h
@@ -21,6 +21,8 @@
#define DEFAULT_DELAY_CHECKS DELAY_CHECKS_OFF
#define DEFAULT_UEVENT_STACKSIZE 256
#define DEFAULT_UXSOCK_TIMEOUT 1000
+#define DEFAULT_RETRIGGER_DELAY 10
+#define DEFAULT_RETRIGGER_TRIES 3
#define DEFAULT_CHECKINT 5
#define MAX_CHECKINT(a) (a << 2)
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index 5c2da43..e3bc67e 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -389,6 +389,12 @@ declare_hw_snprint(deferred_remove, print_yes_no_undef)
declare_mp_handler(deferred_remove, set_yes_no_undef)
declare_mp_snprint(deferred_remove, print_yes_no_undef)
+declare_def_handler(retrigger_tries, set_int)
+declare_def_snprint(retrigger_tries, print_int)
+
+declare_def_handler(retrigger_delay, set_int)
+declare_def_snprint(retrigger_delay, print_int)
+
static int
def_config_dir_handler(vector strvec)
{
@@ -1365,6 +1371,8 @@ init_keywords(void)
install_keyword("delay_wait_checks", &def_delay_wait_checks_handler, &snprint_def_delay_wait_checks);
install_keyword("find_multipaths", &def_find_multipaths_handler, &snprint_def_find_multipaths);
install_keyword("uxsock_timeout", &def_uxsock_timeout_handler, &snprint_def_uxsock_timeout);
+ install_keyword("retrigger_tries", &def_retrigger_tries_handler, &snprint_def_retrigger_tries);
+ install_keyword("retrigger_delay", &def_retrigger_delay_handler, &snprint_def_retrigger_delay);
__deprecated install_keyword("default_selector", &def_selector_handler, NULL);
__deprecated install_keyword("default_path_grouping_policy", &def_pgpolicy_handler, NULL);
__deprecated install_keyword("default_uid_attribute", &def_uid_attribute_handler, NULL);
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 4582a20..bd65354 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1547,7 +1547,7 @@ get_uid (struct path * pp)
pp->dev, strerror(-len));
}
- if (len <= 0 &&
+ if (len <= 0 && pp->retriggers >= conf->retrigger_tries &&
!strcmp(pp->uid_attribute, DEFAULT_UID_ATTRIBUTE)) {
len = get_vpd_uid(pp);
origin = "sysfs";
@@ -1651,11 +1651,19 @@ pathinfo (struct path *pp, vector hwtable, int mask)
}
}
- if ((mask & DI_WWID) && !strlen(pp->wwid))
+ if ((mask & DI_WWID) && !strlen(pp->wwid)) {
get_uid(pp);
+ if (!strlen(pp->wwid)) {
+ pp->initialized = INIT_MISSING_UDEV;
+ pp->tick = conf->retrigger_delay;
+ return PATHINFO_OK;
+ }
+ else
+ pp->tick = 1;
+ }
+
if (mask & DI_BLACKLIST && mask & DI_WWID) {
- if (!strlen(pp->wwid) ||
- filter_wwid(conf->blist_wwid, conf->elist_wwid,
+ if (filter_wwid(conf->blist_wwid, conf->elist_wwid,
pp->wwid, pp->dev) > 0) {
return PATHINFO_SKIPPED;
}
@@ -1665,17 +1673,13 @@ pathinfo (struct path *pp, vector hwtable, int mask)
* Retrieve path priority, even for PATH_DOWN paths if it has never
* been successfully obtained before.
*/
- if ((mask & DI_PRIO) && path_state == PATH_UP) {
+ if ((mask & DI_PRIO) && path_state == PATH_UP && strlen(pp->wwid)) {
if (pp->state != PATH_DOWN || pp->priority == PRIO_UNDEF) {
- if (!strlen(pp->wwid))
- get_uid(pp);
- if (!strlen(pp->wwid))
- return PATHINFO_SKIPPED;
get_prio(pp);
}
}
- pp->initialized = 1;
+ pp->initialized = INIT_OK;
return PATHINFO_OK;
blank:
@@ -1684,7 +1688,7 @@ blank:
*/
memset(pp->wwid, 0, WWID_SIZE);
pp->chkrstate = pp->state = PATH_DOWN;
- pp->initialized = 0;
+ pp->initialized = INIT_FAILED;
- return 0;
+ return PATHINFO_OK;
}
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index c1fce04..85a8fdc 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -145,6 +145,13 @@ enum delay_checks_states {
DELAY_CHECKS_UNDEF = 0,
};
+enum initialized_states {
+ INIT_FAILED,
+ INIT_MISSING_UDEV,
+ INIT_REQUESTED_UDEV,
+ INIT_OK,
+};
+
struct sg_id {
int host_no;
int channel;
@@ -201,6 +208,7 @@ struct path {
struct multipath * mpp;
int fd;
int initialized;
+ int retriggers;
/* configlet pointers */
struct hwentry * hwe;
diff --git a/multipathd/main.c b/multipathd/main.c
index 360a584..ea67324 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -457,11 +457,6 @@ uev_add_path (struct uevent *uev, struct vectors * vecs)
condlog(3, "%s: failed to get path info", uev->kernel);
return 1;
}
- if (!strlen(pp->wwid)) {
- condlog(3, "%s: Failed to get path wwid", uev->kernel);
- free_path(pp);
- return 1;
- }
ret = store_path(vecs->pathvec, pp);
if (!ret) {
pp->checkint = conf->checkint;
@@ -729,6 +724,10 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
uev->kernel);
return 1;
}
+
+ if (pp->initialized == INIT_REQUESTED_UDEV)
+ return uev_add_path(uev, vecs);
+
/* reinit the prio values on change event, in case something is
* different */
prio_init(&pp->prio);
@@ -1164,12 +1163,24 @@ check_path (struct vectors * vecs, struct path * pp)
int add_active;
int oldchkrstate = pp->chkrstate;
- if (pp->initialized && !pp->mpp)
+ if ((pp->initialized == INIT_OK ||
+ pp->initialized == INIT_REQUESTED_UDEV) && !pp->mpp)
return 0;
if (pp->tick && --pp->tick)
return 0; /* don't check this path yet */
+ if (!pp->mpp && pp->initialized == INIT_MISSING_UDEV &&
+ pp->retriggers < conf->retrigger_tries) {
+ condlog(2, "%s: triggering change event to reinitialize",
+ pp->dev);
+ pp->initialized = INIT_REQUESTED_UDEV;
+ pp->retriggers++;
+ sysfs_attr_set_value(pp->udev, "uevent", "change",
+ strlen("change"));
+ return 0;
+ }
+
/*
* provision a next check soonest,
* in case we exit abnormaly from here
@@ -1196,7 +1207,7 @@ check_path (struct vectors * vecs, struct path * pp)
return 1;
}
if (!pp->mpp) {
- if (!strlen(pp->wwid) &&
+ if (!strlen(pp->wwid) && pp->initialized != INIT_MISSING_UDEV &&
(newstate == PATH_UP || newstate == PATH_GHOST)) {
condlog(2, "%s: add missing path", pp->dev);
if (pathinfo(pp, conf->hwtable, DI_ALL) == 0) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 05/10] update multipath rules to deal with partition devices
2015-05-15 23:14 [PATCH 00/10] Multipath path resync Benjamin Marzinski
` (3 preceding siblings ...)
2015-05-15 23:14 ` [PATCH 04/10] retrigger uevents to try and get the uid through udev Benjamin Marzinski
@ 2015-05-15 23:14 ` Benjamin Marzinski
2015-05-15 23:14 ` [PATCH 06/10] change order of multipath.rules Benjamin Marzinski
` (4 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Benjamin Marzinski @ 2015-05-15 23:14 UTC (permalink / raw)
To: device-mapper development; +Cc: Christophe Varoqui
Partition devices should inherit the DM_MULTIPATH_DEVICE_PATH
state of their parents, and if they are part of multipath path devices,
they shouldn't have their own ID_FS_TYPE
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
multipath/multipath.rules | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/multipath/multipath.rules b/multipath/multipath.rules
index c76e6b8..c8fb7e6 100644
--- a/multipath/multipath.rules
+++ b/multipath/multipath.rules
@@ -1,14 +1,22 @@
# Set DM_MULTIPATH_DEVICE_PATH if the device should be handled by multipath
SUBSYSTEM!="block", GOTO="end_mpath"
+ACTION!="add|change", GOTO="end_mpath"
+KERNEL!="sd*|dasd*", GOTO="end_mpath"
+
+ENV{DEVTYPE}!="partition", GOTO="test_dev"
+IMPORT{parent}="DM_MULTIPATH_DEVICE_PATH"
+ENV{DM_MULTIPATH_DEVICE_PATH}=="1", ENV{ID_FS_TYPE}="none", \
+ ENV{SYSTEMD_READY}="0"
+GOTO="end_mpath"
+
+LABEL="test_dev"
ENV{MPATH_SBIN_PATH}="/sbin"
TEST!="$env{MPATH_SBIN_PATH}/multipath", ENV{MPATH_SBIN_PATH}="/usr/sbin"
-SUBSYSTEM=="block", ACTION=="add|change", KERNEL=="sd*|dasd*", \
- ENV{DM_MULTIPATH_DEVICE_PATH}!="1", \
+ENV{DM_MULTIPATH_DEVICE_PATH}!="1", \
PROGRAM=="$env{MPATH_SBIN_PATH}/multipath -u %k", \
- ENV{DM_MULTIPATH_DEVICE_PATH}="1" \
- ENV{ID_FS_TYPE}="none" \
+ ENV{DM_MULTIPATH_DEVICE_PATH}="1", ENV{ID_FS_TYPE}="none", \
ENV{SYSTEMD_READY}="0"
LABEL="end_mpath"
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 06/10] change order of multipath.rules
2015-05-15 23:14 [PATCH 00/10] Multipath path resync Benjamin Marzinski
` (4 preceding siblings ...)
2015-05-15 23:14 ` [PATCH 05/10] update multipath rules to deal with partition devices Benjamin Marzinski
@ 2015-05-15 23:14 ` Benjamin Marzinski
2015-05-15 23:14 ` [PATCH 07/10] make kpartx -d remove all partitions Benjamin Marzinski
` (3 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Benjamin Marzinski @ 2015-05-15 23:14 UTC (permalink / raw)
To: device-mapper development; +Cc: Christophe Varoqui
At least for RedHat, the rule that calls scsi_id is
60-persistent-storage.rules, so the multipath rule needs to come
after this.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
multipath/Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/multipath/Makefile b/multipath/Makefile
index 7f18e9a..4b3de81 100644
--- a/multipath/Makefile
+++ b/multipath/Makefile
@@ -23,7 +23,7 @@ install:
$(INSTALL_PROGRAM) -m 755 $(EXEC) $(DESTDIR)$(bindir)/
$(INSTALL_PROGRAM) -d $(DESTDIR)$(udevrulesdir)
$(INSTALL_PROGRAM) -m 644 11-dm-mpath.rules $(DESTDIR)$(udevrulesdir)
- $(INSTALL_PROGRAM) -m 644 $(EXEC).rules $(DESTDIR)$(libudevdir)/rules.d/56-multipath.rules
+ $(INSTALL_PROGRAM) -m 644 $(EXEC).rules $(DESTDIR)$(libudevdir)/rules.d/62-multipath.rules
$(INSTALL_PROGRAM) -d $(DESTDIR)$(mandir)
$(INSTALL_PROGRAM) -m 644 $(EXEC).8.gz $(DESTDIR)$(mandir)
$(INSTALL_PROGRAM) -d $(DESTDIR)$(man5dir)
@@ -32,7 +32,7 @@ install:
uninstall:
rm $(DESTDIR)$(bindir)/$(EXEC)
rm $(DESTDIR)$(udevrulesdir)/11-dm-mpath.rules
- rm $(DESTDIR)$(libudevdir)/rules.d/56-multipath.rules
+ rm $(DESTDIR)$(libudevdir)/rules.d/62-multipath.rules
rm $(DESTDIR)$(mandir)/$(EXEC).8.gz
rm $(DESTDIR)$(man5dir)/$(EXEC).conf.5.gz
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 07/10] make kpartx -d remove all partitions
2015-05-15 23:14 [PATCH 00/10] Multipath path resync Benjamin Marzinski
` (5 preceding siblings ...)
2015-05-15 23:14 ` [PATCH 06/10] change order of multipath.rules Benjamin Marzinski
@ 2015-05-15 23:14 ` Benjamin Marzinski
2015-05-15 23:14 ` [PATCH 08/10] Fix issues with user_friendly_names initramfs bindings Benjamin Marzinski
` (2 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Benjamin Marzinski @ 2015-05-15 23:14 UTC (permalink / raw)
To: device-mapper development; +Cc: Christophe Varoqui
Currently kpartx -d only removes the partitions that have entries in the
partition table. If you remove a partition from the device, and then
run kpartx -d, it will not delete that partition (kpartx -u will). I don't
see any value in leaving partition devices for partitions that don't even
exist anymore, so this patch makes -d delete all partitions.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
kpartx/kpartx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
index d69f9af..a9d4c98 100644
--- a/kpartx/kpartx.c
+++ b/kpartx/kpartx.c
@@ -426,7 +426,7 @@ main(int argc, char **argv){
break;
case DELETE:
- for (j = n-1; j >= 0; j--) {
+ for (j = MAXSLICES-1; j >= 0; j--) {
if (safe_sprintf(partname, "%s%s%d",
mapname, delim, j+1)) {
fprintf(stderr, "partname too small\n");
@@ -434,7 +434,7 @@ main(int argc, char **argv){
}
strip_slash(partname);
- if (!slices[j].size || !dm_map_present(partname))
+ if (!dm_map_present(partname))
continue;
if (!dm_simplecmd(DM_DEVICE_REMOVE, partname,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 08/10] Fix issues with user_friendly_names initramfs bindings
2015-05-15 23:14 [PATCH 00/10] Multipath path resync Benjamin Marzinski
` (6 preceding siblings ...)
2015-05-15 23:14 ` [PATCH 07/10] make kpartx -d remove all partitions Benjamin Marzinski
@ 2015-05-15 23:14 ` Benjamin Marzinski
2015-05-15 23:14 ` [PATCH 09/10] Make multipath deactivate devices before iscsi shutdown Benjamin Marzinski
2015-05-15 23:14 ` [PATCH 10/10] resize reply buffer for mutipathd help message Benjamin Marzinski
9 siblings, 0 replies; 12+ messages in thread
From: Benjamin Marzinski @ 2015-05-15 23:14 UTC (permalink / raw)
To: device-mapper development; +Cc: Stewart, Sean, Christophe Varoqui
Multipath has an issue with user_friendly_names set in the initramfs.
If the bindings are in the initramfs bindings file, it will create them,
and it may use bindings that are different than the ones in the regular
file system. Once multipathd starts up in the regular file system, it
will try to register the existing bindings, but that may (and in many
cases, is likely to) fail. If it can't reanme it, will pick a new
binding. Since when multipathd starts discovering the existing devices,
it obviously doesn't know all of the existing devices yet, it may very
well pick a binding that's already in use by a device that it hasn't
discovered yet. In this case multipath will be unable to rename the
device to the new binding. Unfortunately, if it fails the rename, it
never resets the alias of the device stored in the mpvec to current
alias of the actual dm device. So multipath will have devices in the
mpvec where the alias and the wwid don't match the actual dm devices
that exist. This can cause all sorts of problems.
This patch does two things to deal with this. First, it makes sure that
if the rename fails, the device will reset its alias to the one that it
currently has.
Second, it adds a new option to help avoid the problem in the first
place. There actually already is a solution to this issue. If
multipathd is started with -B, it makes the bindings file read-only. If
multipathd is started this way in the initramfs, it will never make new
bindings for a device. If the device doesn't already have a binding, it
will simply be named with its wwid. The problem with this method is that
AFAICT it causes a maintenance problem with dracut. At least on RedHat,
dracut is simply copying the regular multipathd.service file into the
initramfs. I don't see a way in multipathd.service to only start
multipathd with the -B option in the initramfs (If there is a way, I'd
be happy to use that). So, the only alternative that lets us use -B is
to have a separate multipathd.service file that dracut uses for the
initramfs. This seems like more work than it's worth.
Instead, this patch pulls some code from systemd to detect if multipathd
is running in the initramfs. If it is, and if new_bindings_in_boot is
not set, multipathd will set the bindings file to read-only, just like
the -B option does.
Cc: "Stewart, Sean" <Sean.Stewart@netapp.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/config.c | 4 ++++
libmultipath/config.h | 1 +
libmultipath/configure.c | 3 +++
libmultipath/dict.c | 4 ++++
libmultipath/util.c | 30 ++++++++++++++++++++++++++++++
libmultipath/util.h | 1 +
6 files changed, 43 insertions(+)
diff --git a/libmultipath/config.c b/libmultipath/config.c
index cfcc685..f0aae53 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -621,6 +621,7 @@ load_config (char * file, struct udev *udev)
conf->uid_attribute = set_default(DEFAULT_UID_ATTRIBUTE);
conf->retrigger_tries = DEFAULT_RETRIGGER_TRIES;
conf->retrigger_delay = DEFAULT_RETRIGGER_DELAY;
+ conf->new_bindings_in_boot = 0;
/*
* preload default hwtable
@@ -735,6 +736,9 @@ load_config (char * file, struct udev *udev)
!conf->wwids_file)
goto out;
+ if (conf->new_bindings_in_boot == 0 && in_initrd())
+ conf->bindings_read_only = 1;
+
return 0;
out:
free_config(conf);
diff --git a/libmultipath/config.h b/libmultipath/config.h
index 372eace..05f1f6d 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -137,6 +137,7 @@ struct config {
int uxsock_timeout;
int retrigger_tries;
int retrigger_delay;
+ int new_bindings_in_boot;
unsigned int version[3];
char * dev;
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 24ad948..3559c01 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -421,6 +421,9 @@ select_action (struct multipath * mpp, vector curmp, int force_reload)
condlog(2, "%s: unable to rename %s to %s (%s is used by %s)",
mpp->wwid, cmpp->alias, mpp->alias,
mpp->alias, cmpp_by_name->wwid);
+ /* reset alias to existing alias */
+ FREE(mpp->alias);
+ mpp->alias = STRDUP(cmpp->alias);
mpp->action = ACT_NOTHING;
return;
}
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index e3bc67e..1952b1e 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -395,6 +395,9 @@ declare_def_snprint(retrigger_tries, print_int)
declare_def_handler(retrigger_delay, set_int)
declare_def_snprint(retrigger_delay, print_int)
+declare_def_handler(new_bindings_in_boot, set_yes_no)
+declare_def_snprint(new_bindings_in_boot, print_yes_no)
+
static int
def_config_dir_handler(vector strvec)
{
@@ -1373,6 +1376,7 @@ init_keywords(void)
install_keyword("uxsock_timeout", &def_uxsock_timeout_handler, &snprint_def_uxsock_timeout);
install_keyword("retrigger_tries", &def_retrigger_tries_handler, &snprint_def_retrigger_tries);
install_keyword("retrigger_delay", &def_retrigger_delay_handler, &snprint_def_retrigger_delay);
+ install_keyword("new_bindings_in_boot", &def_new_bindings_in_boot_handler, &snprint_def_new_bindings_in_boot);
__deprecated install_keyword("default_selector", &def_selector_handler, NULL);
__deprecated install_keyword("default_path_grouping_policy", &def_pgpolicy_handler, NULL);
__deprecated install_keyword("default_uid_attribute", &def_uid_attribute_handler, NULL);
diff --git a/libmultipath/util.c b/libmultipath/util.c
index ac0d1b2..fcab5fc 100644
--- a/libmultipath/util.c
+++ b/libmultipath/util.c
@@ -3,6 +3,8 @@
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
+#include <sys/vfs.h>
+#include <linux/magic.h>
#include "debug.h"
#include "memory.h"
@@ -258,3 +260,31 @@ dev_t parse_devt(const char *dev_t)
return makedev(maj, min);
}
+
+/* This define was taken from systemd. src/shared/macro.h */
+#define F_TYPE_EQUAL(a, b) (a == (typeof(a)) b)
+
+/* This function was taken from systemd. src/shared/util.c */
+int in_initrd(void) {
+ static int saved = -1;
+ struct statfs s;
+
+ if (saved >= 0)
+ return saved;
+
+ /* We make two checks here:
+ *
+ * 1. the flag file /etc/initrd-release must exist
+ * 2. the root file system must be a memory file system
+ * The second check is extra paranoia, since misdetecting an
+ * initrd can have bad bad consequences due the initrd
+ * emptying when transititioning to the main systemd.
+ */
+
+ saved = access("/etc/initrd-release", F_OK) >= 0 &&
+ statfs("/", &s) >= 0 &&
+ (F_TYPE_EQUAL(s.f_type, TMPFS_MAGIC) ||
+ F_TYPE_EQUAL(s.f_type, RAMFS_MAGIC));
+
+ return saved;
+}
diff --git a/libmultipath/util.h b/libmultipath/util.h
index 257912c..a1404f2 100644
--- a/libmultipath/util.h
+++ b/libmultipath/util.h
@@ -10,6 +10,7 @@ size_t strlcat(char *dst, const char *src, size_t size);
int devt2devname (char *, int, char *);
dev_t parse_devt(const char *dev_t);
char *convert_dev(char *dev, int is_path_device);
+int in_initrd(void);
#define safe_sprintf(var, format, args...) \
snprintf(var, sizeof(var), format, ##args) >= sizeof(var)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 09/10] Make multipath deactivate devices before iscsi shutdown
2015-05-15 23:14 [PATCH 00/10] Multipath path resync Benjamin Marzinski
` (7 preceding siblings ...)
2015-05-15 23:14 ` [PATCH 08/10] Fix issues with user_friendly_names initramfs bindings Benjamin Marzinski
@ 2015-05-15 23:14 ` Benjamin Marzinski
2015-05-15 23:14 ` [PATCH 10/10] resize reply buffer for mutipathd help message Benjamin Marzinski
9 siblings, 0 replies; 12+ messages in thread
From: Benjamin Marzinski @ 2015-05-15 23:14 UTC (permalink / raw)
To: device-mapper development; +Cc: Christophe Varoqui
The multipath devices need to be deactivated before iscsi is shutdown,
otherwise the multipath devices will generate a lot of error messages
when it loses its iscsi path devices.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
multipathd/multipathd.service | 1 +
1 file changed, 1 insertion(+)
diff --git a/multipathd/multipathd.service b/multipathd/multipathd.service
index b5b755b..fc2e009 100644
--- a/multipathd/multipathd.service
+++ b/multipathd/multipathd.service
@@ -1,5 +1,6 @@
[Unit]
Description=Device-Mapper Multipath Device Controller
+Requires=blk-availability.service
Before=iscsi.service iscsid.service lvm2-activation-early.service
Before=local-fs-pre.target
After=multipathd.socket
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 10/10] resize reply buffer for mutipathd help message
2015-05-15 23:14 [PATCH 00/10] Multipath path resync Benjamin Marzinski
` (8 preceding siblings ...)
2015-05-15 23:14 ` [PATCH 09/10] Make multipath deactivate devices before iscsi shutdown Benjamin Marzinski
@ 2015-05-15 23:14 ` Benjamin Marzinski
9 siblings, 0 replies; 12+ messages in thread
From: Benjamin Marzinski @ 2015-05-15 23:14 UTC (permalink / raw)
To: device-mapper development; +Cc: Christophe Varoqui
Unlike the other reply messages from multipathd, the generic help
message code couldn't resize the buffer, and the reply had grown too big
for the initial buffer size. This was causing memory corruption and
crashing multipathd instead of printing a help message. This patch
increases the size of the initial reply buffer, and makes the help
message code able to resize the buffer so this doesn't happen in the
future.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
multipathd/cli.c | 88 +++++++++++++++++++++++++++++++++--------------
multipathd/cli.h | 16 ++++++++-
multipathd/cli_handlers.c | 14 --------
3 files changed, 78 insertions(+), 40 deletions(-)
diff --git a/multipathd/cli.c b/multipathd/cli.c
index acc4249..8d26956 100644
--- a/multipathd/cli.c
+++ b/multipathd/cli.c
@@ -320,52 +320,90 @@ alloc_handlers (void)
}
static int
-genhelp_sprint_aliases (char * reply, vector keys, struct key * refkw)
+genhelp_sprint_aliases (char * reply, int maxlen, vector keys,
+ struct key * refkw)
{
- int i, fwd = 0;
+ int i, len = 0;
struct key * kw;
- vector_foreach_slot (keys, kw, i)
- if (kw->code == refkw->code && kw != refkw)
- fwd += sprintf(reply, "|%s", kw->str);
+ vector_foreach_slot (keys, kw, i) {
+ if (kw->code == refkw->code && kw != refkw) {
+ len += snprintf(reply + len, maxlen - len,
+ "|%s", kw->str);
+ if (len >= maxlen)
+ return len;
+ }
+ }
- return fwd;
+ return len;
}
-static char *
-genhelp_handler (void)
-{
+static int
+do_genhelp(char *reply, int maxlen) {
+ int len = 0;
int i, j;
unsigned long fp;
struct handler * h;
struct key * kw;
- char * reply;
- char * p;
-
- reply = MALLOC(INITIAL_REPLY_LEN);
- if (!reply)
- return NULL;
-
- p = reply;
- p += sprintf(p, VERSION_STRING);
- p += sprintf(p, "CLI commands reference:\n");
+ len += snprintf(reply + len, maxlen - len, VERSION_STRING);
+ if (len >= maxlen)
+ goto out;
+ len += snprintf(reply + len, maxlen - len, "CLI commands reference:\n");
+ if (len >= maxlen)
+ goto out;
vector_foreach_slot (handlers, h, i) {
fp = h->fingerprint;
vector_foreach_slot (keys, kw, j) {
if ((kw->code & fp)) {
fp -= kw->code;
- p += sprintf(p, " %s", kw->str);
- p += genhelp_sprint_aliases(p, keys, kw);
-
- if (kw->has_param)
- p += sprintf(p, " $%s", kw->str);
+ len += snprintf(reply + len , maxlen - len,
+ " %s", kw->str);
+ if (len >= maxlen)
+ goto out;
+ len += genhelp_sprint_aliases(reply + len,
+ maxlen - len,
+ keys, kw);
+ if (len >= maxlen)
+ goto out;
+
+ if (kw->has_param) {
+ len += snprintf(reply + len,
+ maxlen - len,
+ " $%s", kw->str);
+ if (len >= maxlen)
+ goto out;
+ }
}
}
- p += sprintf(p, "\n");
+ len += snprintf(reply + len, maxlen - len, "\n");
+ if (len >= maxlen)
+ goto out;
}
+out:
+ return len;
+}
+
+static char *
+genhelp_handler (void)
+{
+ char * reply;
+ char * p = NULL;
+ int maxlen = INITIAL_REPLY_LEN;
+ int again = 1;
+
+ reply = MALLOC(maxlen);
+
+ while (again) {
+ if (!reply)
+ return NULL;
+ p = reply;
+ p += do_genhelp(reply, maxlen);
+ again = ((p - reply) >= maxlen);
+ REALLOC_REPLY(reply, again, maxlen);
+ }
return reply;
}
diff --git a/multipathd/cli.h b/multipathd/cli.h
index 09fdc68..2e0e1da 100644
--- a/multipathd/cli.h
+++ b/multipathd/cli.h
@@ -71,7 +71,21 @@ enum {
#define SETPRSTATUS (1UL << __SETPRSTATUS)
#define UNSETPRSTATUS (1UL << __UNSETPRSTATUS)
-#define INITIAL_REPLY_LEN 1100
+#define INITIAL_REPLY_LEN 1200
+
+#define REALLOC_REPLY(r, a, m) \
+ do { \
+ if ((a)) { \
+ char *tmp = (r); \
+ (r) = REALLOC((r), (m) * 2); \
+ if ((r)) { \
+ memset((r) + (m), 0, (m)); \
+ (m) *= 2; \
+ } \
+ else \
+ free(tmp); \
+ } \
+ } while (0)
struct key {
char * str;
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 305cd7f..8cde810 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -23,20 +23,6 @@
#include "cli.h"
#include "uevent.h"
-#define REALLOC_REPLY(r, a, m) \
- do { \
- if ((a)) { \
- char *tmp = (r); \
- (r) = REALLOC((r), (m) * 2); \
- if ((r)) { \
- memset((r) + (m), 0, (m)); \
- (m) *= 2; \
- } \
- else \
- free(tmp); \
- } \
- } while (0)
-
int
show_paths (char ** r, int * len, struct vectors * vecs, char * style)
{
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread