[PATCH 00/12] multipath-tools: improve processing efficiency for addition and deletion of multipath devices

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 00/12] multipath-tools: improve processing efficiency for addition and deletion of multipath devices
@ 2016-12-27  8:03 tang.junhui
  2016-12-27  8:03 ` [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent tang.junhui
                   ` (11 more replies)
  0 siblings, 12 replies; 33+ messages in thread
From: tang.junhui @ 2016-12-27  8:03 UTC (permalink / raw)
  To: christophe.varoqui, hare, bmarzins, mwilck, bart.vanassche
  Cc: zhang.kai16, dm-devel, tang.junhui, tang.wenjun3

From: tang.junhui <tang.junhui@zte.com.cn>

Hello Christophe, Ben, Hannes, Martin, Bart, and other guys:

The twelve patches in this series are used to improve processing
efficiency for addition and deletion of multipath devices.

These patches are tested pass by ZTE multipath automatic testing syetem.
The modification reduces the system consumption(such as CPU) and shortens
the processing time obviously in scene of massive multipath devices
addition or deletion.

The main processing flow of code is:
1)uevents accumulation in uevents burst scene:
  --[pacth]libmultipath: wait one seconds for more uevents in
    uevent_listen() in uevents burst situations
2)uevents filtering and merging:
  --[pacth]multipathd: merge uevents before proccessing
  --[pacth]libmultipath: filter uevents before proccessing
3)uevents proccessing:
  --[pacth]multipathd: add need_do_map to indicate whether need calling
    domap() in ev_add_path()
  --[pacth]multipathd: add need_do_map to indicate whether need calling
    domap() in ev_remove_path()
  --[pacth]multipathd: proccess merged uevents

There are some patches providing basic struct members or maros which
would used in above pathes:
  --[patch]libmultipath: add wwid for "struct uevent" to record wwid of
    uevent
  --[patch]libmultipath: add merge_node for "struct uevent" to record
    nodes of merged uevents
  --[patch]libmultipath: add two list iteration macros

There are also some independent optimization pathes for further
improvement:
  --[patch]multipathd: move uev_discard() to uevent.c and change its name
    to uevent_can_discard()
  --[patch]multipathd: move calling filter_devnode() from uev_trigger() to
    uevent_can_discard()
  --[patch]libmultipath: use existing wwid when wwid has already been
    existed in uevent

Any comment will be welcome, and it would be appreciated if these
patches would be considered for inclusion in the upstream
multipath-tools.

Thank you all,
Tang Junhui

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent
  2016-12-27  8:03 [PATCH 00/12] multipath-tools: improve processing efficiency for addition and deletion of multipath devices tang.junhui
@ 2016-12-27  8:03 ` tang.junhui
  2017-01-03 22:02   ` Benjamin Marzinski
  2016-12-27  8:03 ` [PATCH 02/12] libmultipath: add merge_node for "struct uevent" to record nodes of merged uevents tang.junhui
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: tang.junhui @ 2016-12-27  8:03 UTC (permalink / raw)
  To: christophe.varoqui, hare, bmarzins, mwilck, bart.vanassche
  Cc: zhang.kai16, dm-devel, tang.junhui, tang.wenjun3

From: tang.junhui <tang.junhui@zte.com.cn>

Add "char *wwid" to point WWID of uevent. This member identifies
the LUN ID which the path belongs to, and it is used for merging
uevents. WWID possibly did not exist in uevent yet, so ->wwid
would be NULL, those uevents would not be merged, but be proccessed
as old way.

Change-Id: Ie6b076363b3735dc7de10184b27fa799b499af0e
Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
---
 libmultipath/uevent.c | 2 ++
 libmultipath/uevent.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 7edcce1..ef1bafe 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -424,6 +424,8 @@ struct uevent *uevent_from_udev_device(struct udev_device *dev)
 			uev->devpath = uev->envp[i] + 8;
 		if (strcmp(name, "ACTION") == 0)
 			uev->action = uev->envp[i] + 7;
+		if (strcmp(name, "ID_SERIAL") == 0)
+			uev->wwid = uev->envp[i] + 10;
 		i++;
 		if (i == HOTPLUG_NUM_ENVP - 1)
 			break;
diff --git a/libmultipath/uevent.h b/libmultipath/uevent.h
index 9d22dcd..7bfccef 100644
--- a/libmultipath/uevent.h
+++ b/libmultipath/uevent.h
@@ -22,6 +22,7 @@ struct uevent {
 	char *devpath;
 	char *action;
 	char *kernel;
+	char *wwid;
 	unsigned long seqnum;
 	char *envp[HOTPLUG_NUM_ENVP];
 };
-- 
2.8.1.windows.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 02/12] libmultipath: add merge_node for "struct uevent" to record nodes of merged uevents
  2016-12-27  8:03 [PATCH 00/12] multipath-tools: improve processing efficiency for addition and deletion of multipath devices tang.junhui
  2016-12-27  8:03 ` [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent tang.junhui
@ 2016-12-27  8:03 ` tang.junhui
  2016-12-27  8:03 ` [PATCH 03/12] libmultipath: add two list iteration macros tang.junhui
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 33+ messages in thread
From: tang.junhui @ 2016-12-27  8:03 UTC (permalink / raw)
  To: christophe.varoqui, hare, bmarzins, mwilck, bart.vanassche
  Cc: zhang.kai16, dm-devel, tang.junhui, tang.wenjun3

From: tang.junhui <tang.junhui@zte.com.cn>

Add merged nodes list to store nodes of merged uevents. By Adding
this member, after merging, the list of uevents would be linked like
this:
uevent
 ---------------------------
|struct list_head node      |----->list node of un-merged uevents
 ---------------------------
|struct list_head merge_node|----->list node of merged uevents, which
----------------------------       moved from the origin un-merged list
|...                        |
 ---------------------------

Change-Id: I5fbfc7656ede77e03ca35c855212e2d2d60706b2
Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
---
 libmultipath/uevent.c | 4 +++-
 libmultipath/uevent.h | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index ef1bafe..181b3b8 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -72,8 +72,10 @@ struct uevent * alloc_uevent (void)
 {
 	struct uevent *uev = MALLOC(sizeof(struct uevent));
 
-	if (uev)
+	if (uev) {
 		INIT_LIST_HEAD(&uev->node);
+		INIT_LIST_HEAD(&uev->merge_node);
+	}
 
 	return uev;
 }
diff --git a/libmultipath/uevent.h b/libmultipath/uevent.h
index 7bfccef..61a4207 100644
--- a/libmultipath/uevent.h
+++ b/libmultipath/uevent.h
@@ -17,6 +17,7 @@ struct udev;
 
 struct uevent {
 	struct list_head node;
+	struct list_head merge_node;
 	struct udev_device *udev;
 	char buffer[HOTPLUG_BUFFER_SIZE + OBJECT_SIZE];
 	char *devpath;
-- 
2.8.1.windows.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 03/12] libmultipath: add two list iteration macros
  2016-12-27  8:03 [PATCH 00/12] multipath-tools: improve processing efficiency for addition and deletion of multipath devices tang.junhui
  2016-12-27  8:03 ` [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent tang.junhui
  2016-12-27  8:03 ` [PATCH 02/12] libmultipath: add merge_node for "struct uevent" to record nodes of merged uevents tang.junhui
@ 2016-12-27  8:03 ` tang.junhui
  2016-12-27  8:03 ` [PATCH 04/12] multipathd: add need_do_map to indicate whether need calling domap() in ev_add_path() tang.junhui
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 33+ messages in thread
From: tang.junhui @ 2016-12-27  8:03 UTC (permalink / raw)
  To: christophe.varoqui, hare, bmarzins, mwilck, bart.vanassche
  Cc: zhang.kai16, dm-devel, tang.junhui, tang.wenjun3

From: tang.junhui <tang.junhui@zte.com.cn>

Add two list iteration macros, they are used to iterate list forwards
or backwards from the given begin node to the given end node, which
would be used in merging uevents.

Change-Id: I8bb53fef9276bb62a5e0f4fdac6455086dc03d9b
Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
---
 libmultipath/list.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/libmultipath/list.h b/libmultipath/list.h
index ceaa381..e728ff0 100644
--- a/libmultipath/list.h
+++ b/libmultipath/list.h
@@ -294,6 +294,18 @@ static inline void list_splice_tail_init(struct list_head *list,
 	     pos = list_entry(pos->member.next, typeof(*pos), member))
 
 /**
+ * list_for_some_entry	-	iterate list of given interval
+ * @pos:	the type * to use as a loop counter.
+ * @from:	the begin node of the iteration.
+ * @to:		the end node of the iteration.
+ * @member:	the name of the list_struct within the struct.
+ */
+#define list_for_some_entry(pos, from, to, member)				\
+	for (pos = list_entry((from)->next, typeof(*pos), member);	\
+	     &pos->member != (to);					\
+	     pos = list_entry(pos->member.next, typeof(*pos), member))
+
+/**
  * list_for_each_entry_reverse - iterate backwards over list of given type.
  * @pos:	the type * to use as a loop counter.
  * @head:	the head for your list.
@@ -305,6 +317,18 @@ static inline void list_splice_tail_init(struct list_head *list,
 	     pos = list_entry(pos->member.prev, typeof(*pos), member))
 
 /**
+ * list_for_some_entry_reverse	-	iterate backwards list of given interval
+ * @pos:	the type * to use as a loop counter.
+ * @from:	the begin node of the iteration.
+ * @to:		the end node of the iteration.
+ * @member:	the name of the list_struct within the struct.
+ */
+#define list_for_some_entry_reverse(pos, from, to, member)			\
+	for (pos = list_entry((from)->prev, typeof(*pos), member);	\
+	     &pos->member != (to);					\
+	     pos = list_entry(pos->member.prev, typeof(*pos), member))
+
+/**
  * list_for_each_entry_safe - iterate over list of given type safe against removal of list entry
  * @pos:	the type * to use as a loop counter.
  * @n:		another type * to use as temporary storage
-- 
2.8.1.windows.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 04/12] multipathd: add need_do_map to indicate whether need calling domap() in ev_add_path()
  2016-12-27  8:03 [PATCH 00/12] multipath-tools: improve processing efficiency for addition and deletion of multipath devices tang.junhui
                   ` (2 preceding siblings ...)
  2016-12-27  8:03 ` [PATCH 03/12] libmultipath: add two list iteration macros tang.junhui
@ 2016-12-27  8:03 ` tang.junhui
  2016-12-27  8:03 ` [PATCH 05/12] multipathd: add need_do_map to indicate whether need calling domap() in ev_remove_path() tang.junhui
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 33+ messages in thread
From: tang.junhui @ 2016-12-27  8:03 UTC (permalink / raw)
  To: christophe.varoqui, hare, bmarzins, mwilck, bart.vanassche
  Cc: zhang.kai16, dm-devel, tang.junhui, tang.wenjun3

From: tang.junhui <tang.junhui@zte.com.cn>

Usually calling domap() in ev_add_path() is needed, but only last path
need to call domap() in processing for merged uevents to reduce the
count of calling domap() and promote efficiency. So add input parameter
need_do_map to indicate whether need calling domap() in ev_add_path().

Change-Id: I7f33de49a1d89d91180dcb95cd94e4944ae7ff36
Signed-off-by: tang.wenjun <tang.wenjun3@zte.com.cn>
---
 multipathd/cli_handlers.c |  3 ++-
 multipathd/main.c         | 25 ++++++++++++++++---------
 multipathd/main.h         |  2 +-
 3 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index b0eeca6..3a46c09 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -670,7 +670,8 @@ cli_add_path (void * v, char ** reply, int * len, void * data)
 		pp->checkint = conf->checkint;
 	}
 	put_multipath_config(conf);
-	return ev_add_path(pp, vecs);
+	return ev_add_path(pp, vecs, 1);
+
 blacklisted:
 	*reply = strdup("blacklisted\n");
 	*len = strlen(*reply) + 1;
diff --git a/multipathd/main.c b/multipathd/main.c
index adc3258..34b1b64 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -608,7 +608,7 @@ ev_remove_map (char * devname, char * alias, int minor, struct vectors * vecs)
 }
 
 static int
-uev_add_path (struct uevent *uev, struct vectors * vecs)
+uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map)
 {
 	struct path *pp;
 	int ret = 0, i;
@@ -641,7 +641,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs)
 				     DI_ALL | DI_BLACKLIST);
 			put_multipath_config(conf);
 			if (r == PATHINFO_OK)
-				ret = ev_add_path(pp, vecs);
+				ret = ev_add_path(pp, vecs, need_do_map);
 			else if (r == PATHINFO_SKIPPED) {
 				condlog(3, "%s: remove blacklisted path",
 					uev->kernel);
@@ -681,7 +681,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs)
 		conf = get_multipath_config();
 		pp->checkint = conf->checkint;
 		put_multipath_config(conf);
-		ret = ev_add_path(pp, vecs);
+		ret = ev_add_path(pp, vecs, need_do_map);
 	} else {
 		condlog(0, "%s: failed to store path info, "
 			"dropping event",
@@ -699,7 +699,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs)
  * 1: error
  */
 int
-ev_add_path (struct path * pp, struct vectors * vecs)
+ev_add_path (struct path * pp, struct vectors * vecs, int need_do_map)
 {
 	struct multipath * mpp;
 	char params[PARAMS_SIZE] = {0};
@@ -767,6 +767,13 @@ rescan:
 	/* persistent reservation check*/
 	mpath_pr_event_handle(pp);
 
+	if (!need_do_map)
+		return 0;
+
+	if (!dm_map_present(mpp->alias)) {
+		mpp->action = ACT_CREATE;
+		start_waiter = 1;
+	}
 	/*
 	 * push the map to the device-mapper
 	 */
@@ -995,7 +1002,7 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
 		}
 
 		if (pp->initialized == INIT_REQUESTED_UDEV)
-			retval = uev_add_path(uev, vecs);
+			retval = uev_add_path(uev, vecs, 1);
 		else if (mpp && ro >= 0) {
 			condlog(2, "%s: update path write_protect to '%d' (uevent)", uev->kernel, ro);
 
@@ -1150,7 +1157,7 @@ uev_trigger (struct uevent * uev, void * trigger_data)
 	put_multipath_config(conf);
 
 	if (!strncmp(uev->action, "add", 3)) {
-		r = uev_add_path(uev, vecs);
+		r = uev_add_path(uev, vecs, 1);
 		goto out;
 	}
 	if (!strncmp(uev->action, "remove", 6)) {
@@ -1570,7 +1577,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
 			conf = get_multipath_config();
 			ret = pathinfo(pp, conf, DI_ALL | DI_BLACKLIST);
 			if (ret == PATHINFO_OK) {
-				ev_add_path(pp, vecs);
+				ev_add_path(pp, vecs, 1);
 				pp->tick = 1;
 			} else if (ret == PATHINFO_SKIPPED) {
 				put_multipath_config(conf);
@@ -1686,7 +1693,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
 		}
 		if (!disable_reinstate && reinstate_path(pp, add_active)) {
 			condlog(3, "%s: reload map", pp->dev);
-			ev_add_path(pp, vecs);
+			ev_add_path(pp, vecs, 1);
 			pp->tick = 1;
 			return 0;
 		}
@@ -1709,7 +1716,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
 			/* Clear IO errors */
 			if (reinstate_path(pp, 0)) {
 				condlog(3, "%s: reload map", pp->dev);
-				ev_add_path(pp, vecs);
+				ev_add_path(pp, vecs, 1);
 				pp->tick = 1;
 				return 0;
 			}
diff --git a/multipathd/main.h b/multipathd/main.h
index f72580d..f810d41 100644
--- a/multipathd/main.h
+++ b/multipathd/main.h
@@ -22,7 +22,7 @@ void exit_daemon(void);
 const char * daemon_status(void);
 int need_to_delay_reconfig (struct vectors *);
 int reconfigure (struct vectors *);
-int ev_add_path (struct path *, struct vectors *);
+int ev_add_path (struct path *, struct vectors *, int);
 int ev_remove_path (struct path *, struct vectors *);
 int ev_add_map (char *, char *, struct vectors *);
 int ev_remove_map (char *, char *, int, struct vectors *);
-- 
2.8.1.windows.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 05/12] multipathd: add need_do_map to indicate whether need calling domap() in ev_remove_path()
  2016-12-27  8:03 [PATCH 00/12] multipath-tools: improve processing efficiency for addition and deletion of multipath devices tang.junhui
                   ` (3 preceding siblings ...)
  2016-12-27  8:03 ` [PATCH 04/12] multipathd: add need_do_map to indicate whether need calling domap() in ev_add_path() tang.junhui
@ 2016-12-27  8:03 ` tang.junhui
  2016-12-27  8:03 ` [PATCH 06/12] multipathd: move uev_discard() to uevent.c and change its name to uevent_can_discard() tang.junhui
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 33+ messages in thread
From: tang.junhui @ 2016-12-27  8:03 UTC (permalink / raw)
  To: christophe.varoqui, hare, bmarzins, mwilck, bart.vanassche
  Cc: zhang.kai16, dm-devel, tang.junhui, tang.wenjun3

From: tang.junhui <tang.junhui@zte.com.cn>

Usually calling domap() in ev_remove_path() is needed, but only last
path need to call domap() in processing for merged uevents to reduce the
count of calling domap() and promote efficiency. So add input parameter
need_do_map to indicate whether need calling domap() in ev_remove_path().

Change-Id: I0a787330a249608396cc3e655465dc820f940539
Signed-off-by: tang.wenjun <tang.wenjun3@zte.com.cn>
---
 multipathd/cli_handlers.c |  2 +-
 multipathd/main.c         | 11 +++++++----
 multipathd/main.h         |  2 +-
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 3a46c09..302fd02 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -693,7 +693,7 @@ cli_del_path (void * v, char ** reply, int * len, void * data)
 		condlog(0, "%s: path already removed", param);
 		return 1;
 	}
-	return ev_remove_path(pp, vecs);
+	return ev_remove_path(pp, vecs, 1);
 }
 
 int
diff --git a/multipathd/main.c b/multipathd/main.c
index 34b1b64..68c8e17 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -840,7 +840,7 @@ fail:
 }
 
 static int
-uev_remove_path (struct uevent *uev, struct vectors * vecs)
+uev_remove_path (struct uevent *uev, struct vectors * vecs, int need_do_map)
 {
 	struct path *pp;
 	int ret;
@@ -851,7 +851,7 @@ uev_remove_path (struct uevent *uev, struct vectors * vecs)
 	pthread_testcancel();
 	pp = find_path_by_dev(vecs->pathvec, uev->kernel);
 	if (pp)
-		ret = ev_remove_path(pp, vecs);
+		ret = ev_remove_path(pp, vecs, need_do_map);
 	lock_cleanup_pop(vecs->lock);
 	if (!pp) {
 		/* Not an error; path might have been purged earlier */
@@ -862,7 +862,7 @@ uev_remove_path (struct uevent *uev, struct vectors * vecs)
 }
 
 int
-ev_remove_path (struct path *pp, struct vectors * vecs)
+ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map)
 {
 	struct multipath * mpp;
 	int i, retval = 0;
@@ -884,6 +884,9 @@ ev_remove_path (struct path *pp, struct vectors * vecs)
 		if ((i = find_slot(mpp->paths, (void *)pp)) != -1)
 			vector_del_slot(mpp->paths, i);
 
+		if(!need_do_map)
+			goto out;
+
 		/*
 		 * remove the map IFF removing the last path
 		 */
@@ -1161,7 +1164,7 @@ uev_trigger (struct uevent * uev, void * trigger_data)
 		goto out;
 	}
 	if (!strncmp(uev->action, "remove", 6)) {
-		r = uev_remove_path(uev, vecs);
+		r = uev_remove_path(uev, vecs, 1);
 		goto out;
 	}
 	if (!strncmp(uev->action, "change", 6)) {
diff --git a/multipathd/main.h b/multipathd/main.h
index f810d41..094b04f 100644
--- a/multipathd/main.h
+++ b/multipathd/main.h
@@ -23,7 +23,7 @@ const char * daemon_status(void);
 int need_to_delay_reconfig (struct vectors *);
 int reconfigure (struct vectors *);
 int ev_add_path (struct path *, struct vectors *, int);
-int ev_remove_path (struct path *, struct vectors *);
+int ev_remove_path (struct path *, struct vectors *, int);
 int ev_add_map (char *, char *, struct vectors *);
 int ev_remove_map (char *, char *, int, struct vectors *);
 void sync_map_state (struct multipath *);
-- 
2.8.1.windows.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 06/12] multipathd: move uev_discard() to uevent.c and change its name to uevent_can_discard()
  2016-12-27  8:03 [PATCH 00/12] multipath-tools: improve processing efficiency for addition and deletion of multipath devices tang.junhui
                   ` (4 preceding siblings ...)
  2016-12-27  8:03 ` [PATCH 05/12] multipathd: add need_do_map to indicate whether need calling domap() in ev_remove_path() tang.junhui
@ 2016-12-27  8:03 ` tang.junhui
  2016-12-27  8:03 ` [PATCH 07/12] multipathd: move calling filter_devnode() from uev_trigger() " tang.junhui
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 33+ messages in thread
From: tang.junhui @ 2016-12-27  8:03 UTC (permalink / raw)
  To: christophe.varoqui, hare, bmarzins, mwilck, bart.vanassche
  Cc: zhang.kai16, dm-devel, tang.junhui, tang.wenjun3

From: tang.junhui <tang.junhui@zte.com.cn>

Move uev_discard() form uevent processing thread to uevent listening
thread to discard unnecessary uevents as soon as possible.

Change-Id: Iaac159ffe3930e53c3325d1069c3ed497e440c0c
Signed-off-by: tang.wenjun <tang.wenjun3@zte.com.cn>
---
 libmultipath/uevent.c | 28 ++++++++++++++++++++++++++++
 multipathd/main.c     | 25 -------------------------
 2 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 181b3b8..ac49cac 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -24,6 +24,7 @@
 
 #include <unistd.h>
 #include <stdio.h>
+#include <stdbool.h>
 #include <errno.h>
 #include <stdlib.h>
 #include <stddef.h>
@@ -80,6 +81,28 @@ struct uevent * alloc_uevent (void)
 	return uev;
 }
 
+bool
+uevent_can_discard(char *devpath)
+{
+	char *tmp;
+	char a[11], b[11];
+
+	/*
+	 * keep only block devices, discard partitions
+	 */
+	tmp = strstr(devpath, "/block/");
+	if (tmp == NULL){
+		condlog(4, "no /block/ in '%s'", devpath);
+		return true;
+	}
+	if (sscanf(tmp, "/block/%10s", a) != 1 ||
+	    sscanf(tmp, "/block/%10[^/]/%10s", a, b) == 2) {
+		condlog(4, "discard event on %s", devpath);
+		return true;
+	}
+	return false;
+}
+
 void
 service_uevq(struct list_head *tmpq)
 {
@@ -527,6 +550,11 @@ int uevent_listen(struct udev *udev)
 			uev = uevent_from_udev_device(dev);
 			if (!uev)
 				continue;
+			if (uevent_can_discard(uev->devpath)) {
+				 udev_device_unref(uev->udev);
+				 FREE(uev);
+				 continue;
+			}
 			list_add_tail(&uev->node, &uevlisten_tmp);
 			events++;
 			continue;
diff --git a/multipathd/main.c b/multipathd/main.c
index 68c8e17..51f7b60 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1087,28 +1087,6 @@ uxsock_trigger (char * str, char ** reply, int * len, void * trigger_data)
 	return r;
 }
 
