[PATCH 0/5] More misc patches

[PATCH 0/5] More misc patches

[Benjamin Marzinski]

Here are a couple of bug fixes and cleanups. Since nobody complained
about the DELL MD3xxx merge, I put that in as well. I've left out the
multipath.rules change from before, since that one shouldn't go in
without an explicit ACK from other maintainers.

Benjamin Marzinski (5):
  multipath: Merge the DELL MD3xxx device configs
  multipath: fix up position independent code
  libmultipath: fix partition detection
  kpartx: default to running in sync mode
  libmultipath: force udev reloads

 Makefile.inc                       |  6 +++-
 kpartx/Makefile                    |  3 +-
 kpartx/kpartx.c                    | 10 +++++--
 kpartx/kpartx.rules                |  2 +-
 libdmmp/Makefile                   |  2 +-
 libmpathcmd/Makefile               |  2 ++
 libmpathpersist/Makefile           |  2 +-
 libmultipath/Makefile              |  2 +-
 libmultipath/checkers/Makefile     |  2 +-
 libmultipath/configure.c           |  7 ++++-
 libmultipath/devmapper.c           | 56 ++++++++++++++++++--------------------
 libmultipath/hwtable.c             | 26 +-----------------
 libmultipath/prioritizers/Makefile |  2 +-
 libmultipath/structs.h             |  1 +
 mpathpersist/Makefile              |  3 +-
 multipath/Makefile                 |  4 +--
 multipathd/Makefile                |  6 ++--
 17 files changed, 63 insertions(+), 73 deletions(-)

-- 
1.8.3.1

[PATCH 1/5] multipath: Merge the DELL MD3xxx device configs

[Benjamin Marzinski]

All of the Dell MD3xxx configs are identical, so we can just use one
config for them.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/hwtable.c | 26 +-------------------------
 1 file changed, 1 insertion(+), 25 deletions(-)

diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
index c944015..54309ef 100644
--- a/libmultipath/hwtable.c
+++ b/libmultipath/hwtable.c
@@ -227,31 +227,7 @@ static struct hwentry default_hw[] = {
 		/* MD Series */
 	{
 		.vendor        = "DELL",
-		.product       = "MD3000",
-		.bl_product    = "Universal Xport",
-		.pgpolicy      = GROUP_BY_PRIO,
-		.checker_name  = RDAC,
-		.features      = "2 pg_init_retries 50",
-		.hwhandler     = "1 rdac",
-		.prio_name     = PRIO_RDAC,
-		.pgfailback    = -FAILBACK_IMMEDIATE,
-		.no_path_retry = 30,
-	},
-	{
-		.vendor        = "DELL",
-		.product       = "(MD32xx|MD36xx)",
-		.bl_product    = "Universal Xport",
-		.pgpolicy      = GROUP_BY_PRIO,
-		.checker_name  = RDAC,
-		.features      = "2 pg_init_retries 50",
-		.hwhandler     = "1 rdac",
-		.prio_name     = PRIO_RDAC,
-		.pgfailback    = -FAILBACK_IMMEDIATE,
-		.no_path_retry = 30,
-	},
-	{
-		.vendor        = "DELL",
-		.product       = "(MD34xx|MD38xx)",
+		.product       = "^MD3",
 		.bl_product    = "Universal Xport",
 		.pgpolicy      = GROUP_BY_PRIO,
 		.checker_name  = RDAC,
-- 
1.8.3.1

[PATCH 2/5] multipath: fix up position independent code

[Benjamin Marzinski]

The multipath binaries were not being compiled as position independent
executables (PIE).  This code fixes that, and makes other minor code
hardening tweaks to make hardening-check happier.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 Makefile.inc                       | 6 +++++-
 kpartx/Makefile                    | 3 ++-
 libdmmp/Makefile                   | 2 +-
 libmpathcmd/Makefile               | 2 ++
 libmpathpersist/Makefile           | 2 +-
 libmultipath/Makefile              | 2 +-
 libmultipath/checkers/Makefile     | 2 +-
 libmultipath/prioritizers/Makefile | 2 +-
 mpathpersist/Makefile              | 3 ++-
 multipath/Makefile                 | 4 ++--
 multipathd/Makefile                | 6 +++---
 11 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/Makefile.inc b/Makefile.inc
index 8361e6c..9f2f963 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -75,8 +75,12 @@ OPTFLAGS	= -O2 -g -pipe -Wall -Wextra -Wformat=2 -Werror=implicit-int \
 		  -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-strong \
 		  --param=ssp-buffer-size=4
 
-CFLAGS		= $(OPTFLAGS) -fPIC -DLIB_STRING=\"${LIB}\" -DRUN_DIR=\"${RUN}\"
+CFLAGS		= $(OPTFLAGS) -DLIB_STRING=\"${LIB}\" -DRUN_DIR=\"${RUN}\"
+BIN_CFLAGS	= -fPIE -DPIE
+LIB_CFLAGS	= -fPIC
 SHARED_FLAGS	= -shared
+LDFLAGS		= -Wl,-z,relro -Wl,-z,now
+BIN_LDFLAGS	= -pie
 
 # Check whether a function with name $1 has been declared in header file $2.
 check_func =								       \
diff --git a/kpartx/Makefile b/kpartx/Makefile
index 9441a2b..7b75032 100644
--- a/kpartx/Makefile
+++ b/kpartx/Makefile
@@ -3,7 +3,8 @@
 #
 include ../Makefile.inc
 
-CFLAGS += -I. -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64
+CFLAGS += $(BIN_CFLAGS) -I. -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64
+LDFLAGS += $(BIN_LDFLAGS)
 
 LIBDEPS += -ldevmapper
 
diff --git a/libdmmp/Makefile b/libdmmp/Makefile
index 1c5329a..a260871 100644
--- a/libdmmp/Makefile
+++ b/libdmmp/Makefile
@@ -15,7 +15,7 @@ HEADERS = libdmmp/libdmmp.h
 
 OBJS = libdmmp.o libdmmp_mp.o libdmmp_pg.o libdmmp_path.o libdmmp_misc.o
 
-CFLAGS += -fvisibility=hidden -I$(libdmmpdir) -I$(mpathcmddir) \
+CFLAGS += $(LIB_CFLAGS) -fvisibility=hidden -I$(libdmmpdir) -I$(mpathcmddir) \
 	  $(shell pkg-config --cflags json-c)
 
 LIBDEPS += $(shell pkg-config --libs json-c) -L$(mpathcmddir) -lmpathcmd -lpthread
diff --git a/libmpathcmd/Makefile b/libmpathcmd/Makefile
index 9cda94c..4f32101 100644
--- a/libmpathcmd/Makefile
+++ b/libmpathcmd/Makefile
@@ -4,6 +4,8 @@ SONAME = 0
 DEVLIB = libmpathcmd.so
 LIBS = $(DEVLIB).$(SONAME)
 
+CFLAGS += $(LIB_CFLAGS)
+
 OBJS = mpath_cmd.o
 
 all: $(LIBS)
diff --git a/libmpathpersist/Makefile b/libmpathpersist/Makefile
index 857c8d8..1b4ec16 100644
--- a/libmpathpersist/Makefile
+++ b/libmpathpersist/Makefile
@@ -4,7 +4,7 @@ SONAME = 0
 DEVLIB = libmpathpersist.so
 LIBS = $(DEVLIB).$(SONAME)
 
-CFLAGS += -I$(multipathdir) -I$(mpathpersistdir) -I$(mpathcmddir)
+CFLAGS += $(LIB_CFLAGS) -I$(multipathdir) -I$(mpathpersistdir) -I$(mpathcmddir)
 
 LIBDEPS += -lpthread -ldevmapper -ldl -L$(multipathdir) -lmultipath \
 	   -L$(mpathcmddir) -lmpathcmd
diff --git a/libmultipath/Makefile b/libmultipath/Makefile
index 1f5ec25..b3244fc 100644
--- a/libmultipath/Makefile
+++ b/libmultipath/Makefile
@@ -7,7 +7,7 @@ SONAME = 0
 DEVLIB = libmultipath.so
 LIBS = $(DEVLIB).$(SONAME)
 
-CFLAGS += -I$(mpathcmddir)
+CFLAGS += $(LIB_CFLAGS) -I$(mpathcmddir)
 
 LIBDEPS += -lpthread -ldl -ldevmapper -ludev -L$(mpathcmddir) -lmpathcmd -lurcu
 
diff --git a/libmultipath/checkers/Makefile b/libmultipath/checkers/Makefile
index 4970fc0..732ca9d 100644
--- a/libmultipath/checkers/Makefile
+++ b/libmultipath/checkers/Makefile
@@ -3,7 +3,7 @@
 #
 include ../../Makefile.inc
 
-CFLAGS += -I..
+CFLAGS += $(LIB_CFLAGS) -I..
 
 # If you add or remove a checker also update multipath/multipath.conf.5
 LIBS= \
diff --git a/libmultipath/prioritizers/Makefile b/libmultipath/prioritizers/Makefile
index 36b42e4..cf6811c 100644
--- a/libmultipath/prioritizers/Makefile
+++ b/libmultipath/prioritizers/Makefile
@@ -3,7 +3,7 @@
 #
 include ../../Makefile.inc
 
-CFLAGS += -I..
+CFLAGS += $(LIB_CFLAGS) -I..
 
 # If you add or remove a prioritizer also update multipath/multipath.conf.5
 LIBS = \
diff --git a/mpathpersist/Makefile b/mpathpersist/Makefile
index 47043bb..bd1c0df 100644
--- a/mpathpersist/Makefile
+++ b/mpathpersist/Makefile
@@ -1,6 +1,7 @@
 include ../Makefile.inc
 
-CFLAGS += -I$(multipathdir) -I$(mpathpersistdir)
+CFLAGS += $(BIN_CFLAGS) -I$(multipathdir) -I$(mpathpersistdir)
+LDFLAGS += $(BIN_LDFLAGS)
 
 LIBDEPS += -lpthread -ldevmapper -L$(mpathpersistdir) -lmpathpersist \
 	   -L$(multipathdir) -L$(mpathcmddir) -lmpathcmd -lmultipath -ludev
diff --git a/multipath/Makefile b/multipath/Makefile
index cad34bf..c85314e 100644
--- a/multipath/Makefile
+++ b/multipath/Makefile
@@ -3,8 +3,8 @@
 #
 include ../Makefile.inc
 
-CFLAGS += -I$(multipathdir) -I$(mpathcmddir)
-
+CFLAGS += $(BIN_CFLAGS) -I$(multipathdir) -I$(mpathcmddir)
+LDFLAGS += $(BIN_LDFLAGS)
 LIBDEPS += -lpthread -ldevmapper -ldl -L$(multipathdir) -lmultipath -ludev \
 	   -L$(mpathcmddir) -lmpathcmd
 
diff --git a/multipathd/Makefile b/multipathd/Makefile
index d57f6d5..d5782a1 100644
--- a/multipathd/Makefile
+++ b/multipathd/Makefile
@@ -6,9 +6,9 @@ include ../Makefile.inc
 #CFLAGS += -DLCKDBG
 #CFLAGS += -D_DEBUG_
 #CFLAGS += -DLOGDBG
-CFLAGS += -I$(multipathdir) -I$(mpathpersistdir) -I$(mpathcmddir) \
-	  -I$(thirdpartydir)
-
+CFLAGS += $(BIN_CFLAGS) -I$(multipathdir) -I$(mpathpersistdir) \
+	  -I$(mpathcmddir) -I$(thirdpartydir)
+LDFLAGS += $(BIN_LDFLAGS)
 LIBDEPS += -ludev -ldl -L$(multipathdir) -lmultipath -L$(mpathpersistdir) \
 	   -lmpathpersist -L$(mpathcmddir) -lmpathcmd -lurcu -lpthread \
 	   -ldevmapper -lreadline
-- 
1.8.3.1

[PATCH 3/5] libmultipath: fix partition detection

[Benjamin Marzinski]

The do_foreach_partmaps code could incorrectly list a device as a
partition of another device, because of two issues.  First, the check to
compare the dm UUID of two devices would allow two partition devices to
match, or a partition device to match with itself, instead of only
having a partition device match with the multipath device that it
belongs to.  Second, the code to check if a multipath device's
major:minor appeared in a partition device's table only used strstr
to check of the existance of the major:minor string. This meant that
any device with a minor number that started with the same digits would
match. for instance, checking for "253:10" would also match "253:102".

This patch fixes these issues.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/devmapper.c | 53 ++++++++++++++++++++++--------------------------
 1 file changed, 24 insertions(+), 29 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 5fb9d9a..c7602cb 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -493,42 +493,35 @@ uuidout:
 
 int dm_get_uuid(char *name, char *uuid)
 {
-	char uuidtmp[WWID_SIZE];
-
-	if (dm_get_prefixed_uuid(name, uuidtmp))
+	if (dm_get_prefixed_uuid(name, uuid))
 		return 1;
 
-	if (!strncmp(uuidtmp, UUID_PREFIX, UUID_PREFIX_LEN))
-		strcpy(uuid, uuidtmp + UUID_PREFIX_LEN);
-	else
-		strcpy(uuid, uuidtmp);
-
+	if (!strncmp(uuid, UUID_PREFIX, UUID_PREFIX_LEN))
+		memmove(uuid, uuid + UUID_PREFIX_LEN,
+			strlen(uuid + UUID_PREFIX_LEN) + 1);
 	return 0;
 }
 
-/*
- * returns:
- *    0 : if both uuids end with same suffix which starts with UUID_PREFIX
- *    1 : otherwise
- */
-int
-dm_compare_uuid(const char* mapname1, const char* mapname2)
+static int
+is_mpath_part(const char *part_name, const char *map_name)
 {
-	char *p1, *p2;
-	char uuid1[WWID_SIZE], uuid2[WWID_SIZE];
+	char *p;
+	char part_uuid[WWID_SIZE], map_uuid[WWID_SIZE];
 
-	if (dm_get_prefixed_uuid(mapname1, uuid1))
-		return 1;
+	if (dm_get_prefixed_uuid(part_name, part_uuid))
+		return 0;
 
-	if (dm_get_prefixed_uuid(mapname2, uuid2))
-		return 1;
+	if (dm_get_prefixed_uuid(map_name, map_uuid))
+		return 0;
 
-	p1 = strstr(uuid1, UUID_PREFIX);
-	p2 = strstr(uuid2, UUID_PREFIX);
-	if (p1 && p2 && !strcmp(p1, p2))
+	if (strncmp(part_uuid, "part", 4) != 0)
 		return 0;
 
-	return 1;
+	p = strstr(part_uuid, UUID_PREFIX);
+	if (p && !strcmp(p, map_uuid))
+		return 1;
+
+	return 0;
 }
 
 int dm_get_status(char *name, char *outstatus)
@@ -1162,6 +1155,7 @@ do_foreach_partmaps (const char * mapname,
 	unsigned long long size;
 	char dev_t[32];
 	int r = 1;
+	char *p;
 
 	if (!(dmt = dm_task_create(DM_DEVICE_LIST)))
 		return 1;
@@ -1190,10 +1184,10 @@ do_foreach_partmaps (const char * mapname,
 		    (dm_type(names->name, TGT_PART) > 0) &&
 
 		    /*
-		     * and both uuid end with same suffix starting
-		     * at UUID_PREFIX
+		     * and the uuid of the target is a partition of the
+		     * uuid of the multipath device
 		     */
-		    (!dm_compare_uuid(names->name, mapname)) &&
+		    is_mpath_part(names->name, mapname) &&
 
 		    /*
 		     * and we can fetch the map table from the kernel
@@ -1203,7 +1197,8 @@ do_foreach_partmaps (const char * mapname,
 		    /*
 		     * and the table maps over the multipath map
 		     */
-		    strstr(params, dev_t)
+		    (p = strstr(params, dev_t)) &&
+		    !isdigit(*(p + strlen(dev_t)))
 		   ) {
 			if (partmap_func(names->name, data) != 0)
 				goto out;
-- 
1.8.3.1

[PATCH 4/5] kpartx: default to running in sync mode

[Benjamin Marzinski]

When users run kpartx, they would naturally assume that when it
completes, the devices have been created. However, kpartx runs in async
mode by default.  This seems like it is likely to trip up users.  So,
switch the default to sync mode, add a -n option to enable async mode,
and set async mode when kpartx is called by the udev rules.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 kpartx/kpartx.c     | 10 +++++++---
 kpartx/kpartx.rules |  2 +-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
index 58e60ff..d1edd5e 100644
--- a/kpartx/kpartx.c
+++ b/kpartx/kpartx.c
@@ -58,7 +58,7 @@ struct pt {
 } pts[MAXTYPES];
 
 int ptct = 0;
-int udev_sync = 0;
+int udev_sync = 1;
 
 static void
 addpts(char *t, ptreader f)
@@ -86,7 +86,7 @@ initpts(void)
 	addpts("ps3", read_ps3_pt);
 }
 
-static char short_opts[] = "rladfgvp:t:su";
+static char short_opts[] = "rladfgvp:t:snu";
 
 /* Used in gpt.c */
 int force_gpt=0;
@@ -105,7 +105,8 @@ usage(void) {
 	printf("\t-g force GUID partition table (GPT)\n");
 	printf("\t-f force devmap create\n");
 	printf("\t-v verbose\n");
-	printf("\t-s sync mode. Don't return until the partitions are created\n");
+	printf("\t-n nosync mode. Return before the partitions are created\n");
+	printf("\t-s sync mode. Don't return until the partitions are created. Default.\n");
 	return 1;
 }
 
@@ -291,6 +292,9 @@ main(int argc, char **argv){
 		case 's':
 			udev_sync = 1;
 			break;
+		case 'n':
+			udev_sync = 0;
+			break;
 		case 'u':
 			what = UPDATE;
 			break;
diff --git a/kpartx/kpartx.rules b/kpartx/kpartx.rules
index 48a4d6c..a958791 100644
--- a/kpartx/kpartx.rules
+++ b/kpartx/kpartx.rules
@@ -40,6 +40,6 @@ ENV{DM_NR_VALID_PATHS}=="0", GOTO="kpartx_end"
 ENV{ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}!="1", IMPORT{db}="DM_SUBSYSTEM_UDEV_FLAG1"
 ENV{DM_SUBSYSTEM_UDEV_FLAG1}=="1", GOTO="kpartx_end"
 ENV{DM_STATE}!="SUSPENDED", ENV{DM_UUID}=="mpath-*", \
-	RUN+="/sbin/kpartx -u -p -part /dev/$name"
+	RUN+="/sbin/kpartx -un -p -part /dev/$name"
 
 LABEL="kpartx_end"
-- 
1.8.3.1

[PATCH 5/5] libmultipath: force udev reloads

[Benjamin Marzinski]

Now that the multipath udev rules try harder to not do lvm scans or
kpartx runs unnecessarily, multipath needs to make sure that these do
run when they are wanted.  This patch makes sure that when a device is
force reloaded, or changes size, the MPATH_UDEV_RELOAD_FLAG isn't set.
This allows lvm scanning and kpartx runs to happen on these uevents.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/configure.c | 7 ++++++-
 libmultipath/devmapper.c | 3 ++-
 libmultipath/structs.h   | 1 +
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index b29a660..bd090d9 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -499,8 +499,10 @@ select_action (struct multipath * mpp, vector curmp, int force_reload)
 				cmpp->alias, mpp->alias);
 			strncpy(mpp->alias_old, cmpp->alias, WWID_SIZE - 1);
 			mpp->action = ACT_RENAME;
-			if (force_reload)
+			if (force_reload) {
+				mpp->force_udev_reload = 1;
 				mpp->action = ACT_FORCERENAME;
+			}
 			return;
 		}
 		mpp->action = ACT_CREATE;
@@ -538,12 +540,14 @@ select_action (struct multipath * mpp, vector curmp, int force_reload)
 		return;
 	}
 	if (force_reload) {
+		mpp->force_udev_reload = 1;
 		mpp->action = ACT_RELOAD;
 		condlog(3, "%s: set ACT_RELOAD (forced by user)",
 			mpp->alias);
 		return;
 	}
 	if (cmpp->size != mpp->size) {
+		mpp->force_udev_reload = 1;
 		mpp->action = ACT_RESIZE;
 		condlog(3, "%s: set ACT_RESIZE (size change)",
 			mpp->alias);
@@ -797,6 +801,7 @@ int domap(struct multipath *mpp, char *params, int is_daemon)
 		 * DM_DEVICE_CREATE, DM_DEVICE_RENAME, or DM_DEVICE_RELOAD
 		 * succeeded
 		 */
+		mpp->force_udev_reload = 0;
 		if (mpp->action == ACT_CREATE)
 			remember_wwid(mpp->wwid);
 		if (!is_daemon) {
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index c7602cb..d73ba1a 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -372,7 +372,8 @@ int dm_addmap_create (struct multipath *mpp, char * params)
 int dm_addmap_reload(struct multipath *mpp, char *params, int flush)
 {
 	int r = 0;
-	uint16_t udev_flags = (flush ? 0 : MPATH_UDEV_RELOAD_FLAG) |
+	uint16_t udev_flags = ((mpp->force_udev_reload)?
+			       0 : MPATH_UDEV_RELOAD_FLAG) |
 			      ((mpp->skip_kpartx == SKIP_KPARTX_ON)?
 			       MPATH_UDEV_NO_KPARTX_FLAG : 0) |
 			      ((mpp->nr_active)? 0 : MPATH_UDEV_NO_PATHS_FLAG);
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 98e13e4..01e031a 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -272,6 +272,7 @@ struct multipath {
 	int skip_kpartx;
 	int max_sectors_kb;
 	int force_readonly;
+	int force_udev_reload;
 	unsigned int dev_loss;
 	uid_t uid;
 	gid_t gid;
-- 
1.8.3.1

Re: [PATCH 4/5] kpartx: default to running in sync mode

[Steffen Maier]

On 04/25/2017 12:39 AM, Benjamin Marzinski wrote:
> When users run kpartx, they would naturally assume that when it
> completes, the devices have been created. However, kpartx runs in async
> mode by default.  This seems like it is likely to trip up users.  So,
> switch the default to sync mode, add a -n option to enable async mode,
> and set async mode when kpartx is called by the udev rules.
>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  kpartx/kpartx.c     | 10 +++++++---
>  kpartx/kpartx.rules |  2 +-
>  2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
> index 58e60ff..d1edd5e 100644
> --- a/kpartx/kpartx.c
> +++ b/kpartx/kpartx.c

> -int udev_sync = 0;
> +int udev_sync = 1;

> -	printf("\t-s sync mode. Don't return until the partitions are created\n");
> +	printf("\t-n nosync mode. Return before the partitions are created\n");
> +	printf("\t-s sync mode. Don't return until the partitions are created. Default.\n");

> +		case 'n':
> +			udev_sync = 0;
> +			break;

> diff --git a/kpartx/kpartx.rules b/kpartx/kpartx.rules
> index 48a4d6c..a958791 100644
> --- a/kpartx/kpartx.rules
> +++ b/kpartx/kpartx.rules
> @@ -40,6 +40,6 @@ ENV{DM_NR_VALID_PATHS}=="0", GOTO="kpartx_end"
>  ENV{ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}!="1", IMPORT{db}="DM_SUBSYSTEM_UDEV_FLAG1"
>  ENV{DM_SUBSYSTEM_UDEV_FLAG1}=="1", GOTO="kpartx_end"
>  ENV{DM_STATE}!="SUSPENDED", ENV{DM_UUID}=="mpath-*", \
> -	RUN+="/sbin/kpartx -u -p -part /dev/$name"
> +	RUN+="/sbin/kpartx -un -p -part /dev/$name"

I understand this was async before and is now still async after the 
default change in kpartx. However, I'm wondering in general how one 
would "wait" for kpartx mappings, since I suppose a "udevadm settle" 
would not suffice. Dracut probably does not care because it would loop 
until its required root-fs devices have appeared.

-- 
Mit freundlichen Grüßen / Kind regards
Steffen Maier

Linux on z Systems Development

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [PATCH 4/5] kpartx: default to running in sync mode

[Benjamin Marzinski]

On Tue, Apr 25, 2017 at 12:12:25PM +0200, Steffen Maier wrote:
> 
> On 04/25/2017 12:39 AM, Benjamin Marzinski wrote:
> >When users run kpartx, they would naturally assume that when it
> >completes, the devices have been created. However, kpartx runs in async
> >mode by default.  This seems like it is likely to trip up users.  So,
> >switch the default to sync mode, add a -n option to enable async mode,
> >and set async mode when kpartx is called by the udev rules.
> >
> >Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> >---
> > kpartx/kpartx.c     | 10 +++++++---
> > kpartx/kpartx.rules |  2 +-
> > 2 files changed, 8 insertions(+), 4 deletions(-)
> >
> >diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
> >index 58e60ff..d1edd5e 100644
> >--- a/kpartx/kpartx.c
> >+++ b/kpartx/kpartx.c
> 
> >-int udev_sync = 0;
> >+int udev_sync = 1;
> 
> >-	printf("\t-s sync mode. Don't return until the partitions are created\n");
> >+	printf("\t-n nosync mode. Return before the partitions are created\n");
> >+	printf("\t-s sync mode. Don't return until the partitions are created. Default.\n");
> 
> >+		case 'n':
> >+			udev_sync = 0;
> >+			break;
> 
> >diff --git a/kpartx/kpartx.rules b/kpartx/kpartx.rules
> >index 48a4d6c..a958791 100644
> >--- a/kpartx/kpartx.rules
> >+++ b/kpartx/kpartx.rules
> >@@ -40,6 +40,6 @@ ENV{DM_NR_VALID_PATHS}=="0", GOTO="kpartx_end"
> > ENV{ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}!="1", IMPORT{db}="DM_SUBSYSTEM_UDEV_FLAG1"
> > ENV{DM_SUBSYSTEM_UDEV_FLAG1}=="1", GOTO="kpartx_end"
> > ENV{DM_STATE}!="SUSPENDED", ENV{DM_UUID}=="mpath-*", \
> >-	RUN+="/sbin/kpartx -u -p -part /dev/$name"
> >+	RUN+="/sbin/kpartx -un -p -part /dev/$name"
> 
> I understand this was async before and is now still async after the default
> change in kpartx. However, I'm wondering in general how one would "wait" for
> kpartx mappings, since I suppose a "udevadm settle" would not suffice.
> Dracut probably does not care because it would loop until its required
> root-fs devices have appeared.

I'm pretty sure that it is inherently impossible in the general case.
There is no way to know when the uevent will occur, and until the uevent
occurs, there would be no way that udev settle could possibly help. You
can know that a dm device has been set up when the first change event
for that device occurs. So, if you knew a device was coming, you could
wait for the change event.  Multipathd currently listens for change
events to know when a multipath device has been fully initialized.

For partition devices explicitly, say you just manually ran multipath to
create a multipath device, and want to know that your partition devices
exist before going on. You could manually run kpartx in the now-default
sync mode, and when it returned, you would know that the devices were
there. However, your kpart call would be racing with the udev version,
so it's possible that kpartx would return with an error because it tried
to create the device and failed because the device already existed. The
good news is that it would continue to try with the rest of the devices,
so it really won't return until they all are there. If it is important
to you, we could add a flag to kpartx to not return an error if a create
fails because the device currently exists. 

-Ben

> 
> -- 
> Mit freundlichen Grüßen / Kind regards
> Steffen Maier
> 
> Linux on z Systems Development
> 
> IBM Deutschland Research & Development GmbH
> Vorsitzende des Aufsichtsrats: Martina Koederitz
> Geschaeftsfuehrung: Dirk Wittkopp
> Sitz der Gesellschaft: Boeblingen
> Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [PATCH 4/5] kpartx: default to running in sync mode

[Steffen Maier]

On 04/25/2017 05:52 PM, Benjamin Marzinski wrote:
> On Tue, Apr 25, 2017 at 12:12:25PM +0200, Steffen Maier wrote:
>> On 04/25/2017 12:39 AM, Benjamin Marzinski wrote:
>>> When users run kpartx, they would naturally assume that when it
>>> completes, the devices have been created. However, kpartx runs in async
>>> mode by default.  This seems like it is likely to trip up users.  So,
>>> switch the default to sync mode, add a -n option to enable async mode,
>>> and set async mode when kpartx is called by the udev rules.

>>> diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c

>>> +	printf("\t-n nosync mode. Return before the partitions are created\n");

>>> diff --git a/kpartx/kpartx.rules b/kpartx/kpartx.rules

>>> @@ -40,6 +40,6 @@ ENV{DM_NR_VALID_PATHS}=="0", GOTO="kpartx_end"
>>> ENV{ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}!="1", IMPORT{db}="DM_SUBSYSTEM_UDEV_FLAG1"
>>> ENV{DM_SUBSYSTEM_UDEV_FLAG1}=="1", GOTO="kpartx_end"
>>> ENV{DM_STATE}!="SUSPENDED", ENV{DM_UUID}=="mpath-*", \
>>> -	RUN+="/sbin/kpartx -u -p -part /dev/$name"
>>> +	RUN+="/sbin/kpartx -un -p -part /dev/$name"
>>
>> I understand this was async before and is now still async after the default
>> change in kpartx. However, I'm wondering in general how one would "wait" for
>> kpartx mappings, since I suppose a "udevadm settle" would not suffice.
>> Dracut probably does not care because it would loop until its required
>> root-fs devices have appeared.
>
> I'm pretty sure that it is inherently impossible in the general case.
> There is no way to know when the uevent will occur, and until the uevent
> occurs, there would be no way that udev settle could possibly help. You

indeed

> can know that a dm device has been set up when the first change event
> for that device occurs. So, if you knew a device was coming, you could
> wait for the change event.  Multipathd currently listens for change
> events to know when a multipath device has been fully initialized.
>
> For partition devices explicitly, say you just manually ran multipath to
> create a multipath device, and want to know that your partition devices
> exist before going on. You could manually run kpartx in the now-default
> sync mode, and when it returned, you would know that the devices were
> there. However, your kpart call would be racing with the udev version,
> so it's possible that kpartx would return with an error because it tried
> to create the device and failed because the device already existed. The
> good news is that it would continue to try with the rest of the devices,
> so it really won't return until they all are there. If it is important
> to you, we could add a flag to kpartx to not return an error if a create
> fails because the device currently exists.

Currently I have no real need for such addition, just wanted to 
understand the implications.

Thanks for your explanation!

-- 
Mit freundlichen Grüßen / Kind regards
Steffen Maier

Linux on z Systems Development

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [PATCH 4/5] kpartx: default to running in sync mode

[Xose Vazquez Perez]

On 04/25/2017 12:39 AM, Benjamin Marzinski wrote:

> When users run kpartx, they would naturally assume that when it
> completes, the devices have been created. However, kpartx runs in async
> mode by default.  This seems like it is likely to trip up users.  So,
> switch the default to sync mode, add a -n option to enable async mode,
> and set async mode when kpartx is called by the udev rules.
>[...]
> diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
> index 58e60ff..d1edd5e 100644
> --- a/kpartx/kpartx.c
> +++ b/kpartx/kpartx.c
>[...]
> -static char short_opts[] = "rladfgvp:t:su";
> +static char short_opts[] = "rladfgvp:t:snu";
>[...]
> -	printf("\t-s sync mode. Don't return until the partitions are created\n");
> +	printf("\t-n nosync mode. Return before the partitions are created\n");
> +	printf("\t-s sync mode. Don't return until the partitions are created. Default.\n");
>  	return 1;
>[...]> +		case 'n':
> +			udev_sync = 0;
> +			break;

New flags should be documented in its man page.

Re: [PATCH 4/5] kpartx: default to running in sync mode

[Martin Wilck]

Hi Ben,

On Mon, 2017-04-24 at 17:39 -0500, Benjamin Marzinski wrote:

> When users run kpartx, they would naturally assume that when it
> completes, the devices have been created. However, kpartx runs in
> async
> mode by default.  This seems like it is likely to trip up users. 

Is this just likely, or do you have evidence that it really did
irritate users? You introduced "sync mode", together with the "-s"
flag, yourself in 2010, and unless I'm mistaken, kpartx operated in
"async" mode before that, too, because there was no "sync" mode. 

I'm not too fond of the idea to change a default which has been
established for such a long time just because user mistakes are
"likely". For one thing, such changes are difficult to document in a
way that doesn't cause confusion. Also, if someone writes a script in
which she wants kpartx to run asynchronously, it will be difficult to
do so in a manner which is portable between distributions if some
distributions include this patch and some don't.

As an alternative, we might simply warn about the default asynchronous
behavior in a prominent place in the man page.

Cheers,
Martin

>  So,
> switch the default to sync mode, add a -n option to enable async
> mode,
> and set async mode when kpartx is called by the udev rules.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  kpartx/kpartx.c     | 10 +++++++---
>  kpartx/kpartx.rules |  2 +-
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
> index 58e60ff..d1edd5e 100644
> --- a/kpartx/kpartx.c
> +++ b/kpartx/kpartx.c
> @@ -58,7 +58,7 @@ struct pt {
>  } pts[MAXTYPES];
>  
>  int ptct = 0;
> -int udev_sync = 0;
> +int udev_sync = 1;
>  
>  static void
>  addpts(char *t, ptreader f)
> @@ -86,7 +86,7 @@ initpts(void)
>  	addpts("ps3", read_ps3_pt);
>  }
>  
> -static char short_opts[] = "rladfgvp:t:su";
> +static char short_opts[] = "rladfgvp:t:snu";
>  
>  /* Used in gpt.c */
>  int force_gpt=0;
> @@ -105,7 +105,8 @@ usage(void) {
>  	printf("\t-g force GUID partition table (GPT)\n");
>  	printf("\t-f force devmap create\n");
>  	printf("\t-v verbose\n");
> -	printf("\t-s sync mode. Don't return until the partitions
> are created\n");
> +	printf("\t-n nosync mode. Return before the partitions are
> created\n");
> +	printf("\t-s sync mode. Don't return until the partitions
> are created. Default.\n");
>  	return 1;
>  }
>  
> @@ -291,6 +292,9 @@ main(int argc, char **argv){
>  		case 's':
>  			udev_sync = 1;
>  			break;
> +		case 'n':
> +			udev_sync = 0;
> +			break;
>  		case 'u':
>  			what = UPDATE;
>  			break;
> diff --git a/kpartx/kpartx.rules b/kpartx/kpartx.rules
> index 48a4d6c..a958791 100644
> --- a/kpartx/kpartx.rules
> +++ b/kpartx/kpartx.rules
> @@ -40,6 +40,6 @@ ENV{DM_NR_VALID_PATHS}=="0", GOTO="kpartx_end"
>  ENV{ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}!="1",
> IMPORT{db}="DM_SUBSYSTEM_UDEV_FLAG1"
>  ENV{DM_SUBSYSTEM_UDEV_FLAG1}=="1", GOTO="kpartx_end"
>  ENV{DM_STATE}!="SUSPENDED", ENV{DM_UUID}=="mpath-*", \
> -	RUN+="/sbin/kpartx -u -p -part /dev/$name"
> +	RUN+="/sbin/kpartx -un -p -part /dev/$name"
>  
>  LABEL="kpartx_end"

-- 
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/5] libmultipath: fix partition detection

[Martin Wilck]

On Mon, 2017-04-24 at 17:39 -0500, Benjamin Marzinski wrote:
> The do_foreach_partmaps code could incorrectly list a device as a
> partition of another device, because of two issues.  First, the check
> to
> compare the dm UUID of two devices would allow two partition devices
> to
> match, or a partition device to match with itself, instead of only
> having a partition device match with the multipath device that it
> belongs to.  Second, the code to check if a multipath device's
> major:minor appeared in a partition device's table only used strstr
> to check of the existance of the major:minor string. This meant that
> any device with a minor number that started with the same digits
> would
> match. for instance, checking for "253:10" would also match
> "253:102".

Good catch! I missed that in my kpartx series although I was trying to
be paranoid ;-)

> -/*
> - * returns:
> - *    0 : if both uuids end with same suffix which starts with
> UUID_PREFIX
> - *    1 : otherwise
> - */
> -int
> -dm_compare_uuid(const char* mapname1, const char* mapname2)
> +static int
> +is_mpath_part(const char *part_name, const char *map_name)
>  {
> -	char *p1, *p2;
> -	char uuid1[WWID_SIZE], uuid2[WWID_SIZE];
> +	char *p;
> +	char part_uuid[WWID_SIZE], map_uuid[WWID_SIZE];
>  
> -	if (dm_get_prefixed_uuid(mapname1, uuid1))
> -		return 1;
> +	if (dm_get_prefixed_uuid(part_name, part_uuid))
> +		return 0;
>  
> -	if (dm_get_prefixed_uuid(mapname2, uuid2))
> -		return 1;
> +	if (dm_get_prefixed_uuid(map_name, map_uuid))
> +		return 0;
>  
> -	p1 = strstr(uuid1, UUID_PREFIX);
> -	p2 = strstr(uuid2, UUID_PREFIX);
> -	if (p1 && p2 && !strcmp(p1, p2))
> +	if (strncmp(part_uuid, "part", 4) != 0)
>  		return 0;
>  
> -	return 1;
> +	p = strstr(part_uuid, UUID_PREFIX);
> +	if (p && !strcmp(p, map_uuid))
> +		return 1;
> +
> +	return 0;
>  }

While we've got this far, we might actually implement proper parsing of
 the UUID instead of "strstr" which would also match something like
"partially_recovered-mpath...".

ACK nonetheless, further checks can be added on top of this.

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 2/5] multipath: fix up position independent code

[Martin Wilck]

On Mon, 2017-04-24 at 17:39 -0500, Benjamin Marzinski wrote:
> The multipath binaries were not being compiled as position
> independent
> executables (PIE).  This code fixes that, and makes other minor code
> hardening tweaks to make hardening-check happier.

Since what version do gcc/ld support -fPIE /-pie? I'm asking because we
at SUSE recently had to revert 596f51f "Replace -fstack-protector with
-fstack-protector-strong" because this parameter isn't supported in all
our gcc versions.

In a way, the recent Makefile patches call for autoconf or something of
the kind ...

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 4/5] kpartx: default to running in sync mode

[Benjamin Marzinski]

On Thu, May 11, 2017 at 02:42:08PM +0200, Martin Wilck wrote:
> Hi Ben,
> 
> On Mon, 2017-04-24 at 17:39 -0500, Benjamin Marzinski wrote:
> 
> > When users run kpartx, they would naturally assume that when it
> > completes, the devices have been created. However, kpartx runs in
> > async
> > mode by default.  This seems like it is likely to trip up users. 
> 
> Is this just likely, or do you have evidence that it really did
> irritate users? You introduced "sync mode", together with the "-s"
> flag, yourself in 2010, and unless I'm mistaken, kpartx operated in
> "async" mode before that, too, because there was no "sync" mode. 

Yes. I made this patch after the second time a customer complained that
kpartx wasn't working correctly, when the real issue was that their
scripts were assuming that after the kpartx command completed, those
partition device nodes would be present. Obviously, 2 isn't a huge
number, but it certainly has caused confusion in some cases.

> I'm not too fond of the idea to change a default which has been
> established for such a long time just because user mistakes are
> "likely". For one thing, such changes are difficult to document in a
> way that doesn't cause confusion. Also, if someone writes a script in
> which she wants kpartx to run asynchronously, it will be difficult to
> do so in a manner which is portable between distributions if some
> distributions include this patch and some don't.

I see your point, but assuming that I am correct that most users do
assume that kpartx runs synchronously, I feel like we shouldn't keep a
bad default forever, simply because it's always been that way. lvm and
dmsetup wait for udev by default (well, on lvm IIRC it is a compile-time
setting, but here is some SUSE documentation showing that it uses udev
synchronization by default
https://www.suse.com/documentation/sles-12/stor_admin/data/sec_lvm_cli.html)
kpartx does seem to be the odd one out.

> As an alternative, we might simply warn about the default asynchronous
> behavior in a prominent place in the man page.

We could, and if you are opposed to this patch, that would probably help
cut down any confusion (assuming that when customers see things go wrong
they read the documentation before calling support).

-Ben

> Cheers,
> Martin
> 
> >  So,
> > switch the default to sync mode, add a -n option to enable async
> > mode,
> > and set async mode when kpartx is called by the udev rules.
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > ---
> >  kpartx/kpartx.c     | 10 +++++++---
> >  kpartx/kpartx.rules |  2 +-
> >  2 files changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
> > index 58e60ff..d1edd5e 100644
> > --- a/kpartx/kpartx.c
> > +++ b/kpartx/kpartx.c
> > @@ -58,7 +58,7 @@ struct pt {
> >  } pts[MAXTYPES];
> >  
> >  int ptct = 0;
> > -int udev_sync = 0;
> > +int udev_sync = 1;
> >  
> >  static void
> >  addpts(char *t, ptreader f)
> > @@ -86,7 +86,7 @@ initpts(void)
> >  	addpts("ps3", read_ps3_pt);
> >  }
> >  
> > -static char short_opts[] = "rladfgvp:t:su";
> > +static char short_opts[] = "rladfgvp:t:snu";
> >  
> >  /* Used in gpt.c */
> >  int force_gpt=0;
> > @@ -105,7 +105,8 @@ usage(void) {
> >  	printf("\t-g force GUID partition table (GPT)\n");
> >  	printf("\t-f force devmap create\n");
> >  	printf("\t-v verbose\n");
> > -	printf("\t-s sync mode. Don't return until the partitions
> > are created\n");
> > +	printf("\t-n nosync mode. Return before the partitions are
> > created\n");
> > +	printf("\t-s sync mode. Don't return until the partitions
> > are created. Default.\n");
> >  	return 1;
> >  }
> >  
> > @@ -291,6 +292,9 @@ main(int argc, char **argv){
> >  		case 's':
> >  			udev_sync = 1;
> >  			break;
> > +		case 'n':
> > +			udev_sync = 0;
> > +			break;
> >  		case 'u':
> >  			what = UPDATE;
> >  			break;
> > diff --git a/kpartx/kpartx.rules b/kpartx/kpartx.rules
> > index 48a4d6c..a958791 100644
> > --- a/kpartx/kpartx.rules
> > +++ b/kpartx/kpartx.rules
> > @@ -40,6 +40,6 @@ ENV{DM_NR_VALID_PATHS}=="0", GOTO="kpartx_end"
> >  ENV{ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}!="1",
> > IMPORT{db}="DM_SUBSYSTEM_UDEV_FLAG1"
> >  ENV{DM_SUBSYSTEM_UDEV_FLAG1}=="1", GOTO="kpartx_end"
> >  ENV{DM_STATE}!="SUSPENDED", ENV{DM_UUID}=="mpath-*", \
> > -	RUN+="/sbin/kpartx -u -p -part /dev/$name"
> > +	RUN+="/sbin/kpartx -un -p -part /dev/$name"
> >  
> >  LABEL="kpartx_end"
> 
> -- 
> 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)

Re: [PATCH 4/5] kpartx: default to running in sync mode

[Martin Wilck]

On Thu, 2017-05-11 at 12:46 -0500, Benjamin Marzinski wrote:
> On Thu, May 11, 2017 at 02:42:08PM +0200, Martin Wilck wrote:
> > Hi Ben,
> > 
> > On Mon, 2017-04-24 at 17:39 -0500, Benjamin Marzinski wrote:
> > 
> > > When users run kpartx, they would naturally assume that when it
> > > completes, the devices have been created. However, kpartx runs in
> > > async
> > > mode by default.  This seems like it is likely to trip up users. 
> > 
> > Is this just likely, or do you have evidence that it really did
> > irritate users? You introduced "sync mode", together with the "-s"
> > flag, yourself in 2010, and unless I'm mistaken, kpartx operated in
> > "async" mode before that, too, because there was no "sync" mode. 
> 
> Yes. I made this patch after the second time a customer complained
> that
> kpartx wasn't working correctly, when the real issue was that their
> scripts were assuming that after the kpartx command completed, those
> partition device nodes would be present. Obviously, 2 isn't a huge
> number, but it certainly has caused confusion in some cases.

2 > "likely", that makes a difference.

> > I'm not too fond of the idea to change a default which has been
> > established for such a long time just because user mistakes are
> > "likely". For one thing, such changes are difficult to document in
> > a
> > way that doesn't cause confusion. Also, if someone writes a script
> > in
> > which she wants kpartx to run asynchronously, it will be difficult
> > to
> > do so in a manner which is portable between distributions if some
> > distributions include this patch and some don't.
> 
> I see your point, but assuming that I am correct that most users do
> assume that kpartx runs synchronously, I feel like we shouldn't keep
> a
> bad default forever, simply because it's always been that way. lvm
> and
> dmsetup wait for udev by default (well, on lvm IIRC it is a compile-
> time
> setting, but here is some SUSE documentation showing that it uses
> udev
> synchronization by default
> https://www.suse.com/documentation/sles-12/stor_admin/data/sec_lvm_cl
> i.html)
> kpartx does seem to be the odd one out.
> 
> > As an alternative, we might simply warn about the default
> > asynchronous
> > behavior in a prominent place in the man page.
> 
> We could, and if you are opposed to this patch, that would probably
> help
> cut down any confusion (assuming that when customers see things go
> wrong
> they read the documentation before calling support).

I still don't like it too much. The scripting issue is my biggest
concern - users need at least a reliable way to test whether or not 
"-n" is supported by kpartx on a given system.

But I, alone, am not in a position to veto this.

Does anyone else have an opinion on this change?

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