It is possible that a path was unavailable and either the registered PR
key was removed or the registered PR key was changed and then that new
key was preempted. In both of these situations, this path will still
have a registered key (just not one that matches mpp->reservation_key)
but it should not have one. If the path becomes usable again in this
state, it may allow the multipath device to access storage that it
shouldn't be allowed to access.

To deal with this, track if a multipath device ever had a registered PR
key. If so, and the device no longer has a registered key, explicitly
clear the key when paths get restored.

Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com>
---
 libmultipath/structs.h |  2 ++
 multipathd/main.c      | 46 ++++++++++++++++++++++++++++++++----------
 2 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 97093675..a206a7d1 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -524,6 +524,8 @@ struct multipath {
        int prflag;
        int prhold;
        int all_tg_pt;
+       bool ever_registered_pr;
+
        struct gen_multipath generic_mp;
        bool fpin_must_reload;
 };
diff --git a/multipathd/main.c b/multipathd/main.c
index c1ed1488..cb00c1f8 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -2638,7 +2638,8 @@ update_path_state (struct vectors * vecs, struct path * 
pp)
 
                /* newstate == PATH_UP || newstate == PATH_GHOST */
 
-               if (pp->mpp->prflag != PR_UNSET) {
+               if (pp->mpp->prflag != PR_UNSET ||
+                   pp->mpp->ever_registered_pr) {
                        int prflag = pp->mpp->prflag;
                        /*
                         * Check Persistent Reservation.
@@ -4246,6 +4247,7 @@ static void check_prhold(struct multipath *mpp, struct 
path *pp)
 
 void set_pr(struct multipath *mpp)
 {
+       mpp->ever_registered_pr = true;
        mpp->prflag = PR_SET;
 }
 
@@ -4256,23 +4258,28 @@ void unset_pr(struct multipath *mpp)
        mpp->sa_flags = 0;
 }
 
+/*
+ * Returns MPATH_PR_SUCCESS unless if fails to read the PR keys. If
+ * MPATH_PR_SUCCESS is returned, mpp->prflag will be either PR_SET or
+ * PR_UNSET.
+ */
 static int update_map_pr(struct multipath *mpp, struct path *pp)
 {
        int noisy=0;
        struct prin_resp resp;
        unsigned int i;
-       int ret = MPATH_PR_OTHER, isFound;
+       int ret, isFound;
        bool was_set = (mpp->prflag == PR_SET);
 
        /* If pr is explicitly unset, it must be manually set */
        if (mpp->prflag == PR_UNSET)
-               return MPATH_PR_SKIP;
+               return MPATH_PR_SUCCESS;
 
        if (!get_be64(mpp->reservation_key)) {
                /* Nothing to do. Assuming pr mgmt feature is disabled*/
                unset_pr(mpp);
                condlog(was_set ? 2 : 4, "%s: reservation_key not set in 
multipath.conf", mpp->alias);
-               return MPATH_PR_SKIP;
+               return MPATH_PR_SUCCESS;
        }
 
        memset(&resp, 0, sizeof(resp));
@@ -4320,6 +4327,7 @@ static void mpath_pr_event_handle(struct path *pp)
        struct multipath *mpp = pp->mpp;
        int ret;
        struct prout_param_descriptor param;
+       bool clear_reg = false;
 
        if (pp->bus != SYSFS_BUS_SCSI) {
                unset_pr(mpp);
@@ -4331,20 +4339,36 @@ static void mpath_pr_event_handle(struct path *pp)
 
        check_prhold(mpp, pp);
 
-       if (mpp->prflag != PR_SET)
-               return;
+       if (mpp->prflag != PR_SET) {
+               if (!mpp->ever_registered_pr)
+                       return;
+               /*
+                * This path may have been unusable and either the
+                * registration was cleared or the registered
+                * key was switched and then that new key was preempted.
+                * In either case, this path should not have a registration
+                * but it might still have one, just with a different
+                * key than mpp->reservation_key is currently set to.
+                * clear it to be sure.
+                */
+               clear_reg = true;
+       }
 
        memset(&param, 0, sizeof(param));
 
-       param.sa_flags = mpp->sa_flags;
-       memcpy(param.sa_key, &mpp->reservation_key, 8);
-       param.num_transportid = 0;
+       if (!clear_reg) {
+               param.sa_flags = mpp->sa_flags;
+               memcpy(param.sa_key, &mpp->reservation_key, 8);
+               param.num_transportid = 0;
+       }
 
-       condlog(3, "device %s:%s", pp->dev, pp->mpp->wwid);
+       condlog(3, "%s registration for device %s:%s",
+               clear_reg ? "Clearing" : "Setting", pp->dev, pp->mpp->wwid);
 
        ret = prout_do_scsi_ioctl(pp->dev, MPATH_PROUT_REG_IGN_SA, 0, 0, 
&param, 0);
        if (ret != MPATH_PR_SUCCESS )
        {
-               condlog(0,"%s: Reservation registration failed. Error: %d", 
pp->dev, ret);
+               condlog(0, "%s: %s reservation registration failed. Error: %d",
+                       clear_reg ? "Clearing" : "Setting", pp->dev, ret);
        }
 }
-- 
2.48.1


Reply via email to