[dm-devel] [PATCH 0/6] multipath-tools history bug in 0.4.9

[dm-devel] [PATCH 0/6] multipath-tools history bug in 0.4.9

[lixiaokeng]

Hi,

There are some old patches in our multipath-tools code. They fix some
bugs in old version (0.4.9). I'm not the auther of these patches,
but I send them. Please review.Thanks.

lixiaokeng (3):
  fix multipathd resize when not all sizes of paths are equal
  fix lun expansion failure when there is offline path
  flush and sync before reboot

sunguoshuai (3):
  check pgp to avoid NULL dereference in enable_group
  fix change reservtion key to uint8 for memcmp
  ignore for clear mismatch key

 libmpathpersist/mpath_persist.c  | 27 ++++++++++++++++++++-------
 libmultipath/configure.c         | 18 ++++++++++++++++++
 libmultipath/prioritizers/alua.c |  4 ++++
 libmultipath/prioritizers/emc.c  |  2 +-
 libmultipath/prioritizers/hds.c  | 19 +++++++++++--------
 libmultipath/structs.c           |  1 +
 libmultipath/structs.h           |  1 +
 libmultipath/structs_vec.c       | 10 +++++++---
 libmultipath/util.c              |  7 +++++++
 multipathd/cli_handlers.c        | 19 ++++++++++++++++++-
 multipathd/main.c                |  5 +++++
 11 files changed, 93 insertions(+), 20 deletions(-)

-- 

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

[dm-devel] [PATCH 1/6] fix multipathd resize when not all sizes of paths are equal

[lixiaokeng]

In history, the "multipathd resize" may fail without failed
messages because of there are different sizes paths in mpp.
Here we check every path size in cli_resize, fail and print
messages if there are some different size paths. This patch
was made in 12 Oct 2017.

Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
---
 multipathd/cli_handlers.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 235e2a2e..18b51875 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -981,9 +981,10 @@ cli_resize(void *v, char **reply, int *len, void *data)
 	char * mapname = get_keyparam(v, MAP);
 	struct multipath *mpp;
 	int minor;
-	unsigned long long size;
+	unsigned long long size, sz;
 	struct pathgroup *pgp;
 	struct path *pp;
+	int i,j;

 	mapname = convert_dev(mapname, 0);
 	condlog(2, "%s: resize map (operator)", mapname);
@@ -1021,6 +1022,22 @@ cli_resize(void *v, char **reply, int *len, void *data)
 			mapname);
 		return 1;
 	}
