[dm-devel] [PATCH 0/3] multipath: fixes for SAS expanders and root FS access

[dm-devel] [PATCH 0/3] multipath: fixes for SAS expanders and root FS access

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Hi Christophe, hi Ben,

here are 3 patches I'd like to get reviewed before we create a pull
request. The first two are related to complex SAS setups, the second
one is to avoid accessing the root file system in a possible dangerous
situation.

Wrt 2/3: while testing it, I discovered that "I_T_nexus_loss_timeout"
is a read-only sysfs attribute, therefore this code does nothing.
I considered removing it altogether, observing that the attribute has
been read-only as long as it existed (v2.6.17, 2006). I'd like some
feedback beforehand, though, perhaps some distros use patched kernels
that make this attribute r/w?

Regards
Martin

Martin Wilck (3):
  libmultipath: use 3rd digit as transport_id for expanders
  libmultipath: sysfs_set_nexus_loss_tmo(): support SAS expanders
  multipathd: add code to initalize unwinder

 libmultipath/discovery.c   | 35 ++++++++++++++++++++++++++++-------
 multipathd/Makefile        |  2 +-
 multipathd/init_unwinder.c | 34 ++++++++++++++++++++++++++++++++++
 multipathd/init_unwinder.h | 21 +++++++++++++++++++++
 multipathd/main.c          |  2 ++
 5 files changed, 86 insertions(+), 8 deletions(-)
 create mode 100644 multipathd/init_unwinder.c
 create mode 100644 multipathd/init_unwinder.h

-- 
2.29.2


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

[dm-devel] [PATCH 1/3] libmultipath: use 3rd digit as transport_id for expanders

[mwilck]

From: Martin Wilck <mwilck@suse.com>

On SAS expanders, node id's have 3 digits. sysfs paths look like this:

/sys/devices/pci0000:80/0000:80:02.0/0000:8b:00.0/0000:8c:09.0/0000:8f:00.0/host9/port-9:0/expander-9:0/port-9:0:13/expander-9:1/port-9:1:12/expander-9:2/port-9:2:4/end_device-9:2:4/target9:0:29/9:0:29:0/block/sdac

