[PATCH 0/6] Various multipath-tools patches

[PATCH 0/6] Various multipath-tools patches

[Bart Van Assche]

Hello Christophe,

This series contains the following patches:
- Four patches address Coverity complaints.
- One patch makes a function argument const.
- One patch addresses a bug that I discovered while inspecting the multipath
  command line tool output.

Please consider these patches for the multipath-tools project.

Thanks,

Bart.

Bart Van Assche (6):
  multipathd/main.c: Fix indentation
  libmultipath, alloc_path_with_pathinfo(): Ensure that pp->wwid is
    '\0'-terminated
  libmultipath, alloc_path_with_pathinfo(): Declare third argument const
  kpartx: Improve reliability of find_loop_by_file()
  libmultipath: Fix sgio_get_vpd()
  Introduce the ibmultipath/unaligned.h header file

 kpartx/lopart.c                       |  7 ++++---
 libmpathpersist/mpath_pr_ioctl.c      |  8 ++++----
 libmultipath/checkers/hp_sw.c         |  4 ++--
 libmultipath/discovery.c              | 22 +++++++++++---------
 libmultipath/discovery.h              |  2 +-
 libmultipath/prioritizers/alua_rtpg.c | 13 ++++++------
 libmultipath/prioritizers/alua_spc3.h | 35 ++------------------------------
 libmultipath/prioritizers/ontap.c     |  4 ++--
 libmultipath/unaligned.h              | 38 +++++++++++++++++++++++++++++++++++
 multipathd/main.c                     |  4 ++--
 10 files changed, 74 insertions(+), 63 deletions(-)
 create mode 100644 libmultipath/unaligned.h

-- 
2.16.2

[PATCH 1/6] multipathd/main.c: Fix indentation

[Bart Van Assche]

This patch avoids that gcc reports the following:

main.c: In function 'uev_pathfail_check':
main.c:1022:5: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
     if (!pp)
     ^~
main.c:1024:2: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
  r = io_err_stat_handle_pathfail(pp);
  ^

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
---
 multipathd/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 27cf234623d0..ccbb0ad13d32 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1020,8 +1020,8 @@ uev_pathfail_check(struct uevent *uev, struct vectors *vecs)
 	lock(&vecs->lock);
 	pthread_testcancel();
 	pp = find_path_by_devt(vecs->pathvec, devt);