-static int
-uev_discard(char * devpath)
-{
-	char *tmp;
-	char a[11], b[11];
-
-	/*
-	 * keep only block devices, discard partitions
-	 */
-	tmp = strstr(devpath, "/block/");
-	if (tmp == NULL){
-		condlog(4, "no /block/ in '%s'", devpath);
-		return 1;
-	}
-	if (sscanf(tmp, "/block/%10s", a) != 1 ||
-	    sscanf(tmp, "/block/%10[^/]/%10s", a, b) == 2) {
-		condlog(4, "discard event on %s", devpath);
-		return 1;
-	}
-	return 0;
-}
-
 int
 uev_trigger (struct uevent * uev, void * trigger_data)
 {
@@ -1118,9 +1096,6 @@ uev_trigger (struct uevent * uev, void * trigger_data)
 
 	vecs = (struct vectors *)trigger_data;
 
-	if (uev_discard(uev->devpath))
-		return 0;
-
 	pthread_cleanup_push(config_cleanup, NULL);
 	pthread_mutex_lock(&config_lock);
 	if (running_state != DAEMON_IDLE &&
-- 
2.8.1.windows.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 07/12] multipathd: move calling filter_devnode() from uev_trigger() to uevent_can_discard()
  2016-12-27  8:03 [PATCH 00/12] multipath-tools: improve processing efficiency for addition and deletion of multipath devices tang.junhui
                   ` (5 preceding siblings ...)
  2016-12-27  8:03 ` [PATCH 06/12] multipathd: move uev_discard() to uevent.c and change its name to uevent_can_discard() tang.junhui
@ 2016-12-27  8:03 ` tang.junhui
  2016-12-27  8:03 ` [PATCH 08/12] libmultipath: wait one seconds for more uevents in uevent_listen() in uevents burst situations tang.junhui
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 33+ messages in thread
From: tang.junhui @ 2016-12-27  8:03 UTC (permalink / raw)
  To: christophe.varoqui, hare, bmarzins, mwilck, bart.vanassche
  Cc: zhang.kai16, dm-devel, tang.junhui, tang.wenjun3

From: tang.junhui <tang.junhui@zte.com.cn>

Move calling filter_devnode() from uev_trigger() to uevent_can_discard()
since they do the similar work.

Change-Id: I0322443fa551b21aa3211bf1ce3fad7d37aeeab4
Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
---
 libmultipath/uevent.c | 25 +++++++++++++++++++++++--
 multipathd/main.c     |  9 ---------
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index ac49cac..cc10d65 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -47,6 +47,9 @@
 #include "list.h"
 #include "uevent.h"
 #include "vector.h"
+#include "structs.h"
+#include "config.h"
+#include "blacklist.h"
 
 typedef int (uev_trigger)(struct uevent *, void * trigger_data);
 
@@ -82,10 +85,11 @@ struct uevent * alloc_uevent (void)
 }
 
 bool
-uevent_can_discard(char *devpath)
+uevent_can_discard(char *devpath, char *kernel)
 {
 	char *tmp;
 	char a[11], b[11];
+	struct config * conf;
 
 	/*
 	 * keep only block devices, discard partitions
@@ -100,6 +104,23 @@ uevent_can_discard(char *devpath)
 		condlog(4, "discard event on %s", devpath);
 		return true;
 	}
+
+	/* 
+	 * do not filter dm devices by devnode
+	 */
+	if (!strncmp(kernel, "dm-", 3))
+		return false;
+	/* 
+	 * filter paths devices by devnode
+	 */
+	conf = get_multipath_config();
+	if (filter_devnode(conf->blist_devnode, conf->elist_devnode,
+			   kernel) > 0) {
+		put_multipath_config(conf);
+		return true;
+	}
+	put_multipath_config(conf);
+
 	return false;
 }
 
@@ -550,7 +571,7 @@ int uevent_listen(struct udev *udev)
 			uev = uevent_from_udev_device(dev);
 			if (!uev)
 				continue;
-			if (uevent_can_discard(uev->devpath)) {
+			if (uevent_can_discard(uev->devpath, uev->kernel)) {
 				 udev_device_unref(uev->udev);
 				 FREE(uev);
 				 continue;
diff --git a/multipathd/main.c b/multipathd/main.c
index 51f7b60..15a6175 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1092,7 +1092,6 @@ uev_trigger (struct uevent * uev, void * trigger_data)
 {
 	int r = 0;
 	struct vectors * vecs;
-	struct config *conf;
 
 	vecs = (struct vectors *)trigger_data;
 
@@ -1126,14 +1125,6 @@ uev_trigger (struct uevent * uev, void * trigger_data)
 	/*
 	 * path add/remove event
 	 */
-	conf = get_multipath_config();
-	if (filter_devnode(conf->blist_devnode, conf->elist_devnode,
-			   uev->kernel) > 0) {
-		put_multipath_config(conf);
-		goto out;
-	}
-	put_multipath_config(conf);
-
 	if (!strncmp(uev->action, "add", 3)) {
 		r = uev_add_path(uev, vecs, 1);
 		goto out;
-- 
2.8.1.windows.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 08/12] libmultipath: wait one seconds for more uevents in uevent_listen() in uevents burst situations
  2016-12-27  8:03 [PATCH 00/12] multipath-tools: improve processing efficiency for addition and deletion of multipath devices tang.junhui
                   ` (6 preceding siblings ...)
  2016-12-27  8:03 ` [PATCH 07/12] multipathd: move calling filter_devnode() from uev_trigger() " tang.junhui
@ 2016-12-27  8:03 ` tang.junhui
  2016-12-28 20:25   ` Martin Wilck
  2017-01-03 22:31   ` Benjamin Marzinski
  2016-12-27  8:03 ` [PATCH 09/12] multipathd: merge uevents before proccessing tang.junhui
                   ` (3 subsequent siblings)
  11 siblings, 2 replies; 33+ messages in thread
From: tang.junhui @ 2016-12-27  8:03 UTC (permalink / raw)
  To: christophe.varoqui, hare, bmarzins, mwilck, bart.vanassche
  Cc: zhang.kai16, dm-devel, tang.junhui, tang.wenjun3

From: tang.junhui <tang.junhui@zte.com.cn>

The more uevents are merged, the higher efficiency program will performs.
So, do not process uevents after receiving immediately in uevents burst
situations, but continue wait 1 seconds for more uevents except that too
much uevents (2048) have already been received or too much time eclipse
(60 seconds).

Change-Id: I763d491540e8114a81d12d603281540a81502742
Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
---
 libmultipath/uevent.c | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index cc10d65..b0b05e9 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -39,6 +39,7 @@
 #include <linux/netlink.h>
 #include <pthread.h>
 #include <sys/mman.h>
+#include <sys/time.h>
 #include <libudev.h>
 #include <errno.h>
 
@@ -490,11 +491,37 @@ struct uevent *uevent_from_udev_device(struct udev_device *dev)
 	return uev;
 }
 
+bool uevent_burst(struct timeval *start_time, int events)
+{
+	struct timeval diff_time, end_time;
+	unsigned long speed;
+	unsigned long eclipse_ms;
+
+	gettimeofday(&end_time, NULL);
+	timersub(&end_time, start_time, &diff_time);
+
+	eclipse_ms = diff_time.tv_sec * 1000 + diff_time.tv_usec / 1000;
+	if (eclipse_ms == 0)
+		return true;
+	/* max wait 60s */
+	if (eclipse_ms > 60*1000) {
+		condlog(1, "burst continued =%lu ms, stoped", eclipse_ms);
+		return false;
+	}
+
+	speed = (events * 1000) / eclipse_ms;
+	if (speed > 10)
+		return true;
+
+	return false;
+}
+
 int uevent_listen(struct udev *udev)
 {
 	int err = 2;
 	struct udev_monitor *monitor = NULL;
 	int fd, socket_flags, events;
+	struct timeval start_time;
 	int need_failback = 1;
 	int timeout = 30;
 	LIST_HEAD(uevlisten_tmp);
@@ -548,6 +575,7 @@ int uevent_listen(struct udev *udev)
 	}
 
 	events = 0;
+	gettimeofday(&start_time, NULL);
 	while (1) {
 		struct uevent *uev;
 		struct udev_device *dev;
@@ -562,7 +590,8 @@ int uevent_listen(struct udev *udev)
 		errno = 0;
 		fdcount = poll(&ev_poll, 1, poll_timeout);
 		if (fdcount && ev_poll.revents & POLLIN) {
-			timeout = 0;
+			timeout = uevent_burst(&start_time, events + 1) ? 1:0;
+
 			dev = udev_monitor_receive_device(monitor);
 			if (!dev) {
 				condlog(0, "failed getting udev device");
@@ -578,7 +607,8 @@ int uevent_listen(struct udev *udev)
 			}
 			list_add_tail(&uev->node, &uevlisten_tmp);
 			events++;
-			continue;
+			if(events < 2048)
+				continue;
 		}
 		if (fdcount < 0) {
 			if (errno == EINTR)
@@ -600,6 +630,7 @@ int uevent_listen(struct udev *udev)
 			pthread_mutex_unlock(uevq_lockp);
 			events = 0;
 		}
+		gettimeofday(&start_time, NULL);
 		timeout = 30;
 	}
 	need_failback = 0;
-- 
2.8.1.windows.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 09/12] multipathd: merge uevents before proccessing
  2016-12-27  8:03 [PATCH 00/12] multipath-tools: improve processing efficiency for addition and deletion of multipath devices tang.junhui
                   ` (7 preceding siblings ...)
  2016-12-27  8:03 ` [PATCH 08/12] libmultipath: wait one seconds for more uevents in uevent_listen() in uevents burst situations tang.junhui
@ 2016-12-27  8:03 ` tang.junhui
  2017-01-04  0:30   ` Benjamin Marzinski
  2016-12-27  8:03 ` [PATCH 10/12] libmultipath: filter " tang.junhui
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: tang.junhui @ 2016-12-27  8:03 UTC (permalink / raw)
  To: christophe.varoqui, hare, bmarzins, mwilck, bart.vanassche
  Cc: zhang.kai16, dm-devel, tang.junhui, tang.wenjun3

From: tang.junhui <tang.junhui@zte.com.cn>

These uevents are going to be merged:
1) uevents come from paths and
2) uevents type is same and
3) uevents type is addition or deletion and
4) uevents wwid is same.

Change-Id: I05ee057391c092aa0c5f989b7a4f9cb550bb4d98
Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
---
 libmultipath/uevent.c | 125 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 114 insertions(+), 11 deletions(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index b0b05e9..114068c 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -85,6 +85,20 @@ struct uevent * alloc_uevent (void)
 	return uev;
 }
 
+void
+uevq_cleanup(struct list_head *tmpq)
+{
+	struct uevent *uev, *tmp;
+
+	list_for_each_entry_safe(uev, tmp, tmpq, node) {
+		list_del_init(&uev->node);
+
+		if (uev->udev)
+			udev_device_unref(uev->udev);
+		FREE(uev);
+	}
+}
+
 bool
 uevent_can_discard(char *devpath, char *kernel)
 {
@@ -125,6 +139,103 @@ uevent_can_discard(char *devpath, char *kernel)
 	return false;
 }
 
+bool
+merge_need_stop(struct uevent *earlier, struct uevent *later)
+{
+	/*
+	 * dm uevent do not try to merge with left uevents
+	 */
+	if (!strncmp(later->kernel, "dm-", 3))
+		return true;
+
+	/*
+	 * we can not make a jugement without wwid,
+	 * so it is sensible to stop merging
+	 */
+	if (!earlier->wwid || !later->wwid)
+		return true;
+	/*
+	 * uevents merging stoped
+	 * when we meet an opposite action uevent from the same LUN to AVOID
+	 * "add path1 |remove path1 |add path2 |remove path2 |add path3"
+	 * to merge as "remove path1, path2" and "add path1, path2, path3"
+	 * OR
+	 * "remove path1 |add path1 |remove path2 |add path2 |remove path3"
+	 * to merge as "add path1, path2" and "remove path1, path2, path3"
+	 * SO
+	 * when we meet a non-change uevent from the same LUN
+	 * with the same wwid and different action
+	 * it would be better to stop merging.
+	 */
+	if (!strcmp(earlier->wwid, later->wwid) &&
+	    strcmp(earlier->action, later->action) &&
+	    strcmp(earlier->action, "change") &&
+	    strcmp(later->action, "change"))
+		return true;
+
+	return false;
+}
+
+bool
+uevent_can_merge(struct uevent *earlier, struct uevent *later)
+{
+	/* merge paths uevents
+	 * whose wwids exsit and are same
+	 * and actions are same,
+	 * and actions are addition or deletion
+	 */
+	if (earlier->wwid && later->wwid &&
+	    !strcmp(earlier->wwid, later->wwid) &&
+	    !strcmp(earlier->action, later->action) &&
+	    strncmp(earlier->action, "change", 6) &&
+	    strncmp(earlier->kernel, "dm-", 3)) {
+		return true;
+	}
+
+	return false;
+}
+
+void
+uevent_merge(struct uevent *later, struct list_head *tmpq)
+{
+	struct uevent *earlier, *temp;
+	/*
+	 * compare the uevent with earlier uevents
+	 */
+	list_for_some_entry_reverse(earlier, &later->node, tmpq, node) {
+next_earlier_node:
+		if (merge_need_stop(earlier, later))
+			break;
+		/*
+		 * try to merge earlier uevents to the later uevent
+		 */
+		if (uevent_can_merge(earlier, later)) {
+			condlog(3, "merged uevent: %s-%s-%s with uevent: %s-%s-%s",
+				earlier->action, earlier->kernel, earlier->wwid,
+				later->action, later->kernel, later->wwid);
+			temp = earlier;
+
+			earlier = list_entry(earlier->node.prev, typeof(struct uevent), node);
+			list_move(&temp->node, &later->merge_node);
+
+			if (earlier ==  list_entry(tmpq, typeof(struct uevent), node))
+				break;
+			else
+				goto next_earlier_node;
+		}
+	}
+}
+
+void
+merge_uevq(struct list_head *tmpq)
+{
+	struct uevent *later;
+
+	list_for_each_entry_reverse(later, tmpq, node) {
+		uevent_merge(later, tmpq);
+	}
+}
+
 void
 service_uevq(struct list_head *tmpq)
 {
@@ -136,6 +247,8 @@ service_uevq(struct list_head *tmpq)
 		if (my_uev_trigger && my_uev_trigger(uev, my_trigger_data))
 			condlog(0, "uevent trigger error");
 
+		uevq_cleanup(&uev->merge_node);
+
 		if (uev->udev)
 			udev_device_unref(uev->udev);
 		FREE(uev);
@@ -150,17 +263,6 @@ static void uevent_cleanup(void *arg)
 	udev_unref(udev);
 }
 
-void
-uevq_cleanup(struct list_head *tmpq)
-{
-	struct uevent *uev, *tmp;
-
-	list_for_each_entry_safe(uev, tmp, tmpq, node) {
-		list_del_init(&uev->node);
-		FREE(uev);
-	}
-}
-
 /*
  * Service the uevent queue.
  */
@@ -189,6 +291,7 @@ int uevent_dispatch(int (*uev_trigger)(struct uevent *, void * trigger_data),
 		pthread_mutex_unlock(uevq_lockp);
 		if (!my_uev_trigger)
 			break;
+		merge_uevq(&uevq_tmp);
 		service_uevq(&uevq_tmp);
 	}
 	condlog(3, "Terminating uev service queue");
-- 
2.8.1.windows.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 10/12] libmultipath: filter uevents before proccessing
  2016-12-27  8:03 [PATCH 00/12] multipath-tools: improve processing efficiency for addition and deletion of multipath devices tang.junhui
                   ` (8 preceding siblings ...)
  2016-12-27  8:03 ` [PATCH 09/12] multipathd: merge uevents before proccessing tang.junhui
@ 2016-12-27  8:03 ` tang.junhui
  2017-01-04  1:21   ` Benjamin Marzinski
  2016-12-27  8:03 ` [PATCH 11/12] multipathd: proccess merged uevents tang.junhui
  2016-12-27  8:03 ` [PATCH 12/12] libmultipath: use existing wwid when wwid has already been existed in uevent tang.junhui
  11 siblings, 1 reply; 33+ messages in thread
From: tang.junhui @ 2016-12-27  8:03 UTC (permalink / raw)
  To: christophe.varoqui, hare, bmarzins, mwilck, bart.vanassche
  Cc: zhang.kai16, dm-devel, tang.junhui, tang.wenjun3

From: tang.junhui <tang.junhui@zte.com.cn>

Before merging uevents, these uevents are going to be filtered:
Change or addition uevent of a removed path (it indicate by an
deletion uevent occurred later).

Change-Id: If00c2c2e23ea466c1d4643c541ed2d8f9a0c8dea
Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
---
 libmultipath/uevent.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 114068c..424f272 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -140,6 +140,28 @@ uevent_can_discard(char *devpath, char *kernel)
 }
 
 bool
+uevent_can_filter(struct uevent *earlier, struct uevent *later)
+{
+
+	/*
+	 * filter earlier uvents if path has removed later, eg:
+	 * "add path3 |chang path3 |add path2 |remove path3"
+	 * can filter as:
+	 * "add path2 |remove path3"
+	 * uevent "add path3" and "chang path3" are filtered out
+	 */
+	if (earlier->wwid && later->wwid &&
+		!strcmp(earlier->wwid, later->wwid) &&
+		strncmp(later->kernel, "dm-", 3) &&
+		!strcmp(later->action, "remove") &&
+		!strcmp(earlier->kernel, later->kernel)) {
+		return true;
+	}
+
+	return false;
+}
+
+bool
 merge_need_stop(struct uevent *earlier, struct uevent *later)
 {
 	/*
@@ -196,6 +218,38 @@ uevent_can_merge(struct uevent *earlier, struct uevent *later)
 }
 
 void
+uevent_filter(struct uevent *later, struct list_head *tmpq)
+{
+	struct uevent *earlier, *temp;
+	/*
+	 * compare the uevent with earlier uevents
+	 */
+	list_for_some_entry_reverse(earlier, &later->node, tmpq, node) {
+next_earlier_node:
+		/*
+		 * filter unnessary earlier uevents by the later uevent
+		 */
+		if (uevent_can_filter(earlier, later)) {
+			condlog(2, "uevent: %s-%s-%s has removed by uevent: %s-%s-%s, filtered",
+				earlier->action, earlier->kernel, earlier->wwid,
+				later->action, later->kernel, later->wwid);
+
+			temp = earlier;
+			earlier = list_entry(earlier->node.prev, typeof(struct uevent), node);
+			list_del_init(&temp->node);
+			if (temp->udev)
+				udev_device_unref(temp->udev);
+			FREE(temp);
+
+			if (earlier ==  list_entry(tmpq, typeof(struct uevent), node))
+				break;
+			else
+				goto next_earlier_node;
+		}
+	}
+}
+
+void
 uevent_merge(struct uevent *later, struct list_head *tmpq)
 {
 	struct uevent *earlier, *temp;
@@ -232,6 +286,7 @@ merge_uevq(struct list_head *tmpq)
 	struct uevent *later;
 
 	list_for_each_entry_reverse(later, tmpq, node) {
+		uevent_filter(later, tmpq);
 		uevent_merge(later, tmpq);
 	}
 }
-- 
2.8.1.windows.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 11/12] multipathd: proccess merged uevents
  2016-12-27  8:03 [PATCH 00/12] multipath-tools: improve processing efficiency for addition and deletion of multipath devices tang.junhui
                   ` (9 preceding siblings ...)
  2016-12-27  8:03 ` [PATCH 10/12] libmultipath: filter " tang.junhui
@ 2016-12-27  8:03 ` tang.junhui
  2017-01-04  1:03   ` Benjamin Marzinski
  2016-12-27  8:03 ` [PATCH 12/12] libmultipath: use existing wwid when wwid has already been existed in uevent tang.junhui
  11 siblings, 1 reply; 33+ messages in thread
From: tang.junhui @ 2016-12-27  8:03 UTC (permalink / raw)
  To: christophe.varoqui, hare, bmarzins, mwilck, bart.vanassche
  Cc: zhang.kai16, dm-devel, tang.junhui, tang.wenjun3

From: "tang.junhui" <tang.junhui@zte.com.cn>

After filtering and merging, then uevents are processed in
uev_trigger(), firstly, each of merged uevents would be processed one
by one with need_do_map in value of 0. Finally, the node “uev” itself
would be processed with need_do_map in value of 1, which would reload
kernel table, and create or remove the dm device.

Change-Id: Ifb4de0ca36180a8af16729c2f70bc250858877bd
Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
---
 multipathd/main.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 15a6175..0b18f6c 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1092,6 +1092,7 @@ uev_trigger (struct uevent * uev, void * trigger_data)
 {
 	int r = 0;
 	struct vectors * vecs;
+	struct uevent *merge_uev, *tmp;
 
 	vecs = (struct vectors *)trigger_data;
 
@@ -1123,20 +1124,21 @@ uev_trigger (struct uevent * uev, void * trigger_data)
 	}
 
 	/*
-	 * path add/remove event
+	 * path add/remove/change event, add/remove maybe merged
 	 */
-	if (!strncmp(uev->action, "add", 3)) {
-		r = uev_add_path(uev, vecs, 1);
-		goto out;
-	}
-	if (!strncmp(uev->action, "remove", 6)) {
-		r = uev_remove_path(uev, vecs, 1);
-		goto out;
-	}
-	if (!strncmp(uev->action, "change", 6)) {
-		r = uev_update_path(uev, vecs);
-		goto out;
-	}
+	list_for_each_entry_safe(merge_uev, tmp, &uev->merge_node, node) {
+		if (!strncmp(merge_uev->action, "add", 3))
+			r += uev_add_path(merge_uev, vecs, 0);
+		if (!strncmp(merge_uev->action, "remove", 6))
+			r += uev_remove_path(merge_uev, vecs, 0);
+	}
+
+	if (!strncmp(uev->action, "add", 3))
+		r += uev_add_path(uev, vecs, 1);
+	if (!strncmp(uev->action, "remove", 6))
+		r += uev_remove_path(uev, vecs, 1);
+	if (!strncmp(uev->action, "change", 6))
+		r += uev_update_path(uev, vecs);
 
 out:
 	return r;
-- 
2.8.1.windows.1



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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 12/12] libmultipath: use existing wwid when wwid has already been existed in uevent
  2016-12-27  8:03 [PATCH 00/12] multipath-tools: improve processing efficiency for addition and deletion of multipath devices tang.junhui
                   ` (10 preceding siblings ...)
  2016-12-27  8:03 ` [PATCH 11/12] multipathd: proccess merged uevents tang.junhui
@ 2016-12-27  8:03 ` tang.junhui
  11 siblings, 0 replies; 33+ messages in thread
