[dm-devel] [PATCH 0/5] Fixes for musl libc / alpine

[dm-devel] [PATCH 0/5] Fixes for musl libc / alpine

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Hi Christophe, Ben, all,

this series of patches fixes the compilation and unit-test
problems of latest upstream with musl libc.

Regards,
Martin

Martin Wilck (5):
  libmultipath: make sure __GLIBC_PREREQ() is defined
  multipath-tools: fix -Wformat errors with musl libc
  libmultipath: always use glibc basename()
  libmultipath tests: fix strerror() difference between musl and glibc
  multipath-tools tests: fix for busybox

 libmultipath/util.c | 19 +++++++++++++++++--
 libmultipath/util.h |  3 +++
 multipathd/waiter.c |  2 +-
 tests/Makefile      |  4 ++--
 tests/alias.c       |  2 +-
 tests/test-log.c    |  5 ++++-
 6 files changed, 28 insertions(+), 7 deletions(-)

-- 
2.29.0


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

[dm-devel] [PATCH 1/5] libmultipath: make sure __GLIBC_PREREQ() is defined

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Otherwise complilation fails on non-glibc systems.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/util.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libmultipath/util.h b/libmultipath/util.h
index 0f0f6cb..2b9703a 100644
--- a/libmultipath/util.h
+++ b/libmultipath/util.h
@@ -55,6 +55,9 @@ struct scandir_result {
 };
 void free_scandir_result(struct scandir_result *);
 
