dm mpath: always return reservation conflict without failing over

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.

This change resolves a show-stopper problem seen with the pNFS SCSI
layout because it is trivial to hit reservation conflict based failover
loops without it.

Doubts were raised about the implications of this change relative to
products like IBM's SVC.  But there is little point withholding a fix
for Linux because a proprietary product may or may not have some issues
in its implementation of how it interfaces with Linux.  In the future,
if there is glaring evidence that this change is certainly problematic
we can revisit it.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Christoph Hellwig <hch@lst.de>
Tested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Mike Snitzer <snitzer@redhat.com> # tweaked header

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index f31fa13..e477af8 100644 (file)
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -1530,6 +1530,14 @@ static void activate_path(struct work_struct *work)
 static int noretry_error(int error)
 {
        switch (error) {
+       case -EBADE:
+               /*
+                * 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.
+                */
        case -EOPNOTSUPP:
        case -EREMOTEIO:
        case -EILSEQ:
@@ -1574,9 +1582,6 @@ static int do_end_io(struct multipath *m, struct request *clone,
                if (!test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags)) {
                        if (!must_push_back_rq(m))
                                r = -EIO;
-               } else {
-                       if (error == -EBADE)
-                               r = error;
                }
        }
 
@@ -1625,9 +1630,6 @@ static int do_end_io_bio(struct multipath *m, struct bio *clone,
                        if (!must_push_back_bio(m))
                                return -EIO;
                        return DM_ENDIO_REQUEUE;
-               } else {
-                       if (error == -EBADE)
-                               return error;
                }
        }