From: tang.junhui @ 2016-12-27  8:03 UTC (permalink / raw)
  To: christophe.varoqui, hare, bmarzins, mwilck, bart.vanassche
  Cc: zhang.kai16, dm-devel, tang.junhui, tang.wenjun3

From: tang.junhui <tang.junhui@zte.com.cn>

Use existing wwid when wwid has already been existed in uevent, it
avoids to get wwid again in pathinfo(), and reduces the count of calling
getuid(), which would promote processing efficiency.

Change-Id: Ia0c7273d33a220e5b415ec42d6b72660018cf4d9
Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
---
 libmultipath/discovery.c | 9 ++++++---
 libmultipath/discovery.h | 4 +++-
 multipathd/main.c        | 4 ++--
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index d1aec31..4985e03 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -32,7 +32,7 @@
 #include "defaults.h"
 
 int
-alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
+alloc_path_with_pathinfo (struct config *conf, struct uevent *uev,
 			  int flag, struct path **pp_ptr)
 {
 	int err = PATHINFO_FAILED;
@@ -42,7 +42,7 @@ alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
 	if (pp_ptr)
 		*pp_ptr = NULL;
 
-	devname = udev_device_get_sysname(udevice);
+	devname = udev_device_get_sysname(uev->udev);
 	if (!devname)
 		return PATHINFO_FAILED;
 
@@ -51,10 +51,13 @@ alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
 	if (!pp)
 		return PATHINFO_FAILED;
 
+	if(uev->wwid)
+		strncpy(pp->wwid, uev->wwid, sizeof(pp->wwid));
+
 	if (safe_sprintf(pp->dev, "%s", devname)) {
 		condlog(0, "pp->dev too small");
 	} else {
-		pp->udev = udev_device_ref(udevice);
+		pp->udev = udev_device_ref(uev->udev);
 		err = pathinfo(pp, conf, flag | DI_BLACKLIST);
 	}
 
diff --git a/libmultipath/discovery.h b/libmultipath/discovery.h
index 3039268..df7de0f 100644
--- a/libmultipath/discovery.h
+++ b/libmultipath/discovery.h
@@ -1,6 +1,8 @@
 #ifndef DISCOVERY_H
 #define DISCOVERY_H
 
+#include "uevent.h"
+
 #define SYSFS_PATH_SIZE 255
 #define INQUIRY_CMDLEN  6
 #define INQUIRY_CMD     0x12
@@ -36,7 +38,7 @@ int do_tur (char *);
 int path_offline (struct path *);
 int get_state (struct path * pp, struct config * conf, int daemon);
 int pathinfo (struct path * pp, struct config * conf, int mask);
-int alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
+int alloc_path_with_pathinfo (struct config *conf, struct uevent *uev,
 			      int flag, struct path **pp_ptr);
 int store_pathinfo (vector pathvec, struct config *conf,
 		    struct udev_device *udevice, int flag,
diff --git a/multipathd/main.c b/multipathd/main.c
index 0b18f6c..7a1cd09 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -664,7 +664,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map)
 	 * get path vital state
 	 */
 	conf = get_multipath_config();
-	ret = alloc_path_with_pathinfo(conf, uev->udev,
+	ret = alloc_path_with_pathinfo(conf, uev,
 				       DI_ALL, &pp);
 	put_multipath_config(conf);
 	if (!pp) {
@@ -1026,7 +1026,7 @@ out:
 			int flag = DI_SYSFS | DI_WWID;
 
 			conf = get_multipath_config();
-			retval = alloc_path_with_pathinfo(conf, uev->udev, flag, NULL);
+			retval = alloc_path_with_pathinfo(conf, uev, flag, NULL);
 			put_multipath_config(conf);
 
 			if (retval == PATHINFO_SKIPPED) {
-- 
2.8.1.windows.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH 08/12] libmultipath: wait one seconds for more uevents in uevent_listen() in uevents burst situations
  2016-12-27  8:03 ` [PATCH 08/12] libmultipath: wait one seconds for more uevents in uevent_listen() in uevents burst situations tang.junhui
@ 2016-12-28 20:25   ` Martin Wilck
  2016-12-29  0:48     ` tang.junhui
  2017-01-03 22:31   ` Benjamin Marzinski
  1 sibling, 1 reply; 33+ messages in thread
From: Martin Wilck @ 2016-12-28 20:25 UTC (permalink / raw)
  To: tang.junhui, christophe.varoqui, hare, bmarzins, bart.vanassche
  Cc: zhang.kai16, dm-devel, tang.wenjun3

On Tue, 2016-12-27 at 16:03 +0800, tang.junhui@zte.com.cn wrote:
> From: tang.junhui <tang.junhui@zte.com.cn>
> 
> The more uevents are merged, the higher efficiency program will
> performs.
> So, do not process uevents after receiving immediately in uevents
> burst
> situations, but continue wait 1 seconds for more uevents except that
> too
> much uevents (2048) have already been received or too much time
> eclipse
> (60 seconds).

Hi Tang,

thanks for this impressive patch set. While I haven't had time to read
through the more complex parts yet, one small nitpick on this patch:
please use named constants rather than explicit numbers in the code.

Regards,
Martin


> 
> Change-Id: I763d491540e8114a81d12d603281540a81502742
> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> ---
>  libmultipath/uevent.c | 35 +++++++++++++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
> index cc10d65..b0b05e9 100644
> --- a/libmultipath/uevent.c
> +++ b/libmultipath/uevent.c
> @@ -39,6 +39,7 @@
>  #include <linux/netlink.h>
>  #include <pthread.h>
>  #include <sys/mman.h>
> +#include <sys/time.h>
>  #include <libudev.h>
>  #include <errno.h>
>  
> @@ -490,11 +491,37 @@ struct uevent *uevent_from_udev_device(struct
> udev_device *dev)
>  	return uev;
>  }
>  
> +bool uevent_burst(struct timeval *start_time, int events)
> +{
> +	struct timeval diff_time, end_time;
> +	unsigned long speed;
> +	unsigned long eclipse_ms;
> +
> +	gettimeofday(&end_time, NULL);
> +	timersub(&end_time, start_time, &diff_time);
> +
> +	eclipse_ms = diff_time.tv_sec * 1000 + diff_time.tv_usec /
> 1000;
> +	if (eclipse_ms == 0)
> +		return true;
> +	/* max wait 60s */
> +	if (eclipse_ms > 60*1000) {
> +		condlog(1, "burst continued =%lu ms, stoped",
> eclipse_ms);
> +		return false;
> +	}
> +
> +	speed = (events * 1000) / eclipse_ms;
> +	if (speed > 10)
> +		return true;
> +
> +	return false;
> +}
> +
>  int uevent_listen(struct udev *udev)
>  {
>  	int err = 2;
>  	struct udev_monitor *monitor = NULL;
>  	int fd, socket_flags, events;
> +	struct timeval start_time;
>  	int need_failback = 1;
>  	int timeout = 30;
>  	LIST_HEAD(uevlisten_tmp);
> @@ -548,6 +575,7 @@ int uevent_listen(struct udev *udev)
>  	}
>  
>  	events = 0;
> +	gettimeofday(&start_time, NULL);
>  	while (1) {
>  		struct uevent *uev;
>  		struct udev_device *dev;
> @@ -562,7 +590,8 @@ int uevent_listen(struct udev *udev)
>  		errno = 0;
>  		fdcount = poll(&ev_poll, 1, poll_timeout);
>  		if (fdcount && ev_poll.revents & POLLIN) {
> -			timeout = 0;
> +			timeout = uevent_burst(&start_time, events +
> 1) ? 1:0;
> +
>  			dev = udev_monitor_receive_device(monitor);
>  			if (!dev) {
>  				condlog(0, "failed getting udev
> device");
> @@ -578,7 +607,8 @@ int uevent_listen(struct udev *udev)
>  			}
>  			list_add_tail(&uev->node, &uevlisten_tmp);
>  			events++;
> -			continue;
> +			if(events < 2048)
> +				continue;
>  		}
>  		if (fdcount < 0) {
>  			if (errno == EINTR)
> @@ -600,6 +630,7 @@ int uevent_listen(struct udev *udev)
>  			pthread_mutex_unlock(uevq_lockp);
>  			events = 0;
>  		}
> +		gettimeofday(&start_time, NULL);
>  		timeout = 30;
>  	}
>  	need_failback = 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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 08/12] libmultipath: wait one seconds for more uevents in uevent_listen() in uevents burst situations
  2016-12-28 20:25   ` Martin Wilck
@ 2016-12-29  0:48     ` tang.junhui
  0 siblings, 0 replies; 33+ messages in thread
From: tang.junhui @ 2016-12-29  0:48 UTC (permalink / raw)
  To: Martin Wilck; +Cc: tang.wenjun3, zhang.kai16, dm-devel, bart.vanassche


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

Hi Martin,

Thank you for your respond, I will modify it and resend it later.

Regards,
Tang



发件人:         Martin Wilck <mwilck@suse.com>
收件人:         tang.junhui@zte.com.cn, christophe.varoqui@opensvc.com, 
hare@suse.de, bmarzins@redhat.com, bart.vanassche@sandisk.com, 
抄送:   dm-devel@redhat.com, zhang.kai16@zte.com.cn, 
tang.wenjun3@zte.com.cn
日期:   2016/12/29 04:26
主题:   Re: [PATCH 08/12] libmultipath: wait one seconds for more uevents 
in uevent_listen() in uevents burst situations



On Tue, 2016-12-27 at 16:03 +0800, tang.junhui@zte.com.cn wrote:
> From: tang.junhui <tang.junhui@zte.com.cn>
> 
> The more uevents are merged, the higher efficiency program will
> performs.
> So, do not process uevents after receiving immediately in uevents
> burst
> situations, but continue wait 1 seconds for more uevents except that
> too
> much uevents (2048) have already been received or too much time
> eclipse
> (60 seconds).

Hi Tang,

thanks for this impressive patch set. While I haven't had time to read
through the more complex parts yet, one small nitpick on this patch:
please use named constants rather than explicit numbers in the code.

Regards,
Martin


> 
> Change-Id: I763d491540e8114a81d12d603281540a81502742
> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> ---
>  libmultipath/uevent.c | 35 +++++++++++++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
> index cc10d65..b0b05e9 100644
> --- a/libmultipath/uevent.c
> +++ b/libmultipath/uevent.c
> @@ -39,6 +39,7 @@
>  #include <linux/netlink.h>
>  #include <pthread.h>
>  #include <sys/mman.h>
> +#include <sys/time.h>
>  #include <libudev.h>
>  #include <errno.h>
>  
> @@ -490,11 +491,37 @@ struct uevent *uevent_from_udev_device(struct
> udev_device *dev)
>                return uev;
>  }
>  
> +bool uevent_burst(struct timeval *start_time, int events)
> +{
> +              struct timeval diff_time, end_time;
> +              unsigned long speed;
> +              unsigned long eclipse_ms;
> +
> +              gettimeofday(&end_time, NULL);
> +              timersub(&end_time, start_time, &diff_time);
> +
> +              eclipse_ms = diff_time.tv_sec * 1000 + diff_time.tv_usec 
/
> 1000;
> +              if (eclipse_ms == 0)
> +                              return true;
> +              /* max wait 60s */
> +              if (eclipse_ms > 60*1000) {
> +                              condlog(1, "burst continued =%lu ms, 
stoped",
> eclipse_ms);
> +                              return false;
> +              }
> +
> +              speed = (events * 1000) / eclipse_ms;
> +              if (speed > 10)
> +                              return true;
> +
> +              return false;
> +}
> +
>  int uevent_listen(struct udev *udev)
>  {
>                int err = 2;
>                struct udev_monitor *monitor = NULL;
>                int fd, socket_flags, events;
> +              struct timeval start_time;
>                int need_failback = 1;
>                int timeout = 30;
>                LIST_HEAD(uevlisten_tmp);
> @@ -548,6 +575,7 @@ int uevent_listen(struct udev *udev)
>                }
>  
>                events = 0;
> +              gettimeofday(&start_time, NULL);
>                while (1) {
>                                struct uevent *uev;
>                                struct udev_device *dev;
> @@ -562,7 +590,8 @@ int uevent_listen(struct udev *udev)
>                                errno = 0;
>                                fdcount = poll(&ev_poll, 1, 
poll_timeout);
>                                if (fdcount && ev_poll.revents & POLLIN) 
{
> -                                              timeout = 0;
> +                                              timeout = 
uevent_burst(&start_time, events +
> 1) ? 1:0;
> +
>                                                dev = 
udev_monitor_receive_device(monitor);
>                                                if (!dev) {
>   condlog(0, "failed getting udev
> device");
> @@ -578,7 +607,8 @@ int uevent_listen(struct udev *udev)
>                                                }
>                                                list_add_tail(&uev->node, 
&uevlisten_tmp);
>                                                events++;
> -                                              continue;
> +                                              if(events < 2048)
> +                                                              continue;
>                                }
>                                if (fdcount < 0) {
>                                                if (errno == EINTR)
> @@ -600,6 +630,7 @@ int uevent_listen(struct udev *udev)
>   pthread_mutex_unlock(uevq_lockp);
>                                                events = 0;
>                                }
> +                              gettimeofday(&start_time, NULL);
>                                timeout = 30;
>                }
>                need_failback = 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)




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

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



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent
  2016-12-27  8:03 ` [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent tang.junhui
@ 2017-01-03 22:02   ` Benjamin Marzinski
  2017-01-04  6:56     ` tang.junhui
  0 siblings, 1 reply; 33+ messages in thread
From: Benjamin Marzinski @ 2017-01-03 22:02 UTC (permalink / raw)
  To: tang.junhui; +Cc: tang.wenjun3, zhang.kai16, dm-devel, bart.vanassche, mwilck

On Tue, Dec 27, 2016 at 04:03:18PM +0800, tang.junhui@zte.com.cn wrote:
> From: tang.junhui <tang.junhui@zte.com.cn>
> 
> Add "char *wwid" to point WWID of uevent. This member identifies
> the LUN ID which the path belongs to, and it is used for merging
> uevents. WWID possibly did not exist in uevent yet, so ->wwid
> would be NULL, those uevents would not be merged, but be proccessed
> as old way.

Right now, multipath users are allowed configure devices to set the wwid
based on any udev environment variable (or even use a callout, although
this is deprecated). With this patch, that breaks. If the udev sets
ID_SERIAL for a device, that is its wwid, right?  Do you know if rbd
devices have ID_SERIAL set? If so, this change will break them.  Even if
this change doesn't break any devices in their default configurations,
we would need to disallow changing how the wwid is set for this patch
to be safe.

-Ben 

> 
> Change-Id: Ie6b076363b3735dc7de10184b27fa799b499af0e
> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> ---
>  libmultipath/uevent.c | 2 ++
>  libmultipath/uevent.h | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
> index 7edcce1..ef1bafe 100644
> --- a/libmultipath/uevent.c
> +++ b/libmultipath/uevent.c
> @@ -424,6 +424,8 @@ struct uevent *uevent_from_udev_device(struct udev_device *dev)
>  			uev->devpath = uev->envp[i] + 8;
>  		if (strcmp(name, "ACTION") == 0)
>  			uev->action = uev->envp[i] + 7;
> +		if (strcmp(name, "ID_SERIAL") == 0)
> +			uev->wwid = uev->envp[i] + 10;
>  		i++;
>  		if (i == HOTPLUG_NUM_ENVP - 1)
>  			break;
> diff --git a/libmultipath/uevent.h b/libmultipath/uevent.h
> index 9d22dcd..7bfccef 100644
> --- a/libmultipath/uevent.h
> +++ b/libmultipath/uevent.h
> @@ -22,6 +22,7 @@ struct uevent {
>  	char *devpath;
>  	char *action;
>  	char *kernel;
> +	char *wwid;
>  	unsigned long seqnum;
>  	char *envp[HOTPLUG_NUM_ENVP];
>  };
> -- 
> 2.8.1.windows.1
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 08/12] libmultipath: wait one seconds for more uevents in uevent_listen() in uevents burst situations
  2016-12-27  8:03 ` [PATCH 08/12] libmultipath: wait one seconds for more uevents in uevent_listen() in uevents burst situations tang.junhui
  2016-12-28 20:25   ` Martin Wilck
@ 2017-01-03 22:31   ` Benjamin Marzinski
  2017-01-04  7:32     ` tang.junhui
  1 sibling, 1 reply; 33+ messages in thread
From: Benjamin Marzinski @ 2017-01-03 22:31 UTC (permalink / raw)
  To: tang.junhui; +Cc: tang.wenjun3, zhang.kai16, dm-devel, bart.vanassche, mwilck

On Tue, Dec 27, 2016 at 04:03:25PM +0800, tang.junhui@zte.com.cn wrote:
> From: tang.junhui <tang.junhui@zte.com.cn>
> 
> The more uevents are merged, the higher efficiency program will performs.
> So, do not process uevents after receiving immediately in uevents burst
> situations, but continue wait 1 seconds for more uevents except that too
> much uevents (2048) have already been received or too much time eclipse
> (60 seconds).

The issue I have with doing this in uevent_listen is that we seperated
receiving the uevents from servicing the uevents specificially to make
sure what we received them as fast as possible. The udev monitor code
is all based on a NETLINK socket. If our socket's receive buffer fills
up, there is no flow control. Events just start getting dropped, which
does cut down on processing time, but not in a way we would like.

This issue applies to a lesser extent to you previous two patches. I
don't really see the benefit of making sure that we drop the uevents
as soon as possible.  As long as we don't process them, that should
be good enough, right?

Now, maybe you put a lot of thought into your timeouts, and feel
confident that we will start processing well before the receive buffer
fills up. But if so, some comments on that would be reassuring, because
from the commit message, they seem fairly arbitrary to me.

-Ben

> 
> Change-Id: I763d491540e8114a81d12d603281540a81502742
> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> ---
>  libmultipath/uevent.c | 35 +++++++++++++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
> index cc10d65..b0b05e9 100644
> --- a/libmultipath/uevent.c
> +++ b/libmultipath/uevent.c
> @@ -39,6 +39,7 @@
>  #include <linux/netlink.h>
>  #include <pthread.h>
>  #include <sys/mman.h>
> +#include <sys/time.h>
>  #include <libudev.h>
>  #include <errno.h>
>  
> @@ -490,11 +491,37 @@ struct uevent *uevent_from_udev_device(struct udev_device *dev)
>  	return uev;
>  }
>  
> +bool uevent_burst(struct timeval *start_time, int events)
> +{
> +	struct timeval diff_time, end_time;
> +	unsigned long speed;
> +	unsigned long eclipse_ms;
> +
> +	gettimeofday(&end_time, NULL);
> +	timersub(&end_time, start_time, &diff_time);
> +
> +	eclipse_ms = diff_time.tv_sec * 1000 + diff_time.tv_usec / 1000;
> +	if (eclipse_ms == 0)
> +		return true;
> +	/* max wait 60s */
> +	if (eclipse_ms > 60*1000) {
> +		condlog(1, "burst continued =%lu ms, stoped", eclipse_ms);
> +		return false;
> +	}
> +
> +	speed = (events * 1000) / eclipse_ms;
> +	if (speed > 10)
> +		return true;
> +
> +	return false;
> +}
> +
>  int uevent_listen(struct udev *udev)
>  {
>  	int err = 2;
>  	struct udev_monitor *monitor = NULL;
>  	int fd, socket_flags, events;
> +	struct timeval start_time;
>  	int need_failback = 1;
>  	int timeout = 30;
>  	LIST_HEAD(uevlisten_tmp);
> @@ -548,6 +575,7 @@ int uevent_listen(struct udev *udev)
>  	}
>  
>  	events = 0;
> +	gettimeofday(&start_time, NULL);
>  	while (1) {
>  		struct uevent *uev;
>  		struct udev_device *dev;
> @@ -562,7 +590,8 @@ int uevent_listen(struct udev *udev)
>  		errno = 0;
>  		fdcount = poll(&ev_poll, 1, poll_timeout);
>  		if (fdcount && ev_poll.revents & POLLIN) {
> -			timeout = 0;
> +			timeout = uevent_burst(&start_time, events + 1) ? 1:0;
> +
>  			dev = udev_monitor_receive_device(monitor);
>  			if (!dev) {
>  				condlog(0, "failed getting udev device");
> @@ -578,7 +607,8 @@ int uevent_listen(struct udev *udev)
>  			}
>  			list_add_tail(&uev->node, &uevlisten_tmp);
>  			events++;
> -			continue;
> +			if(events < 2048)
> +				continue;
>  		}
>  		if (fdcount < 0) {
>  			if (errno == EINTR)
> @@ -600,6 +630,7 @@ int uevent_listen(struct udev *udev)
>  			pthread_mutex_unlock(uevq_lockp);
>  			events = 0;
>  		}
> +		gettimeofday(&start_time, NULL);
>  		timeout = 30;
>  	}
>  	need_failback = 0;
> -- 
> 2.8.1.windows.1
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 09/12] multipathd: merge uevents before proccessing
  2016-12-27  8:03 ` [PATCH 09/12] multipathd: merge uevents before proccessing tang.junhui
@ 2017-01-04  0:30   ` Benjamin Marzinski
  2017-01-04  3:29     ` tang.junhui
  0 siblings, 1 reply; 33+ messages in thread
From: Benjamin Marzinski @ 2017-01-04  0:30 UTC (permalink / raw)
  To: tang.junhui; +Cc: tang.wenjun3, zhang.kai16, dm-devel, bart.vanassche, mwilck

On Tue, Dec 27, 2016 at 04:03:26PM +0800, tang.junhui@zte.com.cn wrote:
> From: tang.junhui <tang.junhui@zte.com.cn>
> 
> These uevents are going to be merged:
> 1) uevents come from paths and
> 2) uevents type is same and
> 3) uevents type is addition or deletion and
> 4) uevents wwid is same.

This is just a nit, and I might be missing something subtle here, but it
seems like instead of adding list_for_some_entry_reverse, and then
breaking the abstraction to manually get previous entries, you could
have just added list_for_some_entry_reverse_safe in your earlier patch,
and hid the work of traversing a list while removing elements behind the
well understood abstraction of a *_safe list traversal method.

-Ben

