[dm-devel] [PATCH] dm-mpath: always return reservation conflict

[Christophe Varoqui]

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

On Wed, Jul 15, 2015 at 1:35 PM, James Bottomley <
James.Bottomley at hansenpartnership.com> wrote:

> 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
>
> > Signed-off-by: Hannes Reinecke <hare at suse.de>
> > ---
> >  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);
>
>
>
> --
> dm-devel mailing list
> dm-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/dm-devel/attachments/20150716/338f2147/attachment.htm>