On 04/30/2016 12:39 AM, Benjamin Marzinski wrote: > On Wed, Apr 27, 2016 at 01:10:21PM +0200, Hannes Reinecke wrote: >> When we remove a path it's totally pointless to add it to >> the path list first; it'll be removed on the next step anyway. >> And we should be cleaning up the comments while we're at it. > > This one causes problems. The easiest way to see that is to run > something like > > # multipathd reload map <map> ; multipathd del path <path-in-that-map> > > This really messes things up. The reason is that at the start of > ev_remove_path, there is no guarantee that any of the paths will be in > mpp->paths. This is because when multipath runs the pgpolicyfn in > setup_map(), all of policy functions free mpp->paths once they have set > up the path groups. I assume that this was done so that there is no > chance that the list of paths in mpp->paths will get out of sync with > the list of paths in the pathgroups. > > I can see why it someone might want to only keep mpp->pg as the > definitive list of paths, and to use update_mpp_paths() to regenerate > mpp->paths when necessary. But that's not what multipathd does. Instead, > mpp->paths is almost always regenerated by calling setup_multipath() > later in the same function that called setup_map(). However not every > function will always do this. ev_remove_path doesn't do this if domap() > fails, and reload_map() never calls setup_multipath(). coalesce_paths() > doesn't call setup_multipath() itself, but some if it's callers do. Even > if mpp->paths isn't restored right away, it will be when check_path > calls update_multipath_strings(). > > So, if you call "cli reload map" and then call "cli del path" before the > checker function restores mpp->paths, and multipath doesn't call > update_mpp_pths() in ev_remove_path, you get into problems. > > The question is, what's the right thing to do? > > Option 1 is to never delete mpp->paths in the first place. Then we can > probably do away with some more of the update_mpp_paths() calls. We just > need to make certain that whenever we update mpp->pg, we are always > either getting the paths from mpp->paths, or we call update_mpp_paths() > afterwards to sync them. > > Option 2 is to say that we will alway regenerate mpp->paths whenever we > need it. In that case, we should probably be freeing it after we're done > with it. > > I don't really care either way, as long as we're consistent. Otherwise > we'll get into bizzare situations like the one above. > Personally, I would opt for 1). Regenerating mpp->paths has the very big risk of running into even more race conditions, and I'd rather have it stable as far as possible.
Thanks for the clarification here. I'll see to come up with a replacement patch. Cheers, Hannes -- Dr. Hannes Reinecke Teamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