> 
> Change-Id: I05ee057391c092aa0c5f989b7a4f9cb550bb4d98
> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> ---
>  libmultipath/uevent.c | 125 +++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 114 insertions(+), 11 deletions(-)
> 
> diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
> index b0b05e9..114068c 100644
> --- a/libmultipath/uevent.c
> +++ b/libmultipath/uevent.c
> @@ -85,6 +85,20 @@ struct uevent * alloc_uevent (void)
>  	return uev;
>  }
>  
> +void
> +uevq_cleanup(struct list_head *tmpq)
> +{
> +	struct uevent *uev, *tmp;
> +
> +	list_for_each_entry_safe(uev, tmp, tmpq, node) {
> +		list_del_init(&uev->node);
> +
> +		if (uev->udev)
> +			udev_device_unref(uev->udev);
> +		FREE(uev);
> +	}
> +}
> +
>  bool
>  uevent_can_discard(char *devpath, char *kernel)
>  {
> @@ -125,6 +139,103 @@ uevent_can_discard(char *devpath, char *kernel)
>  	return false;
>  }
>  
> +bool
> +merge_need_stop(struct uevent *earlier, struct uevent *later)
> +{
> +	/*
> +	 * dm uevent do not try to merge with left uevents
> +	 */
> +	if (!strncmp(later->kernel, "dm-", 3))
> +		return true;
> +
> +	/*
> +	 * we can not make a jugement without wwid,
> +	 * so it is sensible to stop merging
> +	 */
> +	if (!earlier->wwid || !later->wwid)
> +		return true;
> +	/*
> +	 * uevents merging stoped
> +	 * when we meet an opposite action uevent from the same LUN to AVOID
> +	 * "add path1 |remove path1 |add path2 |remove path2 |add path3"
> +	 * to merge as "remove path1, path2" and "add path1, path2, path3"
> +	 * OR
> +	 * "remove path1 |add path1 |remove path2 |add path2 |remove path3"
> +	 * to merge as "add path1, path2" and "remove path1, path2, path3"
> +	 * SO
> +	 * when we meet a non-change uevent from the same LUN
> +	 * with the same wwid and different action
> +	 * it would be better to stop merging.
> +	 */
> +	if (!strcmp(earlier->wwid, later->wwid) &&
> +	    strcmp(earlier->action, later->action) &&
> +	    strcmp(earlier->action, "change") &&
> +	    strcmp(later->action, "change"))
> +		return true;
> +
> +	return false;
> +}
> +
> +bool
> +uevent_can_merge(struct uevent *earlier, struct uevent *later)
> +{
> +	/* merge paths uevents
> +	 * whose wwids exsit and are same
> +	 * and actions are same,
> +	 * and actions are addition or deletion
> +	 */
> +	if (earlier->wwid && later->wwid &&
> +	    !strcmp(earlier->wwid, later->wwid) &&
> +	    !strcmp(earlier->action, later->action) &&
> +	    strncmp(earlier->action, "change", 6) &&
> +	    strncmp(earlier->kernel, "dm-", 3)) {
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +void
> +uevent_merge(struct uevent *later, struct list_head *tmpq)
> +{
> +	struct uevent *earlier, *temp;
> +	/*
> +	 * compare the uevent with earlier uevents
> +	 */
> +	list_for_some_entry_reverse(earlier, &later->node, tmpq, node) {
> +next_earlier_node:
> +		if (merge_need_stop(earlier, later))
> +			break;
> +		/*
> +		 * try to merge earlier uevents to the later uevent
> +		 */
> +		if (uevent_can_merge(earlier, later)) {
> +			condlog(3, "merged uevent: %s-%s-%s with uevent: %s-%s-%s",
> +				earlier->action, earlier->kernel, earlier->wwid,
> +				later->action, later->kernel, later->wwid);
> +			temp = earlier;
> +
> +			earlier = list_entry(earlier->node.prev, typeof(struct uevent), node);
> +			list_move(&temp->node, &later->merge_node);
> +
> +			if (earlier ==  list_entry(tmpq, typeof(struct uevent), node))
> +				break;
> +			else
> +				goto next_earlier_node;
> +		}
> +	}
> +}
> +
> +void
> +merge_uevq(struct list_head *tmpq)
> +{
> +	struct uevent *later;
> +
> +	list_for_each_entry_reverse(later, tmpq, node) {
> +		uevent_merge(later, tmpq);
> +	}
> +}
> +
>  void
>  service_uevq(struct list_head *tmpq)
>  {
> @@ -136,6 +247,8 @@ service_uevq(struct list_head *tmpq)
>  		if (my_uev_trigger && my_uev_trigger(uev, my_trigger_data))
>  			condlog(0, "uevent trigger error");
>  
> +		uevq_cleanup(&uev->merge_node);
> +
>  		if (uev->udev)
>  			udev_device_unref(uev->udev);
>  		FREE(uev);
> @@ -150,17 +263,6 @@ static void uevent_cleanup(void *arg)
>  	udev_unref(udev);
>  }
>  
> -void
> -uevq_cleanup(struct list_head *tmpq)
> -{
> -	struct uevent *uev, *tmp;
> -
> -	list_for_each_entry_safe(uev, tmp, tmpq, node) {
> -		list_del_init(&uev->node);
> -		FREE(uev);
> -	}
> -}
> -
>  /*
>   * Service the uevent queue.
>   */
> @@ -189,6 +291,7 @@ int uevent_dispatch(int (*uev_trigger)(struct uevent *, void * trigger_data),
>  		pthread_mutex_unlock(uevq_lockp);
>  		if (!my_uev_trigger)
>  			break;
> +		merge_uevq(&uevq_tmp);
>  		service_uevq(&uevq_tmp);
>  	}
>  	condlog(3, "Terminating uev service queue");
> -- 
> 2.8.1.windows.1
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 11/12] multipathd: proccess merged uevents
  2016-12-27  8:03 ` [PATCH 11/12] multipathd: proccess merged uevents tang.junhui
@ 2017-01-04  1:03   ` Benjamin Marzinski
  2017-01-04  1:54     ` tang.junhui
  0 siblings, 1 reply; 33+ messages in thread
From: Benjamin Marzinski @ 2017-01-04  1:03 UTC (permalink / raw)
  To: tang.junhui; +Cc: tang.wenjun3, zhang.kai16, dm-devel, bart.vanassche, mwilck

On Tue, Dec 27, 2016 at 04:03:28PM +0800, tang.junhui@zte.com.cn wrote:
> From: "tang.junhui" <tang.junhui@zte.com.cn>
> 
> After filtering and merging, then uevents are processed in
> uev_trigger(), firstly, each of merged uevents would be processed one
> by one with need_do_map in value of 0. Finally, the node “uev” itself
> would be processed with need_do_map in value of 1, which would reload
> kernel table, and create or remove the dm device.

Wait, doesn't this mean that you would change

"add path1 | change path1 | add path2"

to

"change path1 | add path1, path2"

It doesn't make sense to process a change event before an add event.
Looking at uev_update_path, the three things it currently does all can
be safely ignored if you don't process the add event until after you
receive the change event. They are:

disable path on changed wwid: the multipath device hasn't been created
yet, so it will simply be created with the new wwid. That's fine.

attempt to get the pathinfo again if you failed when adding it: Either
you don't have the necessary udev information in the add event, in which
case that uevent won't get merged in the first place and they will
remain in order, or you do have the information, and there's no point in
processing the change event in that case.

change the Read-Only state of the device: since both events (the change
and the add event) have already happened, when you look at the device
for the add event, you will already get the current read-only state.

So, as far as I can tell, if you have change events in your queue as
well as an add event for the same device, you might as well just filter
out the change events, since processing it immediately after the add
event will never actually change anything, and processing it beforehand
makes no sense.

-Ben

> 
> Change-Id: Ifb4de0ca36180a8af16729c2f70bc250858877bd
> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> ---
>  multipathd/main.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 15a6175..0b18f6c 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -1092,6 +1092,7 @@ uev_trigger (struct uevent * uev, void * trigger_data)
>  {
>  	int r = 0;
>  	struct vectors * vecs;
> +	struct uevent *merge_uev, *tmp;
>  
>  	vecs = (struct vectors *)trigger_data;
>  
> @@ -1123,20 +1124,21 @@ uev_trigger (struct uevent * uev, void * trigger_data)
>  	}
>  
>  	/*
> -	 * path add/remove event
> +	 * path add/remove/change event, add/remove maybe merged
>  	 */
> -	if (!strncmp(uev->action, "add", 3)) {
> -		r = uev_add_path(uev, vecs, 1);
> -		goto out;
> -	}
> -	if (!strncmp(uev->action, "remove", 6)) {
> -		r = uev_remove_path(uev, vecs, 1);
> -		goto out;
> -	}
> -	if (!strncmp(uev->action, "change", 6)) {
> -		r = uev_update_path(uev, vecs);
> -		goto out;
> -	}
> +	list_for_each_entry_safe(merge_uev, tmp, &uev->merge_node, node) {
> +		if (!strncmp(merge_uev->action, "add", 3))
> +			r += uev_add_path(merge_uev, vecs, 0);
> +		if (!strncmp(merge_uev->action, "remove", 6))
> +			r += uev_remove_path(merge_uev, vecs, 0);
> +	}
> +
> +	if (!strncmp(uev->action, "add", 3))
> +		r += uev_add_path(uev, vecs, 1);
> +	if (!strncmp(uev->action, "remove", 6))
> +		r += uev_remove_path(uev, vecs, 1);
> +	if (!strncmp(uev->action, "change", 6))
> +		r += uev_update_path(uev, vecs);
>  
>  out:
>  	return r;
> -- 
> 2.8.1.windows.1
> 

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 10/12] libmultipath: filter uevents before proccessing
  2016-12-27  8:03 ` [PATCH 10/12] libmultipath: filter " tang.junhui
@ 2017-01-04  1:21   ` Benjamin Marzinski
  2017-01-04  2:03     ` tang.junhui
  0 siblings, 1 reply; 33+ messages in thread
From: Benjamin Marzinski @ 2017-01-04  1:21 UTC (permalink / raw)
  To: tang.junhui; +Cc: tang.wenjun3, zhang.kai16, dm-devel, bart.vanassche, mwilck

On Tue, Dec 27, 2016 at 04:03:27PM +0800, tang.junhui@zte.com.cn wrote:
> From: tang.junhui <tang.junhui@zte.com.cn>
> 
> Before merging uevents, these uevents are going to be filtered:
> Change or addition uevent of a removed path (it indicate by an
> deletion uevent occurred later).
> 

I think it's safe to remove add and change uevents if they are followed
by a remove event, regardless of whether or not they have a wwid, as
long as the kernel name is the same.  We only get the remove event when
the device is gone. Processing the add and change events will never get
us anything in these cases, because there is no device to act on.

-Ben

> Change-Id: If00c2c2e23ea466c1d4643c541ed2d8f9a0c8dea
> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> ---
>  libmultipath/uevent.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
> index 114068c..424f272 100644
> --- a/libmultipath/uevent.c
> +++ b/libmultipath/uevent.c
> @@ -140,6 +140,28 @@ uevent_can_discard(char *devpath, char *kernel)
>  }
>  
>  bool
> +uevent_can_filter(struct uevent *earlier, struct uevent *later)
> +{
> +
> +	/*
> +	 * filter earlier uvents if path has removed later, eg:
> +	 * "add path3 |chang path3 |add path2 |remove path3"
> +	 * can filter as:
> +	 * "add path2 |remove path3"
> +	 * uevent "add path3" and "chang path3" are filtered out
> +	 */
> +	if (earlier->wwid && later->wwid &&
> +		!strcmp(earlier->wwid, later->wwid) &&
> +		strncmp(later->kernel, "dm-", 3) &&
> +		!strcmp(later->action, "remove") &&
> +		!strcmp(earlier->kernel, later->kernel)) {
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +bool
>  merge_need_stop(struct uevent *earlier, struct uevent *later)
>  {
>  	/*
> @@ -196,6 +218,38 @@ uevent_can_merge(struct uevent *earlier, struct uevent *later)
>  }
>  
>  void
> +uevent_filter(struct uevent *later, struct list_head *tmpq)
> +{
> +	struct uevent *earlier, *temp;
> +	/*
> +	 * compare the uevent with earlier uevents
> +	 */
> +	list_for_some_entry_reverse(earlier, &later->node, tmpq, node) {
> +next_earlier_node:
> +		/*
> +		 * filter unnessary earlier uevents by the later uevent
> +		 */
> +		if (uevent_can_filter(earlier, later)) {
> +			condlog(2, "uevent: %s-%s-%s has removed by uevent: %s-%s-%s, filtered",
> +				earlier->action, earlier->kernel, earlier->wwid,
> +				later->action, later->kernel, later->wwid);
> +
> +			temp = earlier;
> +			earlier = list_entry(earlier->node.prev, typeof(struct uevent), node);
> +			list_del_init(&temp->node);
> +			if (temp->udev)
> +				udev_device_unref(temp->udev);
> +			FREE(temp);
> +
> +			if (earlier ==  list_entry(tmpq, typeof(struct uevent), node))
> +				break;
> +			else
> +				goto next_earlier_node;
> +		}
> +	}
> +}
> +
> +void
>  uevent_merge(struct uevent *later, struct list_head *tmpq)
>  {
>  	struct uevent *earlier, *temp;
> @@ -232,6 +286,7 @@ merge_uevq(struct list_head *tmpq)
>  	struct uevent *later;
>  
>  	list_for_each_entry_reverse(later, tmpq, node) {
> +		uevent_filter(later, tmpq);
>  		uevent_merge(later, tmpq);
>  	}
>  }
> -- 
> 2.8.1.windows.1
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 11/12] multipathd: proccess merged uevents
  2017-01-04  1:03   ` Benjamin Marzinski
@ 2017-01-04  1:54     ` tang.junhui
  0 siblings, 0 replies; 33+ messages in thread
From: tang.junhui @ 2017-01-04  1:54 UTC (permalink / raw)
  To: Benjamin Marzinski
  Cc: tang.wenjun3, zhang.kai16, dm-devel-bounces, dm-devel,
	bart.vanassche, mwilck


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

Hello Ben

Good idea,
I will modify the patch to filter these change uevents.

Thanks
Tang Junhui



发件人:         "Benjamin Marzinski" <bmarzins@redhat.com>
收件人:         tang.junhui@zte.com.cn, 
抄送:   tang.wenjun3@zte.com.cn, zhang.kai16@zte.com.cn, 
dm-devel@redhat.com, bart.vanassche@sandisk.com, mwilck@suse.com
日期:   2017/01/04 09:10
主题:   Re: [dm-devel] [PATCH 11/12] multipathd: proccess merged uevents
发件人: dm-devel-bounces@redhat.com



On Tue, Dec 27, 2016 at 04:03:28PM +0800, tang.junhui@zte.com.cn wrote:
> From: "tang.junhui" <tang.junhui@zte.com.cn>
> 
> After filtering and merging, then uevents are processed in
> uev_trigger(), firstly, each of merged uevents would be processed one
> by one with need_do_map in value of 0. Finally, the node “uev” itself
> would be processed with need_do_map in value of 1, which would reload
> kernel table, and create or remove the dm device.

Wait, doesn't this mean that you would change

"add path1 | change path1 | add path2"

to

"change path1 | add path1, path2"

It doesn't make sense to process a change event before an add event.
Looking at uev_update_path, the three things it currently does all can
be safely ignored if you don't process the add event until after you
receive the change event. They are:

disable path on changed wwid: the multipath device hasn't been created
yet, so it will simply be created with the new wwid. That's fine.

attempt to get the pathinfo again if you failed when adding it: Either
you don't have the necessary udev information in the add event, in which
case that uevent won't get merged in the first place and they will
remain in order, or you do have the information, and there's no point in
processing the change event in that case.

change the Read-Only state of the device: since both events (the change
and the add event) have already happened, when you look at the device
for the add event, you will already get the current read-only state.

So, as far as I can tell, if you have change events in your queue as
well as an add event for the same device, you might as well just filter
out the change events, since processing it immediately after the add
event will never actually change anything, and processing it beforehand
makes no sense.

-Ben

> 
> Change-Id: Ifb4de0ca36180a8af16729c2f70bc250858877bd
> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> ---
>  multipathd/main.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 15a6175..0b18f6c 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -1092,6 +1092,7 @@ uev_trigger (struct uevent * uev, void * 
trigger_data)
>  {
>                int r = 0;
>                struct vectors * vecs;
> +              struct uevent *merge_uev, *tmp;
> 
>                vecs = (struct vectors *)trigger_data;
> 
> @@ -1123,20 +1124,21 @@ uev_trigger (struct uevent * uev, void * 
trigger_data)
>                }
> 
>                /*
> -               * path add/remove event
> +               * path add/remove/change event, add/remove maybe merged
>                 */
> -              if (!strncmp(uev->action, "add", 3)) {
> -                              r = uev_add_path(uev, vecs, 1);
> -                              goto out;
> -              }
> -              if (!strncmp(uev->action, "remove", 6)) {
> -                              r = uev_remove_path(uev, vecs, 1);
> -                              goto out;
> -              }
> -              if (!strncmp(uev->action, "change", 6)) {
> -                              r = uev_update_path(uev, vecs);
> -                              goto out;
> -              }
> +              list_for_each_entry_safe(merge_uev, tmp, 
&uev->merge_node, node) {
> +                              if (!strncmp(merge_uev->action, "add", 
3))
> +                                              r += 
uev_add_path(merge_uev, vecs, 0);
> +                              if (!strncmp(merge_uev->action, "remove", 
6))
> +                                              r += 
uev_remove_path(merge_uev, vecs, 0);
> +              }
> +
> +              if (!strncmp(uev->action, "add", 3))
> +                              r += uev_add_path(uev, vecs, 1);
> +              if (!strncmp(uev->action, "remove", 6))
> +                              r += uev_remove_path(uev, vecs, 1);
> +              if (!strncmp(uev->action, "change", 6))
> +                              r += uev_update_path(uev, vecs);
> 
>  out:
>                return r;
> -- 
> 2.8.1.windows.1
> 

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


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

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



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 10/12] libmultipath: filter uevents before proccessing
  2017-01-04  1:21   ` Benjamin Marzinski
@ 2017-01-04  2:03     ` tang.junhui
  0 siblings, 0 replies; 33+ messages in thread
From: tang.junhui @ 2017-01-04  2:03 UTC (permalink / raw)
  To: Benjamin Marzinski
  Cc: bart.vanassche, dm-devel, mwilck, tang.wenjun3, zhang.kai16


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

Hello Ben,

I add wwid judgment for safe.
I think twice, and as you say, it is safe enough without
this judgment, I will delete these wwid judgment.

Tanks
Tang Junhui



发件人:         "Benjamin Marzinski" <bmarzins@redhat.com>
收件人:         tang.junhui@zte.com.cn, 
抄送:   tang.wenjun3@zte.com.cn, zhang.kai16@zte.com.cn, 
dm-devel@redhat.com, bart.vanassche@sandisk.com, mwilck@suse.com
日期:   2017/01/04 09:28
主题:   Re: [dm-devel] [PATCH 10/12] libmultipath: filter uevents before 
proccessing
发件人: dm-devel-bounces@redhat.com



On Tue, Dec 27, 2016 at 04:03:27PM +0800, tang.junhui@zte.com.cn wrote:
> From: tang.junhui <tang.junhui@zte.com.cn>
> 
> Before merging uevents, these uevents are going to be filtered:
> Change or addition uevent of a removed path (it indicate by an
> deletion uevent occurred later).
> 

I think it's safe to remove add and change uevents if they are followed
by a remove event, regardless of whether or not they have a wwid, as
long as the kernel name is the same.  We only get the remove event when
the device is gone. Processing the add and change events will never get
us anything in these cases, because there is no device to act on.

-Ben

> Change-Id: If00c2c2e23ea466c1d4643c541ed2d8f9a0c8dea
> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> ---
>  libmultipath/uevent.c | 55 
+++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
> index 114068c..424f272 100644
> --- a/libmultipath/uevent.c
> +++ b/libmultipath/uevent.c
> @@ -140,6 +140,28 @@ uevent_can_discard(char *devpath, char *kernel)
>  }
> 
>  bool
> +uevent_can_filter(struct uevent *earlier, struct uevent *later)
> +{
> +
> +              /*
> +               * filter earlier uvents if path has removed later, eg:
> +               * "add path3 |chang path3 |add path2 |remove path3"
> +               * can filter as:
> +               * "add path2 |remove path3"
> +               * uevent "add path3" and "chang path3" are filtered out
> +               */
> +              if (earlier->wwid && later->wwid &&
> +                              !strcmp(earlier->wwid, later->wwid) &&
> +                              strncmp(later->kernel, "dm-", 3) &&
> +                              !strcmp(later->action, "remove") &&
> +                              !strcmp(earlier->kernel, later->kernel)) 
{
> +                              return true;
> +              }
> +
> +              return false;
> +}
> +
> +bool
>  merge_need_stop(struct uevent *earlier, struct uevent *later)
>  {
>                /*
> @@ -196,6 +218,38 @@ uevent_can_merge(struct uevent *earlier, struct 
uevent *later)
>  }
> 
>  void
> +uevent_filter(struct uevent *later, struct list_head *tmpq)
> +{
> +              struct uevent *earlier, *temp;
> +              /*
> +               * compare the uevent with earlier uevents
> +               */
> +              list_for_some_entry_reverse(earlier, &later->node, tmpq, 
node) {
> +next_earlier_node:
> +                              /*
> +                               * filter unnessary earlier uevents by 
the later uevent
> +                               */
> +                              if (uevent_can_filter(earlier, later)) {
> +                                              condlog(2, "uevent: 
%s-%s-%s has removed by uevent: %s-%s-%s, filtered",
> + earlier->action, earlier->kernel, earlier->wwid,
> + later->action, later->kernel, later->wwid);
> +
> +                                              temp = earlier;
> +                                              earlier = 
list_entry(earlier->node.prev, typeof(struct uevent), node);
> + list_del_init(&temp->node);
> +                                              if (temp->udev)
> + udev_device_unref(temp->udev);
> +                                              FREE(temp);
> +
> +                                              if (earlier == 
list_entry(tmpq, typeof(struct uevent), node))
> +                                                              break;
> +                                              else
> +                                                              goto 
next_earlier_node;
> +                              }
> +              }
> +}
> +
> +void
>  uevent_merge(struct uevent *later, struct list_head *tmpq)
>  {
>                struct uevent *earlier, *temp;
> @@ -232,6 +286,7 @@ merge_uevq(struct list_head *tmpq)
>                struct uevent *later;
> 
>                list_for_each_entry_reverse(later, tmpq, node) {
> +                              uevent_filter(later, tmpq);
>                                uevent_merge(later, tmpq);
>                }
>  }
> -- 
> 2.8.1.windows.1
> 

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



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

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



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 09/12] multipathd: merge uevents before proccessing
  2017-01-04  0:30   ` Benjamin Marzinski
@ 2017-01-04  3:29     ` tang.junhui
  0 siblings, 0 replies; 33+ messages in thread
From: tang.junhui @ 2017-01-04  3:29 UTC (permalink / raw)
  To: Benjamin Marzinski
  Cc: bart.vanassche, dm-devel, mwilck, tang.wenjun3, zhang.kai16


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

Hello Ben,

Yes, a *_safe list traversal method can meet the needs,
I will modify it and simplify the codes.

Thanks,
Tang Junhui