+#ifndef __GLIBC_PREREQ
+#define __GLIBC_PREREQ(x, y) 0
+#endif
 /*
  * ffsll() is also available on glibc < 2.27 if _GNU_SOURCE is defined.
  * But relying on that would require that every program using this header file
-- 
2.29.0


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

[dm-devel] [PATCH 2/5] multipath-tools: fix -Wformat errors with musl libc

[mwilck]

From: Martin Wilck <mwilck@suse.com>

rlim_t type is different on musl libc. A cast to unsigned long
should be fine. Also, in musl, pthread_t is a pointer.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/util.c | 6 ++++--
 multipathd/waiter.c | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/libmultipath/util.c b/libmultipath/util.c
index 1dad90f..39ccace 100644
--- a/libmultipath/util.c
+++ b/libmultipath/util.c
@@ -378,11 +378,13 @@ void set_max_fds(rlim_t max_fds)
 		if (setrlimit(RLIMIT_NOFILE, &fd_limit) < 0) {
 			condlog(0, "can't set open fds limit to "
 				"%lu/%lu : %s",
-				fd_limit.rlim_cur, fd_limit.rlim_max,
+				(unsigned long)fd_limit.rlim_cur,
+				(unsigned long)fd_limit.rlim_max,
 				strerror(errno));
 		} else {
 			condlog(3, "set open fds limit to %lu/%lu",
-				fd_limit.rlim_cur, fd_limit.rlim_max);
+				(unsigned long)fd_limit.rlim_cur,
+				(unsigned long)fd_limit.rlim_max);
 		}
 	}
 }
diff --git a/multipathd/waiter.c b/multipathd/waiter.c
index 16fbdc8..3bc6980 100644
--- a/multipathd/waiter.c
+++ b/multipathd/waiter.c
@@ -64,7 +64,7 @@ void stop_waiter_thread (struct multipath *mpp)
 		return;
 
 	condlog(3, "%s: stop event checker thread (%lu)", mpp->alias,
-		mpp->waiter);
+		(unsigned long)mpp->waiter);
 	thread = mpp->waiter;
 	mpp->waiter = (pthread_t)0;
 	pthread_cleanup_push(cleanup_lock, &waiter_lock);
-- 
2.29.0


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

[dm-devel] [PATCH 3/5] libmultipath: always use glibc basename()

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Our code relies on the non-destructive behavior of glibc's basename().
Fortunately that function is very simple, and our unit test makes
it easy to verify its functionality.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/util.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/libmultipath/util.c b/libmultipath/util.c
index 39ccace..1748eaf 100644
--- a/libmultipath/util.c
+++ b/libmultipath/util.c
@@ -34,6 +34,19 @@ strchop(char *str)
 	return i;
 }
 
+#ifndef __GLIBC__
+/*
+ * glibc's non-destructive version of basename()
+ * License: LGPL-2.1-or-later
+ */
+static const char *__basename(const char *filename)
+{
+	char *p = strrchr(filename, '/');
+	return p ? p + 1 : filename;
+}
+#define basename(x) __basename(x)
+#endif
+
 int
 basenamecpy (const char *src, char *dst, size_t size)
 {
-- 
2.29.0


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

[dm-devel] [PATCH 4/5] libmultipath tests: fix strerror() difference between musl and glibc

[mwilck]

From: Martin Wilck <mwilck@suse.com>

If an error occurs with errno=0, strerror() on musl returns a different
string than "Success". Make sure the test doesn't fail for that reason.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 tests/alias.c    | 2 +-
 tests/test-log.c | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/alias.c b/tests/alias.c
index 5624138..7fda679 100644
--- a/tests/alias.c
+++ b/tests/alias.c
@@ -712,7 +712,7 @@ static void al_write_err(void **state)
 	will_return(__wrap_write, strlen(ln) - 1);
 	expect_value(__wrap_ftruncate, length, offset);
 	will_return(__wrap_ftruncate, 0);
-	expect_condlog(0, "Cannot write binding to bindings file : Success\n");
+	expect_condlog(0, "Cannot write binding to bindings file :");
 
 	alias = allocate_binding(0, "WWIDa", 1, "MPATH");
 	assert_ptr_equal(alias, NULL);
diff --git a/tests/test-log.c b/tests/test-log.c
index 051491e..1c901cb 100644
--- a/tests/test-log.c
+++ b/tests/test-log.c
@@ -2,6 +2,7 @@
 #include <stddef.h>
 #include <stdarg.h>
 #include <stdio.h>
+#include <string.h>
 #include <cmocka.h>
 #include "log.h"
 #include "test-log.h"
@@ -11,12 +12,14 @@ void __wrap_dlog (int sink, int prio, const char * fmt, ...)
 {
 	char buff[MAX_MSG_SIZE];
 	va_list ap;
+	char *expected;
 
 	check_expected(prio);
 	va_start(ap, fmt);
 	vsnprintf(buff, MAX_MSG_SIZE, fmt, ap);
 	va_end(ap);
-	assert_string_equal(buff, mock_ptr_type(char *));
+	expected = mock_ptr_type(char *);
+	assert_memory_equal(buff, expected, strlen(expected));
 }
 
 void expect_condlog(int prio, char *string)
-- 
2.29.0


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

[dm-devel] [PATCH 5/5] multipath-tools tests: fix for busybox

[mwilck]

From: Martin Wilck <mwilck@suse.com>

busybox' "ln" command doesn't support the "-t" syntax of GNU ln.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 tests/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/Makefile b/tests/Makefile
index 502377f..3bacabd 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -62,8 +62,8 @@ endif
 	$(CC) $(CFLAGS) $($*-test_FLAGS) -c -o $@ $<
 
 lib/libchecktur.so:
-	mkdir lib
-	ln -t lib ../libmultipath/{checkers,prioritizers,foreign}/*.so
+	mkdir -p lib
+	ln ../libmultipath/*/*.so lib
 
 %.out:	%-test lib/libchecktur.so
 	@echo == running $< ==
-- 
2.29.0


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

Re: [dm-devel] [PATCH 1/5] libmultipath: make sure __GLIBC_PREREQ() is defined

[Xose Vazquez Perez]

On 10/27/20 11:45 PM, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> Otherwise complilation fails on non-glibc systems.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>

Tested-by: Xose Vazquez Perez <xose.vazquez@gmail.com>

> ---
>   libmultipath/util.h | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/libmultipath/util.h b/libmultipath/util.h
> index 0f0f6cb..2b9703a 100644
> --- a/libmultipath/util.h
> +++ b/libmultipath/util.h
> @@ -55,6 +55,9 @@ struct scandir_result {
>   };
>   void free_scandir_result(struct scandir_result *);
>   
> +#ifndef __GLIBC_PREREQ
> +#define __GLIBC_PREREQ(x, y) 0
> +#endif
>   /*
>    * ffsll() is also available on glibc < 2.27 if _GNU_SOURCE is defined.
>    * But relying on that would require that every program using this header file
> 

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

Re: [dm-devel] [PATCH 2/5] multipath-tools: fix -Wformat errors with musl libc

[Xose Vazquez Perez]

On 10/27/20 11:45 PM, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> rlim_t type is different on musl libc. A cast to unsigned long
> should be fine. Also, in musl, pthread_t is a pointer.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>

Tested-by: Xose Vazquez Perez <xose.vazquez@gmail.com>

> ---
>   libmultipath/util.c | 6 ++++--
>   multipathd/waiter.c | 2 +-
>   2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/libmultipath/util.c b/libmultipath/util.c
> index 1dad90f..39ccace 100644
> --- a/libmultipath/util.c
> +++ b/libmultipath/util.c
> @@ -378,11 +378,13 @@ void set_max_fds(rlim_t max_fds)
>   		if (setrlimit(RLIMIT_NOFILE, &fd_limit) < 0) {
>   			condlog(0, "can't set open fds limit to "
>   				"%lu/%lu : %s",
> -				fd_limit.rlim_cur, fd_limit.rlim_max,
> +				(unsigned long)fd_limit.rlim_cur,
> +				(unsigned long)fd_limit.rlim_max,
>   				strerror(errno));
>   		} else {
>   			condlog(3, "set open fds limit to %lu/%lu",
> -				fd_limit.rlim_cur, fd_limit.rlim_max);
> +				(unsigned long)fd_limit.rlim_cur,
> +				(unsigned long)fd_limit.rlim_max);
>   		}
>   	}
>   }
> diff --git a/multipathd/waiter.c b/multipathd/waiter.c
> index 16fbdc8..3bc6980 100644
> --- a/multipathd/waiter.c
> +++ b/multipathd/waiter.c
> @@ -64,7 +64,7 @@ void stop_waiter_thread (struct multipath *mpp)
>   		return;
>   
>   	condlog(3, "%s: stop event checker thread (%lu)", mpp->alias,
> -		mpp->waiter);
> +		(unsigned long)mpp->waiter);
>   	thread = mpp->waiter;
>   	mpp->waiter = (pthread_t)0;
>   	pthread_cleanup_push(cleanup_lock, &waiter_lock);
> 

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

Re: [dm-devel] [PATCH 4/5] libmultipath tests: fix strerror() difference between musl and glibc

[Benjamin Marzinski]

On Tue, Oct 27, 2020 at 11:45:35PM +0100, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> If an error occurs with errno=0, strerror() on musl returns a different
> string than "Success". Make sure the test doesn't fail for that reason.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  tests/alias.c    | 2 +-
>  tests/test-log.c | 5 ++++-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/alias.c b/tests/alias.c
> index 5624138..7fda679 100644
> --- a/tests/alias.c
> +++ b/tests/alias.c
> @@ -712,7 +712,7 @@ static void al_write_err(void **state)
>  	will_return(__wrap_write, strlen(ln) - 1);
>  	expect_value(__wrap_ftruncate, length, offset);
>  	will_return(__wrap_ftruncate, 0);
> -	expect_condlog(0, "Cannot write binding to bindings file : Success\n");
> +	expect_condlog(0, "Cannot write binding to bindings file :");
>  
>  	alias = allocate_binding(0, "WWIDa", 1, "MPATH");
>  	assert_ptr_equal(alias, NULL);
> diff --git a/tests/test-log.c b/tests/test-log.c
> index 051491e..1c901cb 100644
> --- a/tests/test-log.c
> +++ b/tests/test-log.c
> @@ -2,6 +2,7 @@
>  #include <stddef.h>
>  #include <stdarg.h>
>  #include <stdio.h>
> +#include <string.h>
>  #include <cmocka.h>
>  #include "log.h"
>  #include "test-log.h"
> @@ -11,12 +12,14 @@ void __wrap_dlog (int sink, int prio, const char * fmt, ...)
>  {
>  	char buff[MAX_MSG_SIZE];
>  	va_list ap;
> +	char *expected;
>  
>  	check_expected(prio);
>  	va_start(ap, fmt);
>  	vsnprintf(buff, MAX_MSG_SIZE, fmt, ap);
>  	va_end(ap);
> -	assert_string_equal(buff, mock_ptr_type(char *));
> +	expected = mock_ptr_type(char *);
> +	assert_memory_equal(buff, expected, strlen(expected));

This looks fine. I'm just wondering the purpose. Is it just to deal with
bad, non-null terminated strings?

-Ben

>  }
>  
>  void expect_condlog(int prio, char *string)
> -- 
> 2.29.0

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

Re: [dm-devel] [PATCH 0/5] Fixes for musl libc / alpine

[Benjamin Marzinski]

On Tue, Oct 27, 2020 at 11:45:31PM +0100, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>

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

for the set.

> 
> Hi Christophe, Ben, all,
> 
> this series of patches fixes the compilation and unit-test
> problems of latest upstream with musl libc.
> 
> Regards,
> Martin
> 
> Martin Wilck (5):
>   libmultipath: make sure __GLIBC_PREREQ() is defined
>   multipath-tools: fix -Wformat errors with musl libc
>   libmultipath: always use glibc basename()
>   libmultipath tests: fix strerror() difference between musl and glibc
>   multipath-tools tests: fix for busybox
> 
>  libmultipath/util.c | 19 +++++++++++++++++--
>  libmultipath/util.h |  3 +++
>  multipathd/waiter.c |  2 +-
>  tests/Makefile      |  4 ++--
>  tests/alias.c       |  2 +-
>  tests/test-log.c    |  5 ++++-
>  6 files changed, 28 insertions(+), 7 deletions(-)
> 
> -- 
> 2.29.0

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

Re: [dm-devel] [PATCH 4/5] libmultipath tests: fix strerror() difference between musl and glibc

[Martin Wilck]

On Mon, 2020-11-02 at 13:58 -0600, Benjamin Marzinski wrote:
> On Tue, Oct 27, 2020 at 11:45:35PM +0100, mwilck@suse.com wrote:
> > 
> >  	check_expected(prio);
> >  	va_start(ap, fmt);
> >  	vsnprintf(buff, MAX_MSG_SIZE, fmt, ap);
> >  	va_end(ap);
> > -	assert_string_equal(buff, mock_ptr_type(char *));
> > +	expected = mock_ptr_type(char *);
> > +	assert_memory_equal(buff, expected, strlen(expected));
> 
> This looks fine. I'm just wondering the purpose. Is it just to deal
> with
> bad,non-null terminated strings? 

No. In the case at hand, the error strings returned by the C library,
which are at the tail of the log message in this case, were different.
We're looking at an errno==0 case, where glibc returns "Success",
whereas musl returned something else (like "undefined error" or so, I
forgot the exact wording).

The downside of this patch is that if a mismatch occurs,
assert_string_equal() prints the two different strings, whereas
assert_memory_equal() prints differing offsets and values in hex, which
makes it more difficult to assess the cause of the mismatch.

Regards,
Martin


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