If your patch implement a direct path-to-multipath conflict error propagation, won't the common scenario where admins add new (initialy unregistered) paths to a reserved multipath result in application-visible i/o errors ? If so, those admins might hold us a grudge :/ For reference the opensvc crm does use type 5 pr, and aims for all paths registered. It still does not make use of the multipathd pr janitoring features, and uses sg_persist directly for pr status and actions. Best regards, Christophe Varoqui OpenSVC Regards, Christophe Varoqui OpenSVC <div class="gmail_extra"> <div class="gmail_quote">On Wed, Jul 15, 2015 at 1:35 PM, James Bottomley <<a href="mailto:James.Bottomley@hansenpartnership.com" target="_blank">James.Bottomley@hansenpartnership.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Wed, 2015-07-15 at 13:23 +0200, Hannes Reinecke wrote: > If dm-mpath encounters an reservation conflict it should not > fail the path (as communication with the target is not affected) > but should rather retry on another path. > However, in doing so we might be inducing a ping-pong between > paths, with no guarantee of any forward progress. > And arguably a reservation conflict is an unexpected error, > so we should be passing it upwards to allow the application > to take appropriate steps. If I interpret the code correctly, you've changed the behaviour from the current try all paths and fail them, ultimately passing the reservation conflict up if all paths fail to return reservation conflict immediately, keeping all paths running. This assumes that the reservation isn't path specific because if we encounter a path specific reservation, you've altered the behaviour from route around to fail. The case I think the original code was for is SAN Volume controllers which use path specific SCSI-3 reservations effectively to do traffic control and allow favoured paths. Have you verified that nothing we encounter in the enterprise uses path specific reservations for multipath shaping any more? James <div><div> > Signed-off-by: Hannes Reinecke <<a href="mailto:hare@suse.de" target="_blank">hare@suse.de</a>> > --- > drivers/md/dm-mpath.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c > index 5a67671..e65d266 100644 > --- a/drivers/md/dm-mpath.c > +++ b/drivers/md/dm-mpath.c > @@ -1269,7 +1269,16 @@ static int do_end_io(struct multipath *m, struct request *clone, > if (noretry_error(error)) > return error; > > - if (mpio->pgpath) > + /* > + * EBADE signals an reservation conflict. > + * We shouldn't fail the path here as we can communicate with > + * the target. We should failover to the next path, but in > + * doing so we might be causing a ping-pong between paths. > + * So just return the reservation conflict error. > + */ > + if (error == -EBADE) > + r = error; > + else if (mpio->pgpath) > fail_path(mpio->pgpath); > > spin_lock_irqsave(&m->lock, flags); > @@ -1277,9 +1286,6 @@ static int do_end_io(struct multipath *m, struct request *clone, > if (!m->queue_if_no_path) { > if (!__must_push_back(m)) > r = -EIO; > - } else { > - if (error == -EBADE) > - r = error; > } > } > spin_unlock_irqrestore(&m->lock, flags); -- </div></div><div><div>dm-devel mailing list <a href="mailto:dm-devel@redhat.com" target="_blank">dm-devel@redhat.com</a> <a href="https://www.redhat.com/mailman/listinfo/dm-devel" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/dm-devel</a> </div></div></blockquote></div> </div>