<font size=2 face="sans-serif">Hello Ben</font>
<br>
<br><font size=2 face="sans-serif">Good idea,</font>
<br><font size=2 face="sans-serif">I will modify the patch to filter these
change uevents.</font>
<br>
<br><font size=2 face="sans-serif">Thanks</font>
<br><font size=2 face="sans-serif">Tang Junhui</font>
<br>
<br>
<br>
<br><font size=1 color=#5f5f5f face="sans-serif">发件人:    
    </font><font size=1 face="sans-serif">"Benjamin
Marzinski" <bmarzins@redhat.com></font>
<br><font size=1 color=#5f5f5f face="sans-serif">收件人:    
    </font><font size=1 face="sans-serif">tang.junhui@zte.com.cn,
</font>
<br><font size=1 color=#5f5f5f face="sans-serif">抄送:    
   </font><font size=1 face="sans-serif">tang.wenjun3@zte.com.cn,
zhang.kai16@zte.com.cn, dm-devel@redhat.com, bart.vanassche@sandisk.com,
mwilck@suse.com</font>
<br><font size=1 color=#5f5f5f face="sans-serif">日期:    
    </font><font size=1 face="sans-serif">2017/01/04
09:10</font>
<br><font size=1 color=#5f5f5f face="sans-serif">主题:    
   </font><font size=1 face="sans-serif">Re: [dm-devel]
[PATCH 11/12] multipathd: proccess merged uevents</font>
<br><font size=1 color=#5f5f5f face="sans-serif">发件人:    
   </font><font size=1 face="sans-serif">dm-devel-bounces@redhat.com</font>
<br>
<hr noshade>
<br>
<br>
<br><tt><font size=2>On Tue, Dec 27, 2016 at 04:03:28PM +0800, tang.junhui@zte.com.cn
wrote:<br>
> From: "tang.junhui" <tang.junhui@zte.com.cn><br>
> <br>
> After filtering and merging, then uevents are processed in<br>
> uev_trigger(), firstly, each of merged uevents would be processed
one<br>
> by one with need_do_map in value of 0. Finally, the node “uev”
itself<br>
> would be processed with need_do_map in value of 1, which would reload<br>
> kernel table, and create or remove the dm device.<br>
<br>
Wait, doesn't this mean that you would change<br>
<br>
"add path1 | change path1 | add path2"<br>
<br>
to<br>
<br>
"change path1 | add path1, path2"<br>
<br>
It doesn't make sense to process a change event before an add event.<br>
Looking at uev_update_path, the three things it currently does all can<br>
be safely ignored if you don't process the add event until after you<br>
receive the change event. They are:<br>
<br>
disable path on changed wwid: the multipath device hasn't been created<br>
yet, so it will simply be created with the new wwid. That's fine.<br>
<br>
attempt to get the pathinfo again if you failed when adding it: Either<br>
you don't have the necessary udev information in the add event, in which<br>
case that uevent won't get merged in the first place and they will<br>
remain in order, or you do have the information, and there's no point in<br>
processing the change event in that case.<br>
<br>
change the Read-Only state of the device: since both events (the change<br>
and the add event) have already happened, when you look at the device<br>
for the add event, you will already get the current read-only state.<br>
<br>
So, as far as I can tell, if you have change events in your queue as<br>
well as an add event for the same device, you might as well just filter<br>
out the change events, since processing it immediately after the add<br>
event will never actually change anything, and processing it beforehand<br>
makes no sense.<br>
<br>
-Ben<br>
<br>
> <br>
> Change-Id: Ifb4de0ca36180a8af16729c2f70bc250858877bd<br>
> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn><br>
> ---<br>
>  multipathd/main.c | 28 +++++++++++++++-------------<br>
>  1 file changed, 15 insertions(+), 13 deletions(-)<br>
> <br>
> diff --git a/multipathd/main.c b/multipathd/main.c<br>
> index 15a6175..0b18f6c 100644<br>
> --- a/multipathd/main.c<br>
> +++ b/multipathd/main.c<br>
> @@ -1092,6 +1092,7 @@ uev_trigger (struct uevent * uev, void * trigger_data)<br>
>  {<br>
>                  
int r = 0;<br>
>                  
struct vectors * vecs;<br>
> +                
struct uevent *merge_uev, *tmp;<br>
>  <br>
>                  
vecs = (struct vectors *)trigger_data;<br>
>  <br>
> @@ -1123,20 +1124,21 @@ uev_trigger (struct uevent * uev, void * trigger_data)<br>
>                  
}<br>
>  <br>
>                  
/*<br>
> -                
 * path add/remove event<br>
> +                
 * path add/remove/change event, add/remove maybe merged<br>
>                  
 */<br>
> -                
if (!strncmp(uev->action, "add", 3)) {<br>
> -                
                 r
= uev_add_path(uev, vecs, 1);<br>
> -                
                 goto
out;<br>
> -                
}<br>
> -                
if (!strncmp(uev->action, "remove", 6)) {<br>
> -                
                 r
= uev_remove_path(uev, vecs, 1);<br>
> -                
                 goto
out;<br>
> -                
}<br>
> -                
if (!strncmp(uev->action, "change", 6)) {<br>
> -                
                 r
= uev_update_path(uev, vecs);<br>
> -                
                 goto
out;<br>
> -                
}<br>
> +                
list_for_each_entry_safe(merge_uev, tmp, &uev->merge_node, node)
{<br>
> +                
                 if
(!strncmp(merge_uev->action, "add", 3))<br>
> +                
                 
               
r += uev_add_path(merge_uev, vecs, 0);<br>
> +                
                 if
(!strncmp(merge_uev->action, "remove", 6))<br>
> +                
                 
               
r += uev_remove_path(merge_uev, vecs, 0);<br>
> +                
}<br>
> +<br>
> +                
if (!strncmp(uev->action, "add", 3))<br>
> +                
                 r
+= uev_add_path(uev, vecs, 1);<br>
> +                
if (!strncmp(uev->action, "remove", 6))<br>
> +                
                 r
+= uev_remove_path(uev, vecs, 1);<br>
> +                
if (!strncmp(uev->action, "change", 6))<br>
> +                
                 r
+= uev_update_path(uev, vecs);<br>
>  <br>
>  out:<br>
>                  
return r;<br>
> -- <br>
> 2.8.1.windows.1<br>
> <br>
<br>
--<br>
dm-devel mailing list<br>
dm-devel@redhat.com<br>
</font></tt><a href="https://www.redhat.com/mailman/listinfo/dm-devel"><tt><font size=2>https://www.redhat.com/mailman/listinfo/dm-devel</font></tt></a>
<br>