vlserver: do not perform ChangeAddr on mh entries, except for removal
authorMichael Meffie <mmeffie@sinenomine.net>
Tue, 16 Dec 2014 21:13:01 +0000 (16:13 -0500)
committerBenjamin Kaduk <kaduk@mit.edu>
Wed, 25 Feb 2015 17:57:34 +0000 (12:57 -0500)
Fix a long standing bug in the ChangeAddr RPC which damages the vldb,

When vos changeaddr is run with -oldaddr and -newaddr, and the -oldaddr
is present in an multi-homed entry, instead of changing the address in
the mh entry, the server slot is "downgraded" to a single homed entry
and the mh entry is orphaned in the vldb.

Instead, if the -oldaddr is in a multi-home entry, refuse to change the
address with a VL entry not found error and log the event.

Multi-homed addresses can be changed manually using the vos setaddrs
command which calls the RegisterAddrs() RPC.

Change-Id: I20ba3e7fa5ffdb1b1abd2e2a716d8e4bb6594542
Reviewed-on: http://gerrit.openafs.org/11639
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

src/vlserver/vlprocs.c

index 75cf4d7..f69ba58 100644 (file)
@@ -3514,7 +3514,7 @@ ChangeIPAddr(struct vl_ctx *ctx, afs_uint32 ipaddr1, afs_uint32 ipaddr2)
     int i, j;
     afs_int32 code;
     struct extentaddr *exp = NULL;
-    int base;
+    int base = -1;
     int mhidx;
     afsUUID tuuid;
     afs_int32 blockindex, count;
@@ -3524,7 +3524,7 @@ ChangeIPAddr(struct vl_ctx *ctx, afs_uint32 ipaddr1, afs_uint32 ipaddr2)
     char addrbuf1[256];
     char addrbuf2[256];
 
-    /* Don't let addr change to 256.*.*.* : Causes internal error below */
+    /* Don't let addr change to 255.*.*.* : Causes internal error below */
     if ((ipaddr2 & 0xff000000) == 0xff000000)
        return (VL_BADSERVER);
 
@@ -3540,24 +3540,30 @@ ChangeIPAddr(struct vl_ctx *ctx, afs_uint32 ipaddr1, afs_uint32 ipaddr2)
     }
 
     for (i = 0; i <= MAXSERVERID; i++) {
-       code = multiHomedExtentBase(ctx, i, &exp, &base);
+       struct extentaddr *texp = NULL;
+       int tbase;
+
+       code = multiHomedExtentBase(ctx, i, &texp, &tbase);
        if (code)
            return code;
 
-       if (exp) {
+       if (texp) {
            for (mhidx = 0; mhidx < VL_MAXIPADDRS_PERMH; mhidx++) {
-               if (!exp->ex_addrs[mhidx])
+               if (!texp->ex_addrs[mhidx])
                    continue;
-               if (ntohl(exp->ex_addrs[mhidx]) == ipaddr1) {
+               if (ntohl(texp->ex_addrs[mhidx]) == ipaddr1) {
                    ipaddr1_id = i;
+                   exp = texp;
+                   base = tbase;
                }
-               if (ipaddr2 != 0 && ntohl(exp->ex_addrs[mhidx]) == ipaddr2) {
+               if (ipaddr2 != 0 && ntohl(texp->ex_addrs[mhidx]) == ipaddr2) {
                    ipaddr2_id = i;
                }
            }
        } else {
            if (ctx->hostaddress[i] == ipaddr1) {
                exp = NULL;
+               base = -1;
                ipaddr1_id = i;
            }
            if (ipaddr2 != 0 && ctx->hostaddress[i] == ipaddr2) {
@@ -3607,6 +3613,15 @@ ChangeIPAddr(struct vl_ctx *ctx, afs_uint32 ipaddr1, afs_uint32 ipaddr2)
                }
            }
        }
+    } else if (exp) {
+       /* Do not allow changing addresses in multi-homed entries.
+          Older versions of this RPC would silently "downgrade" mh entries
+          to single-homed entries and orphan the mh enties. */
+       addrbuf1[0] = '\0';
+       append_addr(addrbuf1, ipaddr1, sizeof(addrbuf1));
+       VLog(0, ("Refusing to change address %s in multi-homed entry; "
+                "use RegisterAddrs instead.\n", addrbuf1));
+       return VL_NOENT;        /* single-homed entry not found */
     }
 
     /* Log a message saying we are changing/removing an IP address */
@@ -3628,10 +3643,10 @@ ChangeIPAddr(struct vl_ctx *ctx, afs_uint32 ipaddr1, afs_uint32 ipaddr2)
     if (ipaddr2) {
        append_addr(addrbuf2, ipaddr2, sizeof(addrbuf2));
     }
-    VLog(0, ("      entry %d: [%s] -> [%s]\n", i, addrbuf1, addrbuf2));
+    VLog(0, ("      entry %d: [%s] -> [%s]\n", ipaddr1_id, addrbuf1, addrbuf2));
 
     /* Change the registered uuuid addresses */
-    if (exp) {
+    if (exp && base != -1) {
        memset(&tuuid, 0, sizeof(afsUUID));
        afs_htonuuid(&tuuid);
        exp->ex_hostuuid = tuuid;