On Tue, 2006-11-14 at 15:09 -0500, Edward Goggin wrote: > On Tuesday, November 14, 2006 9:27 AM, Dave Wysochanski wrote: > > Are you saying that there's no way in the path checker to distinguish > > between this kind of path and a path that is genuinely down? > > No, I'm certainly not saying that at all. I would rather not > complicate matters by adding yet another path state though. > > The particular problem I'm trying to fix is that the path group > membership for a multipath map can be incorrectly initially set > up if scsi_id(8) is successful but the paths are all failed. > When the paths return to a ready state, the path group membership > is not corrected by reloading the map -- this only happens if the > paths are removed and later added back. > > What do you think of a fix involving changing need_switch_pathgroup() > in multipathd to check and reload a corrected version of the map > in this case? > Maybe something like that yes. It doesn't look like the code handles a switch in priority of a path really at all so putting that in there somewhere (checkerloop?) seems worth doing. Such an approach would have to be careful that triggering off a change in priority wouldn't cause other problems if the priority callout fails in a transient way (e.g. if the priority goes to -1, you probably don't want to modify anything). > > > By "down", > > I mean "fails ioctl" either directly or with an unexpected > > CHECK_CONDITION (this seems to be what PATH_DOWN means in the > > code today > > for all the path checkers). Can you return another state in your path > > checker for this type of path/LUN? > > > > IMO, if at all possible, it would be good to leave > > "PATH_DOWN" with its > > current meaning and not call the priority callouts for paths in this > > state. If the priority callouts could obtain priorities without SG_IO > > succeeding, it might make sense, but this is not the case > > today. If you > > once had a good priority because you could get a command through, now > > you call it when the path is down it will be replaced with an > > incorrect > > one. > > Yes, good point. The converse of this statement is indeed what my > patch is trying to prevent. That is, initially having an incorrect > priority before the path groups are created then having the good or > correct priority calculated only after the path groups are already > created. > One other thought I had was the notion of a "priority valid" flag (or a special priority value) in the path for the case of group_by_prio. Set it to invalid in alloc_path(), then just don't add the path to the multipath struct in coalesce_paths() if the priority value was invalid. Then over in checkerloop(), add the path to the multipath map when you get a valid priority. It is more complicated than that I'm sure (e.g. checkerloop() assumes pp->mpp is non-null in places, etc) but seemed like a half-decent approach to at least consider. This approach doesn't take into consideration the general case of a change in path priority though. > > > > Arguably the states are fuzzy and "types of paths" are mixed in with > > "path states" which leads to the fuzzyness/confusion. Right > > now I don't > > have a good enough feel for it to offer clarifying suggestions though > > other than the attached comment patch which tries to clarify > > the meaning > > of each state as it is in the code today. -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