In that case, we should use the last digit as transport id.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/discovery.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index e818585..f3ce3f8 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -358,9 +358,16 @@ sysfs_get_tgt_nodename(struct path *pp, char *node)
 	if (value) {
 		tgtdev = udev_device_get_parent(parent);
 		while (tgtdev) {
+			char c;
+
 			tgtname = udev_device_get_sysname(tgtdev);
-			if (tgtname && sscanf(tgtname, "end_device-%d:%d",
-				   &host, &tgtid) == 2)
+			if (!tgtname)
+				continue;
+			if (sscanf(tgtname, "end_device-%d:%d:%d%c",
+				   &host, &channel, &tgtid, &c) == 3)
+				break;
+			if (sscanf(tgtname, "end_device-%d:%d%c",
+				   &host, &tgtid, &c) == 2)
 				break;
 			tgtdev = udev_device_get_parent(tgtdev);
 			tgtid = -1;
-- 
2.29.2


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

[dm-devel] [PATCH 2/3] libmultipath: sysfs_set_nexus_loss_tmo(): support SAS expanders

[mwilck]

From: Martin Wilck <mwilck@suse.com>

With SAS expanders, SAS node names have 3 digits. libmultipath
would fail to discover the sas_end_device matching a given SCSI
target in this case. Fix it.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/discovery.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index f3ce3f8..7878454 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -789,14 +789,28 @@ sysfs_set_session_tmo(struct multipath *mpp, struct path *pp)
 static void
 sysfs_set_nexus_loss_tmo(struct multipath *mpp, struct path *pp)
 {
-	struct udev_device *sas_dev = NULL;
-	char end_dev_id[64];
+	struct udev_device *parent, *sas_dev = NULL;
+	const char *end_dev_id = NULL;
 	char value[11];
+	static const char ed_str[] = "end_device-";
 
-	if (mpp->dev_loss == DEV_LOSS_TMO_UNSET)
+	if (!pp->udev || mpp->dev_loss == DEV_LOSS_TMO_UNSET)
 		return;
-	sprintf(end_dev_id, "end_device-%d:%d",
-		pp->sg_id.host_no, pp->sg_id.transport_id);
+
+	for (parent = udev_device_get_parent(pp->udev);
+	     parent;
+	     parent = udev_device_get_parent(parent)) {
+		const char *ed = udev_device_get_sysname(parent);
+
+		if (!strncmp(ed, ed_str, sizeof(ed_str) - 1)) {
+			end_dev_id = ed;
+			break;
+		}
+	}
+	if (!end_dev_id) {
+		condlog(1, "%s: No SAS end device", pp->dev);
+		return;
+	}
 	sas_dev = udev_device_new_from_subsystem_sysname(udev,
 				"sas_end_device", end_dev_id);
 	if (!sas_dev) {
-- 
2.29.2


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

[dm-devel] [PATCH 3/3] multipathd: add code to initalize unwinder

[mwilck]

From: Martin Wilck <mwilck@suse.com>

glibc's implementation of pthread_cancel() loads symbols from
libgcc_s.so using dlopen() when pthread_cancel() is called
for the first time. This happens even with LD_BIND_NOW=1.
This may imply the need for file system access when a thread is
cancelled, which in the case of multipath-tools might be in a
dangerous situation where multipathd must avoid blocking.

Call load_unwinder() during startup to make sure the dynamic
linker has all necessary symbols resolved early on.

This implementation simply creates a dummy thread and cancels
it. This way all necessary symbols for thread cancellation
will be loaded, no matter what the C library needs to implement
cancellation.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/Makefile        |  2 +-
 multipathd/init_unwinder.c | 34 ++++++++++++++++++++++++++++++++++
 multipathd/init_unwinder.h | 21 +++++++++++++++++++++
 multipathd/main.c          |  2 ++
 4 files changed, 58 insertions(+), 1 deletion(-)
 create mode 100644 multipathd/init_unwinder.c
 create mode 100644 multipathd/init_unwinder.h

diff --git a/multipathd/Makefile b/multipathd/Makefile
index 632b82b..d053c1e 100644
--- a/multipathd/Makefile
+++ b/multipathd/Makefile
@@ -30,7 +30,7 @@ ifeq ($(ENABLE_DMEVENTS_POLL),0)
 endif
 
 OBJS = main.o pidfile.o uxlsnr.o uxclnt.o cli.o cli_handlers.o waiter.o \
-       dmevents.o
+       dmevents.o init_unwinder.o
 
 EXEC = multipathd
 
diff --git a/multipathd/init_unwinder.c b/multipathd/init_unwinder.c
new file mode 100644
index 0000000..14467f3
--- /dev/null
+++ b/multipathd/init_unwinder.c
@@ -0,0 +1,34 @@
+#include <pthread.h>
+#include <unistd.h>
+#include "init_unwinder.h"
+
+static pthread_mutex_t dummy_mtx = PTHREAD_MUTEX_INITIALIZER;
+static pthread_cond_t dummy_cond = PTHREAD_COND_INITIALIZER;
+
+static void *dummy_thread(void *arg __attribute__((unused)))
+{
+	pthread_mutex_lock(&dummy_mtx);
+	pthread_cond_broadcast(&dummy_cond);
+	pthread_mutex_unlock(&dummy_mtx);
+	pause();
+	return NULL;
+}
+
+int init_unwinder(void)
+{
+	pthread_t dummy;
+	int rc;
+
+	pthread_mutex_lock(&dummy_mtx);
+
+	rc = pthread_create(&dummy, NULL, dummy_thread, NULL);
+	if (rc != 0) {
+		pthread_mutex_unlock(&dummy_mtx);
+		return rc;
+	}
+
+	pthread_cond_wait(&dummy_cond, &dummy_mtx);
+	pthread_mutex_unlock(&dummy_mtx);
+
+	return pthread_cancel(dummy);
+}
diff --git a/multipathd/init_unwinder.h b/multipathd/init_unwinder.h
new file mode 100644
index 0000000..ada09f8
--- /dev/null
+++ b/multipathd/init_unwinder.h
@@ -0,0 +1,21 @@
+#ifndef _INIT_UNWINDER_H
+#define _INIT_UNWINDER_H 1
+
+/*
+ * init_unwinder(): make sure unwinder symbols are loaded
+ *
+ * libc's implementation of pthread_cancel() loads symbols from
+ * libgcc_s.so using dlopen() when pthread_cancel() is called
+ * for the first time. This happens even with LD_BIND_NOW=1.
+ * This may imply the need for file system access when a thread is
+ * cancelled, which in the case of multipath-tools might be in a
+ * dangerous situation where multipathd must avoid blocking.
+ *
+ * Call load_unwinder() during startup to make sure the dynamic
+ * linker has all necessary symbols resolved early on.
+ *
+ * Return: 0 if successful, an error number otherwise.
+ */
+int init_unwinder(void);
+
+#endif
diff --git a/multipathd/main.c b/multipathd/main.c
index 99a89a6..6f851ae 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -83,6 +83,7 @@
 #include "wwids.h"
 #include "foreign.h"
 #include "../third-party/valgrind/drd.h"
+#include "init_unwinder.h"
 
 #define FILE_NAME_SIZE 256
 #define CMDSIZE 160
@@ -3041,6 +3042,7 @@ child (__attribute__((unused)) void *param)
 	enum daemon_status state;
 	int exit_code = 1;
 
+	init_unwinder();
 	mlockall(MCL_CURRENT | MCL_FUTURE);
 	signal_init();
 	mp_rcu_data = setup_rcu();
-- 
2.29.2


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

Re: [dm-devel] [PATCH 1/3] libmultipath: use 3rd digit as transport_id for expanders

[Benjamin Marzinski]

On Thu, Jan 28, 2021 at 09:45:42PM +0100, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> On SAS expanders, node id's have 3 digits. sysfs paths look like this:
> 
> /sys/devices/pci0000:80/0000:80:02.0/0000:8b:00.0/0000:8c:09.0/0000:8f:00.0/host9/port-9:0/expander-9:0/port-9:0:13/expander-9:1/port-9:1:12/expander-9:2/port-9:2:4/end_device-9:2:4/target9:0:29/9:0:29:0/block/sdac
> 
> In that case, we should use the last digit as transport id.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmultipath/discovery.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index e818585..f3ce3f8 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -358,9 +358,16 @@ sysfs_get_tgt_nodename(struct path *pp, char *node)
>  	if (value) {
>  		tgtdev = udev_device_get_parent(parent);
>  		while (tgtdev) {
> +			char c;
> +
>  			tgtname = udev_device_get_sysname(tgtdev);
> -			if (tgtname && sscanf(tgtname, "end_device-%d:%d",
> -				   &host, &tgtid) == 2)
> +			if (!tgtname)
> +				continue;

won't this make and endless loop if tgtname == NULL

-Ben

> +			if (sscanf(tgtname, "end_device-%d:%d:%d%c",
> +				   &host, &channel, &tgtid, &c) == 3)
> +				break;
> +			if (sscanf(tgtname, "end_device-%d:%d%c",
> +				   &host, &tgtid, &c) == 2)
>  				break;
>  			tgtdev = udev_device_get_parent(tgtdev);
>  			tgtid = -1;
> -- 
> 2.29.2

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

Re: [dm-devel] [PATCH 0/3] multipath: fixes for SAS expanders and root FS access

[Benjamin Marzinski]

On Thu, Jan 28, 2021 at 09:45:41PM +0100, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> Hi Christophe, hi Ben,
> 
> here are 3 patches I'd like to get reviewed before we create a pull
> request. The first two are related to complex SAS setups, the second
> one is to avoid accessing the root file system in a possible dangerous
> situation.
> 
> Wrt 2/3: while testing it, I discovered that "I_T_nexus_loss_timeout"
> is a read-only sysfs attribute, therefore this code does nothing.
> I considered removing it altogether, observing that the attribute has
> been read-only as long as it existed (v2.6.17, 2006). I'd like some
> feedback beforehand, though, perhaps some distros use patched kernels
> that make this attribute r/w?

I_T_nexus_loss_timeout appears to have always been read-only on RHEL and
Fedora.

-Ben

> 
> Regards
> Martin
> 
> Martin Wilck (3):
>   libmultipath: use 3rd digit as transport_id for expanders
>   libmultipath: sysfs_set_nexus_loss_tmo(): support SAS expanders
>   multipathd: add code to initalize unwinder
> 
>  libmultipath/discovery.c   | 35 ++++++++++++++++++++++++++++-------
>  multipathd/Makefile        |  2 +-
>  multipathd/init_unwinder.c | 34 ++++++++++++++++++++++++++++++++++
>  multipathd/init_unwinder.h | 21 +++++++++++++++++++++
>  multipathd/main.c          |  2 ++
>  5 files changed, 86 insertions(+), 8 deletions(-)
>  create mode 100644 multipathd/init_unwinder.c
>  create mode 100644 multipathd/init_unwinder.h
> 
> -- 
> 2.29.2

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

Re: [dm-devel] [PATCH 2/3] libmultipath: sysfs_set_nexus_loss_tmo(): support SAS expanders

[Benjamin Marzinski]

On Thu, Jan 28, 2021 at 09:45:43PM +0100, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> With SAS expanders, SAS node names have 3 digits. libmultipath
> would fail to discover the sas_end_device matching a given SCSI
> target in this case. Fix it.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/discovery.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index f3ce3f8..7878454 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -789,14 +789,28 @@ sysfs_set_session_tmo(struct multipath *mpp, struct path *pp)
>  static void
>  sysfs_set_nexus_loss_tmo(struct multipath *mpp, struct path *pp)
>  {
> -	struct udev_device *sas_dev = NULL;
> -	char end_dev_id[64];
> +	struct udev_device *parent, *sas_dev = NULL;
> +	const char *end_dev_id = NULL;
>  	char value[11];
> +	static const char ed_str[] = "end_device-";
>  
> -	if (mpp->dev_loss == DEV_LOSS_TMO_UNSET)
> +	if (!pp->udev || mpp->dev_loss == DEV_LOSS_TMO_UNSET)
>  		return;
> -	sprintf(end_dev_id, "end_device-%d:%d",
> -		pp->sg_id.host_no, pp->sg_id.transport_id);
> +
> +	for (parent = udev_device_get_parent(pp->udev);
> +	     parent;
> +	     parent = udev_device_get_parent(parent)) {
> +		const char *ed = udev_device_get_sysname(parent);
> +
> +		if (!strncmp(ed, ed_str, sizeof(ed_str) - 1)) {
> +			end_dev_id = ed;
> +			break;
> +		}
> +	}
> +	if (!end_dev_id) {
> +		condlog(1, "%s: No SAS end device", pp->dev);
> +		return;
> +	}
>  	sas_dev = udev_device_new_from_subsystem_sysname(udev,
>  				"sas_end_device", end_dev_id);
>  	if (!sas_dev) {
> -- 
> 2.29.2

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

Re: [dm-devel] [PATCH 3/3] multipathd: add code to initalize unwinder

[Benjamin Marzinski]

On Thu, Jan 28, 2021 at 09:45:44PM +0100, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> glibc's implementation of pthread_cancel() loads symbols from
> libgcc_s.so using dlopen() when pthread_cancel() is called
> for the first time. This happens even with LD_BIND_NOW=1.
> This may imply the need for file system access when a thread is
> cancelled, which in the case of multipath-tools might be in a
> dangerous situation where multipathd must avoid blocking.
> 
> Call load_unwinder() during startup to make sure the dynamic
> linker has all necessary symbols resolved early on.
> 
> This implementation simply creates a dummy thread and cancels
> it. This way all necessary symbols for thread cancellation
> will be loaded, no matter what the C library needs to implement
> cancellation.
> 
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  multipathd/Makefile        |  2 +-
>  multipathd/init_unwinder.c | 34 ++++++++++++++++++++++++++++++++++
>  multipathd/init_unwinder.h | 21 +++++++++++++++++++++
>  multipathd/main.c          |  2 ++
>  4 files changed, 58 insertions(+), 1 deletion(-)
>  create mode 100644 multipathd/init_unwinder.c
>  create mode 100644 multipathd/init_unwinder.h
> 
> diff --git a/multipathd/Makefile b/multipathd/Makefile
> index 632b82b..d053c1e 100644
> --- a/multipathd/Makefile
> +++ b/multipathd/Makefile
> @@ -30,7 +30,7 @@ ifeq ($(ENABLE_DMEVENTS_POLL),0)
>  endif
>  
>  OBJS = main.o pidfile.o uxlsnr.o uxclnt.o cli.o cli_handlers.o waiter.o \
> -       dmevents.o
> +       dmevents.o init_unwinder.o
>  
>  EXEC = multipathd
>  
> diff --git a/multipathd/init_unwinder.c b/multipathd/init_unwinder.c
> new file mode 100644
> index 0000000..14467f3
> --- /dev/null
> +++ b/multipathd/init_unwinder.c
> @@ -0,0 +1,34 @@
> +#include <pthread.h>
> +#include <unistd.h>
> +#include "init_unwinder.h"
> +
> +static pthread_mutex_t dummy_mtx = PTHREAD_MUTEX_INITIALIZER;
> +static pthread_cond_t dummy_cond = PTHREAD_COND_INITIALIZER;
> +
> +static void *dummy_thread(void *arg __attribute__((unused)))
> +{
> +	pthread_mutex_lock(&dummy_mtx);
> +	pthread_cond_broadcast(&dummy_cond);
> +	pthread_mutex_unlock(&dummy_mtx);
> +	pause();
> +	return NULL;
> +}
> +
> +int init_unwinder(void)
> +{
> +	pthread_t dummy;
> +	int rc;
> +
> +	pthread_mutex_lock(&dummy_mtx);
> +
> +	rc = pthread_create(&dummy, NULL, dummy_thread, NULL);
> +	if (rc != 0) {
> +		pthread_mutex_unlock(&dummy_mtx);
> +		return rc;
> +	}
> +
> +	pthread_cond_wait(&dummy_cond, &dummy_mtx);
> +	pthread_mutex_unlock(&dummy_mtx);
> +
> +	return pthread_cancel(dummy);
> +}
> diff --git a/multipathd/init_unwinder.h b/multipathd/init_unwinder.h
> new file mode 100644
> index 0000000..ada09f8
> --- /dev/null
> +++ b/multipathd/init_unwinder.h
> @@ -0,0 +1,21 @@
> +#ifndef _INIT_UNWINDER_H
> +#define _INIT_UNWINDER_H 1
> +
> +/*
> + * init_unwinder(): make sure unwinder symbols are loaded
> + *
> + * libc's implementation of pthread_cancel() loads symbols from
> + * libgcc_s.so using dlopen() when pthread_cancel() is called
> + * for the first time. This happens even with LD_BIND_NOW=1.
> + * This may imply the need for file system access when a thread is
> + * cancelled, which in the case of multipath-tools might be in a
> + * dangerous situation where multipathd must avoid blocking.
> + *
> + * Call load_unwinder() during startup to make sure the dynamic
> + * linker has all necessary symbols resolved early on.
> + *
> + * Return: 0 if successful, an error number otherwise.
> + */
> +int init_unwinder(void);
> +
> +#endif
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 99a89a6..6f851ae 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -83,6 +83,7 @@
>  #include "wwids.h"
>  #include "foreign.h"
>  #include "../third-party/valgrind/drd.h"
> +#include "init_unwinder.h"
>  
>  #define FILE_NAME_SIZE 256
>  #define CMDSIZE 160
> @@ -3041,6 +3042,7 @@ child (__attribute__((unused)) void *param)
>  	enum daemon_status state;
>  	int exit_code = 1;
>  
> +	init_unwinder();
>  	mlockall(MCL_CURRENT | MCL_FUTURE);
>  	signal_init();
>  	mp_rcu_data = setup_rcu();
> -- 
> 2.29.2

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

Re: [dm-devel] [PATCH 1/3] libmultipath: use 3rd digit as transport_id for expanders

[Martin Wilck]

On Mon, 2021-02-01 at 20:22 -0600, Benjamin Marzinski wrote:
> On Thu, Jan 28, 2021 at 09:45:42PM +0100, mwilck@suse.com wrote:
> > From: Martin Wilck <mwilck@suse.com>
> > 
> > On SAS expanders, node id's have 3 digits. sysfs paths look like
> > this:
> > 
> > /sys/devices/pci0000:80/0000:80:02.0/0000:8b:00.0/0000:8c:09.0/0000
> > :8f:00.0/host9/port-9:0/expander-9:0/port-9:0:13/expander-9:1/port-
> > 9:1:12/expander-9:2/port-9:2:4/end_device-
> > 9:2:4/target9:0:29/9:0:29:0/block/sdac
> > 
> > In that case, we should use the last digit as transport id.
> > 
> > Signed-off-by: Martin Wilck <mwilck@suse.com>
> > ---
> >  libmultipath/discovery.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> > index e818585..f3ce3f8 100644
> > --- a/libmultipath/discovery.c
> > +++ b/libmultipath/discovery.c
> > @@ -358,9 +358,16 @@ sysfs_get_tgt_nodename(struct path *pp, char
> > *node)
> >         if (value) {
> >                 tgtdev = udev_device_get_parent(parent);
> >                 while (tgtdev) {
> > +                       char c;
> > +
> >                         tgtname = udev_device_get_sysname(tgtdev);
> > -                       if (tgtname && sscanf(tgtname, "end_device-
> > %d:%d",
> > -                                  &host, &tgtid) == 2)
> > +                       if (!tgtname)
> > +                               continue;
> 
> won't this make and endless loop if tgtname == NULL

Ouch. Thanks for spotting this.

Martin




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

Re: [dm-devel] [PATCH 0/3] multipath: fixes for SAS expanders and root FS access

[Martin Wilck]

On Mon, 2021-02-01 at 21:15 -0600, Benjamin Marzinski wrote:
> On Thu, Jan 28, 2021 at 09:45:41PM +0100, mwilck@suse.com wrote:
> > From: Martin Wilck <mwilck@suse.com>
> > 
> > Hi Christophe, hi Ben,
> > 
> > here are 3 patches I'd like to get reviewed before we create a pull
> > request. The first two are related to complex SAS setups, the
> > second
> > one is to avoid accessing the root file system in a possible
> > dangerous
> > situation.
> > 
> > Wrt 2/3: while testing it, I discovered that
> > "I_T_nexus_loss_timeout"
> > is a read-only sysfs attribute, therefore this code does nothing.
> > I considered removing it altogether, observing that the attribute
> > has
> > been read-only as long as it existed (v2.6.17, 2006). I'd like some
> > feedback beforehand, though, perhaps some distros use patched
> > kernels
> > that make this attribute r/w?
> 
> I_T_nexus_loss_timeout appears to have always been read-only on RHEL
> and
> Fedora.

Thanks. Anyway, given that we're preparing a submission to Christophe,
I'd like to give people more time for responding. We can remove this
code later.

Regards
Martin



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