[PATCH 0/4] misc multipath patches

[PATCH 0/4] misc multipath patches

[Benjamin Marzinski]

These are a couple of disconnected multipath patches.

Benjamin Marzinski (4):
  mpathpersist: remove broken/unused code
  libmultipath: EMC PowerMax NVMe device config
  mpathpersist: fix leaks
  libmultipath: fix mpcontext initialization

 libmpathpersist/mpath_persist.c      | 16 ++++----------
 libmultipath/checkers.c              | 29 ++++++++++++++++++++++++--
 libmultipath/checkers.h              |  1 +
 libmultipath/checkers/cciss_tur.c    |  5 +++++
 libmultipath/checkers/directio.c     |  5 +++++
 libmultipath/checkers/emc_clariion.c |  7 +++++++
 libmultipath/checkers/hp_sw.c        |  5 +++++
 libmultipath/checkers/rdac.c         |  5 +++++
 libmultipath/checkers/readsector0.c  |  5 +++++
 libmultipath/checkers/tur.c          |  5 +++++
 libmultipath/discovery.c             |  2 ++
 libmultipath/hwtable.c               |  6 ++++++
 mpathpersist/main.c                  | 31 ++++++++++++++++++----------
 13 files changed, 97 insertions(+), 25 deletions(-)

-- 
2.17.2

[PATCH 1/4] mpathpersist: remove broken/unused code

[Benjamin Marzinski]