-    if (!pp)
-        goto out_lock;
+	if (!pp)
+		goto out_lock;
 	r = io_err_stat_handle_pathfail(pp);
 	if (r)
 		condlog(3, "io_err_stat: %s: cannot handle pathfail uevent",
-- 
2.16.2

Re: [PATCH 1/6] multipathd/main.c: Fix indentation

[Martin Wilck]

On Thu, 2018-03-01 at 11:29 -0800, Bart Van Assche wrote:
> This patch avoids that gcc reports the following:
> 
> main.c: In function 'uev_pathfail_check':
> main.c:1022:5: warning: this 'if' clause does not guard... [-
> Wmisleading-indentation]
>      if (!pp)
>      ^~
> main.c:1024:2: note: ...this statement, but the latter is
> misleadingly indented as if it were guarded by the 'if'
>   r = io_err_stat_handle_pathfail(pp);
>   ^
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>

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

> ---
>  multipathd/main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 27cf234623d0..ccbb0ad13d32 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -1020,8 +1020,8 @@ uev_pathfail_check(struct uevent *uev, struct
> vectors *vecs)
>  	lock(&vecs->lock);
>  	pthread_testcancel();
>  	pp = find_path_by_devt(vecs->pathvec, devt);
> -    if (!pp)
> -        goto out_lock;
> +	if (!pp)
> +		goto out_lock;
>  	r = io_err_stat_handle_pathfail(pp);
>  	if (r)
>  		condlog(3, "io_err_stat: %s: cannot handle pathfail
> uevent",

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

[PATCH 2/6] libmultipath, alloc_path_with_pathinfo(): Ensure that pp->wwid is '\0'-terminated

[Bart Van Assche]

Discovered by Coverity (CID 173257).

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
---
 libmultipath/discovery.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 4b31ddef23f0..fe50ce5062ea 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -52,8 +52,8 @@ alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
 	if (!pp)
 		return PATHINFO_FAILED;
 
-	if(wwid)
-		strncpy(pp->wwid, wwid, sizeof(pp->wwid));
+	if (wwid)
+		strlcpy(pp->wwid, wwid, sizeof(pp->wwid));
 
 	if (safe_sprintf(pp->dev, "%s", devname)) {
 		condlog(0, "pp->dev too small");
-- 
2.16.2

Re: [PATCH 2/6] libmultipath, alloc_path_with_pathinfo(): Ensure that pp->wwid is '\0'-terminated

[Martin Wilck]

On Thu, 2018-03-01 at 11:29 -0800, Bart Van Assche wrote:
> Discovered by Coverity (CID 173257).
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>

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

> ---
>  libmultipath/discovery.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index 4b31ddef23f0..fe50ce5062ea 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -52,8 +52,8 @@ alloc_path_with_pathinfo (struct config *conf,
> struct udev_device *udevice,
>  	if (!pp)
>  		return PATHINFO_FAILED;
>  
> -	if(wwid)
> -		strncpy(pp->wwid, wwid, sizeof(pp->wwid));
> +	if (wwid)
> +		strlcpy(pp->wwid, wwid, sizeof(pp->wwid));
>  
>  	if (safe_sprintf(pp->dev, "%s", devname)) {
>  		condlog(0, "pp->dev too small");



-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

[PATCH 3/6] libmultipath, alloc_path_with_pathinfo(): Declare third argument const

[Bart Van Assche]

Since the third argument of this function is not modified inside that
function, declare it const.

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
---
 libmultipath/discovery.c | 2 +-
 libmultipath/discovery.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index fe50ce5062ea..d84715e15db1 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -34,7 +34,7 @@
 
 int
 alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
-			  char *wwid, int flag, struct path **pp_ptr)
+			  const char *wwid, int flag, struct path **pp_ptr)
 {
 	int err = PATHINFO_FAILED;
 	struct path * pp;
diff --git a/libmultipath/discovery.h b/libmultipath/discovery.h
index b151cc7a39a8..bd5e6678a26d 100644
--- a/libmultipath/discovery.h
+++ b/libmultipath/discovery.h
@@ -38,7 +38,7 @@ int get_state (struct path * pp, struct config * conf, int daemon, int state);
 int get_vpd_sgio (int fd, int pg, char * str, int maxlen);
 int pathinfo (struct path * pp, struct config * conf, int mask);
 int alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
-			      char *wwid, int flag, struct path **pp_ptr);
+			      const char *wwid, int flag, struct path **pp_ptr);
 int store_pathinfo (vector pathvec, struct config *conf,
 		    struct udev_device *udevice, int flag,
 		    struct path **pp_ptr);
-- 
2.16.2

Re: [PATCH 3/6] libmultipath, alloc_path_with_pathinfo(): Declare third argument const

[Martin Wilck]

On Thu, 2018-03-01 at 11:29 -0800, Bart Van Assche wrote:
> Since the third argument of this function is not modified inside that
> function, declare it const.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>

ACK, but this hunk is part of my previously posted patch "libmultipath:
const qualifier for wwid and alias", which includes some more changes
along similar lines and is part of a larger patch set that tries to
improve "const" usage in libmultipath.


> ---
>  libmultipath/discovery.c | 2 +-
>  libmultipath/discovery.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index fe50ce5062ea..d84715e15db1 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -34,7 +34,7 @@
>  
>  int
>  alloc_path_with_pathinfo (struct config *conf, struct udev_device
> *udevice,
> -			  char *wwid, int flag, struct path
> **pp_ptr)
> +			  const char *wwid, int flag, struct path
> **pp_ptr)
>  {
>  	int err = PATHINFO_FAILED;
>  	struct path * pp;
> diff --git a/libmultipath/discovery.h b/libmultipath/discovery.h
> index b151cc7a39a8..bd5e6678a26d 100644
> --- a/libmultipath/discovery.h
> +++ b/libmultipath/discovery.h
> @@ -38,7 +38,7 @@ int get_state (struct path * pp, struct config *
> conf, int daemon, int state);
>  int get_vpd_sgio (int fd, int pg, char * str, int maxlen);
>  int pathinfo (struct path * pp, struct config * conf, int mask);
>  int alloc_path_with_pathinfo (struct config *conf, struct
> udev_device *udevice,
> -			      char *wwid, int flag, struct path
> **pp_ptr);
> +			      const char *wwid, int flag, struct
> path **pp_ptr);
>  int store_pathinfo (vector pathvec, struct config *conf,
>  		    struct udev_device *udevice, int flag,
>  		    struct path **pp_ptr);

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

Re: [PATCH 3/6] libmultipath, alloc_path_with_pathinfo(): Declare third argument const

[Bart Van Assche]

On Mon, 2018-03-05 at 17:18 +0100, Martin Wilck wrote:
> On Thu, 2018-03-01 at 11:29 -0800, Bart Van Assche wrote:
> > Since the third argument of this function is not modified inside that
> > function, declare it const.
> > 
> > Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> 
> ACK, but this hunk is part of my previously posted patch "libmultipath:
> const qualifier for wwid and alias", which includes some more changes
> along similar lines and is part of a larger patch set that tries to
> improve "const" usage in libmultipath.

Hello Martin,

If this patch duplicates some of your work then I'm fine with dropping this
patch.

Bart.

[PATCH 4/6] kpartx: Improve reliability of find_loop_by_file()

[Bart Van Assche]

Avoid that the strchr() call in this function examines uninitialized
data on the stack. This patch avoids that Coverity reports the following:

    CID 173252:  Error handling issues  (CHECKED_RETURN)
    "read(int, void *, size_t)" returns the number of bytes read, but it is ignored.

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
---
 kpartx/lopart.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/kpartx/lopart.c b/kpartx/lopart.c
index 02b29e8cf91d..69c1eda1cc5c 100644
--- a/kpartx/lopart.c
+++ b/kpartx/lopart.c
@@ -64,7 +64,7 @@ char *find_loop_by_file(const char *filename)
 	DIR *dir;
 	struct dirent *dent;
 	char dev[64], *found = NULL, *p;
-	int fd;
+	int fd, bytes_read;
 	struct stat statbuf;
 	struct loop_info loopinfo;
 	const char VIRT_BLOCK[] = "/sys/devices/virtual/block";
@@ -86,14 +86,15 @@ char *find_loop_by_file(const char *filename)
 		if (fd < 0)
 			continue;
 
-		if (read(fd, dev, sizeof(dev)) <= 0) {
+		bytes_read = read(fd, dev, sizeof(dev) - 1);
+		if (bytes_read <= 0) {
 			close(fd);
 			continue;
 		}
 
 		close(fd);
 
-		dev[sizeof(dev)-1] = '\0';
+		dev[bytes_read] = '\0';
 		p = strchr(dev, '\n');
 		if (p != NULL)
 			*p = '\0';
-- 
2.16.2

Re: [PATCH 4/6] kpartx: Improve reliability of find_loop_by_file()

[Martin Wilck]

On Thu, 2018-03-01 at 11:29 -0800, Bart Van Assche wrote:
> Avoid that the strchr() call in this function examines uninitialized
> data on the stack. This patch avoids that Coverity reports the
> following:
> 
>     CID 173252:  Error handling issues  (CHECKED_RETURN)
>     "read(int, void *, size_t)" returns the number of bytes read, but
> it is ignored.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>

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


> ---
>  kpartx/lopart.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/kpartx/lopart.c b/kpartx/lopart.c
> index 02b29e8cf91d..69c1eda1cc5c 100644
> --- a/kpartx/lopart.c
> +++ b/kpartx/lopart.c
> @@ -64,7 +64,7 @@ char *find_loop_by_file(const char *filename)
>  	DIR *dir;
>  	struct dirent *dent;
>  	char dev[64], *found = NULL, *p;
> -	int fd;
> +	int fd, bytes_read;
>  	struct stat statbuf;
>  	struct loop_info loopinfo;
>  	const char VIRT_BLOCK[] = "/sys/devices/virtual/block";
> @@ -86,14 +86,15 @@ char *find_loop_by_file(const char *filename)
>  		if (fd < 0)
>  			continue;
>  
> -		if (read(fd, dev, sizeof(dev)) <= 0) {
> +		bytes_read = read(fd, dev, sizeof(dev) - 1);
> +		if (bytes_read <= 0) {
>  			close(fd);
>  			continue;
>  		}
>  
>  		close(fd);
>  
> -		dev[sizeof(dev)-1] = '\0';
> +		dev[bytes_read] = '\0';
>  		p = strchr(dev, '\n');
>  		if (p != NULL)
>  			*p = '\0';

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

[PATCH 5/6] libmultipath: Fix sgio_get_vpd()

[Bart Van Assche]

Pass the VPD page number to sgio_get_vpd() such that the page needed
by the caller is queried instead of page 0x83. Fix the statement that
computes the length of the page returned by do_inq(). Fix the return
code check in the caller of sgio_get_vpd().

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
---
 libmultipath/discovery.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index d84715e15db1..780feb253797 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -836,8 +836,9 @@ detect_alua(struct path * pp, struct config *conf)
 
 #define DEFAULT_SGIO_LEN 254
 
+/* Query VPD page @pg. Returns 0 upon success and -1 upon failure. */
 static int
-sgio_get_vpd (unsigned char * buff, int maxlen, int fd)
+sgio_get_vpd (unsigned char * buff, int maxlen, int fd, int pg)
 {
 	int len = DEFAULT_SGIO_LEN;
 
@@ -846,8 +847,8 @@ sgio_get_vpd (unsigned char * buff, int maxlen, int fd)
 		return -1;
 	}
 retry:
-	if (0 == do_inq(fd, 0, 1, 0x83, buff, len)) {
-		len = buff[3] + (buff[2] << 8);
+	if (0 == do_inq(fd, 0, 1, pg, buff, len)) {
+		len = buff[3] + (buff[2] << 8) + 4;
 		if (len >= maxlen)
 			return len;
 		if (len > DEFAULT_SGIO_LEN)
@@ -1099,7 +1100,7 @@ get_vpd_sgio (int fd, int pg, char * str, int maxlen)
 	unsigned char buff[4096];
 
 	memset(buff, 0x0, 4096);
-	if (sgio_get_vpd(buff, 4096, fd) <= 0) {
+	if (sgio_get_vpd(buff, 4096, fd, pg) < 0) {
 		condlog(3, "failed to issue vpd inquiry for pg%02x",
 			pg);
 		return -errno;
-- 
2.16.2

Re: [PATCH 5/6] libmultipath: Fix sgio_get_vpd()

[Martin Wilck]

On Thu, 2018-03-01 at 11:29 -0800, Bart Van Assche wrote:
> Pass the VPD page number to sgio_get_vpd() such that the page needed
> by the caller is queried instead of page 0x83. Fix the statement that
> computes the length of the page returned by do_inq(). Fix the return
> code check in the caller of sgio_get_vpd().
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> ---

Bart, thanks for the patch. Please consider rebasing your work on my
previously submitted patches which have already been positively
reviewed ([1], [2]):

 libmultipath: get_vpd_sgio: support VPD 0xc9
 libmultipath: sgio_get_vpd: add page argument
 libmultipath: fix return code of sgio_get_vpd()

Otherwise, ACK.

[1] https://www.redhat.com/archives/dm-devel/2018-January/msg00241.html
[2] https://www.redhat.com/archives/dm-devel/2018-January/msg00370.html

Martin

>  libmultipath/discovery.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index d84715e15db1..780feb253797 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -836,8 +836,9 @@ detect_alua(struct path * pp, struct config
> *conf)
>  
>  #define DEFAULT_SGIO_LEN 254
>  
> +/* Query VPD page @pg. Returns 0 upon success and -1 upon failure.
> */
>  static int
> -sgio_get_vpd (unsigned char * buff, int maxlen, int fd)
> +sgio_get_vpd (unsigned char * buff, int maxlen, int fd, int pg)
>  {
>  	int len = DEFAULT_SGIO_LEN;
>  
> @@ -846,8 +847,8 @@ sgio_get_vpd (unsigned char * buff, int maxlen,
> int fd)
>  		return -1;
>  	}
>  retry:
> -	if (0 == do_inq(fd, 0, 1, 0x83, buff, len)) {
> -		len = buff[3] + (buff[2] << 8);
> +	if (0 == do_inq(fd, 0, 1, pg, buff, len)) {
> +		len = buff[3] + (buff[2] << 8) + 4;
>  		if (len >= maxlen)
>  			return len;
>  		if (len > DEFAULT_SGIO_LEN)
> @@ -1099,7 +1100,7 @@ get_vpd_sgio (int fd, int pg, char * str, int
> maxlen)
>  	unsigned char buff[4096];
>  
>  	memset(buff, 0x0, 4096);
> -	if (sgio_get_vpd(buff, 4096, fd) <= 0) {
> +	if (sgio_get_vpd(buff, 4096, fd, pg) < 0) {
>  		condlog(3, "failed to issue vpd inquiry for pg%02x",
>  			pg);
>  		return -errno;


-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

Re: [PATCH 5/6] libmultipath: Fix sgio_get_vpd()

[Martin Wilck]

On Mon, 2018-03-05 at 17:53 +0100, Martin Wilck wrote:
> On Thu, 2018-03-01 at 11:29 -0800, Bart Van Assche wrote:
> > Pass the VPD page number to sgio_get_vpd() such that the page
> > needed
> > by the caller is queried instead of page 0x83. Fix the statement
> > that
> > computes the length of the page returned by do_inq(). Fix the
> > return
> > code check in the caller of sgio_get_vpd().
> > 
> > Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> > ---
> 
> Bart, thanks for the patch. Please consider rebasing your work on my
> previously submitted patches which have already been positively
> reviewed ([1], [2]):
> 
>  libmultipath: get_vpd_sgio: support VPD 0xc9
>  libmultipath: sgio_get_vpd: add page argument
>  libmultipath: fix return code of sgio_get_vpd()
> 
> Otherwise, ACK.

Sorry, I found a glitch.

With the change of the "len" calculation and the return value check, we
  need to make sure sufficient data has been read:

@@ -1100,13 +1101,19 @@ get_vpd_sgio (int fd, int pg, char * str, int maxlen)
 	unsigned char buff[4096];
 
 	memset(buff, 0x0, 4096);
-	if (sgio_get_vpd(buff, 4096, fd, pg) <= 0) {
+	buff_len = sgio_get_vpd(buff, 4096, fd, pg);
+	if (buff_len < 0) {
 		condlog(3, "failed to issue vpd inquiry for pg%02x",
 			pg);
-		return -errno;
+		return errno != 0 ? -errno : -1;
 	}
 
-	if (buff[1] != pg) {
+	if (buff_len < 4) {
+		condlog(3, "vpd pg%02x error, short read", pg);
+		return -ENODATA;
+	}
+
+	if ( buff[1] != pg) {
 		condlog(3, "vpd pg%02x error, invalid vpd page %02x",
 			pg, buff[1]);
 		return -ENODATA;

As you can see I added also a small hunk that makes it explcicit that
we don't return 0 on failure by accident.

Regards,
Martin

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

Re: [PATCH 5/6] libmultipath: Fix sgio_get_vpd()

[Martin Wilck]

On Mon, 2018-03-05 at 18:09 +0100, Martin Wilck wrote:
> On Mon, 2018-03-05 at 17:53 +0100, Martin Wilck wrote:
> > On Thu, 2018-03-01 at 11:29 -0800, Bart Van Assche wrote:
> > > Pass the VPD page number to sgio_get_vpd() such that the page
> > > needed
> > > by the caller is queried instead of page 0x83. Fix the statement
> > > that
> > > computes the length of the page returned by do_inq(). Fix the
> > > return
> > > code check in the caller of sgio_get_vpd().
> > > 
> > > Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> > > ---
> > 
> > Bart, thanks for the patch. Please consider rebasing your work on
> > my
> > previously submitted patches which have already been positively
> > reviewed ([1], [2]):
> > 
> >  libmultipath: get_vpd_sgio: support VPD 0xc9
> >  libmultipath: sgio_get_vpd: add page argument
> >  libmultipath: fix return code of sgio_get_vpd()
> > 
> > Otherwise, ACK.
> 
> Sorry, I found a glitch.
> 
> With the change of the "len" calculation and the return value check,
> we
>   need to make sure sufficient data has been read:

Replying to self - this is actually not necessary, as we're looking at
the response buffer of a successful VPD INQUIRY, so we have to assume
that at least the header has been read correctly.

All we need to change is the return code and the respective comment,
because the function actually returns the number of INQUIRY bytes if
INQUIRY was successful (before my patch "libmultipath: fix return code
of sgio_get_vpd()", the function behaved rather strange, returning -1
on failure, 0 on success, and len > buffer length on buffer overflow).

Unless you object, I'll repost your series rebased on mine.

Martin

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

Re: [PATCH 5/6] libmultipath: Fix sgio_get_vpd()

[Bart Van Assche]

On Mon, 2018-03-05 at 20:14 +0100, Martin Wilck wrote:
> Unless you object, I'll repost your series rebased on mine.

Hello Martin,

Before you start working on that: has your patch series already been posted
on the dm-devel mailing list?

Thanks,

Bart.

Re: [PATCH 5/6] libmultipath: Fix sgio_get_vpd()

[Martin Wilck]

On Mon, 2018-03-05 at 19:18 +0000, Bart Van Assche wrote:
> On Mon, 2018-03-05 at 20:14 +0100, Martin Wilck wrote:
> > Unless you object, I'll repost your series rebased on mine.
> 
> Hello Martin,
> 
> Before you start working on that: has your patch series already been
> posted
> on the dm-devel mailing list?

Yes. "PATCH v2 00/20] Various multipath-tools fixes" ff.
https://www.redhat.com/archives/dm-devel/2018-January/msg00219.html

Regards,
Martin

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

Re: [PATCH 5/6] libmultipath: Fix sgio_get_vpd()

[Bart Van Assche]

On Mon, 2018-03-05 at 21:47 +0100, Martin Wilck wrote:
> On Mon, 2018-03-05 at 19:18 +0000, Bart Van Assche wrote:
> > On Mon, 2018-03-05 at 20:14 +0100, Martin Wilck wrote:
> > > Unless you object, I'll repost your series rebased on mine.
> > 
> > Hello Martin,
> > 
> > Before you start working on that: has your patch series already been
> > posted
> > on the dm-devel mailing list?
> 
> Yes. "PATCH v2 00/20] Various multipath-tools fixes" ff.
> https://www.redhat.com/archives/dm-devel/2018-January/msg00219.html

Ah, thanks, but unfortunately these patches are no longer in my mailbox. I
pulled these from https://github.com/openSUSE/multipath-tools. I'm fine with
my patches being rebased on top of your series, whether or not the following
issues get addressed:
* Several patches that are on the upstream-queue branch introduce trailing
  whitespace.
* The macro FREE_CONST() should never have been introduced. Introducing such
  a macro namely introduces the risk of calling free() for a string constant,
  something that should never happen. Have you considered to declare
  dynamically allocated strings, e.g. the result of strdup(), as char *
  instead of const char * ? I think with that change the FREE_CONST() macro
  definitions can be removed again.

Thanks,

Bart.

Re: [PATCH 5/6] libmultipath: Fix sgio_get_vpd()

[Martin Wilck]

On Mon, 2018-03-05 at 21:13 +0000, Bart Van Assche wrote:
> On Mon, 2018-03-05 at 21:47 +0100, Martin Wilck wrote:
> > On Mon, 2018-03-05 at 19:18 +0000, Bart Van Assche wrote:
> > > On Mon, 2018-03-05 at 20:14 +0100, Martin Wilck wrote:
> > > > Unless you object, I'll repost your series rebased on mine.
> > > 
> > > Hello Martin,
> > > 
> > > Before you start working on that: has your patch series already
> > > been
> > > posted
> > > on the dm-devel mailing list?
> > 
> > Yes. "PATCH v2 00/20] Various multipath-tools fixes" ff.
> > https://www.redhat.com/archives/dm-devel/2018-January/msg00219.html
> 
> Ah, thanks, but unfortunately these patches are no longer in my
> mailbox. I
> pulled these from https://github.com/openSUSE/multipath-tools. I'm
> fine with
> my patches being rebased on top of your series, whether or not the
> following
> issues get addressed:
> * Several patches that are on the upstream-queue branch introduce
> trailing
>   whitespace.
> * The macro FREE_CONST() should never have been introduced.
> Introducing such
>   a macro namely introduces the risk of calling free() for a string
> constant,
>   something that should never happen. Have you considered to declare
>   dynamically allocated strings, e.g. the result of strdup(), as char
> *
>   instead of const char * ? I think with that change the FREE_CONST()
> macro
>   definitions can be removed again.

Another philosophical issue :-) I disagree with you here. The name
FREE_CONST is obvious enough that people should think twice before
using it. I have indeed considered what you propse, but I like being
able to use "const char*" when I work with strings that shouldn't be
changed. An accidental write to a const location is much easier to
overlook than a FREE_CONST(). Freeing a "const char*" isn't always
wrong, just if the pointer was passed in from elsewhere.

This issue is philosophical but not religious to me, so if you and
maybe others are offended so much by FREE_CONST, I may ditch it again.

Regards,
Martin

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

Re: [PATCH 5/6] libmultipath: Fix sgio_get_vpd()

[Martin Wilck]

On Tue, 2018-03-06 at 00:24 +0100, Martin Wilck wrote:
> On Mon, 2018-03-05 at 21:13 +0000, Bart Van Assche wrote:
> > Yes. "PATCH v2 00/20] Various multipath-tools fixes" ff.
> > > https://www.redhat.com/archives/dm-devel/2018-January/msg00219.ht
> > > ml
> > 
> > Ah, thanks, but unfortunately these patches are no longer in my
> > mailbox. I
> > pulled these from https://github.com/openSUSE/multipath-tools. I'm
> > fine with
> > my patches being rebased on top of your series, whether or not the
> > following
> > issues get addressed:

I pushed my latest state, including my rebase of your patches, 
to https://github.com/openSUSE/multipath-tools/commits/upstream-queue.
I plan to merge also Ben's and Chongyun's late submissions, but
I didn't get around to it yet.

Regards,
Martin

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

[PATCH 6/6] Introduce the ibmultipath/unaligned.h header file

[Bart Van Assche]

This patch avoids that Coverity reports the following for the code
in libmultipath/prioritizers/alua_rtpg.c:

   CID 173256:  Integer handling issues  (SIGN_EXTENSION)
    Suspicious implicit sign extension: "buf[0]" with type "unsigned char" (8 bits, unsigned) is promoted in "((buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3]) + 4" to type "int" (32 bits, signed), then sign-extended to type "unsigned long" (64 bits, unsigned).  If "((buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3]) + 4" is greater than 0x7FFFFFFF, the upper bits of the result will all be 1.

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
---
 libmpathpersist/mpath_pr_ioctl.c      |  8 ++++----
 libmultipath/checkers/hp_sw.c         |  4 ++--
 libmultipath/discovery.c              |  9 +++++----
 libmultipath/prioritizers/alua_rtpg.c | 13 ++++++------
 libmultipath/prioritizers/alua_spc3.h | 35 ++------------------------------
 libmultipath/prioritizers/ontap.c     |  4 ++--
 libmultipath/unaligned.h              | 38 +++++++++++++++++++++++++++++++++++
 7 files changed, 60 insertions(+), 51 deletions(-)
 create mode 100644 libmultipath/unaligned.h

diff --git a/libmpathpersist/mpath_pr_ioctl.c b/libmpathpersist/mpath_pr_ioctl.c
index 29df8c6f2da1..dbed4ca7fad4 100644
--- a/libmpathpersist/mpath_pr_ioctl.c
+++ b/libmpathpersist/mpath_pr_ioctl.c
@@ -13,6 +13,7 @@
 #include <libudev.h>
 #include "mpath_pr_ioctl.h"
 #include "mpath_persist.h"
+#include "unaligned.h"
 
 #include "debug.h"
 
@@ -243,10 +244,9 @@ void mpath_format_readfullstatus(struct prin_resp *pr_buff, int len, int noisy)
 		memcpy(&fdesc.key, p, 8 );
 		fdesc.flag = p[12];
 		fdesc.scope_type =  p[13];
-		fdesc.rtpi = ((p[18] << 8) | p[19]);
+		fdesc.rtpi = get_unaligned_be16(&p[18]);
 
-		tid_len_len = ((p[20] << 24) | (p[21] << 16) |
-				(p[22] << 8) | p[23]);
+		tid_len_len = get_unaligned_be32(&p[20]);
 
 		if (tid_len_len > 0)
 			decode_transport_id( &fdesc, &p[24], tid_len_len);
@@ -277,7 +277,7 @@ decode_transport_id(struct prin_fulldescr *fdesc, unsigned char * p, int length)
 			jump = 24;
 			break;
 		case MPATH_PROTOCOL_ID_ISCSI:
-			num = ((p[2] << 8) | p[3]);
+			num = get_unaligned_be16(&p[2]);
 			memcpy(&fdesc->trnptid.iscsi_name, &p[4], num);
 			jump = (((num + 4) < 24) ? 24 : num + 4);
 			break;
diff --git a/libmultipath/checkers/hp_sw.c b/libmultipath/checkers/hp_sw.c
index 6019c9dbcd9d..cee9aab8d089 100644
--- a/libmultipath/checkers/hp_sw.c
+++ b/libmultipath/checkers/hp_sw.c
@@ -14,6 +14,7 @@
 #include "checkers.h"
 
 #include "../libmultipath/sg_include.h"
+#include "../libmultipath/unaligned.h"
 
 #define TUR_CMD_LEN		6
 #define INQUIRY_CMDLEN		6
@@ -63,8 +64,7 @@ do_inq(int sg_fd, int cmddt, int evpd, unsigned int pg_op,
 	if (evpd)
 		inqCmdBlk[1] |= 1;
 	inqCmdBlk[2] = (unsigned char) pg_op;
-	inqCmdBlk[3] = (unsigned char)((mx_resp_len >> 8) & 0xff);
-	inqCmdBlk[4] = (unsigned char) (mx_resp_len & 0xff);
+	put_unaligned_be16(mx_resp_len, &inqCmdBlk[3]);
 	memset(&io_hdr, 0, sizeof (struct sg_io_hdr));
 	memset(sense_b, 0, SENSE_BUFF_LEN);
 	io_hdr.interface_id = 'S';
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 780feb253797..1d027927bd98 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -30,6 +30,7 @@
 #include "discovery.h"
 #include "prio.h"
 #include "defaults.h"
+#include "unaligned.h"
 #include "prioritizers/alua_rtpg.h"
 
 int
@@ -848,7 +849,7 @@ sgio_get_vpd (unsigned char * buff, int maxlen, int fd, int pg)
 	}
 retry:
 	if (0 == do_inq(fd, 0, 1, pg, buff, len)) {
-		len = buff[3] + (buff[2] << 8) + 4;
+		len = get_unaligned_be16(&buff[2]) + 4;
 		if (len >= maxlen)
 			return len;
 		if (len > DEFAULT_SGIO_LEN)
@@ -879,7 +880,7 @@ static int
 parse_vpd_pg80(const unsigned char *in, char *out, size_t out_len)
 {
 	char *p = NULL;
-	int len = in[3] + (in[2] << 8);
+	int len = get_unaligned_be16(&in[2]);
 
 	if (len >= out_len) {
 		condlog(2, "vpd pg80 overflow, %d/%d bytes required",
@@ -1079,7 +1080,7 @@ get_vpd_sysfs (struct udev_device *parent, int pg, char * str, int maxlen)
 			pg, buff[1]);
 		return -ENODATA;
 	}
-	buff_len = (buff[2] << 8) + buff[3] + 4;
+	buff_len = get_unaligned_be16(&buff[2]) + 4;
 	if (buff_len > 4096)
 		condlog(3, "vpd pg%02x page truncated", pg);
 
@@ -1111,7 +1112,7 @@ get_vpd_sgio (int fd, int pg, char * str, int maxlen)
 			pg, buff[1]);
 		return -ENODATA;
 	}
-	buff_len = (buff[2] << 8) + buff[3] + 4;
+	buff_len = get_unaligned_be16(&buff[2]) + 4;
 	if (buff_len > 4096)
 		condlog(3, "vpd pg%02x page truncated", pg);
 
diff --git a/libmultipath/prioritizers/alua_rtpg.c b/libmultipath/prioritizers/alua_rtpg.c
index e9d83286ff16..e43150200ffc 100644
--- a/libmultipath/prioritizers/alua_rtpg.c
+++ b/libmultipath/prioritizers/alua_rtpg.c
@@ -26,6 +26,7 @@
 #include "../structs.h"
 #include "../prio.h"
 #include "../discovery.h"
+#include "../unaligned.h"
 #include "alua_rtpg.h"
 
 #define SENSE_BUFF_LEN  32
@@ -128,7 +129,7 @@ do_inquiry(int fd, int evpd, unsigned int codepage,
 		inquiry_command_set_evpd(&cmd);
 		cmd.page = codepage;
 	}
-	set_uint16(cmd.length, resplen);
+	put_unaligned_be16(resplen, cmd.length);
 	PRINT_HEX((unsigned char *) &cmd, sizeof(cmd));
 
 	memset(&hdr, 0, sizeof(hdr));
@@ -220,7 +221,7 @@ get_target_port_group(struct path * pp, unsigned int timeout)
 		if (rc < 0)
 			goto out;
 
-		scsi_buflen = (buf[2] << 8 | buf[3]) + 4;
+		scsi_buflen = get_unaligned_be16(&buf[2]) + 4;
 		/* Paranoia */
 		if (scsi_buflen >= USHRT_MAX)
 			scsi_buflen = USHRT_MAX;
@@ -251,7 +252,7 @@ get_target_port_group(struct path * pp, unsigned int timeout)
 				continue;
 			}
 			p  = (struct vpd83_tpg_dscr *)dscr->data;
-			rc = get_uint16(p->tpg);
+			rc = get_unaligned_be16(p->tpg);
 		}
 	}
 
@@ -274,7 +275,7 @@ do_rtpg(int fd, void* resp, long resplen, unsigned int timeout)
 	memset(&cmd, 0, sizeof(cmd));
 	cmd.op			= OPERATION_CODE_RTPG;
 	rtpg_command_set_service_action(&cmd);
-	set_uint32(cmd.length, resplen);
+	put_unaligned_be32(resplen, cmd.length);
 	PRINT_HEX((unsigned char *) &cmd, sizeof(cmd));
 
 	memset(&hdr, 0, sizeof(hdr));
@@ -321,7 +322,7 @@ get_asymmetric_access_state(int fd, unsigned int tpg, unsigned int timeout)
 	rc = do_rtpg(fd, buf, buflen, timeout);
 	if (rc < 0)
 		goto out;
-	scsi_buflen = (buf[0] << 24 | buf[1] << 16 | buf[2] << 8 | buf[3]) + 4;
+	scsi_buflen = get_unaligned_be32(&buf[0]) + 4;
 	if (scsi_buflen > UINT_MAX)
 		scsi_buflen = UINT_MAX;
 	if (buflen < scsi_buflen) {
@@ -342,7 +343,7 @@ get_asymmetric_access_state(int fd, unsigned int tpg, unsigned int timeout)
 	tpgd = (struct rtpg_data *) buf;
 	rc   = -RTPG_TPG_NOT_FOUND;
 	RTPG_FOR_EACH_PORT_GROUP(tpgd, dscr) {
-		if (get_uint16(dscr->tpg) == tpg) {
+		if (get_unaligned_be16(dscr->tpg) == tpg) {
 			if (rc != -RTPG_TPG_NOT_FOUND) {
 				PRINT_DEBUG("get_asymmetric_access_state: "
 					"more than one entry with same port "
diff --git a/libmultipath/prioritizers/alua_spc3.h b/libmultipath/prioritizers/alua_spc3.h
index 13a0924719b4..58d507a5cbc9 100644
--- a/libmultipath/prioritizers/alua_spc3.h
+++ b/libmultipath/prioritizers/alua_spc3.h
@@ -14,37 +14,6 @@
  */
 #ifndef __SPC3_H__
 #define __SPC3_H__
-/*=============================================================================
- * Some helper functions for getting and setting 16 and 32 bit values.
- *=============================================================================
- */
-static inline unsigned short
-get_uint16(unsigned char *p)
-{
-	return (p[0] << 8) + p[1];
-}
-
-static inline void
-set_uint16(unsigned char *p, unsigned short v)
-{
-	p[0] = (v >> 8) & 0xff;
-	p[1] = v & 0xff;
-}
-
-static inline unsigned int
-get_uint32(unsigned char *p)
-{
-	return (p[0] << 24) + (p[1] << 16) + (p[2] << 8) + p[3];
-}
-
-static inline void
-set_uint32(unsigned char *p, unsigned int v)
-{
-	p[0] = (v >> 24) & 0xff;
-	p[1] = (v >> 16) & 0xff;
-	p[2] = (v >>  8) & 0xff;
-	p[3] = v & 0xff;
-}
 
 /*=============================================================================
  * Definitions to support the standard inquiry command as defined in SPC-3.
@@ -232,7 +201,7 @@ struct vpd83_data {
 		for( \
 			d = p->data; \
 			(((char *) d) - ((char *) p)) < \
-			get_uint16(p->length); \
+			get_unaligned_be16(p->length); \
 			d = (struct vpd83_dscr *) \
 				((char *) d + d->length + 4) \
 		)
@@ -315,7 +284,7 @@ struct rtpg_data {
 #define RTPG_FOR_EACH_PORT_GROUP(p, g) \
 		for( \
 			g = &(p->data[0]); \
-			(((char *) g) - ((char *) p)) < get_uint32(p->length); \
+			(((char *) g) - ((char *) p)) < get_unaligned_be32(p->length); \
 			g = (struct rtpg_tpg_dscr *) ( \
 				((char *) g) + \
 				sizeof(struct rtpg_tpg_dscr) + \
diff --git a/libmultipath/prioritizers/ontap.c b/libmultipath/prioritizers/ontap.c
index ca06d6ce3a9f..6505033f44c9 100644
--- a/libmultipath/prioritizers/ontap.c
+++ b/libmultipath/prioritizers/ontap.c
@@ -22,6 +22,7 @@
 #include "debug.h"
 #include "prio.h"
 #include "structs.h"
+#include "unaligned.h"
 
 #define INQUIRY_CMD	0x12
 #define INQUIRY_CMDLEN	6
@@ -197,8 +198,7 @@ static int ontap_prio(const char *dev, int fd, unsigned int timeout)
 	memset(&results, 0, sizeof (results));
 	rc = send_gva(dev, fd, 0x41, results, &results_size, timeout);
 	if (rc >= 0) {
-		tot_len = results[0] << 24 | results[1] << 16 |
-			  results[2] << 8 | results[3];
+		tot_len = get_unaligned_be32(&results[0]);
 		if (tot_len <= 8) {
 			goto try_fcp_proxy;
 		}
diff --git a/libmultipath/unaligned.h b/libmultipath/unaligned.h
new file mode 100644
index 000000000000..14ec8b23e397
--- /dev/null
+++ b/libmultipath/unaligned.h
@@ -0,0 +1,38 @@
+#ifndef _UNALIGNED_H_
+#define _UNALIGNED_H_
+
+#include <stdint.h>
+
+static inline uint16_t get_unaligned_be16(const void *ptr)
+{
+	const uint8_t *p = ptr;
+
+	return p[0] << 8 | p[1];
+}
+
+static inline uint32_t get_unaligned_be32(void *ptr)
+{
+	const uint8_t *p = ptr;
+
+	return p[0] << 24 | p[1] << 16 | p[2] << 8 | p[3];
+}
+
+static inline void put_unaligned_be16(uint16_t val, void *ptr)
+{
+	uint8_t *p = ptr;
+
+	p[0] = val >> 8;
+	p[1] = val;
+}
+
+static inline void put_unaligned_be32(uint32_t val, void *ptr)
+{
+	uint8_t *p = ptr;
+
+	p[0] = val >> 24;
+	p[1] = val >> 16;
+	p[2] = val >> 8;
+	p[3] = val;
+}
+
+#endif /* _UNALIGNED_H_ */
-- 
2.16.2

Re: [PATCH 6/6] Introduce the ibmultipath/unaligned.h header file

[Martin Wilck]

On Thu, 2018-03-01 at 11:29 -0800, Bart Van Assche wrote:
> This patch avoids that Coverity reports the following for the code
> in libmultipath/prioritizers/alua_rtpg.c:
> 
>    CID 173256:  Integer handling issues  (SIGN_EXTENSION)
>     Suspicious implicit sign extension: "buf[0]" with type "unsigned
> char" (8 bits, unsigned) is promoted in "((buf[0] << 24) | (buf[1] <<
> 16) | (buf[2] << 8) | buf[3]) + 4" to type "int" (32 bits, signed),
> then sign-extended to type "unsigned long" (64 bits, unsigned).  If
> "((buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3]) + 4" is
> greater than 0x7FFFFFFF, the upper bits of the result will all be 1.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>

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

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

Re: [PATCH 0/6] Various multipath-tools patches

[Benjamin Marzinski]

On Thu, Mar 01, 2018 at 11:29:29AM -0800, Bart Van Assche wrote:
> Hello Christophe,
> 
> This series contains the following patches:
> - Four patches address Coverity complaints.
> - One patch makes a function argument const.
> - One patch addresses a bug that I discovered while inspecting the multipath
>   command line tool output.
> 
> Please consider these patches for the multipath-tools project.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

For the set. Buy I agree with Martin that we should fold some of these
into his already posted patches.

-Ben

> 
> Thanks,
> 
> Bart.
> 
> Bart Van Assche (6):
>   multipathd/main.c: Fix indentation
>   libmultipath, alloc_path_with_pathinfo(): Ensure that pp->wwid is
>     '\0'-terminated
>   libmultipath, alloc_path_with_pathinfo(): Declare third argument const
>   kpartx: Improve reliability of find_loop_by_file()
>   libmultipath: Fix sgio_get_vpd()
>   Introduce the ibmultipath/unaligned.h header file
> 
>  kpartx/lopart.c                       |  7 ++++---
>  libmpathpersist/mpath_pr_ioctl.c      |  8 ++++----
>  libmultipath/checkers/hp_sw.c         |  4 ++--
>  libmultipath/discovery.c              | 22 +++++++++++---------
>  libmultipath/discovery.h              |  2 +-
>  libmultipath/prioritizers/alua_rtpg.c | 13 ++++++------
>  libmultipath/prioritizers/alua_spc3.h | 35 ++------------------------------
>  libmultipath/prioritizers/ontap.c     |  4 ++--
>  libmultipath/unaligned.h              | 38 +++++++++++++++++++++++++++++++++++
>  multipathd/main.c                     |  4 ++--
>  10 files changed, 74 insertions(+), 63 deletions(-)
>  create mode 100644 libmultipath/unaligned.h
> 
> -- 
> 2.16.2
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

Re: [PATCH 0/6] Various multipath-tools patches

[Christophe Varoqui]

[-- Attachment #1.1: Type: text/plain, Size: 2091 bytes --]

Hi Bart,

I've finally merged the queued patches, so you can rebase this patchset
over the new head.

Thanks.


On Tue, Mar 6, 2018 at 4:25 PM, Benjamin Marzinski <bmarzins@redhat.com>
wrote:

> On Thu, Mar 01, 2018 at 11:29:29AM -0800, Bart Van Assche wrote:
> > Hello Christophe,
> >
> > This series contains the following patches:
> > - Four patches address Coverity complaints.
> > - One patch makes a function argument const.
> > - One patch addresses a bug that I discovered while inspecting the
> multipath
> >   command line tool output.
> >
> > Please consider these patches for the multipath-tools project.
>
> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
>
> For the set. Buy I agree with Martin that we should fold some of these
> into his already posted patches.
>
> -Ben
>
> >
> > Thanks,
> >
> > Bart.
> >
> > Bart Van Assche (6):
> >   multipathd/main.c: Fix indentation
> >   libmultipath, alloc_path_with_pathinfo(): Ensure that pp->wwid is
> >     '\0'-terminated
> >   libmultipath, alloc_path_with_pathinfo(): Declare third argument const
> >   kpartx: Improve reliability of find_loop_by_file()
> >   libmultipath: Fix sgio_get_vpd()
> >   Introduce the ibmultipath/unaligned.h header file
> >
> >  kpartx/lopart.c                       |  7 ++++---
> >  libmpathpersist/mpath_pr_ioctl.c      |  8 ++++----
> >  libmultipath/checkers/hp_sw.c         |  4 ++--
> >  libmultipath/discovery.c              | 22 +++++++++++---------
> >  libmultipath/discovery.h              |  2 +-
> >  libmultipath/prioritizers/alua_rtpg.c | 13 ++++++------
> >  libmultipath/prioritizers/alua_spc3.h | 35
> ++------------------------------
> >  libmultipath/prioritizers/ontap.c     |  4 ++--
> >  libmultipath/unaligned.h              | 38
> +++++++++++++++++++++++++++++++++++
> >  multipathd/main.c                     |  4 ++--
> >  10 files changed, 74 insertions(+), 63 deletions(-)
> >  create mode 100644 libmultipath/unaligned.h
> >
> > --
> > 2.16.2
> >
> > --
> > dm-devel mailing list
> > dm-devel@redhat.com
> > https://www.redhat.com/mailman/listinfo/dm-devel
>

[-- Attachment #1.2: Type: text/html, Size: 3179 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 0/6] Various multipath-tools patches

[Bart Van Assche]

On Wed, 2018-03-07 at 10:56 +0100, Christophe Varoqui wrote:
> I've finally merged the queued patches, so you can rebase this patchset over the new head.

Hello Christophe,

I will rebase my patch series, retest and repost it.

Bart.