发件人:         "Benjamin Marzinski" <bmarzins@redhat.com>
收件人:         tang.junhui@zte.com.cn, 
抄送:   tang.wenjun3@zte.com.cn, zhang.kai16@zte.com.cn, 
dm-devel@redhat.com, bart.vanassche@sandisk.com, mwilck@suse.com
日期:   2017/01/04 08:39
主题:   Re: [dm-devel] [PATCH 09/12] multipathd: merge uevents before 
proccessing
发件人: dm-devel-bounces@redhat.com



On Tue, Dec 27, 2016 at 04:03:26PM +0800, tang.junhui@zte.com.cn wrote:
> From: tang.junhui <tang.junhui@zte.com.cn>
> 
> These uevents are going to be merged:
> 1) uevents come from paths and
> 2) uevents type is same and
> 3) uevents type is addition or deletion and
> 4) uevents wwid is same.

This is just a nit, and I might be missing something subtle here, but it
seems like instead of adding list_for_some_entry_reverse, and then
breaking the abstraction to manually get previous entries, you could
have just added list_for_some_entry_reverse_safe in your earlier patch,
and hid the work of traversing a list while removing elements behind the
well understood abstraction of a *_safe list traversal method.

-Ben

> 
> Change-Id: I05ee057391c092aa0c5f989b7a4f9cb550bb4d98
> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> ---
>  libmultipath/uevent.c | 125 
+++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 114 insertions(+), 11 deletions(-)
> 
> diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
> index b0b05e9..114068c 100644
> --- a/libmultipath/uevent.c
> +++ b/libmultipath/uevent.c
> @@ -85,6 +85,20 @@ struct uevent * alloc_uevent (void)
>                return uev;
>  }
> 
> +void
> +uevq_cleanup(struct list_head *tmpq)
> +{
> +              struct uevent *uev, *tmp;
> +
> +              list_for_each_entry_safe(uev, tmp, tmpq, node) {
> +                              list_del_init(&uev->node);
> +
> +                              if (uev->udev)
> + udev_device_unref(uev->udev);
> +                              FREE(uev);
> +              }
> +}
> +
>  bool
>  uevent_can_discard(char *devpath, char *kernel)
>  {
> @@ -125,6 +139,103 @@ uevent_can_discard(char *devpath, char *kernel)
>                return false;
>  }
> 
> +bool
> +merge_need_stop(struct uevent *earlier, struct uevent *later)
> +{
> +              /*
> +               * dm uevent do not try to merge with left uevents
> +               */
> +              if (!strncmp(later->kernel, "dm-", 3))
> +                              return true;
> +
> +              /*
> +               * we can not make a jugement without wwid,
> +               * so it is sensible to stop merging
> +               */
> +              if (!earlier->wwid || !later->wwid)
> +                              return true;
> +              /*
> +               * uevents merging stoped
> +               * when we meet an opposite action uevent from the same 
LUN to AVOID
> +               * "add path1 |remove path1 |add path2 |remove path2 |add 
path3"
> +               * to merge as "remove path1, path2" and "add path1, 
path2, path3"
> +               * OR
> +               * "remove path1 |add path1 |remove path2 |add path2 
|remove path3"
> +               * to merge as "add path1, path2" and "remove path1, 
path2, path3"
> +               * SO
> +               * when we meet a non-change uevent from the same LUN
> +               * with the same wwid and different action
> +               * it would be better to stop merging.
> +               */
> +              if (!strcmp(earlier->wwid, later->wwid) &&
> +                  strcmp(earlier->action, later->action) &&
> +                  strcmp(earlier->action, "change") &&
> +                  strcmp(later->action, "change"))
> +                              return true;
> +
> +              return false;
> +}
> +
> +bool
> +uevent_can_merge(struct uevent *earlier, struct uevent *later)
> +{
> +              /* merge paths uevents
> +               * whose wwids exsit and are same
> +               * and actions are same,
> +               * and actions are addition or deletion
> +               */
> +              if (earlier->wwid && later->wwid &&
> +                  !strcmp(earlier->wwid, later->wwid) &&
> +                  !strcmp(earlier->action, later->action) &&
> +                  strncmp(earlier->action, "change", 6) &&
> +                  strncmp(earlier->kernel, "dm-", 3)) {
> +                              return true;
> +              }
> +
> +              return false;
> +}
> +
> +void
> +uevent_merge(struct uevent *later, struct list_head *tmpq)
> +{
> +              struct uevent *earlier, *temp;
> +              /*
> +               * compare the uevent with earlier uevents
> +               */
> +              list_for_some_entry_reverse(earlier, &later->node, tmpq, 
node) {
> +next_earlier_node:
> +                              if (merge_need_stop(earlier, later))
> +                                              break;
> +                              /*
> +                               * try to merge earlier uevents to the 
later uevent
> +                               */
> +                              if (uevent_can_merge(earlier, later)) {
> +                                              condlog(3, "merged 
uevent: %s-%s-%s with uevent: %s-%s-%s",
> + earlier->action, earlier->kernel, earlier->wwid,
> + later->action, later->kernel, later->wwid);
> +                                              temp = earlier;
> +
> +                                              earlier = 
list_entry(earlier->node.prev, typeof(struct uevent), node);
> +                                              list_move(&temp->node, 
&later->merge_node);
> +
> +                                              if (earlier == 
list_entry(tmpq, typeof(struct uevent), node))
> +                                                              break;
> +                                              else
> +                                                              goto 
next_earlier_node;
> +                              }
> +              }
> +}
> +
> +void
> +merge_uevq(struct list_head *tmpq)
> +{
> +              struct uevent *later;
> +
> +              list_for_each_entry_reverse(later, tmpq, node) {
> +                              uevent_merge(later, tmpq);
> +              }
> +}
> +
>  void
>  service_uevq(struct list_head *tmpq)
>  {
> @@ -136,6 +247,8 @@ service_uevq(struct list_head *tmpq)
>                                if (my_uev_trigger && my_uev_trigger(uev, 
my_trigger_data))
>                                                condlog(0, "uevent 
trigger error");
> 
> +                              uevq_cleanup(&uev->merge_node);
> +
>                                if (uev->udev)
> udev_device_unref(uev->udev);
>                                FREE(uev);
> @@ -150,17 +263,6 @@ static void uevent_cleanup(void *arg)
>                udev_unref(udev);
>  }
> 
> -void
> -uevq_cleanup(struct list_head *tmpq)
> -{
> -              struct uevent *uev, *tmp;
> -
> -              list_for_each_entry_safe(uev, tmp, tmpq, node) {
> -                              list_del_init(&uev->node);
> -                              FREE(uev);
> -              }
> -}
> -
>  /*
>   * Service the uevent queue.
>   */
> @@ -189,6 +291,7 @@ int uevent_dispatch(int (*uev_trigger)(struct uevent 
*, void * trigger_data),
>                                pthread_mutex_unlock(uevq_lockp);
>                                if (!my_uev_trigger)
>                                                break;
> +                              merge_uevq(&uevq_tmp);
>                                service_uevq(&uevq_tmp);
>                }
>                condlog(3, "Terminating uev service queue");
> -- 
> 2.8.1.windows.1
> 

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




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

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



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent
  2017-01-03 22:02   ` Benjamin Marzinski
@ 2017-01-04  6:56     ` tang.junhui
  2017-01-04 18:14       ` Benjamin Marzinski
  0 siblings, 1 reply; 33+ messages in thread
From: tang.junhui @ 2017-01-04  6:56 UTC (permalink / raw)
  To: Benjamin Marzinski
  Cc: tang.wenjun3, zhang.kai16, dm-devel, bart.vanassche, mwilck


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

Hello Ben,

> Right now, multipath users are allowed configure devices to set the wwid
> based on any udev environment variable (or even use a callout, although
> this is deprecated). With this patch, that breaks.
Does WWID obtained by different methods change? If it changes, we would 
better to modify code to keep it no change. 

> If the udev sets
> ID_SERIAL for a device, that is its wwid, right? 
Yes

> Do you know if rbd
> devices have ID_SERIAL set? 
WWID has different label in uevents for different devices, I only test for
SCSI devices. Now we do not support rbd divice for uevents merging, these 
device process as old way, it has no harm in logic. If we need to merge 
rbd uevents for these devices, we can add code to get wwid from uevents
and it can be supported easily. 


Regards
Tang Junhui





发件人:         "Benjamin Marzinski" <bmarzins@redhat.com>
收件人:         tang.junhui@zte.com.cn, 
抄送:   christophe.varoqui@opensvc.com, hare@suse.de, mwilck@suse.com, 
bart.vanassche@sandisk.com, dm-devel@redhat.com, zhang.kai16@zte.com.cn, 
tang.wenjun3@zte.com.cn
日期:   2017/01/04 06:03
主题:   Re: [PATCH 01/12] libmultipath: add wwid for "struct uevent" to 
record wwid of uevent



On Tue, Dec 27, 2016 at 04:03:18PM +0800, tang.junhui@zte.com.cn wrote:
> From: tang.junhui <tang.junhui@zte.com.cn>
> 
> Add "char *wwid" to point WWID of uevent. This member identifies
> the LUN ID which the path belongs to, and it is used for merging
> uevents. WWID possibly did not exist in uevent yet, so ->wwid
> would be NULL, those uevents would not be merged, but be proccessed
> as old way.

Right now, multipath users are allowed configure devices to set the wwid
based on any udev environment variable (or even use a callout, although
this is deprecated). With this patch, that breaks. If the udev sets
ID_SERIAL for a device, that is its wwid, right?  Do you know if rbd
devices have ID_SERIAL set? If so, this change will break them.  Even if
this change doesn't break any devices in their default configurations,
we would need to disallow changing how the wwid is set for this patch
to be safe.

-Ben 

> 
> Change-Id: Ie6b076363b3735dc7de10184b27fa799b499af0e
> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> ---
>  libmultipath/uevent.c | 2 ++
>  libmultipath/uevent.h | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
> index 7edcce1..ef1bafe 100644
> --- a/libmultipath/uevent.c
> +++ b/libmultipath/uevent.c
> @@ -424,6 +424,8 @@ struct uevent *uevent_from_udev_device(struct 
udev_device *dev)
>                                                uev->devpath = 
uev->envp[i] + 8;
>                                if (strcmp(name, "ACTION") == 0)
>                                                uev->action = 
uev->envp[i] + 7;
> +                              if (strcmp(name, "ID_SERIAL") == 0)
> +                                              uev->wwid = uev->envp[i] 
+ 10;
>                                i++;
>                                if (i == HOTPLUG_NUM_ENVP - 1)
>                                                break;
> diff --git a/libmultipath/uevent.h b/libmultipath/uevent.h
> index 9d22dcd..7bfccef 100644
> --- a/libmultipath/uevent.h
> +++ b/libmultipath/uevent.h
> @@ -22,6 +22,7 @@ struct uevent {
>                char *devpath;
>                char *action;
>                char *kernel;
> +              char *wwid;
>                unsigned long seqnum;
>                char *envp[HOTPLUG_NUM_ENVP];
>  };
> -- 
> 2.8.1.windows.1
> 



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

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



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 08/12] libmultipath: wait one seconds for more uevents in uevent_listen() in uevents burst situations
  2017-01-03 22:31   ` Benjamin Marzinski
@ 2017-01-04  7:32     ` tang.junhui
  0 siblings, 0 replies; 33+ messages in thread
From: tang.junhui @ 2017-01-04  7:32 UTC (permalink / raw)
  To: Benjamin Marzinski
  Cc: tang.wenjun3, zhang.kai16, dm-devel-bounces, dm-devel,
	bart.vanassche, mwilck


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

Hello Ben,

I know the issue of socket's buffer fills up, 
but I think uevent_can_discard() totally process in memory,
it is light-weight and low cpu consumption, and it reduce the 
iteration count in merging, the test result is also good in
massive multipath devices scene. 

But if you still stick to it, I will revert it to uevents 
processing thread, and call it in uevent_dispatch() before 
calling merge_uevq(), actually, I'm going to do that.

Regards
Tang Junhui





发件人:         "Benjamin Marzinski" <bmarzins@redhat.com>
收件人:         tang.junhui@zte.com.cn, 
抄送:   tang.wenjun3@zte.com.cn, zhang.kai16@zte.com.cn, 
dm-devel@redhat.com, bart.vanassche@sandisk.com, mwilck@suse.com
日期:   2017/01/04 06:38
主题:   Re: [dm-devel] [PATCH 08/12] libmultipath: wait one seconds for 
more uevents in uevent_listen() in uevents burst situations
发件人: dm-devel-bounces@redhat.com



On Tue, Dec 27, 2016 at 04:03:25PM +0800, tang.junhui@zte.com.cn wrote:
> From: tang.junhui <tang.junhui@zte.com.cn>
> 
> The more uevents are merged, the higher efficiency program will 
performs.
> So, do not process uevents after receiving immediately in uevents burst
> situations, but continue wait 1 seconds for more uevents except that too
> much uevents (2048) have already been received or too much time eclipse
> (60 seconds).

The issue I have with doing this in uevent_listen is that we seperated
receiving the uevents from servicing the uevents specificially to make
sure what we received them as fast as possible. The udev monitor code
is all based on a NETLINK socket. If our socket's receive buffer fills
up, there is no flow control. Events just start getting dropped, which
does cut down on processing time, but not in a way we would like.

This issue applies to a lesser extent to you previous two patches. I
don't really see the benefit of making sure that we drop the uevents
as soon as possible.  As long as we don't process them, that should
be good enough, right?

Now, maybe you put a lot of thought into your timeouts, and feel
confident that we will start processing well before the receive buffer
fills up. But if so, some comments on that would be reassuring, because
from the commit message, they seem fairly arbitrary to me.

-Ben

> 
> Change-Id: I763d491540e8114a81d12d603281540a81502742
> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> ---
>  libmultipath/uevent.c | 35 +++++++++++++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
> index cc10d65..b0b05e9 100644
> --- a/libmultipath/uevent.c
> +++ b/libmultipath/uevent.c
> @@ -39,6 +39,7 @@
>  #include <linux/netlink.h>
>  #include <pthread.h>
>  #include <sys/mman.h>
> +#include <sys/time.h>
>  #include <libudev.h>
>  #include <errno.h>
> 
> @@ -490,11 +491,37 @@ struct uevent *uevent_from_udev_device(struct 
udev_device *dev)
>                return uev;
>  }
> 
> +bool uevent_burst(struct timeval *start_time, int events)
> +{
> +              struct timeval diff_time, end_time;
> +              unsigned long speed;
> +              unsigned long eclipse_ms;
> +
> +              gettimeofday(&end_time, NULL);
> +              timersub(&end_time, start_time, &diff_time);
> +
> +              eclipse_ms = diff_time.tv_sec * 1000 + diff_time.tv_usec 
/ 1000;
> +              if (eclipse_ms == 0)
> +                              return true;
> +              /* max wait 60s */
> +              if (eclipse_ms > 60*1000) {
> +                              condlog(1, "burst continued =%lu ms, 
stoped", eclipse_ms);
> +                              return false;
> +              }
> +
> +              speed = (events * 1000) / eclipse_ms;
> +              if (speed > 10)
> +                              return true;
> +
> +              return false;
> +}
> +
>  int uevent_listen(struct udev *udev)
>  {
>                int err = 2;
>                struct udev_monitor *monitor = NULL;
>                int fd, socket_flags, events;
> +              struct timeval start_time;
>                int need_failback = 1;
>                int timeout = 30;
>                LIST_HEAD(uevlisten_tmp);
> @@ -548,6 +575,7 @@ int uevent_listen(struct udev *udev)
>                }
> 
>                events = 0;
> +              gettimeofday(&start_time, NULL);
>                while (1) {
>                                struct uevent *uev;
>                                struct udev_device *dev;
> @@ -562,7 +590,8 @@ int uevent_listen(struct udev *udev)
>                                errno = 0;
>                                fdcount = poll(&ev_poll, 1, 
poll_timeout);
>                                if (fdcount && ev_poll.revents & POLLIN) 
{
> -                                              timeout = 0;
> +                                              timeout = 
uevent_burst(&start_time, events + 1) ? 1:0;
> +
>                                                dev = 
udev_monitor_receive_device(monitor);
>                                                if (!dev) {
> condlog(0, "failed getting udev device");
> @@ -578,7 +607,8 @@ int uevent_listen(struct udev *udev)
>                                                }
>                                                list_add_tail(&uev->node, 
&uevlisten_tmp);
>                                                events++;
> -                                              continue;
> +                                              if(events < 2048)
> +                                                              continue;
>                                }
>                                if (fdcount < 0) {
>                                                if (errno == EINTR)
> @@ -600,6 +630,7 @@ int uevent_listen(struct udev *udev)
> pthread_mutex_unlock(uevq_lockp);
>                                                events = 0;
>                                }
> +                              gettimeofday(&start_time, NULL);
>                                timeout = 30;
>                }
>                need_failback = 0;
> -- 
> 2.8.1.windows.1
> 

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



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

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



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent
  2017-01-04  6:56     ` tang.junhui
@ 2017-01-04 18:14       ` Benjamin Marzinski
  2017-01-04 20:33         ` Martin Wilck
  2017-01-05  3:10         ` tang.junhui
  0 siblings, 2 replies; 33+ messages in thread
From: Benjamin Marzinski @ 2017-01-04 18:14 UTC (permalink / raw)
  To: tang.junhui; +Cc: tang.wenjun3, zhang.kai16, dm-devel, bart.vanassche, mwilck

On Wed, Jan 04, 2017 at 02:56:46PM +0800, tang.junhui@zte.com.cn wrote:
>    Hello Ben,
> 
>    > Right now, multipath users are allowed configure devices to set the wwid
>    > based on any udev environment variable (or even use a callout, although
>    > this is deprecated). With this patch, that breaks.
>    Does WWID obtained by different methods change? If it changes, we would
>    better to modify code to keep it no change.

Yes. users can pick any udev environment variable (and currently, any
callout program that they write) to use as the wwid.

>    > If the udev sets
>    > ID_SERIAL for a device, that is its wwid, right?
>    Yes
> 
>    > Do you know if rbd
>    > devices have ID_SERIAL set?
>    WWID has different label in uevents for different devices, I only test for
>    SCSI devices.

Where is that check? I only see a check for whether ID_SERIAL exists. If
it exists on things that are not scsi devices, you will set the wwid for
these devices with it as well, even if it doesn't work for them.

>    Now we do not support rbd divice for uevents merging, these
>    device process as old way, it has no harm in logic. If we need to merge
>    rbd uevents for these devices, we can add code to get wwid from uevents
>    and it can be supported easily.

Look at get_rbd_uid(), and how it's called. rbd devices don't even use
the getuid callout or uid_attribute. They use special code called by
get_uid.

Now you could add explicit checks so we only check ID_SERIAL for scsi
devices, ID_UID for dasd devices, and never set the wwid otherwise.  You
could even make the attribute we check configurable by device type with
an option like

uid_attrs "sd:ID_SERIAL dasd:ID_UID"

in the defaults section, that would set up mappings between device types
and uevent attributes to check for the uid. But this would be on per
device types, not per storage array, like it currently is. uid_attribute
and getuid attribute would only ever be used for device types that
weren't in the uid_attrs list.

The other option would be to not actually merge the uevents, but simply
run through the filtered but unmerged list of uevents, and skip the
domap stuff but remember the maps that need pushing to device-mapper.
Once you are done processing all the uevents, except for updating the
maps in device-mapper, you go back and update all the maps that need
updating. There's more code refactoring in this approach, but it keeps
the uid being set in pathinfo, where you have all the information
necessary to set it using uid_attribute, getuid, or specialized code
like rbd uses.

As long as we make sure that we are only checking specific uevent
attributes for specific device types, I'm o.k. with your way, but we are
losing flexibility that multipath has always had in regards to setting
the wwid. I want to point that out so that anyone who needs this knows
that it is changing.

-Ben 

>    Regards
>    Tang Junhui
> 
>    ������:         "Benjamin Marzinski" <bmarzins@redhat.com>
>    �ռ���:         tang.junhui@zte.com.cn,
>    ����:        christophe.varoqui@opensvc.com, hare@suse.de,
>    mwilck@suse.com, bart.vanassche@sandisk.com, dm-devel@redhat.com,
>    zhang.kai16@zte.com.cn, tang.wenjun3@zte.com.cn
>    ����:         2017/01/04 06:03
>    ����:        Re: [PATCH 01/12] libmultipath: add wwid for "struct uevent"
>    to record wwid of uevent
> 
>    --------------------------------------------------------------------------
> 
>    On Tue, Dec 27, 2016 at 04:03:18PM +0800, tang.junhui@zte.com.cn wrote:
>    > From: tang.junhui <tang.junhui@zte.com.cn>
>    >
>    > Add "char *wwid" to point WWID of uevent. This member identifies
>    > the LUN ID which the path belongs to, and it is used for merging
>    > uevents. WWID possibly did not exist in uevent yet, so ->wwid
>    > would be NULL, those uevents would not be merged, but be proccessed
>    > as old way.
> 
>    Right now, multipath users are allowed configure devices to set the wwid
>    based on any udev environment variable (or even use a callout, although
>    this is deprecated). With this patch, that breaks. If the udev sets
>    ID_SERIAL for a device, that is its wwid, right?  Do you know if rbd
>    devices have ID_SERIAL set? If so, this change will break them.  Even if
>    this change doesn't break any devices in their default configurations,
>    we would need to disallow changing how the wwid is set for this patch
>    to be safe.
> 
>    -Ben
> 
>    >
>    > Change-Id: Ie6b076363b3735dc7de10184b27fa799b499af0e
>    > Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
>    > ---
>    >  libmultipath/uevent.c | 2 ++
>    >  libmultipath/uevent.h | 1 +
>    >  2 files changed, 3 insertions(+)
>    >
>    > diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
>    > index 7edcce1..ef1bafe 100644
>    > --- a/libmultipath/uevent.c
>    > +++ b/libmultipath/uevent.c
>    > @@ -424,6 +424,8 @@ struct uevent *uevent_from_udev_device(struct
>    udev_device *dev)
>    >                                                     uev->devpath =
>    uev->envp[i] + 8;
>    >                                    if (strcmp(name, "ACTION") == 0)
>    >                                                     uev->action =
>    uev->envp[i] + 7;
>    > +                                  if (strcmp(name, "ID_SERIAL") == 0)
>    > +                                                   uev->wwid =
>    uev->envp[i] + 10;
>    >                                    i++;
>    >                                    if (i == HOTPLUG_NUM_ENVP - 1)
>    >                                                     break;
>    > diff --git a/libmultipath/uevent.h b/libmultipath/uevent.h
>    > index 9d22dcd..7bfccef 100644
>    > --- a/libmultipath/uevent.h
>    > +++ b/libmultipath/uevent.h
>    > @@ -22,6 +22,7 @@ struct uevent {
>    >                   char *devpath;
>    >                   char *action;
>    >                   char *kernel;
>    > +                 char *wwid;
>    >                   unsigned long seqnum;
>    >                   char *envp[HOTPLUG_NUM_ENVP];
>    >  };
>    > --
>    > 2.8.1.windows.1
>    >

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent
  2017-01-04 18:14       ` Benjamin Marzinski
@ 2017-01-04 20:33         ` Martin Wilck
  2017-01-05  3:00           ` Benjamin Marzinski
  2017-01-05  3:10         ` tang.junhui
  1 sibling, 1 reply; 33+ messages in thread
From: Martin Wilck @ 2017-01-04 20:33 UTC (permalink / raw)
  To: Benjamin Marzinski, tang.junhui
  Cc: tang.wenjun3, zhang.kai16, dm-devel, bart.vanassche

On Wed, 2017-01-04 at 12:14 -0600, Benjamin Marzinski wrote:
> 
> The other option would be to not actually merge the uevents, but
> simply
> run through the filtered but unmerged list of uevents, and skip the
> domap stuff but remember the maps that need pushing to device-mapper.
> Once you are done processing all the uevents, except for updating the
> maps in device-mapper, you go back and update all the maps that need
> updating. There's more code refactoring in this approach, but it
> keeps
> the uid being set in pathinfo, where you have all the information
> necessary to set it using uid_attribute, getuid, or specialized code
> like rbd uses.

That sounds a lot like configure()/coalesce_paths() to me. Would it
perhaps make sense, instead of refactoring/rewriting a whole lot of
code, to re-use that mature code path?

Cheers,
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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent
  2017-01-04 20:33         ` Martin Wilck
@ 2017-01-05  3:00           ` Benjamin Marzinski
  0 siblings, 0 replies; 33+ messages in thread
From: Benjamin Marzinski @ 2017-01-05  3:00 UTC (permalink / raw)
  To: Martin Wilck
  Cc: tang.junhui, tang.wenjun3, zhang.kai16, dm-devel, bart.vanassche

On Wed, Jan 04, 2017 at 09:33:26PM +0100, Martin Wilck wrote:
> On Wed, 2017-01-04 at 12:14 -0600, Benjamin Marzinski wrote:
> > 
> > The other option would be to not actually merge the uevents, but
> > simply
> > run through the filtered but unmerged list of uevents, and skip the
> > domap stuff but remember the maps that need pushing to device-mapper.
> > Once you are done processing all the uevents, except for updating the
> > maps in device-mapper, you go back and update all the maps that need
> > updating. There's more code refactoring in this approach, but it
> > keeps
> > the uid being set in pathinfo, where you have all the information
> > necessary to set it using uid_attribute, getuid, or specialized code
> > like rbd uses.
> 
> That sounds a lot like configure()/coalesce_paths() to me. Would it
> perhaps make sense, instead of refactoring/rewriting a whole lot of
> code, to re-use that mature code path?

Like I mentioned before, configure does a lot of extra unnecessary work,
and currently has the problem of dropping information about failed paths.

-Ben

> 
> Cheers,
> 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)

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent
  2017-01-04 18:14       ` Benjamin Marzinski
  2017-01-04 20:33         ` Martin Wilck
@ 2017-01-05  3:10         ` tang.junhui
  2017-01-05 17:36           ` Benjamin Marzinski
  1 sibling, 1 reply; 33+ messages in thread
From: tang.junhui @ 2017-01-05  3:10 UTC (permalink / raw)
  To: Benjamin Marzinski, mwilck
  Cc: bart.vanassche, dm-devel, tang.wenjun3, dm-devel-bounces, zhang.kai16


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

Hello Ben,

I know what you concern about. Maybe we can change the "wwid" member
in "struct uevent" to another name such as "merge_id" to only 
identify which uevents can merge together. The value of "merge_id" can
set by "ID_SERIAL" or "ID_UID" in uevent. If we get the "merge_id"
from uevents, we can merge and process it as merging way, otherwise
we process it as old way (processed one by one).

And we revert this patch:
"[dm-devel] [PATCH 12/12] libmultipath: use existing wwid when wwid
has already been existed in uevent"
thus wwid of each path will get by methods of configuration as old
ways.

The premise is that, devices whose "merge_id" is the same have the
same "wwid". I think this premise can be established.

how do you think about the above proposal?

>    The other option would be to not actually merge the uevents, but 
simply
>    run through the filtered but unmerged list of uevents, and skip the
>    domap stuff but remember the maps that need pushing to device-mapper.
>    Once you are done processing all the uevents, except for updating the
>    maps in device-mapper, you go back and update all the maps that need
>    updating. There's more code refactoring in this approach, but it 
keeps
>    the uid being set in pathinfo, where you have all the information
>    necessary to set it using uid_attribute, getuid, or specialized code
>    like rbd uses.
I think it is ok, but a little complex. and if we can get rid of the 
"wwid" issue, we do not need to do so.

Regards
Tang Junhui





发件人:         "Benjamin Marzinski" <bmarzins@redhat.com>
收件人:         tang.junhui@zte.com.cn, 
抄送:   tang.wenjun3@zte.com.cn, zhang.kai16@zte.com.cn, 
dm-devel@redhat.com, bart.vanassche@sandisk.com, mwilck@suse.com
日期:   2017/01/05 02:23
主题:   Re: [dm-devel] [PATCH 01/12] libmultipath: add wwid for "struct 
uevent" to record wwid of uevent
发件人: dm-devel-bounces@redhat.com



On Wed, Jan 04, 2017 at 02:56:46PM +0800, tang.junhui@zte.com.cn wrote:
>    Hello Ben,
> 
>    > Right now, multipath users are allowed configure devices to set the 
wwid
>    > based on any udev environment variable (or even use a callout, 
although
>    > this is deprecated). With this patch, that breaks.
>    Does WWID obtained by different methods change? If it changes, we 
would
>    better to modify code to keep it no change.

Yes. users can pick any udev environment variable (and currently, any
callout program that they write) to use as the wwid.

>    > If the udev sets
>    > ID_SERIAL for a device, that is its wwid, right?
>    Yes
> 
>    > Do you know if rbd
>    > devices have ID_SERIAL set?
>    WWID has different label in uevents for different devices, I only 
test for
>    SCSI devices.

Where is that check? I only see a check for whether ID_SERIAL exists. If
it exists on things that are not scsi devices, you will set the wwid for
these devices with it as well, even if it doesn't work for them.

>    Now we do not support rbd divice for uevents merging, these
>    device process as old way, it has no harm in logic. If we need to 
merge
>    rbd uevents for these devices, we can add code to get wwid from 
uevents
>    and it can be supported easily.

Look at get_rbd_uid(), and how it's called. rbd devices don't even use
the getuid callout or uid_attribute. They use special code called by
get_uid.

Now you could add explicit checks so we only check ID_SERIAL for scsi
devices, ID_UID for dasd devices, and never set the wwid otherwise.  You
could even make the attribute we check configurable by device type with
an option like

uid_attrs "sd:ID_SERIAL dasd:ID_UID"

in the defaults section, that would set up mappings between device types
and uevent attributes to check for the uid. But this would be on per
device types, not per storage array, like it currently is. uid_attribute
and getuid attribute would only ever be used for device types that
weren't in the uid_attrs list.

The other option would be to not actually merge the uevents, but simply
run through the filtered but unmerged list of uevents, and skip the
domap stuff but remember the maps that need pushing to device-mapper.
Once you are done processing all the uevents, except for updating the
maps in device-mapper, you go back and update all the maps that need
updating. There's more code refactoring in this approach, but it keeps
the uid being set in pathinfo, where you have all the information
necessary to set it using uid_attribute, getuid, or specialized code
like rbd uses.

As long as we make sure that we are only checking specific uevent
attributes for specific device types, I'm o.k. with your way, but we are
losing flexibility that multipath has always had in regards to setting
the wwid. I want to point that out so that anyone who needs this knows
that it is changing.

-Ben 

>    Regards
>    Tang Junhui
> 
>    ������:         "Benjamin Marzinski" <bmarzins@redhat.com>
>    �ռ���:         tang.junhui@zte.com.cn,
>    ����:        christophe.varoqui@opensvc.com, hare@suse.de,
>    mwilck@suse.com, bart.vanassche@sandisk.com, dm-devel@redhat.com,
>    zhang.kai16@zte.com.cn, tang.wenjun3@zte.com.cn
>    ����:         2017/01/04 06:03
>    ����:        Re: [PATCH 01/12] libmultipath: add wwid for "struct 
uevent"
>    to record wwid of uevent
> 
> 
--------------------------------------------------------------------------
> 
>    On Tue, Dec 27, 2016 at 04:03:18PM +0800, tang.junhui@zte.com.cn 
wrote:
>    > From: tang.junhui <tang.junhui@zte.com.cn>
>    >
>    > Add "char *wwid" to point WWID of uevent. This member identifies
>    > the LUN ID which the path belongs to, and it is used for merging
>    > uevents. WWID possibly did not exist in uevent yet, so ->wwid
>    > would be NULL, those uevents would not be merged, but be proccessed
>    > as old way.
> 
>    Right now, multipath users are allowed configure devices to set the 
wwid
>    based on any udev environment variable (or even use a callout, 
although
>    this is deprecated). With this patch, that breaks. If the udev sets
>    ID_SERIAL for a device, that is its wwid, right?  Do you know if rbd
>    devices have ID_SERIAL set? If so, this change will break them.  Even 
if
>    this change doesn't break any devices in their default 
configurations,
>    we would need to disallow changing how the wwid is set for this patch
>    to be safe.
> 
>    -Ben
> 
>    >
>    > Change-Id: Ie6b076363b3735dc7de10184b27fa799b499af0e
>    > Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
>    > ---
>    >  libmultipath/uevent.c | 2 ++
>    >  libmultipath/uevent.h | 1 +
>    >  2 files changed, 3 insertions(+)
>    >
>    > diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
>    > index 7edcce1..ef1bafe 100644
>    > --- a/libmultipath/uevent.c
>    > +++ b/libmultipath/uevent.c
>    > @@ -424,6 +424,8 @@ struct uevent *uevent_from_udev_device(struct
>    udev_device *dev)
>    >                                                     uev->devpath =
>    uev->envp[i] + 8;
>    >                                    if (strcmp(name, "ACTION") == 0)
>    >                                                     uev->action =
>    uev->envp[i] + 7;
>    > +                                  if (strcmp(name, "ID_SERIAL") == 
0)
>    > +                                                   uev->wwid =
>    uev->envp[i] + 10;
>    >                                    i++;
>    >                                    if (i == HOTPLUG_NUM_ENVP - 1)
>    >                                                     break;
>    > diff --git a/libmultipath/uevent.h b/libmultipath/uevent.h
>    > index 9d22dcd..7bfccef 100644
>    > --- a/libmultipath/uevent.h
>    > +++ b/libmultipath/uevent.h
>    > @@ -22,6 +22,7 @@ struct uevent {
>    >                   char *devpath;
>    >                   char *action;
>    >                   char *kernel;
>    > +                 char *wwid;
>    >                   unsigned long seqnum;
>    >                   char *envp[HOTPLUG_NUM_ENVP];
>    >  };
>    > --
>    > 2.8.1.windows.1
>    >

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


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

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



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent
  2017-01-05  3:10         ` tang.junhui
@ 2017-01-05 17:36           ` Benjamin Marzinski
  2017-01-06  0:59             ` tang.junhui
  0 siblings, 1 reply; 33+ messages in thread
From: Benjamin Marzinski @ 2017-01-05 17:36 UTC (permalink / raw)
  To: tang.junhui
  Cc: tang.wenjun3, zhang.kai16, dm-devel-bounces, dm-devel,
	bart.vanassche, mwilck

On Thu, Jan 05, 2017 at 11:10:43AM +0800, tang.junhui@zte.com.cn wrote:
>    Hello Ben,
> 
>    I know what you concern about. Maybe we can change the "wwid" member
>    in "struct uevent" to another name such as "merge_id" to only
>    identify which uevents can merge together. The value of "merge_id" can
>    set by "ID_SERIAL" or "ID_UID" in uevent. If we get the "merge_id"
>    from uevents, we can merge and process it as merging way, otherwise
>    we process it as old way (processed one by one).

 
>    And we revert this patch:
>    "[dm-devel] [PATCH 12/12] libmultipath: use existing wwid when wwid
>    has already been existed in uevent"
>    thus wwid of each path will get by methods of configuration as old
>    ways.
> 
>    The premise is that, devices whose "merge_id" is the same have the
>    same "wwid". I think this premise can be established.

So, if you have device where the merge_id doesn't equal the wwid, you
call domap on that device, as it it wasn't part of a merged set?

That would work, but you would have to be careful to deal with the case
where the last uevent in the set, the one that would be used to call
domap for the whole merged set, suddenly gets processed all on it's own,
because it had a different wwid from the rest of the set. In that case
You would need to make sure that something called domap on the other
members of the merged set.  Now, I admit that this is a very unlikely
thing to happen.

I thought about proposing that we revert patch 12, but it does add
complexity if you find that only some of your merged devices change
wwid. Also, if all of them change to the same new wwid, you just skipped
merging when you should be able to. But I'm not against reverting it, as
long as we handle everything o.k. Reverting it would mean that we don't
have to worry about removing a capability that multipath previously had.
There's just a trade-off between supporting a capability (that we don't
know if anyone uses), and maintaining less complex code.