+
+	vector_foreach_slot (mpp->pg, pgp, i) {
+		vector_foreach_slot (pgp->paths, pp, j) {
+			if (!pp->udev || sysfs_get_size(pp, &sz)) {
+				condlog(0, "%s: couldn't get size for sysfs. cannot resize",
+					mapname);
+				return 1;
+			}
+			if (size != sz) {
+				condlog(0, "%s: not all path sizes are equal, please check. cannot resize",
+					mapname);
+				return 1;
+			}
+		}
+	}
+
 	if (size == mpp->size) {
 		condlog(0, "%s: map is still the same size (%llu)", mapname,
 			mpp->size);
-- 

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

[dm-devel] [PATCH 2/6] fix lun expansion failure when there is offline path

[lixiaokeng]

There are two paths for one multipath device; one path
is up and the other path is down. Expand the multipath
device in server. Scan the path size in client. It will
cause failure expansion in client. The reason:
When two different-size paths for one device pass to
kernel. Kerenl will return fail. Multipathd will try to
delete the multipath device but fail because the
device is used. Here we check the offline path; if its'
size is different with other path, delete it.

Signed-off-by: Yanfei Zhang <yanfei.zhang@huawei.com>
Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
---
 libmultipath/configure.c   | 18 ++++++++++++++++++
 libmultipath/structs.c     |  1 +
 libmultipath/structs.h     |  1 +
 libmultipath/structs_vec.c |  6 ++++--
 4 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 9d6eeba1..00c12929 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -1176,6 +1176,7 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid,

 	vector_foreach_slot (pathvec, pp1, k) {
 		int invalid;
+		condlog(3, "%s %s: start coalesce", pp1->dev, pp1->dev_t);
 		/* skip this path for some reason */

 		/* 1. if path has no unique id or wwid blacklisted */
@@ -1213,6 +1214,18 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid,
 			continue;
 		}

+		/* if path is offline */
+		if (pp1->state == PATH_DOWN) {
+			orphan_path(pp1, "skip offline path");
+			continue;
+		}
+
+		/* if path is handled before */
+		if (pp1->handled) {
+			condlog(3, "%s: skip handled path.", pp1->dev_t);
+			continue;
+		}
+
 		/*
 		 * at this point, we know we really got a new mp
 		 */
@@ -1230,10 +1243,15 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid,

 		for (i = k + 1; i < VECTOR_SIZE(pathvec); i++) {
 			pp2 = VECTOR_SLOT(pathvec, i);
+			if (pp2->handled)
+				continue;

 			if (strcmp(pp1->wwid, pp2->wwid))
 				continue;

+			if (!pp2->size || pp2->state == PATH_DOWN)
+				continue;
+
 			if (!mpp->size && pp2->size)
 				mpp->size = pp2->size;

diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index 464596fc..e5de0a7d 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -101,6 +101,7 @@ alloc_path (void)
 		pp->fd = -1;
 		pp->tpgs = TPGS_UNDEF;
 		pp->priority = PRIO_UNDEF;
+		pp->handled = 0;
 		pp->checkint = CHECKINT_UNDEF;
 		checker_clear(&pp->checker);
 		dm_path_to_gen(pp)->ops = &dm_gen_path_ops;
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 7de93d6c..022ba126 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -280,6 +280,7 @@ struct path {
 	struct checker checker;
 	struct multipath * mpp;
 	int fd;
+	int handled;
 	int initialized;
 	int retriggers;
 	unsigned int path_failures;
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 8895fa77..5a73d4ce 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -738,11 +738,13 @@ int verify_paths(struct multipath *mpp)
 		return 0;

 	vector_foreach_slot (mpp->paths, pp, i) {
+		pp->handled = 1;
 		/*
 		 * see if path is in sysfs
 		 */
-		if (!pp->udev || sysfs_attr_get_value(pp->udev, "dev",
-					 pp->dev_t, BLK_DEV_SIZE) < 0) {
+		if ((!pp->udev || sysfs_attr_get_value(pp->udev, "dev",
+		    pp->dev_t, BLK_DEV_SIZE) < 0) ||
+		    (pp->state == PATH_DOWN && pp->size != mpp->size)) {
 			if (pp->state != PATH_DOWN) {
 				condlog(1, "%s: removing valid path %s in state %d",
 					mpp->alias, pp->dev, pp->state);
-- 

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

[dm-devel] [PATCH 3/6] check pgp to avoid NULL dereference in enable_group

[lixiaokeng]

If the pgp is NULL, this code will lead to coredump.
Here we check the pgp before dereference. And there
are some code enhance.

Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
---
 libmultipath/prioritizers/alua.c |  4 ++++
 libmultipath/prioritizers/emc.c  |  2 +-
 libmultipath/prioritizers/hds.c  | 19 +++++++++++--------
 libmultipath/structs_vec.c       |  4 +++-
 multipathd/main.c                |  5 +++++
 5 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/libmultipath/prioritizers/alua.c b/libmultipath/prioritizers/alua.c
index 0ab06e2b..a6307d69 100644
--- a/libmultipath/prioritizers/alua.c
+++ b/libmultipath/prioritizers/alua.c
@@ -131,15 +131,19 @@ int getprio (struct path * pp, char * args, unsigned int timeout)
 		switch(-rc) {
 		case ALUA_PRIO_NOT_SUPPORTED:
 			condlog(0, "%s: alua not supported", pp->dev);
+			rc = 0;
 			break;
 		case ALUA_PRIO_RTPG_FAILED:
 			condlog(0, "%s: couldn't get target port group", pp->dev);
+			rc = 0;
 			break;
 		case ALUA_PRIO_GETAAS_FAILED:
 			condlog(0, "%s: couldn't get asymmetric access state", pp->dev);
+			rc = 0;
 			break;
 		case ALUA_PRIO_TPGS_FAILED:
 			condlog(3, "%s: couldn't get supported alua states", pp->dev);
+			rc = 0;
 			break;
 		}
 	}
diff --git a/libmultipath/prioritizers/emc.c b/libmultipath/prioritizers/emc.c
index 3b63cca0..c40f88a0 100644
--- a/libmultipath/prioritizers/emc.c
+++ b/libmultipath/prioritizers/emc.c
@@ -19,7 +19,7 @@ int emc_clariion_prio(const char *dev, int fd, unsigned int timeout)
 	unsigned char inqCmdBlk[INQUIRY_CMDLEN] = {INQUIRY_CMD, 1, 0xC0, 0,
 						sizeof(sense_buffer), 0};
 	struct sg_io_hdr io_hdr;
-	int ret = PRIO_UNDEF;
+	int ret = 0;

 	memset(&io_hdr, 0, sizeof (struct sg_io_hdr));
 	memset(&sense_buffer, 0, 128);
diff --git a/libmultipath/prioritizers/hds.c b/libmultipath/prioritizers/hds.c
index 88cac5f0..634ae73a 100644
--- a/libmultipath/prioritizers/hds.c
+++ b/libmultipath/prioritizers/hds.c
@@ -98,10 +98,11 @@ int hds_modular_prio (const char *dev, int fd, unsigned int timeout)
 	unsigned char *inqBuffp = inqBuff;
 	unsigned char sense_buffer[32];
 	sg_io_hdr_t io_hdr;
+        int ret = 0;

 	if ((ioctl (fd, SG_GET_VERSION_NUM, &k) < 0) || (k < 30000)) {
 		pp_hds_log(0, "can't use SG ioctl interface");
-		return -1;
+		goto out;
 	}

 	memset (&io_hdr, 0, sizeof (sg_io_hdr_t));
@@ -118,11 +119,11 @@ int hds_modular_prio (const char *dev, int fd, unsigned int timeout)

 	if (ioctl (fd, SG_IO, &io_hdr) < 0) {
 		pp_hds_log(0, "SG_IO error");
-		return -1;
+		goto out;
 	}
 	if ((io_hdr.info & SG_INFO_OK_MASK) != SG_INFO_OK) {
 		pp_hds_log(0, "SCSI error");
-		return -1;
+		goto out;
 	}

 	snprintf (vendor, 9, "%.8s", inqBuffp + 8);
@@ -144,11 +145,11 @@ int hds_modular_prio (const char *dev, int fd, unsigned int timeout)
 		switch (ldev[3]) {
 		case '0': case '2': case '4': case '6': case '8': case 'A': case 'C': case 'E':
 			pp_hds_log(4, "CTRL EVEN, LDEV EVEN, PRIO 1");
-			return 1;
+			ret = 1;
 			break;
 		case '1': case '3': case '5': case '7': case '9': case 'B': case 'D': case 'F':
 			pp_hds_log(4, "CTRL EVEN, LDEV ODD, PRIO 0");
-			return 0;
+			ret = 0;
 			break;
 		}
 		break;
@@ -156,16 +157,18 @@ int hds_modular_prio (const char *dev, int fd, unsigned int timeout)
 		switch (ldev[3]) {
 		case '0': case '2': case '4': case '6': case '8': case 'A': case 'C': case 'E':
 			pp_hds_log(4, "CTRL ODD, LDEV EVEN, PRIO 0");
-			return 0;
+			ret = 0;
 			break;
 		case '1': case '3': case '5': case '7': case '9': case 'B': case 'D': case 'F':
 			pp_hds_log(4, "CTRL ODD, LDEV ODD, PRIO 1");
-			return 1;
+			ret = 1;
 			break;
 		}
 		break;
 	}
-	return -1;
+
+out:
+	return ret;
 }

 int getprio (struct path * pp, __attribute__((unused)) char *args,
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 5a73d4ce..51696e36 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -35,8 +35,10 @@ int update_mpp_paths(struct multipath *mpp, vector pathvec)
 		return 0;

 	if (!mpp->paths &&
-	    !(mpp->paths = vector_alloc()))
+	    !(mpp->paths = vector_alloc())) {
+		condlog(2, "mpp->paths alloc failed");
 		return 1;
+	}

 	vector_foreach_slot (mpp->pg, pgp, i) {
 		vector_foreach_slot (pgp->paths, pp, j) {
diff --git a/multipathd/main.c b/multipathd/main.c
index a4abbb27..13cb4dec 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1725,6 +1725,11 @@ enable_group(struct path * pp)

 	pgp = VECTOR_SLOT(pp->mpp->pg, pp->pgindex - 1);

+	if (!pgp) {
+		condlog(2, "%s: pgp is NULL", pp->mpp->alias);
+		return;
+	}
+
 	if (pgp->status == PGSTATE_DISABLED) {
 		condlog(2, "%s: enable group #%i", pp->mpp->alias, pp->pgindex);
 		dm_enablegroup(pp->mpp->alias, pp->pgindex);
-- 

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

[dm-devel] [PATCH 4/6] fix change reservtion key to uint8 for memcmp

[lixiaokeng]

The mpp->reservation_key is uint64_t and the paramp->key
is uint8_t. For example, paramp->key is 0x7d0 and the
mpp->reservation_key is 0x00000000000007d0. When memcmp
is called, it will not return 0. We change  reservtion_key
to uint8 to fix that.

Signed-off-by: Jianbing Jiao <jiaojianbing@huawei.com>
Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
---
 libmpathpersist/mpath_persist.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index 1f9817ed..02d173dc 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -249,6 +249,9 @@ int __mpath_persistent_reserve_out ( int fd, int rq_servact, int rq_scope,
 	int ret;
 	uint64_t prkey;
 	struct config *conf;
+	uint8_t  uitmp[8] = {0};
+	uint64_t uireservation = {0};
+	int j;

 	ret = mpath_get_map(fd, &alias, &mpp);
 	if (ret != MPATH_PR_SUCCESS)
@@ -274,8 +277,14 @@ int __mpath_persistent_reserve_out ( int fd, int rq_servact, int rq_scope,
 		}
 	}

-	if (memcmp(paramp->key, &mpp->reservation_key, 8) &&
-	    memcmp(paramp->sa_key, &mpp->reservation_key, 8)) {
+	uireservation = get_be64(mpp->reservation_key);
+	for (j = 7; j >= 0; --j) {
+		uitmp[j] = (uireservation & 0xff);
+		uireservation >>= 8;
+	}
+
+	if (memcmp(paramp->key, uitmp, 8) &&
+	    memcmp(paramp->sa_key, uitmp, 8)) {
 		condlog(0, "%s: configured reservation key doesn't match: 0x%" PRIx64, alias, get_be64(mpp->reservation_key));
 		ret = MPATH_PR_SYNTAX_ERROR;
 		goto out1;
--

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

[dm-devel] [PATCH 5/6] ignore for clear mismatch key

[lixiaokeng]

The mpathpersist -I and -G does not work.
Here we fix that.

Signed-off-by: Jianbing Jiao <jiaojianbing@huawei.com>
Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
---
 libmpathpersist/mpath_persist.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index 02d173dc..95018ae5 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -265,8 +265,10 @@ int __mpath_persistent_reserve_out ( int fd, int rq_servact, int rq_scope,
 	memcpy(&prkey, paramp->sa_key, 8);
 	if (mpp->prkey_source == PRKEY_SOURCE_FILE && prkey &&
 	    ((!get_be64(mpp->reservation_key) &&
-	      rq_servact == MPATH_PROUT_REG_SA) ||
-	     rq_servact == MPATH_PROUT_REG_IGN_SA)) {
+	    rq_servact == MPATH_PROUT_REG_SA) ||
+	    rq_servact == MPATH_PROUT_REG_IGN_SA ||
+	    (!memcmp(paramp->key, &mpp->reservation_key, 8) &&
+	    rq_servact == MPATH_PROUT_REG_SA))) {
 		memcpy(&mpp->reservation_key, paramp->sa_key, 8);
 		if (update_prkey_flags(alias, get_be64(mpp->reservation_key),
 				       paramp->sa_flags)) {
@@ -282,12 +284,14 @@ int __mpath_persistent_reserve_out ( int fd, int rq_servact, int rq_scope,
 		uitmp[j] = (uireservation & 0xff);
 		uireservation >>= 8;
 	}
-
-	if (memcmp(paramp->key, uitmp, 8) &&
-	    memcmp(paramp->sa_key, uitmp, 8)) {
-		condlog(0, "%s: configured reservation key doesn't match: 0x%" PRIx64, alias, get_be64(mpp->reservation_key));
-		ret = MPATH_PR_SYNTAX_ERROR;
-		goto out1;
+	/* pass -I option */
+	if (rq_servact != MPATH_PROUT_REG_IGN_SA) {
+		if (memcmp(paramp->key, uitmp, 8) &&
+		    memcmp(paramp->sa_key, uitmp, 8)) {
+			condlog(0, "%s: configured reservation key doesn't match: 0x%" PRIx64, alias, get_be64(mpp->reservation_key));
+			ret = MPATH_PR_SYNTAX_ERROR;
+			goto out1;
+		}
 	}

 	switch(rq_servact)
--

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

[dm-devel] [PATCH 6/6] flush and sync before reboot

[lixiaokeng]

Write to /etc/multipath/prkeys, for example
0x0000000000000001 3620f17c1007609293677bb210000058e.
Power-off and restart in 5 seconds. There will
be some 0 byte in /etc/multipath/prkeys.
Here we flush and sync before reboot.

Signed-off-by: Jianbing Jiao <jiangjianbiang@huawei.com>
Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
---
 libmultipath/util.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/libmultipath/util.c b/libmultipath/util.c
index 1dad90f2..11d4a398 100644
--- a/libmultipath/util.c
+++ b/libmultipath/util.c
@@ -355,6 +355,13 @@ int safe_write(int fd, const void *buf, size_t count)
 		count -= r;
 		buf = (const char *)buf + r;
 	}
+
+	/* sync->disk */
+	if (fsync(fd) < 0) {
+		condlog(0, "failed to fsync fd :%d", fd);
+		return -errno;
+	}
+
 	return 0;
 }

-- 

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

Re: [dm-devel] [PATCH 0/6] multipath-tools history bug in 0.4.9

[Martin Wilck]

Hello Lixiaokeng,

On Mon, 2020-10-26 at 17:23 +0800, lixiaokeng wrote:
> Hi,
> 
> There are some old patches in our multipath-tools code. They fix some
> bugs in old version (0.4.9). I'm not the auther of these patches,
> but I send them. Please review.Thanks.

Thanks for the patches, but unless you can provide evidence that the
issues are still present in recent upstream code, we likely won't
bother with this on dm-devel. Please report this to the distribution(s)
you are using that are still shipping the old multipath-tools versions.

Regards,
Martin

> 
> lixiaokeng (3):
>   fix multipathd resize when not all sizes of paths are equal
>   fix lun expansion failure when there is offline path
>   flush and sync before reboot
> 
> sunguoshuai (3):
>   check pgp to avoid NULL dereference in enable_group
>   fix change reservtion key to uint8 for memcmp
>   ignore for clear mismatch key
> 
>  libmpathpersist/mpath_persist.c  | 27 ++++++++++++++++++++-------
>  libmultipath/configure.c         | 18 ++++++++++++++++++
>  libmultipath/prioritizers/alua.c |  4 ++++
>  libmultipath/prioritizers/emc.c  |  2 +-
>  libmultipath/prioritizers/hds.c  | 19 +++++++++++--------
>  libmultipath/structs.c           |  1 +
>  libmultipath/structs.h           |  1 +
>  libmultipath/structs_vec.c       | 10 +++++++---
>  libmultipath/util.c              |  7 +++++++
>  multipathd/cli_handlers.c        | 19 ++++++++++++++++++-
>  multipathd/main.c                |  5 +++++
>  11 files changed, 93 insertions(+), 20 deletions(-)
> 


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

Re: [dm-devel] [PATCH 0/6] multipath-tools history bug in 0.4.9

[lixiaokeng]

> 
> Thanks for the patches, but unless you can provide evidence that the
> issues are still present in recent upstream code, we likely won't
> bother with this on dm-devel. Please report this to the distribution(s)
> you are using that are still shipping the old multipath-tools versions.
> 
> Regards,
> Martin
> 
Hi
  Martin. Thanks for your reply. I'm sorry for sending these old patches.
Before these patches, I sent two patch "libmultipath: fix memory leaks in
coalesce_paths" (v2) and "libmultipath: call udev_device_unref before
return". But I did not get a reply. Do you notice them?

Regards,
Martin

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