multipath-tools: prevent unexpected swapping of underlying LUNs
authorJun'ichi Nomura <j-nomura@ce.jp.nec.com>
Fri, 17 Aug 2012 08:40:39 +0000 (17:40 +0900)
committerChristophe Varoqui <christophe.varoqui@opensvc.com>
Fri, 17 Aug 2012 19:53:12 +0000 (21:53 +0200)
When you want to rename a multipath device
but the new alias is already used by other multipath device,
multipath-tools mistakenly reload a table for the original multipath device
to the other multipath device.
That could lead to very bad result, such as I/O error and data corruption.

This patch checks such a condition and gives up renaming with error log.

For example, suppose you have following 'bindings' file:

   # cat /etc/multipath/bindings
   mpatha 212140084abcd0000
   mpathb 212150084abcd0000

and a logical volume 'VG/LV0' on top of mpathb,
which is on top of /dev/sde(8:64) and /dev/sdk(8:160):

   # dmsetup ls --tree
   mpatha (253:1)
    ├─ (8:144)
    └─ (8:48)
   VG-LV0 (253:2)
    └─mpathb (253:0)
       ├─ (8:160)
       └─ (8:64)

Then you decide to swap their names and change the 'bindings' as follows:

   # cat /etc/multipath/bindings
   mpathb 212140084abcd0000
   mpatha 212150084abcd0000

you'll get this after 'service multipathd reload':

   # dmsetup ls --tree
   mpatha (253:1)
    ├─ (8:160)
    └─ (8:64)
   VG-LV0 (253:2)
    └─mpathb (253:0)
       ├─ (8:144)
       └─ (8:48)

Now you suddenly have 'VG/LV0' on top of /dev/sdd(8:48) and /dev/sdj(8:144),
that is obviously wrong and will corrupt data if you write to 'VG/LV0'.

libmultipath/configure.c

index 46910f3..9977296 100644 (file)
@@ -152,12 +152,12 @@ static void
 select_action (struct multipath * mpp, vector curmp, int force_reload)
 {
        struct multipath * cmpp;
+       struct multipath * cmpp_by_name;
 
-       cmpp = find_mp_by_alias(curmp, mpp->alias);
-
-       if (!cmpp) {
-               cmpp = find_mp_by_wwid(curmp, mpp->wwid);
+       cmpp = find_mp_by_wwid(curmp, mpp->wwid);
+       cmpp_by_name = find_mp_by_alias(curmp, mpp->alias);
 
+       if (!cmpp_by_name) {
                if (cmpp) {
                        condlog(2, "%s: rename %s to %s", mpp->wwid,
                                cmpp->alias, mpp->alias);
@@ -171,17 +171,25 @@ select_action (struct multipath * mpp, vector curmp, int force_reload)
                return;
        }
 
-       if (!find_mp_by_wwid(curmp, mpp->wwid)) {
-               condlog(2, "%s: remove (wwid changed)", cmpp->alias);
+       if (!cmpp) {
+               condlog(2, "%s: remove (wwid changed)", mpp->alias);
                dm_flush_map(mpp->alias);
-               strncpy(cmpp->wwid, mpp->wwid, WWID_SIZE);
-               drop_multipath(curmp, cmpp->wwid, KEEP_PATHS);
+               strncpy(cmpp_by_name->wwid, mpp->wwid, WWID_SIZE);
+               drop_multipath(curmp, cmpp_by_name->wwid, KEEP_PATHS);
                mpp->action = ACT_CREATE;
                condlog(3, "%s: set ACT_CREATE (map wwid change)",
                        mpp->alias);
                return;
        }
 
+       if (cmpp != cmpp_by_name) {
+               condlog(2, "%s: unable to rename %s to %s (%s is used by %s)",
+                       mpp->wwid, cmpp->alias, mpp->alias,
+                       mpp->alias, cmpp_by_name->wwid);
+               mpp->action = ACT_NOTHING;
+               return;
+       }
+
        if (pathcount(mpp, PATH_UP) == 0) {
                mpp->action = ACT_NOTHING;
                condlog(3, "%s: set ACT_NOTHING (no usable path)",