As long as it is possible to configure what uevent variable you check
for by device type, I don't know of a situation where this will cause
real world problems. It's possible that some users are changing
uid_attribute for only certain scsi-based arrays, but I don't know of
any cases. If somebody reading this knows of any cases, please speak up.
But I would like to have the variable you check be configurable so that
it's flexible with regards to new device types or the possiblity of
differing uevent variable names either between distributions or over
time. Does that sound reasonable?

-Ben

> 
>    how do you think about the above proposal?
> 
>    >    The other option would be to not actually merge the uevents, but
>    simply
>    >    run through the filtered but unmerged list of uevents, and skip the
>    >    domap stuff but remember the maps that need pushing to device-mapper.
>    >    Once you are done processing all the uevents, except for updating the
>    >    maps in device-mapper, you go back and update all the maps that need
>    >    updating. There's more code refactoring in this approach, but it
>    keeps
>    >    the uid being set in pathinfo, where you have all the information
>    >    necessary to set it using uid_attribute, getuid, or specialized code
>    >    like rbd uses.
>    I think it is ok, but a little complex. and if we can get rid of the
>    "wwid" issue, we do not need to do so.
> 
>    Regards
>    Tang Junhui
> 
>    发件人:         "Benjamin Marzinski" <bmarzins@redhat.com>
>    收件人:         tang.junhui@zte.com.cn,
>    抄送:        tang.wenjun3@zte.com.cn, zhang.kai16@zte.com.cn,
>    dm-devel@redhat.com, bart.vanassche@sandisk.com, mwilck@suse.com
>    日期:         2017/01/05 02:23
>    主题:        Re: [dm-devel] [PATCH 01/12] libmultipath: add wwid for
>    "struct uevent" to record wwid of uevent
>    发件人:        dm-devel-bounces@redhat.com
> 
>    --------------------------------------------------------------------------
> 
>    On Wed, Jan 04, 2017 at 02:56:46PM +0800, tang.junhui@zte.com.cn wrote:
>    >    Hello Ben,
>    >
>    >    > Right now, multipath users are allowed configure devices to set the
>    wwid
>    >    > based on any udev environment variable (or even use a callout,
>    although
>    >    > this is deprecated). With this patch, that breaks.
>    >    Does WWID obtained by different methods change? If it changes, we
>    would
>    >    better to modify code to keep it no change.
> 
>    Yes. users can pick any udev environment variable (and currently, any
>    callout program that they write) to use as the wwid.
> 
>    >    > If the udev sets
>    >    > ID_SERIAL for a device, that is its wwid, right?
>    >    Yes
>    >
>    >    > Do you know if rbd
>    >    > devices have ID_SERIAL set?
>    >    WWID has different label in uevents for different devices, I only
>    test for
>    >    SCSI devices.
> 
>    Where is that check? I only see a check for whether ID_SERIAL exists. If
>    it exists on things that are not scsi devices, you will set the wwid for
>    these devices with it as well, even if it doesn't work for them.
> 
>    >    Now we do not support rbd divice for uevents merging, these
>    >    device process as old way, it has no harm in logic. If we need to
>    merge
>    >    rbd uevents for these devices, we can add code to get wwid from
>    uevents
>    >    and it can be supported easily.
> 
>    Look at get_rbd_uid(), and how it's called. rbd devices don't even use
>    the getuid callout or uid_attribute. They use special code called by
>    get_uid.
> 
>    Now you could add explicit checks so we only check ID_SERIAL for scsi
>    devices, ID_UID for dasd devices, and never set the wwid otherwise.  You
>    could even make the attribute we check configurable by device type with
>    an option like
> 
>    uid_attrs "sd:ID_SERIAL dasd:ID_UID"
> 
>    in the defaults section, that would set up mappings between device types
>    and uevent attributes to check for the uid. But this would be on per
>    device types, not per storage array, like it currently is. uid_attribute
>    and getuid attribute would only ever be used for device types that
>    weren't in the uid_attrs list.
> 
>    The other option would be to not actually merge the uevents, but simply
>    run through the filtered but unmerged list of uevents, and skip the
>    domap stuff but remember the maps that need pushing to device-mapper.
>    Once you are done processing all the uevents, except for updating the
>    maps in device-mapper, you go back and update all the maps that need
>    updating. There's more code refactoring in this approach, but it keeps
>    the uid being set in pathinfo, where you have all the information
>    necessary to set it using uid_attribute, getuid, or specialized code
>    like rbd uses.
> 
>    As long as we make sure that we are only checking specific uevent
>    attributes for specific device types, I'm o.k. with your way, but we are
>    losing flexibility that multipath has always had in regards to setting
>    the wwid. I want to point that out so that anyone who needs this knows
>    that it is changing.
> 
>    -Ben
> 
>    >    Regards
>    >    Tang Junhui
>    >
>    >    ������:         "Benjamin Marzinski" <bmarzins@redhat.com>
>    >    �ռ���:         tang.junhui@zte.com.cn,
>    >    ����:        christophe.varoqui@opensvc.com, hare@suse.de,
>    >    mwilck@suse.com, bart.vanassche@sandisk.com, dm-devel@redhat.com,
>    >    zhang.kai16@zte.com.cn, tang.wenjun3@zte.com.cn
>    >    ����:         2017/01/04 06:03
>    >    ����:        Re: [PATCH 01/12] libmultipath: add wwid for "struct
>    uevent"
>    >    to record wwid of uevent
>    >
>    >  
>     --------------------------------------------------------------------------
>    >
>    >    On Tue, Dec 27, 2016 at 04:03:18PM +0800, tang.junhui@zte.com.cn
>    wrote:
>    >    > From: tang.junhui <tang.junhui@zte.com.cn>
>    >    >
>    >    > Add "char *wwid" to point WWID of uevent. This member identifies
>    >    > the LUN ID which the path belongs to, and it is used for merging
>    >    > uevents. WWID possibly did not exist in uevent yet, so ->wwid
>    >    > would be NULL, those uevents would not be merged, but be proccessed
>    >    > as old way.
>    >
>    >    Right now, multipath users are allowed configure devices to set the
>    wwid
>    >    based on any udev environment variable (or even use a callout,
>    although
>    >    this is deprecated). With this patch, that breaks. If the udev sets
>    >    ID_SERIAL for a device, that is its wwid, right?  Do you know if rbd
>    >    devices have ID_SERIAL set? If so, this change will break them.  Even
>    if
>    >    this change doesn't break any devices in their default
>    configurations,
>    >    we would need to disallow changing how the wwid is set for this patch
>    >    to be safe.
>    >
>    >    -Ben
>    >
>    >    >
>    >    > Change-Id: Ie6b076363b3735dc7de10184b27fa799b499af0e
>    >    > Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
>    >    > ---
>    >    >  libmultipath/uevent.c | 2 ++
>    >    >  libmultipath/uevent.h | 1 +
>    >    >  2 files changed, 3 insertions(+)
>    >    >
>    >    > diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
>    >    > index 7edcce1..ef1bafe 100644
>    >    > --- a/libmultipath/uevent.c
>    >    > +++ b/libmultipath/uevent.c
>    >    > @@ -424,6 +424,8 @@ struct uevent *uevent_from_udev_device(struct
>    >    udev_device *dev)
>    >    >                                                     uev->devpath =
>    >    uev->envp[i] + 8;
>    >    >                                    if (strcmp(name, "ACTION") == 0)
>    >    >                                                     uev->action =
>    >    uev->envp[i] + 7;
>    >    > +                                  if (strcmp(name, "ID_SERIAL") ==
>    0)
>    >    > +                                                   uev->wwid =
>    >    uev->envp[i] + 10;
>    >    >                                    i++;
>    >    >                                    if (i == HOTPLUG_NUM_ENVP - 1)
>    >    >                                                     break;
>    >    > diff --git a/libmultipath/uevent.h b/libmultipath/uevent.h
>    >    > index 9d22dcd..7bfccef 100644
>    >    > --- a/libmultipath/uevent.h
>    >    > +++ b/libmultipath/uevent.h
>    >    > @@ -22,6 +22,7 @@ struct uevent {
>    >    >                   char *devpath;
>    >    >                   char *action;
>    >    >                   char *kernel;
>    >    > +                 char *wwid;
>    >    >                   unsigned long seqnum;
>    >    >                   char *envp[HOTPLUG_NUM_ENVP];
>    >    >  };
>    >    > --
>    >    > 2.8.1.windows.1
>    >    >
> 
>    --
>    dm-devel mailing list
>    dm-devel@redhat.com
>    [1]https://www.redhat.com/mailman/listinfo/dm-devel
> 
> References
> 
>    Visible links
>    1. https://www.redhat.com/mailman/listinfo/dm-devel

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent
  2017-01-05 17:36           ` Benjamin Marzinski
@ 2017-01-06  0:59             ` tang.junhui
  2017-01-06 16:02               ` Benjamin Marzinski
  0 siblings, 1 reply; 33+ messages in thread
From: tang.junhui @ 2017-01-06  0:59 UTC (permalink / raw)
  To: Benjamin Marzinski
  Cc: tang.wenjun3, zhang.kai16, dm-devel-bounces, dm-devel,
	bart.vanassche, mwilck


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

Hello Ben,

>    >    The premise is that, devices whose "merge_id" is the same have 
the
>    >    same "wwid". I think this premise can be established.

>    So, if you have device where the merge_id doesn't equal the wwid, you
>    call domap on that device, as it it wasn't part of a merged set?

Maybe I did not say clearly. "merge_id" is only used for uevents merging, 
and
it has nothing to do with "wwid". I only primse devices whose "merge_id" 
are 
same have same "wwid" (Sorry, maybe I had a misleading statement in 
previous email). 
"merge_id" and "wwid" can be same or not same. The merged uevents are 
still 
processed as bellow patch I send previous:
[dm-devel] [PATCH 11/12] multipathd: proccess merged uevents


Regards,
Tang Junhui




发件人:         "Benjamin Marzinski" <bmarzins@redhat.com>
收件人:         tang.junhui@zte.com.cn, 
抄送:   mwilck@suse.com, bart.vanassche@sandisk.com, dm-devel@redhat.com, 
dm-devel-bounces@redhat.com, tang.wenjun3@zte.com.cn, 
zhang.kai16@zte.com.cn
日期:   2017/01/06 01:37
主题:   Re: Re: [dm-devel] [PATCH 01/12] libmultipath: add wwid for 
"struct uevent" to record wwid of uevent



On Thu, Jan 05, 2017 at 11:10:43AM +0800, tang.junhui@zte.com.cn wrote:
>    Hello Ben,
> 
>    I know what you concern about. Maybe we can change the "wwid" member
>    in "struct uevent" to another name such as "merge_id" to only
>    identify which uevents can merge together. The value of "merge_id" 
can
>    set by "ID_SERIAL" or "ID_UID" in uevent. If we get the "merge_id"
>    from uevents, we can merge and process it as merging way, otherwise
>    we process it as old way (processed one by one).

 
>    And we revert this patch:
>    "[dm-devel] [PATCH 12/12] libmultipath: use existing wwid when wwid
>    has already been existed in uevent"
>    thus wwid of each path will get by methods of configuration as old
>    ways.
> 
>    The premise is that, devices whose "merge_id" is the same have the
>    same "wwid". I think this premise can be established.

So, if you have device where the merge_id doesn't equal the wwid, you
call domap on that device, as it it wasn't part of a merged set?

That would work, but you would have to be careful to deal with the case
where the last uevent in the set, the one that would be used to call
domap for the whole merged set, suddenly gets processed all on it's own,
because it had a different wwid from the rest of the set. In that case
You would need to make sure that something called domap on the other
members of the merged set.  Now, I admit that this is a very unlikely
thing to happen.

I thought about proposing that we revert patch 12, but it does add
complexity if you find that only some of your merged devices change
wwid. Also, if all of them change to the same new wwid, you just skipped
merging when you should be able to. But I'm not against reverting it, as
long as we handle everything o.k. Reverting it would mean that we don't
have to worry about removing a capability that multipath previously had.
There's just a trade-off between supporting a capability (that we don't
know if anyone uses), and maintaining less complex code.

As long as it is possible to configure what uevent variable you check
for by device type, I don't know of a situation where this will cause
real world problems. It's possible that some users are changing
uid_attribute for only certain scsi-based arrays, but I don't know of
any cases. If somebody reading this knows of any cases, please speak up.
But I would like to have the variable you check be configurable so that
it's flexible with regards to new device types or the possiblity of
differing uevent variable names either between distributions or over
time. Does that sound reasonable?

-Ben

> 
>    how do you think about the above proposal?
> 
>    >    The other option would be to not actually merge the uevents, but
>    simply
>    >    run through the filtered but unmerged list of uevents, and skip 
the
>    >    domap stuff but remember the maps that need pushing to 
device-mapper.
>    >    Once you are done processing all the uevents, except for 
updating the
>    >    maps in device-mapper, you go back and update all the maps that 
need
>    >    updating. There's more code refactoring in this approach, but it
>    keeps
>    >    the uid being set in pathinfo, where you have all the 
information
>    >    necessary to set it using uid_attribute, getuid, or specialized 
code
>    >    like rbd uses.
>    I think it is ok, but a little complex. and if we can get rid of the
>    "wwid" issue, we do not need to do so.
> 
>    Regards
>    Tang Junhui
> 
>    发件人:         "Benjamin Marzinski" <bmarzins@redhat.com>
>    收件人:         tang.junhui@zte.com.cn,
>    抄送:        tang.wenjun3@zte.com.cn, zhang.kai16@zte.com.cn,
>    dm-devel@redhat.com, bart.vanassche@sandisk.com, mwilck@suse.com
>    日期:         2017/01/05 02:23
>    主题:        Re: [dm-devel] [PATCH 01/12] libmultipath: add wwid for
>    "struct uevent" to record wwid of uevent
>    发件人:        dm-devel-bounces@redhat.com
> 
> 
--------------------------------------------------------------------------
> 
>    On Wed, Jan 04, 2017 at 02:56:46PM +0800, tang.junhui@zte.com.cn 
wrote:
>    >    Hello Ben,
>    >
>    >    > Right now, multipath users are allowed configure devices to 
set the
>    wwid
>    >    > based on any udev environment variable (or even use a callout,
>    although
>    >    > this is deprecated). With this patch, that breaks.
>    >    Does WWID obtained by different methods change? If it changes, 
we
>    would
>    >    better to modify code to keep it no change.
> 
>    Yes. users can pick any udev environment variable (and currently, any
>    callout program that they write) to use as the wwid.
> 
>    >    > If the udev sets
>    >    > ID_SERIAL for a device, that is its wwid, right?
>    >    Yes
>    >
>    >    > Do you know if rbd
>    >    > devices have ID_SERIAL set?
>    >    WWID has different label in uevents for different devices, I 
only
>    test for
>    >    SCSI devices.
> 
>    Where is that check? I only see a check for whether ID_SERIAL exists. 
If
>    it exists on things that are not scsi devices, you will set the wwid 
for
>    these devices with it as well, even if it doesn't work for them.
> 
>    >    Now we do not support rbd divice for uevents merging, these
>    >    device process as old way, it has no harm in logic. If we need 
to
>    merge
>    >    rbd uevents for these devices, we can add code to get wwid from
>    uevents
>    >    and it can be supported easily.
> 
>    Look at get_rbd_uid(), and how it's called. rbd devices don't even 
use
>    the getuid callout or uid_attribute. They use special code called by
>    get_uid.
> 
>    Now you could add explicit checks so we only check ID_SERIAL for scsi
>    devices, ID_UID for dasd devices, and never set the wwid otherwise. 
 You
>    could even make the attribute we check configurable by device type 
with
>    an option like
> 
>    uid_attrs "sd:ID_SERIAL dasd:ID_UID"
> 
>    in the defaults section, that would set up mappings between device 
types
>    and uevent attributes to check for the uid. But this would be on per
>    device types, not per storage array, like it currently is. 
uid_attribute
>    and getuid attribute would only ever be used for device types that
>    weren't in the uid_attrs list.
> 
>    The other option would be to not actually merge the uevents, but 
simply
>    run through the filtered but unmerged list of uevents, and skip the
>    domap stuff but remember the maps that need pushing to device-mapper.
>    Once you are done processing all the uevents, except for updating the
>    maps in device-mapper, you go back and update all the maps that need
>    updating. There's more code refactoring in this approach, but it 
keeps
>    the uid being set in pathinfo, where you have all the information
>    necessary to set it using uid_attribute, getuid, or specialized code
>    like rbd uses.
> 
>    As long as we make sure that we are only checking specific uevent
>    attributes for specific device types, I'm o.k. with your way, but we 
are
>    losing flexibility that multipath has always had in regards to 
setting
>    the wwid. I want to point that out so that anyone who needs this 
knows
>    that it is changing.
> 
>    -Ben
> 
>    >    Regards
>    >    Tang Junhui
>    >
>    >    ������:         "Benjamin Marzinski" <bmarzins@redhat.com>
>    >    �ռ���:         tang.junhui@zte.com.cn,
>    >    ����:        christophe.varoqui@opensvc.com, hare@suse.de,
>    >    mwilck@suse.com, bart.vanassche@sandisk.com, 
dm-devel@redhat.com,
>    >    zhang.kai16@zte.com.cn, tang.wenjun3@zte.com.cn
>    >    ����:         2017/01/04 06:03
>    >    ����:        Re: [PATCH 01/12] libmultipath: add wwid for 
"struct
>    uevent"
>    >    to record wwid of uevent
>    >
>    >  
> 
 --------------------------------------------------------------------------
>    >
>    >    On Tue, Dec 27, 2016 at 04:03:18PM +0800, tang.junhui@zte.com.cn
>    wrote:
>    >    > From: tang.junhui <tang.junhui@zte.com.cn>
>    >    >
>    >    > Add "char *wwid" to point WWID of uevent. This member 
identifies
>    >    > the LUN ID which the path belongs to, and it is used for 
merging
>    >    > uevents. WWID possibly did not exist in uevent yet, so ->wwid
>    >    > would be NULL, those uevents would not be merged, but be 
proccessed
>    >    > as old way.
>    >
>    >    Right now, multipath users are allowed configure devices to set 
the
>    wwid
>    >    based on any udev environment variable (or even use a callout,
>    although
>    >    this is deprecated). With this patch, that breaks. If the udev 
sets
>    >    ID_SERIAL for a device, that is its wwid, right?  Do you know if 
rbd
>    >    devices have ID_SERIAL set? If so, this change will break them. 
 Even
>    if
>    >    this change doesn't break any devices in their default
>    configurations,
>    >    we would need to disallow changing how the wwid is set for this 
patch
>    >    to be safe.
>    >
>    >    -Ben
>    >
>    >    >
>    >    > Change-Id: Ie6b076363b3735dc7de10184b27fa799b499af0e
>    >    > Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
>    >    > ---
>    >    >  libmultipath/uevent.c | 2 ++
>    >    >  libmultipath/uevent.h | 1 +
>    >    >  2 files changed, 3 insertions(+)
>    >    >
>    >    > diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
>    >    > index 7edcce1..ef1bafe 100644
>    >    > --- a/libmultipath/uevent.c
>    >    > +++ b/libmultipath/uevent.c
>    >    > @@ -424,6 +424,8 @@ struct uevent 
*uevent_from_udev_device(struct
>    >    udev_device *dev)
>    >    >                                                     
uev->devpath =
>    >    uev->envp[i] + 8;
>    >    >                                    if (strcmp(name, "ACTION") 
== 0)
>    >    >                                                     
uev->action =
>    >    uev->envp[i] + 7;
>    >    > +                                  if (strcmp(name, 
"ID_SERIAL") ==
>    0)
>    >    > +                                                   uev->wwid 
=
>    >    uev->envp[i] + 10;
>    >    >                                    i++;
>    >    >                                    if (i == HOTPLUG_NUM_ENVP - 
1)
>    >    >                                                     break;
>    >    > diff --git a/libmultipath/uevent.h b/libmultipath/uevent.h
>    >    > index 9d22dcd..7bfccef 100644
>    >    > --- a/libmultipath/uevent.h
>    >    > +++ b/libmultipath/uevent.h
>    >    > @@ -22,6 +22,7 @@ struct uevent {
>    >    >                   char *devpath;
>    >    >                   char *action;
>    >    >                   char *kernel;
>    >    > +                 char *wwid;
>    >    >                   unsigned long seqnum;
>    >    >                   char *envp[HOTPLUG_NUM_ENVP];
>    >    >  };
>    >    > --
>    >    > 2.8.1.windows.1
>    >    >
> 
>    --
>    dm-devel mailing list
>    dm-devel@redhat.com
>    [1]https://www.redhat.com/mailman/listinfo/dm-devel
> 
> References
> 
>    Visible links
>    1. https://www.redhat.com/mailman/listinfo/dm-devel



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

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



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent
  2017-01-06  0:59             ` tang.junhui
