Re: [PATCH 11/12] multipathd: proccess merged uevents

["Benjamin Marzinski" <bmarzins@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