DEVEL15-viced-host-uuid-and-addr-hashing-corrections-20090530
authorJeffrey Altman <jaltman@secure-endpoints.com>
Sat, 30 May 2009 18:27:17 +0000 (18:27 +0000)
committerDerrick Brashear <shadow@dementia.org>
Sat, 30 May 2009 18:27:17 +0000 (18:27 +0000)
LICENSE IPL10
FIXES 124634

only valid addr/port pairs are registered in the hash table.
add then remove when changing addresses.
make host restoral properly hash hosts.
remove should remove the address we asked for and not simply the
primary address.

(cherry picked from commit ca613599a2537756462a420ae1a632747a433226)

src/viced/host.c

index da75189..b00d072 100644 (file)
@@ -903,12 +903,10 @@ h_LookupUuid_r(afsUUID * uuidp)
        assert(host);
        if (!(host->hostFlags & HOSTDELETED) && host->interface
            && afs_uuid_equal(&host->interface->uuid, uuidp)) {
-           break;
+            return host;
        }
-       host = NULL;
     }
-    return host;
-
+    return NULL;
 }                              /*h_Lookup */
 
 
@@ -939,7 +937,7 @@ h_TossStuff_r(register struct host *host)
     if (h_NBLock_r(host) != 0) {
        char hoststr[16];
        ViceLog(0,
-               ("Warning:  h_TossStuff_r failed; Host %x (%s:%d) was locked.\n",
+               ("Warning:  h_TossStuff_r failed; Host %" AFS_PTR_FMT " (%s:%d) was locked.\n",
                 host, afs_inet_ntoa_r(host->host, hoststr), ntohs(host->port)));
        return;
     } else {
@@ -954,7 +952,7 @@ h_TossStuff_r(register struct host *host)
            if (code < 0) {
                char hoststr[16];
                ViceLog(0,
-                       ("Warning: h_TossStuff_r failed: Host %x (%s:%d) client %x was locked.\n",
+                       ("Warning: h_TossStuff_r failed: Host %" AFS_PTR_FMT " (%s:%d) client %x was locked.\n",
                         host, afs_inet_ntoa_r(host->host, hoststr),
                         ntohs(host->port), client));
                return;
@@ -963,7 +961,7 @@ h_TossStuff_r(register struct host *host)
            if (client->refCount) {
                char hoststr[16];
                ViceLog(0,
-                       ("Warning: h_TossStuff_r failed: Host %x (%s:%d) client %x refcount %d.\n",
+                       ("Warning: h_TossStuff_r failed: Host %" AFS_PTR_FMT " (%s:%d) client %x refcount %d.\n",
                         host, afs_inet_ntoa_r(host->host, hoststr),
                         ntohs(host->port), client, client->refCount));
                /* This is the same thing we do if the host is locked */
@@ -986,9 +984,7 @@ h_TossStuff_r(register struct host *host)
     host->hostFlags &= ~CLIENTDELETED;
 
     if (host->hostFlags & HOSTDELETED) {
-        register struct h_AddrHashChain **ahp, *ath;
        register struct rx_connection *rxconn;
-       afsUUID *uuidp;
        struct AddrPort hostAddrPort;
        int i;
 
@@ -1007,44 +1003,22 @@ h_TossStuff_r(register struct host *host)
 
        /* if alternate addresses do not exist */
        if (!(host->interface)) {
-           for (ahp = &hostAddrHashTable[h_HashIndex(host->host)]; (ath = *ahp);
-                ahp = &ath->next) {
-               assert(ath->hostPtr);
-               if (ath->hostPtr == host) {
-                   *ahp = ath->next;
-                   free(ath);
-                   break;
-               }
-           }
+           h_DeleteHostFromAddrHashTable_r(host->host, host->port, host);
        } else {
-            register struct h_UuidHashChain **uhp, *uth;
-           /* delete the hash entry for the UUID */
-           uuidp = &host->interface->uuid;
-           for (uhp = &hostUuidHashTable[h_UuidHashIndex(uuidp)]; (uth = *uhp);
-                uhp = &uth->next) {
-               assert(uth->hostPtr);
-               if (uth->hostPtr == host) {
-                   *uhp = uth->next;
-                   free(uth);
-                   break;
-               }
-           }
-           /* delete the hash entry for each alternate addresses */
+            h_DeleteHostFromUuidHashTable_r(host);
+           h_DeleteHostFromAddrHashTable_r(host->host, host->port, host);
+           /* delete the hash entry for each valid alternate addresses */
            for (i = 0; i < host->interface->numberOfInterfaces; i++) {
                hostAddrPort = host->interface->interface[i];
-
-                if (!hostAddrPort.valid)
-                    continue;
-
-               for (ahp = &hostAddrHashTable[h_HashIndex(hostAddrPort.addr)]; (ath = *ahp);
-                    ahp = &ath->next) {
-                   assert(ath->hostPtr);
-                   if (ath->hostPtr == host) {
-                       *ahp = ath->next;
-                       free(ath);
-                       break;
-                   }
-               }
+                /* 
+                 * if the interface addr/port is the primary, we already
+                 * removed it.  If the addr/port is not valid, its not
+                 * in the hash table.
+                 */
+                if (hostAddrPort.valid &&
+                    (host->host != hostAddrPort.addr ||
+                     host->port != hostAddrPort.port))
+                    h_DeleteHostFromAddrHashTable_r(hostAddrPort.addr, hostAddrPort.port, host);
            }
            free(host->interface);
            host->interface = NULL;
@@ -1150,15 +1124,30 @@ h_AddHostToUuidHashTable_r(struct afsUUID *uuid, struct host *host)
 {
     int index;
     struct h_UuidHashChain *chain;
+    char uuid1[128], uuid2[128];
+    char hoststr[16];
 
     /* hash into proper bucket */
     index = h_UuidHashIndex(uuid);
 
     /* don't add the same entry multiple times */
     for (chain = hostUuidHashTable[index]; chain; chain = chain->next) {
+       if (!chain->hostPtr)
+           continue;
+
        if (chain->hostPtr->interface && 
-           afs_uuid_equal(&chain->hostPtr->interface->uuid, uuid))
+           afs_uuid_equal(&chain->hostPtr->interface->uuid, uuid)) {
+           if (LogLevel >= 125) {
+               afsUUID_to_string(&chain->hostPtr->interface->uuid, uuid1, 
+                                 127);
+               afsUUID_to_string(uuid, uuid2, 127);
+               ViceLog(125, ("h_AddHostToUuidHashTable_r: host %" AFS_PTR_FMT " (uuid %s) exists as %s:%d (uuid %s)\n", 
+                             host, uuid1,
+                             afs_inet_ntoa_r(chain->hostPtr->host, hoststr), 
+                             ntohs(chain->hostPtr->port), uuid2));
+           }
            return;
+       }
     }
 
     /* insert into beginning of list for this bucket */
@@ -1167,12 +1156,53 @@ h_AddHostToUuidHashTable_r(struct afsUUID *uuid, struct host *host)
        ViceLog(0, ("Failed malloc in h_AddHostToUuidHashTable_r\n"));
        assert(0);
     }
-    assert(chain);
     chain->hostPtr = host;
     chain->next = hostUuidHashTable[index];
     hostUuidHashTable[index] = chain;
+         if (LogLevel < 125)
+              return;
+     afsUUID_to_string(uuid, uuid2, 127);
+     ViceLog(125, 
+            ("h_AddHostToUuidHashTable_r: host %" AFS_PTR_FMT " (%s:%d) added as uuid %s\n",
+             host, afs_inet_ntoa_r(chain->hostPtr->host, hoststr), 
+             ntohs(chain->hostPtr->port), uuid));
 }
 
+/* deletes a HashChain structure corresponding to this host */
+int
+h_DeleteHostFromUuidHashTable_r(struct host *host)
+{
+     int index;
+     register struct h_UuidHashChain **uhp, *uth;
+     char uuid1[128];
+     char hoststr[16];
+     if (!host->interface)
+       return 0;
+     /* hash into proper bucket */
+     index = h_UuidHashIndex(&host->interface->uuid);
+     
+     if (LogLevel >= 125)
+        afsUUID_to_string(&host->interface->uuid, uuid1, 127);
+     for (uhp = &hostUuidHashTable[index]; (uth = *uhp); uhp = &uth->next) {
+         assert(uth->hostPtr);
+        if (uth->hostPtr == host) {
+            ViceLog(125, 
+                    ("h_DeleteHostFromUuidHashTable_r: host %" AFS_PTR_FMT " (uuid %s %s:%d)\n",
+                     host, uuid1, afs_inet_ntoa_r(host->host, hoststr), 
+                     ntohs(host->port)));
+            *uhp = uth->next;
+            free(uth);
+            return 1;
+        }
+     }
+     ViceLog(125, 
+            ("h_DeleteHostFromUuidHashTable_r: host %" AFS_PTR_FMT " (uuid %s %s:%d) not found\n",
+             host, uuid1, afs_inet_ntoa_r(host->host, hoststr), 
+             ntohs(host->port)));
+     return 0;
+}
 
 /* inserts a new HashChain structure corresponding to this address */
 void
@@ -1181,26 +1211,30 @@ h_AddHostToAddrHashTable_r(afs_uint32 addr, afs_uint16 port, struct host *host)
     int index;
     struct h_AddrHashChain *chain;
     int found = 0;
-    char hoststr[16];
+    char hoststr[16], hoststr2[16];
 
     /* hash into proper bucket */
     index = h_HashIndex(addr);
 
     /* don't add the same entry multiple times */
     for (chain = hostAddrHashTable[index]; chain; chain = chain->next) {
-       if (chain->addr == addr && chain->port == port) {
-            if (chain->hostPtr == host)
-                found = 1;
-            else if (!(host->hostFlags & HOSTDELETED))
-                ViceLog(125, ("Addr %s:%d assigned to %x and %x.\n",
-                            afs_inet_ntoa_r(addr, hoststr), ntohs(port),
-                            host, chain));
-        }
+       if (chain->hostPtr == host) {
+           if (chain->addr != addr || chain->port != port) {
+               ViceLog(0, 
+                       ("h_AddHostToAddrHashTable_r: host %" AFS_PTR_FMT " exists as %s:%d when adding %s:%d\n",
+                        host, afs_inet_ntoa_r(chain->addr, hoststr), 
+                        ntohs(chain->port), afs_inet_ntoa_r(addr, hoststr2), 
+                        ntohs(port)));
+           } else
+               ViceLog(125, 
+                       ("h_AddHostToAddrHashTable_r: host %" AFS_PTR_FMT " (%s:%d) already hashed\n",
+                        host, afs_inet_ntoa_r(chain->addr, hoststr), 
+                        ntohs(chain->port)));
+           
+           return;
+       }
     }
 
-    if (found)
-        return;
-
     /* insert into beginning of list for this bucket */
     chain = (struct h_AddrHashChain *)malloc(sizeof(struct h_AddrHashChain));
     if (!chain) {
@@ -1212,13 +1246,14 @@ h_AddHostToAddrHashTable_r(afs_uint32 addr, afs_uint16 port, struct host *host)
     chain->addr = addr;
     chain->port = port;
     hostAddrHashTable[index] = chain;
+    ViceLog(125, ("h_AddHostToAddrHashTable_r: host %" AFS_PTR_FMT " added as %s:%d\n",
+                 host, afs_inet_ntoa_r(addr, hoststr), ntohs(port)));
 }
 
 /*
- * This is called with host locked and held. At this point, the
- * hostAddrHashTable should not have entries for the alternate
- * interfaces. This function has to insert these entries in the
- * hostAddrHashTable.
+ * This is called with host locked and held. 
+ * It is called to either validate or add an additional interface
+ * address/port on the specified host.  
  *
  * All addresses are in network byte order.
  */
@@ -1227,10 +1262,9 @@ addInterfaceAddr_r(struct host *host, afs_uint32 addr, afs_uint16 port)
 {
     int i;
     int number;
-    int found;
     struct Interface *interface;
     char hoststr[16], hoststr2[16];
-
+                                                   
     assert(host);
     assert(host->interface);
 
@@ -1239,45 +1273,54 @@ addInterfaceAddr_r(struct host *host, afs_uint32 addr, afs_uint16 port)
      * for this host.
      */
     number = host->interface->numberOfInterfaces;
-    for (i = 0, found = 0; i < number && !found; i++) {
+    for (i = 0; i < number; i++) {
        if (host->interface->interface[i].addr == addr &&
              host->interface->interface[i].port == port) {
-           found = 1;
-            host->interface->interface[i].valid = 1;
+           ViceLog(125, 
+                   ("addInterfaceAddr : found host %" AFS_PTR_FMT " (%s:%d) adding %s:%d%s\n",
+                    host, afs_inet_ntoa_r(host->host, hoststr), 
+                    ntohs(host->port), afs_inet_ntoa_r(addr, hoststr2), 
+                    ntohs(port), host->interface->interface[i].valid ? "" : 
+                    ", validating"));
+     
+           if (host->interface->interface[i].valid == 0) {
+               host->interface->interface[i].valid = 1;
+               h_AddHostToAddrHashTable_r(addr, port, host);
+           }
+           return 0;
         }
     }
 
-    ViceLog(125, ("addInterfaceAddr : host %s:%d addr %s:%d : found:%d\n", 
-                  afs_inet_ntoa_r(host->host, hoststr), ntohs(host->port), 
-                  afs_inet_ntoa_r(addr, hoststr2), ntohs(port), found));
+    ViceLog(125, ("addInterfaceAddr : host %" AFS_PTR_FMT " (%s:%d) adding %s:%d\n", 
+                 host, afs_inet_ntoa_r(host->host, hoststr), 
+                 ntohs(host->port), afs_inet_ntoa_r(addr, hoststr2), 
+                 ntohs(port)));
     
-    if (!found) {
-       interface = (struct Interface *)
-           malloc(sizeof(struct Interface) + (sizeof(struct AddrPort) * number));
-       if (!interface) {
-           ViceLog(0, ("Failed malloc in addInterfaceAddr_r\n"));
-           assert(0);
-       }
-       interface->numberOfInterfaces = number + 1;
-       interface->uuid = host->interface->uuid;
-       for (i = 0; i < number; i++)
-           interface->interface[i] = host->interface->interface[i];
-       interface->interface[number].addr = addr;
-       interface->interface[number].port = port;
-        interface->interface[number].valid = 1;
-       free(host->interface);
-       host->interface = interface;
+    interface = (struct Interface *)
+       malloc(sizeof(struct Interface) + (sizeof(struct AddrPort) * number));
+    if (!interface) {
+       ViceLog(0, ("Failed malloc in addInterfaceAddr_r\n"));
+       assert(0);
     }
-
+    interface->numberOfInterfaces = number + 1;
+    interface->uuid = host->interface->uuid;
+    for (i = 0; i < number; i++)
+       interface->interface[i] = host->interface->interface[i];
+    
+    /* Add the new valid interface */
+    interface->interface[number].addr = addr;
+    interface->interface[number].port = port;
+    interface->interface[number].valid = 1;
+    h_AddHostToAddrHashTable_r(addr, port, host);
+    free(host->interface);
+    host->interface = interface;
+    
     return 0;
 }
 
 
 /*
- * This is called with host locked and held. At this point, the
- * hostAddrHashTable should not be having entries for the alternate
- * interfaces. This function has to insert these entries in the
- * hostAddrHashTable.
+ * This is called with host locked and held.
  *
  * All addresses are in network byte order.
  */
@@ -1286,16 +1329,16 @@ removeInterfaceAddr_r(struct host *host, afs_uint32 addr, afs_uint16 port)
 {
     int i;
     int number;
-    int found;
     struct Interface *interface;
     char hoststr[16], hoststr2[16];
 
     assert(host);
     assert(host->interface);
 
-    ViceLog(125, ("removeInterfaceAddr : host %s:%d addr %s:%d\n", 
-                  afs_inet_ntoa_r(host->host, hoststr), ntohs(host->port), 
-                  afs_inet_ntoa_r(addr, hoststr2), ntohs(port)));
+    ViceLog(125, ("removeInterfaceAddr : host %" AFS_PTR_FMT " (%s:%d) addr %s:%d\n", 
+                 host, afs_inet_ntoa_r(host->host, hoststr), 
+                 ntohs(host->port), afs_inet_ntoa_r(addr, hoststr2), 
+                 ntohs(port)));
 
     /*
      * Make sure this address is on the list of known addresses
@@ -1303,26 +1346,62 @@ removeInterfaceAddr_r(struct host *host, afs_uint32 addr, afs_uint16 port)
      */
     interface = host->interface;
     number = host->interface->numberOfInterfaces;
-    for (i = 0, found = 0; i < number && !found; i++) {
+    for (i = 0; i < number; i++) {
        if (interface->interface[i].addr == addr &&
            interface->interface[i].port == port) {
-           found = 1;
-            interface->interface[i].valid = 0;
-       }
-    }
-    if (found) {
-       number--;
-       for (; i < number; i++) {
-           interface->interface[i] = interface->interface[i+1];
+           if (interface->interface[i].valid)
+               h_DeleteHostFromAddrHashTable_r(addr, port, host);
+           number--;
+           for (; i < number; i++) {
+               interface->interface[i] = interface->interface[i+1];
+           }
+           interface->numberOfInterfaces = number;
+           return 0;
        }
-       interface->numberOfInterfaces = number;
-    }
+    }  
+    /* not found */
+    return 0;
+}
 
+/*
+ * This is called with host locked and held.
+ *
+ * All addresses are in network byte order.
+ */
+int
+invalidateInterfaceAddr_r(struct host *host, afs_uint32 addr, afs_uint16 port)
+{
+    int i;
+    int number;
+    struct Interface *interface;
+    char hoststr[16], hoststr2[16];
+    
+    assert(host);
+    assert(host->interface);
+    
+    ViceLog(125, ("invalidateInterfaceAddr : host %" AFS_PTR_FMT " (%s:%d) addr %s:%d\n", 
+                 host, afs_inet_ntoa_r(host->host, hoststr), 
+                 ntohs(host->port), afs_inet_ntoa_r(addr, hoststr2), 
+                 ntohs(port)));
+    
     /*
-     * Remove the hash table entry for this address
+     * Make sure this address is on the list of known addresses
+     * for this host.
      */
-    h_DeleteHostFromAddrHashTable_r(addr, port, host);
-
+    interface = host->interface;
+    number = host->interface->numberOfInterfaces;
+    for (i = 0; i < number; i++) {
+       if (interface->interface[i].addr == addr &&
+           interface->interface[i].port == port) {
+            if (interface->interface[i].valid) {
+                h_DeleteHostFromAddrHashTable_r(addr, port, host);
+               interface->interface[i].valid = 0;
+           }
+           return 0;
+       }
+    }
+    
+    /* not found */
     return 0;
 }
 
@@ -1340,23 +1419,29 @@ removeAddress_r(struct host *host, afs_uint32 addr, afs_uint16 port)
 {
     int i;
     char hoststr[16], hoststr2[16];
+    struct rx_connection *rxconn;
 
-    if (!host->interface) {
+    if (!host->interface || host->interface->numberOfInterfaces == 1) {
         if (host->host == addr && host->port == port) {
             ViceLog(25,
-                    ("Removing only address for host %x (%s:%d), deleting host.\n",
+                    ("Removing only address for host %" AFS_PTR_FMT " (%s:%d), deleting host.\n",
                      host, afs_inet_ntoa_r(host->host, hoststr), ntohs(host->port)));
             host->hostFlags |= HOSTDELETED;
+            /* 
+             * Do not remove the primary addr/port from the hash table.
+             * It will be ignored due to the HOSTDELETED flag and will
+             * be removed when h_TossStuff_r() cleans up the HOSTDELETED
+             * host.  Removing it here will only result in a search for 
+             * the host/addr/port in the hash chain which will fail.
+             */
+        } else {
+            ViceLog(0,
+                    ("Removing address that does not belong to host %" AFS_PTR_FMT " (%s:%d).\n",
+                     host, afs_inet_ntoa_r(host->host, hoststr), ntohs(host->port)));
         }
     } else {
-        removeInterfaceAddr_r(host, host->host, host->port);
-        if (host->interface->numberOfInterfaces == 0) {
-            ViceLog(25,
-                     ("Removed only address for host %x (%s:%d), no alternate interfaces, deleting host.\n",
-                       host, afs_inet_ntoa_r(host->host, hoststr), ntohs(host->port)));
-            host->hostFlags |= HOSTDELETED;
-        } else {
-            struct rx_connection *rxconn;
+        if (host->host == addr && host->port == port)  {
+            removeInterfaceAddr_r(host, addr, port);
 
             rxconn = host->callback_rxcon;
             host->callback_rxcon = NULL;
@@ -1364,11 +1449,11 @@ removeAddress_r(struct host *host, afs_uint32 addr, afs_uint16 port)
             if (rxconn) {
                 struct client *client;
                 /*
-                * If rx_DestroyConnection calls h_FreeConnection we will
-                * deadlock on the host_glock_mutex. Work around the problem
-                * by unhooking the client from the connection before
-                * destroying the connection.
-                */
+                 * If rx_DestroyConnection calls h_FreeConnection we will
+                 * deadlock on the host_glock_mutex. Work around the problem
+                 * by unhooking the client from the connection before
+                 * destroying the connection.
+                 */
                 client = rx_GetSpecific(rxconn, rxcon_client_key);
                 rx_SetSpecific(rxconn, rxcon_client_key, (void *)0);
                 rx_DestroyConnection(rxconn);
@@ -1377,7 +1462,7 @@ removeAddress_r(struct host *host, afs_uint32 addr, afs_uint16 port)
             for (i=0; i < host->interface->numberOfInterfaces; i++) {
                 if (host->interface->interface[i].valid) {
                     ViceLog(25,
-                             ("Removed address for host %x (%s:%d), new primary interface %s:%d.\n",
+                             ("Removed address for host %" AFS_PTR_FMT " (%s:%d), new primary interface %s:%d.\n",
                                host, afs_inet_ntoa_r(host->host, hoststr), ntohs(host->port),
                                afs_inet_ntoa_r(host->interface->interface[i].addr, hoststr2), 
                                ntohs(host->interface->interface[i].port)));
@@ -1390,9 +1475,12 @@ removeAddress_r(struct host *host, afs_uint32 addr, afs_uint16 port)
 
             if (i == host->interface->numberOfInterfaces) {
                 ViceLog(25,
-                         ("Removed only address for host %x (%s:%d), no valid alternate interfaces, deleting host.\n",
+                         ("Removed only address for host %" AFS_PTR_FMT " (%s:%d), no valid alternate interfaces, deleting host.\n",
                            host, afs_inet_ntoa_r(host->host, hoststr), ntohs(host->port)));
                 host->hostFlags |= HOSTDELETED;
+                /* addr/port was removed from the hash table */
+                host->host = 0;
+                host->port = 0;
             } else {
                 if (!sc)
                     sc = rxnull_NewClientSecurityObject();
@@ -1401,6 +1489,9 @@ removeAddress_r(struct host *host, afs_uint32 addr, afs_uint16 port)
                 rx_SetConnDeadTime(host->callback_rxcon, 50);
                 rx_SetConnHardDeadTime(host->callback_rxcon, AFS_HARDDEADTIME);
             }
+        } else {
+            /* not the primary addr/port, just invalidate it */
+            invalidateInterfaceAddr_r(host, addr, port);
         }
     }
 
@@ -1479,7 +1570,7 @@ h_GetHost_r(struct rx_connection *tcon)
            if (!held)
                h_Release_r(host);
            ViceLog(125,
-                   ("Host %x (%s:%d) starting h_Lookup again\n",
+                   ("Host %" AFS_PTR_FMT " (%s:%d) starting h_Lookup again\n",
                     host, afs_inet_ntoa_r(host->host, hoststr),
                     ntohs(host->port)));
            goto retry;
@@ -1544,7 +1635,7 @@ h_GetHost_r(struct rx_connection *tcon)
                 * that we maintain some extra callback state information */
                if (host->interface) {
                    ViceLog(0,
-                           ("Host %x (%s:%d) used to support WhoAreYou, deleting.\n",
+                           ("Host %" AFS_PTR_FMT " (%s:%d) used to support WhoAreYou, deleting.\n",
                             host, 
                             afs_inet_ntoa_r(host->host, hoststr),
                             ntohs(host->port)));
@@ -1595,7 +1686,7 @@ h_GetHost_r(struct rx_connection *tcon)
                     removeAddress_r(host, haddr, hport);
                } else {
                    ViceLog(25,
-                           ("Uuid doesn't match host %x (%s:%d).\n",
+                           ("Uuid doesn't match host %" AFS_PTR_FMT " (%s:%d).\n",
                             host, afs_inet_ntoa_r(host->host, hoststr), ntohs(host->port)));
                    
                    removeAddress_r(host, host->host, host->port);
@@ -1633,13 +1724,18 @@ h_GetHost_r(struct rx_connection *tcon)
                      * callback connection, and destroy the old one.
                      */
                     struct rx_connection *rxconn;
-                    ViceLog(0,("CB: ProbeUuid for host %x (%s:%d) failed %d\n",
+                    ViceLog(0,("CB: ProbeUuid for host %" AFS_PTR_FMT " (%s:%d) failed %d\n",
                               host, 
                               afs_inet_ntoa_r(host->host, hoststr),
                               ntohs(host->port),code2));
-                   
-                    removeInterfaceAddr_r(host, host->host, host->port);
+
+                    /* 
+                     * make sure we add and then remove.  otherwise, we
+                     * might end up with no valid interfaces after the 
+                     * remove and the host will have been marked deleted.
+                     */
                     addInterfaceAddr_r(host, haddr, hport);
+                    removeInterfaceAddr_r(host, host->host, host->port);
                     host->host = haddr;
                     host->port = hport;
                     rxconn = host->callback_rxcon;
@@ -1680,7 +1776,7 @@ h_GetHost_r(struct rx_connection *tcon)
                 return 0;
            } else {
                ViceLog(0,
-                       ("CB: WhoAreYou failed for host %x (%s:%d), error %d\n",
+                       ("CB: WhoAreYou failed for host %" AFS_PTR_FMT " (%s:%d), error %d\n",
                         host, afs_inet_ntoa_r(host->host, hoststr),
                         ntohs(host->port), code));
                host->hostFlags |= VENUSDOWN;
@@ -1698,7 +1794,7 @@ h_GetHost_r(struct rx_connection *tcon)
        if (!(host->hostFlags & ALTADDR)) {
            /* another thread is doing the initialisation */
            ViceLog(125,
-                   ("Host %x (%s:%d) waiting for host-init to complete\n",
+                   ("Host %" AFS_PTR_FMT " (%s:%d) waiting for host-init to complete\n",
                     host, afs_inet_ntoa_r(host->host, hoststr),
                     ntohs(host->port)));
            h_Lock_r(host);
@@ -1706,7 +1802,7 @@ h_GetHost_r(struct rx_connection *tcon)
            if (!held)
                h_Release_r(host);
            ViceLog(125,
-                   ("Host %x (%s:%d) starting h_Lookup again\n",
+                   ("Host %" AFS_PTR_FMT " (%s:%d) starting h_Lookup again\n",
                     host, afs_inet_ntoa_r(host->host, hoststr),
                     ntohs(host->port)));
            goto retry;
@@ -1726,7 +1822,7 @@ h_GetHost_r(struct rx_connection *tcon)
            if (host->interface)
                afsUUID_to_string(&host->interface->uuid, uuid2, 127);
            ViceLog(0,
-                   ("CB: new identity for host %x (%s:%d), deleting(%x %x %s %s)\n",
+                   ("CB: new identity for host %" AFS_PTR_FMT " (%s:%d), deleting(%x %x %s %s)\n",
                     host, afs_inet_ntoa_r(host->host, hoststr), ntohs(host->port),
                     identP->valid, host->interface,
                     identP->valid ? uuid1 : "no_uuid",
@@ -1772,7 +1868,7 @@ h_GetHost_r(struct rx_connection *tcon)
                if (!pident)
                    rx_SetSpecific(tcon, rxcon_ident_key, identP);
                ViceLog(25,
-                       ("Host %x (%s:%d) does not support WhoAreYou.\n",
+                       ("Host %" AFS_PTR_FMT " (%s:%d) does not support WhoAreYou.\n",
                         host, afs_inet_ntoa_r(host->host, hoststr),
                         ntohs(host->port)));
                code = 0;
@@ -1793,7 +1889,7 @@ h_GetHost_r(struct rx_connection *tcon)
                if (!pident)
                    rx_SetSpecific(tcon, rxcon_ident_key, identP);
                ViceLog(25,
-                       ("WhoAreYou success on host %x (%s:%d)\n",
+                       ("WhoAreYou success on host %" AFS_PTR_FMT " (%s:%d)\n",
                         host, afs_inet_ntoa_r(host->host, hoststr),
                         ntohs(host->port)));
            }
@@ -1835,7 +1931,7 @@ h_GetHost_r(struct rx_connection *tcon)
                             * MultiProbeAlternateAddress_r() will remove the
                             * alternate interfaces that do not have the same
                             * Uuid. */
-                           ViceLog(0,("CB: ProbeUuid for host %x (%s:%d) failed %d\n",
+                           ViceLog(0,("CB: ProbeUuid for host %" AFS_PTR_FMT " (%s:%d) failed %d\n",
                                         oldHost, 
                                          afs_inet_ntoa_r(oldHost->host, hoststr),
                                         ntohs(oldHost->port),code2));
@@ -1857,39 +1953,46 @@ h_GetHost_r(struct rx_connection *tcon)
                        struct rx_connection *rxconn;
 
                        ViceLog(25,
-                                 ("CB: Host %x (%s:%d) has new addr %s:%d\n",
+                                 ("CB: Host %" AFS_PTR_FMT " (%s:%d) has new addr %s:%d\n",
                                    oldHost, 
                                    afs_inet_ntoa_r(oldHost->host, hoststr2),
                                    ntohs(oldHost->port),
                                    afs_inet_ntoa_r(haddr, hoststr),
                                    ntohs(hport)));
+                       /* 
+                        * add then remove.  otherwise the host may get marked
+                        * deleted if we removed the only valid address.
+                        */
+                       addInterfaceAddr_r(oldHost, haddr, hport);
                        if (probefail || oldHost->host == haddr) {
-                           /* The probe failed which means that the old address is 
-                            * either unreachable or is not the same host we were just
-                            * contacted by.  We will also remove addresses if only
-                            * the port has changed because that indicates the client
+                           /* 
+                            * The probe failed which means that the old 
+                            * address is either unreachable or is not the 
+                            * same host we were just contacted by.  We will 
+                            * also remove addresses if only the port has 
+                            * changed because that indicates the client
                             * is behind a NAT. 
                             */
                            removeInterfaceAddr_r(oldHost, oldHost->host, oldHost->port);
                        } else {
-                           int i, found;
+                           int i;
                            struct Interface *interface = oldHost->interface;
                            int number = oldHost->interface->numberOfInterfaces;
-                           for (i = 0, found = 0; i < number; i++) {
+                           for (i = 0; i < number; i++) {
                                if (interface->interface[i].addr == haddr &&
                                    interface->interface[i].port != hport) {
-                                   found = 1;
+                                   /* 
+                                    * We have just been contacted by a client
+                                    * that has been seen from behind a NAT 
+                                    * and at least one other address.
+                                    */
+                                   removeInterfaceAddr_r(oldHost, haddr, 
+                                                         interface->interface[i].port);
                                    break;
                                }
                            }
-                           if (found) {
-                               /* We have just been contacted by a client that has been
-                                * seen from behind a NAT and at least one other address.
-                                */
-                               removeInterfaceAddr_r(oldHost, haddr, interface->interface[i].port);
-                           }
                        }
-                       addInterfaceAddr_r(oldHost, haddr, hport);
+                       h_AddHostToAddrHashTable_r(haddr, hport, oldHost);
                        oldHost->host = haddr;
                        oldHost->port = hport;
                        rxconn = oldHost->callback_rxcon;
@@ -1929,7 +2032,7 @@ h_GetHost_r(struct rx_connection *tcon)
                    H_LOCK;
                    if (code == 0) {
                        ViceLog(25,
-                               ("InitCallBackState3 success on host %x (%s:%d)\n",
+                               ("InitCallBackState3 success on host %" AFS_PTR_FMT " (%s:%d)\n",
                                 host, afs_inet_ntoa_r(host->host, hoststr),
                                 ntohs(host->port)));
                        assert(interfValid == 1);
@@ -1939,12 +2042,12 @@ h_GetHost_r(struct rx_connection *tcon)
            }
            if (code) {
                ViceLog(0,
-                       ("CB: RCallBackConnectBack failed for %x (%s:%d)\n",
+                       ("CB: RCallBackConnectBack failed for %" AFS_PTR_FMT " (%s:%d)\n",
                         host, afs_inet_ntoa_r(host->host, hoststr), ntohs(host->port)));
                host->hostFlags |= VENUSDOWN;
            } else {
                ViceLog(125,
-                       ("CB: RCallBackConnectBack succeeded for %x (%s:%d)\n",
+                       ("CB: RCallBackConnectBack succeeded for %" AFS_PTR_FMT " (%s:%d)\n",
                         host, afs_inet_ntoa_r(host->host, hoststr), ntohs(host->port)));
                host->hostFlags |= RESETDONE;
            }
@@ -2271,7 +2374,7 @@ h_FindClient_r(struct rx_connection *tcon)
            if (code) {
                char hoststr[16];
                ViceLog(0,
-                       ("pr_GetCPS failed(%d) for user %d, host %x (%s:%d)\n",
+                       ("pr_GetCPS failed(%d) for user %d, host %" AFS_PTR_FMT " (%s:%d)\n",
                         code, viceid, client->host, 
                          afs_inet_ntoa_r(client->host->host,hoststr),
                         ntohs(client->host->port)));
@@ -3075,13 +3178,20 @@ h_stateRestoreHost(struct fs_dump_state * state)
     h_diskEntryToHost_r(&hdsk, host);
     h_SetupCallbackConn_r(host);
 
+    h_AddHostToAddrHashTable_r(host->host, host->port, host);
     if (ifp) {
        int i;
        for (i = ifp->numberOfInterfaces-1; i >= 0; i--) {
+            if (ifp->interface[i].valid && 
+                !(ifp->interface[i].addr == host->host &&
+                  ifp->interface[i].port == host->port)) {
+                h_AddHostToAddrHashTable_r(ifp->interface[i].addr, 
+                                           ifp->interface[i].port, 
+                                           host);
+            }
        }
        h_AddHostToUuidHashTable_r(&ifp->uuid, host);
     }
-    h_AddHostToAddrHashTable_r(host->host, host->port, host);
     h_InsertList_r(host);
 
     /* setup host id map entry */
@@ -3436,7 +3546,7 @@ CheckHost(register struct host *host, int held, void *rock)
                     * back state, because break delayed callbacks (called when a
                     * message is received from the workstation) will always send a 
                     * break all call backs to the workstation if there is no
-                    *callback.
+                    * callback.
                     */
                }
            } else {
@@ -3539,7 +3649,7 @@ CheckHost_r(register struct host *host, int held, char *dummy)
                     * back state, because break delayed callbacks (called when a
                     * message is received from the workstation) will always send a 
                     * break all call backs to the workstation if there is no
-                    *callback.
+                    * callback.
                     */
                }
            } else {
@@ -3612,9 +3722,8 @@ h_CheckHosts(void)
 
 /*
  * This is called with host locked and held. At this point, the
- * hostAddrHashTable should not have any entries for the alternate
- * interfaces. This function has to insert these entries in the
- * hostAddrHashTable.
+ * hostAddrHashTable has an entry for the primary addr/port inserted
+ * by h_Alloc_r().  No other interfaces should be considered valid.
  *
  * The addresses in the interfaceAddr list are in host byte order.
  */
@@ -3732,7 +3841,8 @@ initInterfaceAddr_r(struct host *host, struct interfaceAddr *interf)
         * are coming from fully connected hosts (no NAT/PATs)
         */
        interface->interface[i].port = port7001;
-        interface->interface[i].valid = 1;      /* valid until a conflict is found */
+        interface->interface[i].valid = 
+            (interf->addr_in[i] == myAddr && port7001 == myPort) ? 1 : 0;
     }
 
     interface->uuid = interf->uuid;
@@ -3740,13 +3850,15 @@ initInterfaceAddr_r(struct host *host, struct interfaceAddr *interf)
     assert(!host->interface);
     host->interface = interface;
 
-    afsUUID_to_string(&interface->uuid, uuidstr, 127);
-
-    ViceLog(125, ("--- uuid %s\n", uuidstr));
-    for (i = 0; i < host->interface->numberOfInterfaces; i++) {
-       ViceLog(125, ("--- alt address %s:%d\n", 
-                      afs_inet_ntoa_r(host->interface->interface[i].addr, hoststr),
-                      ntohs(host->interface->interface[i].port)));
+    if (LogLevel >= 125) {
+       afsUUID_to_string(&interface->uuid, uuidstr, 127);
+       
+       ViceLog(125, ("--- uuid %s\n", uuidstr));
+       for (i = 0; i < host->interface->numberOfInterfaces; i++) {
+           ViceLog(125, ("--- alt address %s:%d\n", 
+                         afs_inet_ntoa_r(host->interface->interface[i].addr, hoststr),
+                         ntohs(host->interface->interface[i].port)));
+       }
     }
 
     return 0;
@@ -3755,23 +3867,32 @@ initInterfaceAddr_r(struct host *host, struct interfaceAddr *interf)
 /* deleted a HashChain structure for this address and host */
 /* returns 1 on success */
 int
-h_DeleteHostFromAddrHashTable_r(afs_uint32 addr, afs_uint16 port, struct host *host)
+h_DeleteHostFromAddrHashTable_r(afs_uint32 addr, afs_uint16 port, 
+                               struct host *host)
 {
-    int flag = 0;
+    char hoststr[16];
     register struct h_AddrHashChain **hp, *th;
 
-    for (hp = &hostAddrHashTable[h_HashIndex(addr)]; (th = *hp);) {
-       assert(th->hostPtr);
-       if (th->hostPtr == host && th->addr == addr && th->port == port) {
-           *hp = th->next;
-           free(th);
-           flag = 1;
-           break;
-       } else {
-           hp = &th->next;
-       }
+    if (addr == 0 && port == 0)
+       return 1;
+
+    for (hp = &hostAddrHashTable[h_HashIndex(addr)]; (th = *hp); 
+        hp = &th->next) {
+        assert(th->hostPtr);
+        if (th->hostPtr == host && th->addr == addr && th->port == port) {
+           ViceLog(125, ("h_DeleteHostFromAddrHashTable_r: host %" AFS_PTR_FMT " (%s:%d)\n",
+                         host, afs_inet_ntoa_r(host->host, hoststr),
+                         ntohs(host->port)));
+            *hp = th->next;
+            free(th);
+           return 1;
+        }
     }
-    return flag;
+    ViceLog(125, 
+           ("h_DeleteHostFromAddrHashTable_r: host %" AFS_PTR_FMT " (%s:%d) not found\n",
+            host, afs_inet_ntoa_r(host->host, hoststr), 
+            ntohs(host->port)));
+    return 0;
 }