@ 2017-01-06 16:02               ` Benjamin Marzinski
  2017-01-09  7:22                 ` tang.junhui
  0 siblings, 1 reply; 33+ messages in thread
From: Benjamin Marzinski @ 2017-01-06 16:02 UTC (permalink / raw)
  To: tang.junhui
  Cc: tang.wenjun3, zhang.kai16, dm-devel-bounces, dm-devel,
	bart.vanassche, mwilck

On Fri, Jan 06, 2017 at 08:59:27AM +0800, tang.junhui@zte.com.cn wrote:
>    Hello Ben,
> 
>    >    >    The premise is that, devices whose "merge_id" is the same have
>    the
>    >    >    same "wwid". I think this premise can be established.
> 
>    >    So, if you have device where the merge_id doesn't equal the wwid, you
>    >    call domap on that device, as it it wasn't part of a merged set?
> 
>    Maybe I did not say clearly. "merge_id" is only used for uevents merging,
>    and
>    it has nothing to do with "wwid". I only primse devices whose "merge_id"
>    are
>    same have same "wwid" (Sorry, maybe I had a misleading statement in
>    previous email).
>    "merge_id" and "wwid" can be same or not same. The merged uevents are
>    still
>    processed as bellow patch I send previous:
>    [dm-devel] [PATCH 11/12] multipathd: proccess merged uevents

Let me back up. Say you have a batch of merged uevents

"add path1, path2, path3"

where path1 and path2 are on path3's merge_node list. This means that
when you call uev_add_path() for path1 and path2, you will call it with
need_do_map set to 0, and no multipath device will be created. The
multipath device will be created when you call uev_add_path() for path3,
with need_no_map set to 1. Now lets say that when you call pathinfo on
path2, you get a different wwid than your merge_id.  path1 and path3 get
the same wwid as their merge_id. In this situation you would call domap
on path1 and path3, but nothing would create the multipath device for
path2.

Like I said, this is unlikely to happen.  But it could. Imagine that
there was some device that presented itself like a scsi device, but
ID_SERIAL could have the same value for devices that should not be
multipathed together. To figure out which devices actually should be
multipathed together, you need to call pathinfo and get the correct
wwid. In this case you would find that you had incorrectly batched
devices with different wwids together.

To solve this, you would need to make sure to call domap on the device
if its wwid ended up being different from its merge_id.  The issue
that my last email pointed out was that if the main uevent, path3 in the
above example, was the only one to have a different wwid, then you would
need to make sure to call domap on the other devices as well. Otherwise
you would create a multipath device for path3, but not path1 and path2.

If you don't handle these cases, you will only work correctly on the
(admittedly vastly more common) case where even if the wwid and merge_id
don't match, the wwid is still the same for all the batched devices.
But if you are already assuming that the merge_id will correctly group
the devices, then what is the purpose in fetching the wwid at all? It
may be different from the merge_id, but it will still group the devices
the same way. There's nothing special about the wwid value itself. It is
just a tool to correctly group the devices. So, I would say that you
should either not grab the wwid at all if you got it from the uevent, or
do grab the wwid, but be prepared to deal with the case where not all of
the devices that you have batched having the same wwid.

Does that make more sense?

-Ben

> 
>    Regards,
>    Tang Junhui
> 
>    发件人:         "Benjamin Marzinski" <bmarzins@redhat.com>
>    收件人:         tang.junhui@zte.com.cn,
>    抄送:        mwilck@suse.com, bart.vanassche@sandisk.com,
>    dm-devel@redhat.com, dm-devel-bounces@redhat.com, tang.wenjun3@zte.com.cn,
>    zhang.kai16@zte.com.cn
>    日期:         2017/01/06 01:37
>    主题:        Re: Re: [dm-devel] [PATCH 01/12] libmultipath: add wwid for
>    "struct uevent" to record wwid of uevent
> 
>    --------------------------------------------------------------------------
> 
>    On Thu, Jan 05, 2017 at 11:10:43AM +0800, tang.junhui@zte.com.cn wrote:
>    >    Hello Ben,
>    >
>    >    I know what you concern about. Maybe we can change the "wwid" member
>    >    in "struct uevent" to another name such as "merge_id" to only
>    >    identify which uevents can merge together. The value of "merge_id"
>    can
>    >    set by "ID_SERIAL" or "ID_UID" in uevent. If we get the "merge_id"
>    >    from uevents, we can merge and process it as merging way, otherwise
>    >    we process it as old way (processed one by one).
> 
>    >    And we revert this patch:
>    >    "[dm-devel] [PATCH 12/12] libmultipath: use existing wwid when wwid
>    >    has already been existed in uevent"
>    >    thus wwid of each path will get by methods of configuration as old
>    >    ways.
>    >
>    >    The premise is that, devices whose "merge_id" is the same have the
>    >    same "wwid". I think this premise can be established.
> 
>    So, if you have device where the merge_id doesn't equal the wwid, you
>    call domap on that device, as it it wasn't part of a merged set?
> 
>    That would work, but you would have to be careful to deal with the case
>    where the last uevent in the set, the one that would be used to call
>    domap for the whole merged set, suddenly gets processed all on it's own,
>    because it had a different wwid from the rest of the set. In that case
>    You would need to make sure that something called domap on the other
>    members of the merged set.  Now, I admit that this is a very unlikely
>    thing to happen.
> 
>    I thought about proposing that we revert patch 12, but it does add
>    complexity if you find that only some of your merged devices change
>    wwid. Also, if all of them change to the same new wwid, you just skipped
>    merging when you should be able to. But I'm not against reverting it, as
>    long as we handle everything o.k. Reverting it would mean that we don't
>    have to worry about removing a capability that multipath previously had.
>    There's just a trade-off between supporting a capability (that we don't
>    know if anyone uses), and maintaining less complex code.
> 
>    As long as it is possible to configure what uevent variable you check
>    for by device type, I don't know of a situation where this will cause
>    real world problems. It's possible that some users are changing
>    uid_attribute for only certain scsi-based arrays, but I don't know of
>    any cases. If somebody reading this knows of any cases, please speak up.
>    But I would like to have the variable you check be configurable so that
>    it's flexible with regards to new device types or the possiblity of
>    differing uevent variable names either between distributions or over
>    time. Does that sound reasonable?
> 
>    -Ben
> 
>    >
>    >    how do you think about the above proposal?
>    >
>    >    >    The other option would be to not actually merge the uevents, but
>    >    simply
>    >    >    run through the filtered but unmerged list of uevents, and skip
>    the
>    >    >    domap stuff but remember the maps that need pushing to
>    device-mapper.
>    >    >    Once you are done processing all the uevents, except for
>    updating the
>    >    >    maps in device-mapper, you go back and update all the maps that
>    need
>    >    >    updating. There's more code refactoring in this approach, but it
>    >    keeps
>    >    >    the uid being set in pathinfo, where you have all the
>    information
>    >    >    necessary to set it using uid_attribute, getuid, or specialized
>    code
>    >    >    like rbd uses.
>    >    I think it is ok, but a little complex. and if we can get rid of the
>    >    "wwid" issue, we do not need to do so.
>    >
>    >    Regards
>    >    Tang Junhui
>    >
>    >    发件人:         "Benjamin Marzinski" <bmarzins@redhat.com>
>    >    收件人:         tang.junhui@zte.com.cn,
>    >    抄送:        tang.wenjun3@zte.com.cn, zhang.kai16@zte.com.cn,
>    >    dm-devel@redhat.com, bart.vanassche@sandisk.com, mwilck@suse.com
>    >    日期:         2017/01/05 02:23
>    >    主题:        Re: [dm-devel] [PATCH 01/12] libmultipath: add wwid for
>    >    "struct uevent" to record wwid of uevent
>    >    发件人:        dm-devel-bounces@redhat.com
>    >
>    >  
>     --------------------------------------------------------------------------
>    >
>    >    On Wed, Jan 04, 2017 at 02:56:46PM +0800, tang.junhui@zte.com.cn
>    wrote:
>    >    >    Hello Ben,
>    >    >
>    >    >    > Right now, multipath users are allowed configure devices to
>    set the
>    >    wwid
>    >    >    > based on any udev environment variable (or even use a callout,
>    >    although
>    >    >    > this is deprecated). With this patch, that breaks.
>    >    >    Does WWID obtained by different methods change? If it changes,
>    we
>    >    would
>    >    >    better to modify code to keep it no change.
>    >
>    >    Yes. users can pick any udev environment variable (and currently, any
>    >    callout program that they write) to use as the wwid.
>    >
>    >    >    > If the udev sets
>    >    >    > ID_SERIAL for a device, that is its wwid, right?
>    >    >    Yes
>    >    >
>    >    >    > Do you know if rbd
>    >    >    > devices have ID_SERIAL set?
>    >    >    WWID has different label in uevents for different devices, I
>    only
>    >    test for
>    >    >    SCSI devices.
>    >
>    >    Where is that check? I only see a check for whether ID_SERIAL exists.
>    If
>    >    it exists on things that are not scsi devices, you will set the wwid
>    for
>    >    these devices with it as well, even if it doesn't work for them.
>    >
>    >    >    Now we do not support rbd divice for uevents merging, these
>    >    >    device process as old way, it has no harm in logic. If we need
>    to
>    >    merge
>    >    >    rbd uevents for these devices, we can add code to get wwid from
>    >    uevents
>    >    >    and it can be supported easily.
>    >
>    >    Look at get_rbd_uid(), and how it's called. rbd devices don't even
>    use
>    >    the getuid callout or uid_attribute. They use special code called by
>    >    get_uid.
>    >
>    >    Now you could add explicit checks so we only check ID_SERIAL for scsi
>    >    devices, ID_UID for dasd devices, and never set the wwid otherwise.
>     You
>    >    could even make the attribute we check configurable by device type
>    with
>    >    an option like
>    >
>    >    uid_attrs "sd:ID_SERIAL dasd:ID_UID"
>    >
>    >    in the defaults section, that would set up mappings between device
>    types
>    >    and uevent attributes to check for the uid. But this would be on per
>    >    device types, not per storage array, like it currently is.
>    uid_attribute
>    >    and getuid attribute would only ever be used for device types that
>    >    weren't in the uid_attrs list.
>    >
>    >    The other option would be to not actually merge the uevents, but
>    simply
>    >    run through the filtered but unmerged list of uevents, and skip the
>    >    domap stuff but remember the maps that need pushing to device-mapper.
>    >    Once you are done processing all the uevents, except for updating the
>    >    maps in device-mapper, you go back and update all the maps that need
>    >    updating. There's more code refactoring in this approach, but it
>    keeps
>    >    the uid being set in pathinfo, where you have all the information
>    >    necessary to set it using uid_attribute, getuid, or specialized code
>    >    like rbd uses.
>    >
>    >    As long as we make sure that we are only checking specific uevent
>    >    attributes for specific device types, I'm o.k. with your way, but we
>    are
>    >    losing flexibility that multipath has always had in regards to
>    setting
>    >    the wwid. I want to point that out so that anyone who needs this
>    knows
>    >    that it is changing.
>    >
>    >    -Ben
>    >
>    >    >    Regards
>    >    >    Tang Junhui
>    >    >
>    >    >    ������:         "Benjamin Marzinski" <bmarzins@redhat.com>
>    >    >    �ռ���:         tang.junhui@zte.com.cn,
>    >    >    ����:        christophe.varoqui@opensvc.com, hare@suse.de,
>    >    >    mwilck@suse.com, bart.vanassche@sandisk.com,
>    dm-devel@redhat.com,
>    >    >    zhang.kai16@zte.com.cn, tang.wenjun3@zte.com.cn
>    >    >    ����:         2017/01/04 06:03
>    >    >    ����:        Re: [PATCH 01/12] libmultipath: add wwid for
>    "struct
>    >    uevent"
>    >    >    to record wwid of uevent
>    >    >
>    >    >  
>    >  
>      --------------------------------------------------------------------------
>    >    >
>    >    >    On Tue, Dec 27, 2016 at 04:03:18PM +0800, tang.junhui@zte.com.cn
>    >    wrote:
>    >    >    > From: tang.junhui <tang.junhui@zte.com.cn>
>    >    >    >
>    >    >    > Add "char *wwid" to point WWID of uevent. This member
>    identifies
>    >    >    > the LUN ID which the path belongs to, and it is used for
>    merging
>    >    >    > uevents. WWID possibly did not exist in uevent yet, so ->wwid
>    >    >    > would be NULL, those uevents would not be merged, but be
>    proccessed
>    >    >    > as old way.
>    >    >
>    >    >    Right now, multipath users are allowed configure devices to set
>    the
>    >    wwid
>    >    >    based on any udev environment variable (or even use a callout,
>    >    although
>    >    >    this is deprecated). With this patch, that breaks. If the udev
>    sets
>    >    >    ID_SERIAL for a device, that is its wwid, right?  Do you know if
>    rbd
>    >    >    devices have ID_SERIAL set? If so, this change will break them.
>     Even
>    >    if
>    >    >    this change doesn't break any devices in their default
>    >    configurations,
>    >    >    we would need to disallow changing how the wwid is set for this
>    patch
>    >    >    to be safe.
>    >    >
>    >    >    -Ben
>    >    >
>    >    >    >
>    >    >    > Change-Id: Ie6b076363b3735dc7de10184b27fa799b499af0e
>    >    >    > Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
>    >    >    > ---
>    >    >    >  libmultipath/uevent.c | 2 ++
>    >    >    >  libmultipath/uevent.h | 1 +
>    >    >    >  2 files changed, 3 insertions(+)
>    >    >    >
>    >    >    > diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
>    >    >    > index 7edcce1..ef1bafe 100644
>    >    >    > --- a/libmultipath/uevent.c
>    >    >    > +++ b/libmultipath/uevent.c
>    >    >    > @@ -424,6 +424,8 @@ struct uevent
>    *uevent_from_udev_device(struct
>    >    >    udev_device *dev)
>    >    >    >                                                    
>    uev->devpath =
>    >    >    uev->envp[i] + 8;
>    >    >    >                                    if (strcmp(name, "ACTION")
>    == 0)
>    >    >    >                                                    
>    uev->action =
>    >    >    uev->envp[i] + 7;
>    >    >    > +                                  if (strcmp(name,
>    "ID_SERIAL") ==
>    >    0)
>    >    >    > +                                                   uev->wwid
>    =
>    >    >    uev->envp[i] + 10;
>    >    >    >                                    i++;
>    >    >    >                                    if (i == HOTPLUG_NUM_ENVP -
>    1)
>    >    >    >                                                     break;
>    >    >    > diff --git a/libmultipath/uevent.h b/libmultipath/uevent.h
>    >    >    > index 9d22dcd..7bfccef 100644
>    >    >    > --- a/libmultipath/uevent.h
>    >    >    > +++ b/libmultipath/uevent.h
>    >    >    > @@ -22,6 +22,7 @@ struct uevent {
>    >    >    >                   char *devpath;
>    >    >    >                   char *action;
>    >    >    >                   char *kernel;
>    >    >    > +                 char *wwid;
>    >    >    >                   unsigned long seqnum;
>    >    >    >                   char *envp[HOTPLUG_NUM_ENVP];
>    >    >    >  };
>    >    >    > --
>    >    >    > 2.8.1.windows.1
>    >    >    >
>    >
>    >    --
>    >    dm-devel mailing list
>    >    dm-devel@redhat.com
>    >    [1][1]https://www.redhat.com/mailman/listinfo/dm-devel
>    >
>    > References
>    >
>    >    Visible links
>    >    1. [2]https://www.redhat.com/mailman/listinfo/dm-devel
> 
> References
> 
>    Visible links
>    1. https://www.redhat.com/mailman/listinfo/dm-devel
>    2. https://www.redhat.com/mailman/listinfo/dm-devel

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent
  2017-01-06 16:02               ` Benjamin Marzinski
@ 2017-01-09  7:22                 ` tang.junhui
  0 siblings, 0 replies; 33+ messages in thread