The test for an empty pp->dev in updatepaths() dates back to when
disassemble_map() didn't fill in pp->dev for newly added paths, and it
was meant to catch paths that got added by disassemble_map().  With the
mpathpersist speedup code, all paths get added by disassemble_map().
However, disassemble_map() now calls devt2devname() to set pp->dev if
possible.  This means that there is no point in calling devt2devname()
again in updatepaths(). If for some reason it did return success, the
current code would still fail, since it doesn't set pp->udev in this
code path. The best thing to do if disassemble_map() couldn't set
pp->dev is simply to fail the path.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmpathpersist/mpath_persist.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index 53022f5b..603cfc3b 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -82,18 +82,10 @@ updatepaths (struct multipath * mpp)
 
 		vector_foreach_slot (pgp->paths, pp, j){
 			if (!strlen(pp->dev)){
-				if (devt2devname(pp->dev, FILE_NAME_SIZE,
-						 pp->dev_t)){
-					/*
-					 * path is not in sysfs anymore
-					 */
-					pp->state = PATH_DOWN;
-					continue;
-				}
-				pp->mpp = mpp;
-				conf = get_multipath_config();
-				pathinfo(pp, conf, DI_ALL);
-				put_multipath_config(conf);
+				/*
+				 * path is not in sysfs anymore
+				 */
+				pp->state = PATH_DOWN;
 				continue;
 			}
 			pp->mpp = mpp;
-- 
2.17.2

[PATCH 2/4] libmultipath: EMC PowerMax NVMe device config

[Benjamin Marzinski]

Got this config from Dell.

Cc: heyi <yi.he@dell.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/hwtable.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
index 96e8b25d..ca217e65 100644
--- a/libmultipath/hwtable.c
+++ b/libmultipath/hwtable.c
@@ -360,6 +360,12 @@ static struct hwentry default_hw[] = {
 		.pgfailback    = -FAILBACK_IMMEDIATE,
 		.no_path_retry = 30,
 	},
+	{
+		/* EMC PowerMax NVMe */
+		.vendor        = "NVME",
+		.product       = "^EMC PowerMax_",
+		.pgpolicy      = MULTIBUS,
+	},
 	/*
 	 * Fujitsu
 	 */
-- 
2.17.2

[PATCH 3/4] mpathpersist: fix leaks

[Benjamin Marzinski]

If handle_args() fails while looping through the argument list, it needs
to free batch_fn, if it has been set. Also handle_args() needs to make
sure to free the file descriptor after it has been opened.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 mpathpersist/main.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/mpathpersist/main.c b/mpathpersist/main.c
index 5ad06a97..2368b429 100644
--- a/mpathpersist/main.c
+++ b/mpathpersist/main.c
@@ -155,7 +155,8 @@ static int do_batch_file(const char *batch_fn)
 
 static int handle_args(int argc, char * argv[], int nline)
 {
-	int fd, c;
+	int c;
+	int fd = -1;
 	const char *device_name = NULL;
 	int num_prin_sa = 0;
 	int num_prout_sa = 0;
@@ -213,7 +214,8 @@ static int handle_args(int argc, char * argv[], int nline)
 				if (nline == 0 && 1 != sscanf (optarg, "%d", &loglevel))
 				{
 					fprintf (stderr, "bad argument to '--verbose'\n");
-					return MPATH_PR_SYNTAX_ERROR;
+					ret = MPATH_PR_SYNTAX_ERROR;
+					goto out;
 				}
 				break;
 
@@ -228,6 +230,7 @@ static int handle_args(int argc, char * argv[], int nline)
 
 			case 'h':
 				usage ();
+				free(batch_fn);
 				return 0;
 
 			case 'H':
@@ -254,7 +257,8 @@ static int handle_args(int argc, char * argv[], int nline)
 				if (1 != sscanf (optarg, "%" SCNx64 "", &param_rk))
 				{
 					fprintf (stderr, "bad argument to '--param-rk'\n");
-					return MPATH_PR_SYNTAX_ERROR;
+					ret = MPATH_PR_SYNTAX_ERROR;
+					goto out;
 				}
 				++num_prout_param;
 				break;
@@ -263,7 +267,8 @@ static int handle_args(int argc, char * argv[], int nline)
 				if (1 != sscanf (optarg, "%" SCNx64 "", &param_sark))
 				{
 					fprintf (stderr, "bad argument to '--param-sark'\n");
-					return MPATH_PR_SYNTAX_ERROR;
+					ret = MPATH_PR_SYNTAX_ERROR;
+					goto out;
 				}
 				++num_prout_param;
 				break;
@@ -282,7 +287,8 @@ static int handle_args(int argc, char * argv[], int nline)
 				if (1 != sscanf (optarg, "%x", &prout_type))
 				{
 					fprintf (stderr, "bad argument to '--prout-type'\n");
-					return MPATH_PR_SYNTAX_ERROR;
+					ret = MPATH_PR_SYNTAX_ERROR;
+					goto out;
 				}
 				++num_prout_param;
 				break;
@@ -330,7 +336,8 @@ static int handle_args(int argc, char * argv[], int nline)
 			case 'X':
 				if (0 != construct_transportid(optarg, transportids, num_transport)) {
 					fprintf(stderr, "bad argument to '--transport-id'\n");
-					return MPATH_PR_SYNTAX_ERROR;
+					ret = MPATH_PR_SYNTAX_ERROR;
+					goto out;
 				}
 
 				++num_transport;
@@ -339,11 +346,13 @@ static int handle_args(int argc, char * argv[], int nline)
 			case 'l':
 				if (1 != sscanf(optarg, "%u", &mpath_mx_alloc_len)) {
 					fprintf(stderr, "bad argument to '--alloc-length'\n");
-					return MPATH_PR_SYNTAX_ERROR;
+					ret = MPATH_PR_SYNTAX_ERROR;
+					goto out;
 				} else if (MPATH_MAX_PARAM_LEN < mpath_mx_alloc_len) {
 					fprintf(stderr, "'--alloc-length' argument exceeds maximum"
 							" limit(%d)\n", MPATH_MAX_PARAM_LEN);
-					return MPATH_PR_SYNTAX_ERROR;
+					ret = MPATH_PR_SYNTAX_ERROR;
+					goto out;
 				}
 				break;
 
@@ -481,14 +490,14 @@ static int handle_args(int argc, char * argv[], int nline)
 		{
 			fprintf (stderr, "failed to allocate PRIN response buffer\n");
 			ret = MPATH_PR_OTHER;
-			goto out;
+			goto out_fd;
 		}
 
 		ret = __mpath_persistent_reserve_in (fd, prin_sa, resp, noisy);
 		if (ret != MPATH_PR_SUCCESS )
 		{
 			fprintf (stderr, "Persistent Reserve IN command failed\n");
-			goto out;
+			goto out_fd;
 		}
 
 		switch(prin_sa)
@@ -568,8 +577,8 @@ static int handle_args(int argc, char * argv[], int nline)
 		printf("PR out: command failed\n");
 	}
 
+out_fd:
 	close (fd);
-
 out :
 	if (ret == MPATH_PR_SYNTAX_ERROR) {
 		free(batch_fn);
-- 
2.17.2

[PATCH 4/4] libmultipath: fix mpcontext initialization

[Benjamin Marzinski]

If a path is discovered before there is a multipath device for it to
belong to, the checker will not have its mpcontext initialized, even if
that path later belongs to a multipath device. A checker's mpcontext is
only set when the checker is selected, and is set to NULL if there is no
multipath device associated with the path. This only impacts the emc
checker. However, it makes the emc checker unable to determine if a
passive path is connected to an inactive snapshot or not.

This can be solved by adding a new checker class function, mp_init().
This is called when the checker is first initialized, and whenever the
checker is called, if the checker's mpcontext hasn't been initialized.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/checkers.c              | 29 ++++++++++++++++++++++++++--
 libmultipath/checkers.h              |  1 +
 libmultipath/checkers/cciss_tur.c    |  5 +++++
 libmultipath/checkers/directio.c     |  5 +++++
 libmultipath/checkers/emc_clariion.c |  7 +++++++
 libmultipath/checkers/hp_sw.c        |  5 +++++
 libmultipath/checkers/rdac.c         |  5 +++++
 libmultipath/checkers/readsector0.c  |  5 +++++
 libmultipath/checkers/tur.c          |  5 +++++
 libmultipath/discovery.c             |  2 ++
 10 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
index f4fdcae9..240b0f29 100644
--- a/libmultipath/checkers.c
+++ b/libmultipath/checkers.c
@@ -16,6 +16,7 @@ struct checker_class {
 	char name[CHECKER_NAME_LEN];
 	int (*check)(struct checker *);
 	int (*init)(struct checker *);       /* to allocate the context */
+	int (*mp_init)(struct checker *);    /* to allocate the mpcontext */
 	void (*free)(struct checker *);      /* to free the context */
 	const char **msgtable;
 	short msgtable_size;
@@ -140,6 +141,13 @@ static struct checker_class *add_checker_class(const char *multipath_dir,
 	if (!c->init)
 		goto out;
 
+	c->mp_init = (int (*)(struct checker *)) dlsym(c->handle, "libcheck_mp_init");
+	errstr = dlerror();
+	if (errstr != NULL)
+		condlog(0, "A dynamic linking error occurred: (%s)", errstr);
+	if (!c->mp_init)
+		goto out;
+
 	c->free = (void (*)(struct checker *)) dlsym(c->handle, "libcheck_free");
 	errstr = dlerror();
 	if (errstr != NULL)
@@ -212,8 +220,25 @@ int checker_init (struct checker * c, void ** mpctxt_addr)
 	if (!c || !c->cls)
 		return 1;
 	c->mpcontext = mpctxt_addr;
-	if (c->cls->init)
-		return c->cls->init(c);
+	if (c->cls->init && c->cls->init(c) != 0)
+		return 1;
+	if (mpctxt_addr && *mpctxt_addr == NULL && c->cls->mp_init &&
+	    c->cls->mp_init(c) != 0) /* continue even if mp_init fails */
+		c->mpcontext = NULL;
+	return 0;
+}
+
+int checker_mp_init(struct checker * c, void ** mpctxt_addr)
+{
+	if (!c || !c->cls)
+		return 1;
+	if (c->cls->mp_init && !c->mpcontext && mpctxt_addr) {
+		c->mpcontext = mpctxt_addr;
+		if (c->cls->mp_init(c) != 0) {
+			c->mpcontext = NULL;
+			return 1;
+		}
+	}
 	return 0;
 }
 
diff --git a/libmultipath/checkers.h b/libmultipath/checkers.h
index dab197f9..5237e7ec 100644
--- a/libmultipath/checkers.h
+++ b/libmultipath/checkers.h
@@ -138,6 +138,7 @@ const char *checker_state_name(int);
 int init_checkers(const char *);
 void cleanup_checkers (void);
 int checker_init (struct checker *, void **);
+int checker_mp_init(struct checker *, void **);
 void checker_clear (struct checker *);
 void checker_put (struct checker *);
 void checker_reset (struct checker *);
diff --git a/libmultipath/checkers/cciss_tur.c b/libmultipath/checkers/cciss_tur.c
index ea843742..b570ed65 100644
--- a/libmultipath/checkers/cciss_tur.c
+++ b/libmultipath/checkers/cciss_tur.c
@@ -51,6 +51,11 @@ int libcheck_init (struct checker * c)
 	return 0;
 }
 
+int libcheck_mp_init (struct checker * c)
+{
+	return 0;
+}
+
 void libcheck_free (struct checker * c)
 {
 	return;
diff --git a/libmultipath/checkers/directio.c b/libmultipath/checkers/directio.c
index 1b00b775..96f223b2 100644
--- a/libmultipath/checkers/directio.c
+++ b/libmultipath/checkers/directio.c
@@ -103,6 +103,11 @@ out:
 	return 1;
 }
 
+int libcheck_mp_init(struct checker * c)
+{
+	return 0;
+}
+
 void libcheck_free (struct checker * c)
 {
 	struct directio_context * ct = (struct directio_context *)c->context;
diff --git a/libmultipath/checkers/emc_clariion.c b/libmultipath/checkers/emc_clariion.c
index 6fc89113..5cd63aca 100644
--- a/libmultipath/checkers/emc_clariion.c
+++ b/libmultipath/checkers/emc_clariion.c
@@ -107,11 +107,18 @@ int libcheck_init (struct checker * c)
 		return 1;
 	((struct emc_clariion_checker_path_context *)c->context)->wwn_set = 0;
 
+	return 0;
+}
+
+int libcheck_mp_init (struct checker * c)
+{
 	/*
 	 * Allocate and initialize the multi-path global context.
 	 */
 	if (c->mpcontext && *c->mpcontext == NULL) {
 		void * mpctxt = malloc(sizeof(int));
+		if (!mpctxt)
+			return 1;
 		*c->mpcontext = mpctxt;
 		CLR_INACTIVE_SNAP(c);
 	}
diff --git a/libmultipath/checkers/hp_sw.c b/libmultipath/checkers/hp_sw.c
index 1a820223..35aca204 100644
--- a/libmultipath/checkers/hp_sw.c
+++ b/libmultipath/checkers/hp_sw.c
@@ -37,6 +37,11 @@ int libcheck_init (struct checker * c)
 	return 0;
 }
 
+int libcheck_mp_init(struct checker * c)
+{
+	return 0;
+}
+
 void libcheck_free (struct checker * c)
 {
 	return;
diff --git a/libmultipath/checkers/rdac.c b/libmultipath/checkers/rdac.c
index 8a3b73ec..805d153e 100644
--- a/libmultipath/checkers/rdac.c
+++ b/libmultipath/checkers/rdac.c
@@ -133,6 +133,11 @@ out:
 	return 0;
 }
 
+int libcheck_mp_init(struct checker * c)
+{
+	return 0;
+}
+
 void libcheck_free (struct checker * c)
 {
 	return;
diff --git a/libmultipath/checkers/readsector0.c b/libmultipath/checkers/readsector0.c
index cf79e067..71db9f80 100644
--- a/libmultipath/checkers/readsector0.c
+++ b/libmultipath/checkers/readsector0.c
@@ -15,6 +15,11 @@ int libcheck_init (struct checker * c)
 	return 0;
 }
 
+int libcheck_mp_init(struct checker * c)
+{
+	return 0;
+}
+
 void libcheck_free (struct checker * c)
 {
 	return;
diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c
index 6b08dbbb..138b9e58 100644
--- a/libmultipath/checkers/tur.c
+++ b/libmultipath/checkers/tur.c
@@ -79,6 +79,11 @@ int libcheck_init (struct checker * c)
 	return 0;
 }
 
+int libcheck_mp_init(struct checker * c)
+{
+	return 0;
+}
+
 static void cleanup_context(struct tur_checker_context *ct)
 {
 	pthread_mutex_destroy(&ct->lock);
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index acca466c..72f455e8 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1608,6 +1608,8 @@ get_state (struct path * pp, struct config *conf, int daemon, int oldstate)
 			return PATH_UNCHECKED;
 		}
 	}
+	if (pp->mpp && !c->mpcontext)
+		checker_mp_init(c, &pp->mpp->mpcontext);
 	checker_clear_message(c);
 	if (daemon) {
 		if (conf->force_sync == 0)
-- 
2.17.2

Re: [PATCH 1/4] mpathpersist: remove broken/unused code

[Martin Wilck]

On Tue, 2019-09-10 at 17:39 -0500, Benjamin Marzinski wrote:
> The test for an empty pp->dev in updatepaths() dates back to when
> disassemble_map() didn't fill in pp->dev for newly added paths, and
> it
> was meant to catch paths that got added by disassemble_map().  With
> the
> mpathpersist speedup code, all paths get added by disassemble_map().
> However, disassemble_map() now calls devt2devname() to set pp->dev if
> possible.  This means that there is no point in calling
> devt2devname()
> again in updatepaths(). If for some reason it did return success, the
> current code would still fail, since it doesn't set pp->udev in this
> code path. The best thing to do if disassemble_map() couldn't set
> pp->dev is simply to fail the path.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmpathpersist/mpath_persist.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)

Reviewed-by: Martin Wilck <mwilck@suse.com>

Re: [PATCH 2/4] libmultipath: EMC PowerMax NVMe device config

[Martin Wilck]

On Tue, 2019-09-10 at 17:39 -0500, Benjamin Marzinski wrote:
> Got this config from Dell.
> 
> Cc: heyi <yi.he@dell.com>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/hwtable.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Reviewed-by: Martin Wilck <mwilck@suse.com>

Re: [PATCH 3/4] mpathpersist: fix leaks

[Martin Wilck]

On Tue, 2019-09-10 at 17:39 -0500, Benjamin Marzinski wrote:
> If handle_args() fails while looping through the argument list, it
> needs
> to free batch_fn, if it has been set. Also handle_args() needs to
> make
> sure to free the file descriptor after it has been opened.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  mpathpersist/main.c | 31 ++++++++++++++++++++-----------
>  1 file changed, 20 insertions(+), 11 deletions(-)
> 


Looking at this code again, I wonder why we don't "goto out" in this
code in the "else if (prin_flag)" case:

		if (0 == num_prin_sa)
		{
			fprintf (stderr,
					" No service action given for Persistent Reserve IN\n");
			ret = MPATH_PR_SYNTAX_ERROR;
		}
		else if (num_prin_sa > 1)
		{
			fprintf (stderr, " Too many service actions given; choose "
					"one only\n");
			ret = MPATH_PR_SYNTAX_ERROR;
		}

At least in the first case, prin_sa would be -1, and trying to continue
looks unhealthy. In the second case, the error message might be treated
as a warning and the second prin action would override the first. I
personally would think that it would be better to assume the user made
a mistake, and do nothing, in this case as well.

Anyway, that's not your patch's fault. So:

Reviewed-by: Martin Wilck <mwilck@suse.com>

Re: [PATCH 4/4] libmultipath: fix mpcontext initialization

[Martin Wilck]

On Tue, 2019-09-10 at 17:39 -0500, Benjamin Marzinski wrote:
> If a path is discovered before there is a multipath device for it to
> belong to, the checker will not have its mpcontext initialized, even
> if
> that path later belongs to a multipath device. A checker's mpcontext
> is
> only set when the checker is selected, and is set to NULL if there is
> no
> multipath device associated with the path. This only impacts the emc
> checker. However, it makes the emc checker unable to determine if a
> passive path is connected to an inactive snapshot or not.
> 
> This can be solved by adding a new checker class function, mp_init().
> This is called when the checker is first initialized, and whenever
> the
> checker is called, if the checker's mpcontext hasn't been
> initialized.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/checkers.c              | 29
> ++++++++++++++++++++++++++--
>  libmultipath/checkers.h              |  1 +
>  libmultipath/checkers/cciss_tur.c    |  5 +++++
>  libmultipath/checkers/directio.c     |  5 +++++
>  libmultipath/checkers/emc_clariion.c |  7 +++++++
>  libmultipath/checkers/hp_sw.c        |  5 +++++
>  libmultipath/checkers/rdac.c         |  5 +++++
>  libmultipath/checkers/readsector0.c  |  5 +++++
>  libmultipath/checkers/tur.c          |  5 +++++
>  libmultipath/discovery.c             |  2 ++
>  10 files changed, 67 insertions(+), 2 deletions(-)
> 

I see the problem and agree the solution is correct. But I'd prefer if
the mp_init() method was optional and, if not defined in the
libcheckX.so, would simply be set to NULL. So you wouldn't need to
define empty methods for all checkers except emc. 

checker_mp_init() checks for mp_init() being non-NULL anyway.

Thanks,
Martin

Re: [PATCH 3/4] mpathpersist: fix leaks

[Benjamin Marzinski]

On Fri, Sep 13, 2019 at 07:56:13AM +0000, Martin Wilck wrote:
> On Tue, 2019-09-10 at 17:39 -0500, Benjamin Marzinski wrote:
> > If handle_args() fails while looping through the argument list, it
> > needs
> > to free batch_fn, if it has been set. Also handle_args() needs to
> > make
> > sure to free the file descriptor after it has been opened.
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > ---
> >  mpathpersist/main.c | 31 ++++++++++++++++++++-----------
> >  1 file changed, 20 insertions(+), 11 deletions(-)
> > 
> 
> 
> Looking at this code again, I wonder why we don't "goto out" in this
> code in the "else if (prin_flag)" case:
> 
> 		if (0 == num_prin_sa)
> 		{
> 			fprintf (stderr,
> 					" No service action given for Persistent Reserve IN\n");
> 			ret = MPATH_PR_SYNTAX_ERROR;
> 		}
> 		else if (num_prin_sa > 1)
> 		{
> 			fprintf (stderr, " Too many service actions given; choose "
> 					"one only\n");
> 			ret = MPATH_PR_SYNTAX_ERROR;
> 		}
> 
> At least in the first case, prin_sa would be -1, and trying to continue
> looks unhealthy. In the second case, the error message might be treated
> as a warning and the second prin action would override the first. I
> personally would think that it would be better to assume the user made
> a mistake, and do nothing, in this case as well.

I agree. I'll respin the patches with this included.

-Ben

> Anyway, that's not your patch's fault. So:
> 
> Reviewed-by: Martin Wilck <mwilck@suse.com>
> 
> 

Re: [PATCH 4/4] libmultipath: fix mpcontext initialization

[Benjamin Marzinski]

On Fri, Sep 13, 2019 at 08:28:59AM +0000, Martin Wilck wrote:
> On Tue, 2019-09-10 at 17:39 -0500, Benjamin Marzinski wrote:
> > If a path is discovered before there is a multipath device for it to
> > belong to, the checker will not have its mpcontext initialized, even
> > if
> > that path later belongs to a multipath device. A checker's mpcontext
> > is
> > only set when the checker is selected, and is set to NULL if there is
> > no
> > multipath device associated with the path. This only impacts the emc
> > checker. However, it makes the emc checker unable to determine if a
> > passive path is connected to an inactive snapshot or not.
> > 
> > This can be solved by adding a new checker class function, mp_init().
> > This is called when the checker is first initialized, and whenever
> > the
> > checker is called, if the checker's mpcontext hasn't been
> > initialized.
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > ---
> >  libmultipath/checkers.c              | 29
> > ++++++++++++++++++++++++++--
> >  libmultipath/checkers.h              |  1 +
> >  libmultipath/checkers/cciss_tur.c    |  5 +++++
> >  libmultipath/checkers/directio.c     |  5 +++++
> >  libmultipath/checkers/emc_clariion.c |  7 +++++++
> >  libmultipath/checkers/hp_sw.c        |  5 +++++
> >  libmultipath/checkers/rdac.c         |  5 +++++
> >  libmultipath/checkers/readsector0.c  |  5 +++++
> >  libmultipath/checkers/tur.c          |  5 +++++
> >  libmultipath/discovery.c             |  2 ++
> >  10 files changed, 67 insertions(+), 2 deletions(-)
> > 
> 
> I see the problem and agree the solution is correct. But I'd prefer if
> the mp_init() method was optional and, if not defined in the
> libcheckX.so, would simply be set to NULL. So you wouldn't need to
> define empty methods for all checkers except emc. 

Sure.

-Ben

> checker_mp_init() checks for mp_init() being non-NULL anyway.
> 
> Thanks,
> Martin
>