From: tang.junhui @ 2017-01-09  7:22 UTC (permalink / raw)
  To: Benjamin Marzinski
  Cc: tang.wenjun3, zhang.kai16, dm-devel-bounces, dm-devel,
	bart.vanassche, mwilck


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

Hello Ben,


OK, I will modify code to call domap() on the device
if its wwid is different from its merge_id.

I'll resend a serials of patch later,
Thank you for your patience. 

Regards,
Tang Junhui





发件人:         "Benjamin Marzinski" <bmarzins@redhat.com>
收件人:         tang.junhui@zte.com.cn, 
抄送:   tang.wenjun3@zte.com.cn, zhang.kai16@zte.com.cn, 
dm-devel-bounces@redhat.com, dm-devel@redhat.com, 
bart.vanassche@sandisk.com, mwilck@suse.com
日期:   2017/01/07 00:11
主题:   Re: [dm-devel] [PATCH 01/12] libmultipath: add wwid for "struct 
uevent" to record wwid of uevent
发件人: dm-devel-bounces@redhat.com



On Fri, Jan 06, 2017 at 08:59:27AM +0800, tang.junhui@zte.com.cn wrote:
>    Hello Ben,
> 
>    >    >    The premise is that, devices whose "merge_id" is the same 
have
>    the
>    >    >    same "wwid". I think this premise can be established.
> 
>    >    So, if you have device where the merge_id doesn't equal the 
wwid, you
>    >    call domap on that device, as it it wasn't part of a merged set?
> 
>    Maybe I did not say clearly. "merge_id" is only used for uevents 
merging,
>    and
>    it has nothing to do with "wwid". I only primse devices whose 
"merge_id"
>    are
>    same have same "wwid" (Sorry, maybe I had a misleading statement in
>    previous email).
>    "merge_id" and "wwid" can be same or not same. The merged uevents are
>    still
>    processed as bellow patch I send previous:
>    [dm-devel] [PATCH 11/12] multipathd: proccess merged uevents

Let me back up. Say you have a batch of merged uevents

"add path1, path2, path3"

where path1 and path2 are on path3's merge_node list. This means that
when you call uev_add_path() for path1 and path2, you will call it with
need_do_map set to 0, and no multipath device will be created. The
multipath device will be created when you call uev_add_path() for path3,
with need_no_map set to 1. Now lets say that when you call pathinfo on
path2, you get a different wwid than your merge_id.  path1 and path3 get
the same wwid as their merge_id. In this situation you would call domap
on path1 and path3, but nothing would create the multipath device for
path2.

Like I said, this is unlikely to happen.  But it could. Imagine that
there was some device that presented itself like a scsi device, but
ID_SERIAL could have the same value for devices that should not be
multipathed together. To figure out which devices actually should be
multipathed together, you need to call pathinfo and get the correct
wwid. In this case you would find that you had incorrectly batched
devices with different wwids together.

To solve this, you would need to make sure to call domap on the device
if its wwid ended up being different from its merge_id.  The issue
that my last email pointed out was that if the main uevent, path3 in the
above example, was the only one to have a different wwid, then you would
need to make sure to call domap on the other devices as well. Otherwise
you would create a multipath device for path3, but not path1 and path2.

If you don't handle these cases, you will only work correctly on the
(admittedly vastly more common) case where even if the wwid and merge_id
don't match, the wwid is still the same for all the batched devices.
But if you are already assuming that the merge_id will correctly group
the devices, then what is the purpose in fetching the wwid at all? It
may be different from the merge_id, but it will still group the devices
the same way. There's nothing special about the wwid value itself. It is
just a tool to correctly group the devices. So, I would say that you
should either not grab the wwid at all if you got it from the uevent, or
do grab the wwid, but be prepared to deal with the case where not all of
the devices that you have batched having the same wwid.

Does that make more sense?

-Ben

> 
>    Regards,
>    Tang Junhui
> 
>    发件人:         "Benjamin Marzinski" <bmarzins@redhat.com>
>    收件人:         tang.junhui@zte.com.cn,
>    抄送:        mwilck@suse.com, bart.vanassche@sandisk.com,
>    dm-devel@redhat.com, dm-devel-bounces@redhat.com, 
tang.wenjun3@zte.com.cn,
>    zhang.kai16@zte.com.cn
>    日期:         2017/01/06 01:37
>    主题:        Re: Re: [dm-devel] [PATCH 01/12] libmultipath: add wwid 
for
>    "struct uevent" to record wwid of uevent
> 
> 
--------------------------------------------------------------------------
> 
>    On Thu, Jan 05, 2017 at 11:10:43AM +0800, tang.junhui@zte.com.cn 
wrote:
>    >    Hello Ben,
>    >
>    >    I know what you concern about. Maybe we can change the "wwid" 
member
>    >    in "struct uevent" to another name such as "merge_id" to only
>    >    identify which uevents can merge together. The value of 
"merge_id"
>    can
>    >    set by "ID_SERIAL" or "ID_UID" in uevent. If we get the 
"merge_id"
>    >    from uevents, we can merge and process it as merging way, 
otherwise
>    >    we process it as old way (processed one by one).
> 
>    >    And we revert this patch:
>    >    "[dm-devel] [PATCH 12/12] libmultipath: use existing wwid when 
wwid
>    >    has already been existed in uevent"
>    >    thus wwid of each path will get by methods of configuration as 
old
>    >    ways.
>    >
>    >    The premise is that, devices whose "merge_id" is the same have 
the
>    >    same "wwid". I think this premise can be established.
> 
>    So, if you have device where the merge_id doesn't equal the wwid, you
>    call domap on that device, as it it wasn't part of a merged set?
> 
>    That would work, but you would have to be careful to deal with the 
case
>    where the last uevent in the set, the one that would be used to call
>    domap for the whole merged set, suddenly gets processed all on it's 
own,
>    because it had a different wwid from the rest of the set. In that 
case
>    You would need to make sure that something called domap on the other
>    members of the merged set.  Now, I admit that this is a very unlikely
>    thing to happen.
> 
>    I thought about proposing that we revert patch 12, but it does add
>    complexity if you find that only some of your merged devices change
>    wwid. Also, if all of them change to the same new wwid, you just 
skipped
>    merging when you should be able to. But I'm not against reverting it, 
as
>    long as we handle everything o.k. Reverting it would mean that we 
don't
>    have to worry about removing a capability that multipath previously 
had.
>    There's just a trade-off between supporting a capability (that we 
don't
>    know if anyone uses), and maintaining less complex code.
> 
>    As long as it is possible to configure what uevent variable you check
>    for by device type, I don't know of a situation where this will cause
>    real world problems. It's possible that some users are changing
>    uid_attribute for only certain scsi-based arrays, but I don't know of
>    any cases. If somebody reading this knows of any cases, please speak 
up.
>    But I would like to have the variable you check be configurable so 
that
>    it's flexible with regards to new device types or the possiblity of
>    differing uevent variable names either between distributions or over
>    time. Does that sound reasonable?
> 
>    -Ben
> 
>    >
>    >    how do you think about the above proposal?
>    >
>    >    >    The other option would be to not actually merge the 
uevents, but
>    >    simply
>    >    >    run through the filtered but unmerged list of uevents, and 
skip
>    the
>    >    >    domap stuff but remember the maps that need pushing to
>    device-mapper.
>    >    >    Once you are done processing all the uevents, except for
>    updating the
>    >    >    maps in device-mapper, you go back and update all the maps 
that
>    need
>    >    >    updating. There's more code refactoring in this approach, 
but it
>    >    keeps
>    >    >    the uid being set in pathinfo, where you have all the
>    information
>    >    >    necessary to set it using uid_attribute, getuid, or 
specialized
>    code
>    >    >    like rbd uses.
>    >    I think it is ok, but a little complex. and if we can get rid of 
the
>    >    "wwid" issue, we do not need to do so.
>    >
>    >    Regards
>    >    Tang Junhui
>    >
>    >    发件人:         "Benjamin Marzinski" <bmarzins@redhat.com>
>    >    收件人:         tang.junhui@zte.com.cn,
>    >    抄送:        tang.wenjun3@zte.com.cn, zhang.kai16@zte.com.cn,
>    >    dm-devel@redhat.com, bart.vanassche@sandisk.com, mwilck@suse.com
>    >    日期:         2017/01/05 02:23
>    >    主题:        Re: [dm-devel] [PATCH 01/12] libmultipath: add wwid 
for
>    >    "struct uevent" to record wwid of uevent
>    >    发件人:        dm-devel-bounces@redhat.com
>    >
>    >  
> 
 --------------------------------------------------------------------------
>    >
>    >    On Wed, Jan 04, 2017 at 02:56:46PM +0800, tang.junhui@zte.com.cn
>    wrote:
>    >    >    Hello Ben,
>    >    >
>    >    >    > Right now, multipath users are allowed configure devices 
to
>    set the
>    >    wwid
>    >    >    > based on any udev environment variable (or even use a 
callout,
>    >    although
>    >    >    > this is deprecated). With this patch, that breaks.
>    >    >    Does WWID obtained by different methods change? If it 
changes,
>    we
>    >    would
>    >    >    better to modify code to keep it no change.
>    >
>    >    Yes. users can pick any udev environment variable (and 
currently, any
>    >    callout program that they write) to use as the wwid.
>    >
>    >    >    > If the udev sets
>    >    >    > ID_SERIAL for a device, that is its wwid, right?
>    >    >    Yes
>    >    >
>    >    >    > Do you know if rbd
>    >    >    > devices have ID_SERIAL set?
>    >    >    WWID has different label in uevents for different devices, 
I
>    only
>    >    test for
>    >    >    SCSI devices.
>    >
>    >    Where is that check? I only see a check for whether ID_SERIAL 
exists.
>    If
>    >    it exists on things that are not scsi devices, you will set the 
wwid
>    for
>    >    these devices with it as well, even if it doesn't work for them.
>    >
>    >    >    Now we do not support rbd divice for uevents merging, these
>    >    >    device process as old way, it has no harm in logic. If we 
need
>    to
>    >    merge
>    >    >    rbd uevents for these devices, we can add code to get wwid 
from
>    >    uevents
>    >    >    and it can be supported easily.
>    >
>    >    Look at get_rbd_uid(), and how it's called. rbd devices don't 
even
>    use
>    >    the getuid callout or uid_attribute. They use special code 
called by
>    >    get_uid.
>    >
>    >    Now you could add explicit checks so we only check ID_SERIAL for 
scsi
>    >    devices, ID_UID for dasd devices, and never set the wwid 
otherwise.
>     You
>    >    could even make the attribute we check configurable by device 
type
>    with
>    >    an option like
>    >
>    >    uid_attrs "sd:ID_SERIAL dasd:ID_UID"
>    >
>    >    in the defaults section, that would set up mappings between 
device
>    types
>    >    and uevent attributes to check for the uid. But this would be on 
per
>    >    device types, not per storage array, like it currently is.
>    uid_attribute
>    >    and getuid attribute would only ever be used for device types 
that
>    >    weren't in the uid_attrs list.
>    >
>    >    The other option would be to not actually merge the uevents, but
>    simply
>    >    run through the filtered but unmerged list of uevents, and skip 
the
>    >    domap stuff but remember the maps that need pushing to 
device-mapper.
>    >    Once you are done processing all the uevents, except for 
updating the
>    >    maps in device-mapper, you go back and update all the maps that 
need
>    >    updating. There's more code refactoring in this approach, but it
>    keeps
>    >    the uid being set in pathinfo, where you have all the 
information
>    >    necessary to set it using uid_attribute, getuid, or specialized 
code
>    >    like rbd uses.
>    >
>    >    As long as we make sure that we are only checking specific 
uevent
>    >    attributes for specific device types, I'm o.k. with your way, 
but we
>    are
>    >    losing flexibility that multipath has always had in regards to
>    setting
>    >    the wwid. I want to point that out so that anyone who needs this
>    knows
>    >    that it is changing.
>    >
>    >    -Ben
>    >
>    >    >    Regards
>    >    >    Tang Junhui
>    >    >
>    >    >    ������:         "Benjamin Marzinski" 
<bmarzins@redhat.com>
>    >    >    �ռ���:         tang.junhui@zte.com.cn,
>    >    >    ����:        christophe.varoqui@opensvc.com, 
hare@suse.de,
>    >    >    mwilck@suse.com, bart.vanassche@sandisk.com,
>    dm-devel@redhat.com,
>    >    >    zhang.kai16@zte.com.cn, tang.wenjun3@zte.com.cn
>    >    >    ����:         2017/01/04 06:03
>    >    >    ����:        Re: [PATCH 01/12] libmultipath: add wwid 
for
>    "struct
>    >    uevent"
>    >    >    to record wwid of uevent
>    >    >
>    >    >  
>    >  
> 
  --------------------------------------------------------------------------
>    >    >
>    >    >    On Tue, Dec 27, 2016 at 04:03:18PM +0800, 
tang.junhui@zte.com.cn
>    >    wrote:
>    >    >    > From: tang.junhui <tang.junhui@zte.com.cn>
>    >    >    >
>    >    >    > Add "char *wwid" to point WWID of uevent. This member
>    identifies
>    >    >    > the LUN ID which the path belongs to, and it is used for
>    merging
>    >    >    > uevents. WWID possibly did not exist in uevent yet, so 
->wwid
>    >    >    > would be NULL, those uevents would not be merged, but be
>    proccessed
>    >    >    > as old way.
>    >    >
>    >    >    Right now, multipath users are allowed configure devices to 
set
>    the
>    >    wwid
>    >    >    based on any udev environment variable (or even use a 
callout,
>    >    although
>    >    >    this is deprecated). With this patch, that breaks. If the 
udev
>    sets
>    >    >    ID_SERIAL for a device, that is its wwid, right?  Do you 
know if
>    rbd
>    >    >    devices have ID_SERIAL set? If so, this change will break 
them.
>     Even
>    >    if
>    >    >    this change doesn't break any devices in their default
>    >    configurations,
>    >    >    we would need to disallow changing how the wwid is set for 
this
>    patch
>    >    >    to be safe.
>    >    >
>    >    >    -Ben
>    >    >
>    >    >    >
>    >    >    > Change-Id: Ie6b076363b3735dc7de10184b27fa799b499af0e
>    >    >    > Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
>    >    >    > ---
>    >    >    >  libmultipath/uevent.c | 2 ++
>    >    >    >  libmultipath/uevent.h | 1 +
>    >    >    >  2 files changed, 3 insertions(+)
>    >    >    >
>    >    >    > diff --git a/libmultipath/uevent.c 
b/libmultipath/uevent.c
>    >    >    > index 7edcce1..ef1bafe 100644
>    >    >    > --- a/libmultipath/uevent.c
>    >    >    > +++ b/libmultipath/uevent.c
>    >    >    > @@ -424,6 +424,8 @@ struct uevent
>    *uevent_from_udev_device(struct
>    >    >    udev_device *dev)
>    >    >    >                                                    
>    uev->devpath =
>    >    >    uev->envp[i] + 8;
>    >    >    >                                    if (strcmp(name, 
"ACTION")
>    == 0)
>    >    >    >                                                    
>    uev->action =
>    >    >    uev->envp[i] + 7;
>    >    >    > +                                  if (strcmp(name,
>    "ID_SERIAL") ==
>    >    0)
>    >    >    > +                                                   
uev->wwid
>    =
>    >    >    uev->envp[i] + 10;
>    >    >    >                                    i++;
>    >    >    >                                    if (i == 
HOTPLUG_NUM_ENVP -
>    1)
>    >    >    >                                                     
break;
>    >    >    > diff --git a/libmultipath/uevent.h 
b/libmultipath/uevent.h
>    >    >    > index 9d22dcd..7bfccef 100644
>    >    >    > --- a/libmultipath/uevent.h
>    >    >    > +++ b/libmultipath/uevent.h
>    >    >    > @@ -22,6 +22,7 @@ struct uevent {
>    >    >    >                   char *devpath;
>    >    >    >                   char *action;
>    >    >    >                   char *kernel;
>    >    >    > +                 char *wwid;
>    >    >    >                   unsigned long seqnum;
>    >    >    >                   char *envp[HOTPLUG_NUM_ENVP];
>    >    >    >  };
>    >    >    > --
>    >    >    > 2.8.1.windows.1
>    >    >    >
>    >
>    >    --
>    >    dm-devel mailing list
>    >    dm-devel@redhat.com
>    >    [1][1]https://www.redhat.com/mailman/listinfo/dm-devel
>    >
>    > References
>    >
>    >    Visible links
>    >    1. [2]https://www.redhat.com/mailman/listinfo/dm-devel
> 
> References
> 
>    Visible links
>    1. https://www.redhat.com/mailman/listinfo/dm-devel
>    2. https://www.redhat.com/mailman/listinfo/dm-devel

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


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

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



^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2017-01-09  7:22 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-27  8:03 [PATCH 00/12] multipath-tools: improve processing efficiency for addition and deletion of multipath devices tang.junhui
2016-12-27  8:03 ` [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent tang.junhui
2017-01-03 22:02   ` Benjamin Marzinski
2017-01-04  6:56     ` tang.junhui
2017-01-04 18:14       ` Benjamin Marzinski
2017-01-04 20:33         ` Martin Wilck
2017-01-05  3:00           ` Benjamin Marzinski
2017-01-05  3:10         ` tang.junhui
2017-01-05 17:36           ` Benjamin Marzinski
2017-01-06  0:59             ` tang.junhui
2017-01-06 16:02               ` Benjamin Marzinski
2017-01-09  7:22                 ` tang.junhui
2016-12-27  8:03 ` [PATCH 02/12] libmultipath: add merge_node for "struct uevent" to record nodes of merged uevents tang.junhui
2016-12-27  8:03 ` [PATCH 03/12] libmultipath: add two list iteration macros tang.junhui
2016-12-27  8:03 ` [PATCH 04/12] multipathd: add need_do_map to indicate whether need calling domap() in ev_add_path() tang.junhui
2016-12-27  8:03 ` [PATCH 05/12] multipathd: add need_do_map to indicate whether need calling domap() in ev_remove_path() tang.junhui
2016-12-27  8:03 ` [PATCH 06/12] multipathd: move uev_discard() to uevent.c and change its name to uevent_can_discard() tang.junhui
2016-12-27  8:03 ` [PATCH 07/12] multipathd: move calling filter_devnode() from uev_trigger() " tang.junhui
2016-12-27  8:03 ` [PATCH 08/12] libmultipath: wait one seconds for more uevents in uevent_listen() in uevents burst situations tang.junhui
2016-12-28 20:25   ` Martin Wilck
2016-12-29  0:48     ` tang.junhui
2017-01-03 22:31   ` Benjamin Marzinski
2017-01-04  7:32     ` tang.junhui
2016-12-27  8:03 ` [PATCH 09/12] multipathd: merge uevents before proccessing tang.junhui
2017-01-04  0:30   ` Benjamin Marzinski
2017-01-04  3:29     ` tang.junhui
2016-12-27  8:03 ` [PATCH 10/12] libmultipath: filter " tang.junhui
2017-01-04  1:21   ` Benjamin Marzinski
2017-01-04  2:03     ` tang.junhui
2016-12-27  8:03 ` [PATCH 11/12] multipathd: proccess merged uevents tang.junhui
2017-01-04  1:03   ` Benjamin Marzinski
2017-01-04  1:54     ` tang.junhui
2016-12-27  8:03 ` [PATCH 12/12] libmultipath: use existing wwid when wwid has already been existed in uevent tang.junhui

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.