viced: Fix host enumeration flags

Do not give uninitialized flags values to h_Enumerate callback
functions. In fact, do not give a flags value to h_Enumerate or
h_Enumerate_r callback functions at all, since they are not actually
used.

Fix host enumeration callback functions to just return 0 or the
relevant flags, instead of basing the return value off of the given
flags value. Update MultiBreakVolumeCallBack_r to use the correct
return values, since it currently tries to use the old meanings of the
host enumeration return values.

FIXES 129376

(cherry picked from commit 5b9d427141f0a6fd0e83de9564e70ef2cfebf656)

Change-Id: Icae627318cd523fa225beb8b53449f61532c4a90
Reviewed-on: http://gerrit.openafs.org/4531
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementia.org>

diff --git a/src/viced/callback.c b/src/viced/callback.c
index c2d9d65e990b23db0a6d07d0b554351183b8f7bb..fdbd5b55c26dd23e672df06324947c1686fc4aa4 100644 (file)
--- a/src/viced/callback.c
+++ b/src/viced/callback.c
@@ -191,10 +191,9 @@ static int AddCallBack1_r(struct host *host, AFSFid * fid, afs_uint32 * thead,
                          int type, int locked);
 static void MultiBreakCallBack_r(struct cbstruct cba[], int ncbas,
                                 struct AFSCBFids *afidp, struct host *xhost);
-static int MultiBreakVolumeCallBack_r(struct host *host, int isheld,
+static int MultiBreakVolumeCallBack_r(struct host *host,
                                      struct VCBParams *parms, int deletefe);
-static int MultiBreakVolumeLaterCallBack(struct host *host, int isheld,
-                                        void *rock);
+static int MultiBreakVolumeLaterCallBack(struct host *host, void *rock);
 static int GetSomeSpace_r(struct host *hostp, int locked);
 static int ClearHostCallbacks_r(struct host *hp, int locked);
 static int DumpCallBackState_r(void);
@@ -1113,18 +1112,14 @@ BreakDelayedCallBacks_r(struct host *host)
     return (host->hostFlags & VENUSDOWN);
 }
 
-/*
-** isheld is 0 if the host is held in h_Enumerate
-** isheld is 1 if the host is held in BreakVolumeCallBacks
-*/
 static int
-MultiBreakVolumeCallBack_r(struct host *host, int isheld,
+MultiBreakVolumeCallBack_r(struct host *host,
                           struct VCBParams *parms, int deletefe)
 {
     char hoststr[16];
 
     if (host->hostFlags & HOSTDELETED)
-       return 0;               /* host is deleted, release hold */
+       return 0;
 
     if (!(host->hostFlags & HCBREAK))
        return 0;               /* host is not flagged to notify */
@@ -1145,7 +1140,7 @@ MultiBreakVolumeCallBack_r(struct host *host, int isheld,
                                                 * later */
        host->hostFlags &= ~(RESETDONE|HCBREAK);        /* Do InitCallBackState when host returns */
        h_Unlock_r(host);
-       return 0;               /* parent will release hold */
+       return 0;
     }
     osi_Assert(parms->ncbas <= MAX_CB_HOSTS);
 
@@ -1166,20 +1161,21 @@ MultiBreakVolumeCallBack_r(struct host *host, int isheld,
     parms->cba[parms->ncbas].hp = host;
     parms->cba[(parms->ncbas)++].thead = parms->thead;
     host->hostFlags &= ~HCBREAK;
-    return 1;          /* parent shouldn't release hold, more work to do */
+
+    /* we have more work to do on this host, so make sure we keep a reference
+     * to it */
+    h_Hold_r(host);
+
+    return 0;
 }
 
-/*
-** isheld is 0 if the host is held in h_Enumerate
-** isheld is 1 if the host is held in BreakVolumeCallBacks
-*/
 static int
-MultiBreakVolumeLaterCallBack(struct host *host, int isheld, void *rock)
+MultiBreakVolumeLaterCallBack(struct host *host, void *rock)
 {
     struct VCBParams *parms = (struct VCBParams *)rock;
     int retval;
     H_LOCK;
-    retval = MultiBreakVolumeCallBack_r(host, isheld, parms, 0);
+    retval = MultiBreakVolumeCallBack_r(host, parms, 0);
     H_UNLOCK;
     return retval;
 }
@@ -1446,7 +1442,7 @@ struct lih_params {
  * theory not give these to us anyway, but be paranoid.
  */
 static int
-lih0_r(struct host *host, int flags, void *rock)
+lih0_r(struct host *host, void *rock)
 {
     struct lih_params *params = (struct lih_params *)rock;
 
@@ -1464,12 +1460,12 @@ lih0_r(struct host *host, int flags, void *rock)
        h_Hold_r(host);
        params->lih = host;
     }
-    return flags;
+    return 0;
 }
 
 /* same as lih0_r, except we do not prevent held hosts from being selected. */
 static int
-lih1_r(struct host *host, int flags, void *rock)
+lih1_r(struct host *host, void *rock)
 {
     struct lih_params *params = (struct lih_params *)rock;
 
@@ -1485,7 +1481,7 @@ lih1_r(struct host *host, int flags, void *rock)
        h_Hold_r(host);
        params->lih = host;
     }
-    return flags;
+    return 0;
 }
 
 /* This could be upgraded to get more space each time */

diff --git a/src/viced/host.c b/src/viced/host.c
index 0698c0d3eec4e0a5da1ca1169916103308f1f892..c41d896b78dd7b7502399337dfdef1fc6898d89b 100644 (file)
--- a/src/viced/host.c
+++ b/src/viced/host.c
@@ -962,23 +962,18 @@ h_TossStuff_r(struct host *host)
 
 
 
-/* h_Enumerate: Calls (*proc)(host, held, param) for at least each host in the
+/* h_Enumerate: Calls (*proc)(host, param) for at least each host in the
  * system at the start of the enumeration (perhaps more).  Hosts may be deleted
  * (have delete flag set); ditto for clients.  refCount is always incremented
- * before (*proc) is called.  The param flags is passed to (*proc) as the
- * param flags, permitting (*proc) to stop the enumeration (BAIL).
+ * before (*proc) is called.
  *
- * Needed?  Why not always h_Hold_r and h_Release_r in (*proc), or even -never-
- * h_Hold_r or h_Release_r in (*proc)?
- *
- * **The proc should return 0 if the host should be released, 1 if it should
- * be held after enumeration.
+ * The return value of the proc is a set of flags. The proc should set
+ * H_ENUMERATE_BAIL(foo) if the enumeration of hosts should be stopped early.
  */
 void
-h_Enumerate(int (*proc) (struct host*, int, void *), void *param)
+h_Enumerate(int (*proc) (struct host*, void *), void *param)
 {
     struct host *host, **list;
-    int *flags;
     int i, count;
     int totalCount;
 
@@ -992,11 +987,6 @@ h_Enumerate(int (*proc) (struct host*, int, void *), void *param)
        ViceLog(0, ("Failed malloc in h_Enumerate (list)\n"));
        osi_Panic("Failed malloc in h_Enumerate (list)\n");
     }
-    flags = (int *)malloc(hostCount * sizeof(int));
-    if (!flags) {
-       ViceLog(0, ("Failed malloc in h_Enumerate (flags)\n"));
-       osi_Panic("Failed malloc in h_Enumerate (flags)\n");
-    }
     for (totalCount = count = 0, host = hostList;
          host && totalCount < hostCount;
         host = host->next, totalCount++) {
@@ -1015,43 +1005,37 @@ h_Enumerate(int (*proc) (struct host*, int, void *), void *param)
     }
     H_UNLOCK;
     for (i = 0; i < count; i++) {
-       flags[i] = (*proc) (list[i], flags[i], param);
+       int flags;
+       flags = (*proc) (list[i], param);
        H_LOCK;
        h_Release_r(list[i]);
        H_UNLOCK;
        /* bail out of the enumeration early */
-       if (H_ENUMERATE_ISSET_BAIL(flags[i]))
+       if (H_ENUMERATE_ISSET_BAIL(flags))
            break;
     }
     free((void *)list);
-    free((void *)flags);
 }      /* h_Enumerate */
 
 
 /* h_Enumerate_r (revised):
- * Calls (*proc)(host, flags, param) for each host in hostList, starting
+ * Calls (*proc)(host, param) for each host in hostList, starting
  * at enumstart. Called only under H_LOCK.  Hosts may be deleted (have
  * delete flag set); ditto for clients.  refCount is always incremented
- * before (*proc) is called.  The param flags is passed to (*proc) as the
- * param flags, permitting (*proc) to stop the enumeration (BAIL).
- *
- * Needed?  Why not always h_Hold_r and h_Release_r in (*proc), or even -never-
- * h_Hold_r or h_Release_r in (*proc)?
+ * before (*proc) is called.
  *
  * @note Assumes that hostList is only prepended to, that a host is never
  *       inserted into the middle. Otherwise this would not be guaranteed to
  *       terminate.
  *
- * **The proc should return 0 if the host should be released, 1 if it should
- * be held after enumeration.
+ * The return value of the proc is a set of flags. The proc should set
+ * H_ENUMERATE_BAIL(foo) if the enumeration of hosts should be stopped early.
  */
 void
-h_Enumerate_r(int (*proc) (struct host *, int, void *),
+h_Enumerate_r(int (*proc) (struct host *, void *),
              struct host *enumstart, void *param)
 {
     struct host *host, *next;
-    int flags = 0;
-    int nflags = 0;
     int count;
     int origHostCount;
 
@@ -1090,7 +1074,7 @@ h_Enumerate_r(int (*proc) (struct host *, int, void *),
      * h_Release_r */
     origHostCount = hostCount;
 
-    for (count = 0, host = enumstart; host && count < origHostCount; host = next, flags = nflags, count++) {
+    for (count = 0, host = enumstart; host && count < origHostCount; host = next, count++) {
        next = host->next;
 
        /* find the next non-deleted host */
@@ -1102,11 +1086,12 @@ h_Enumerate_r(int (*proc) (struct host *, int, void *),
                ShutDownAndCore(PANIC);
            }
        }
-       if (next && !H_ENUMERATE_ISSET_BAIL(flags))
+       if (next)
            h_Hold_r(next);
 
        if (!(host->hostFlags & HOSTDELETED)) {
-           flags = (*proc) (host, flags, param);
+           int flags;
+           flags = (*proc) (host, param);
            if (H_ENUMERATE_ISSET_BAIL(flags)) {
                h_Release_r(host); /* this might free up the host */
                break;
@@ -2734,7 +2719,7 @@ h_PrintStats(void)
 
 
 static int
-h_PrintClient(struct host *host, int flags, void *rock)
+h_PrintClient(struct host *host, void *rock)
 {
     StreamHandle_t *file = (StreamHandle_t *)rock;
     struct client *client;
@@ -2748,7 +2733,7 @@ h_PrintClient(struct host *host, int flags, void *rock)
     LastCall = host->LastCall;
     if (host->hostFlags & HOSTDELETED) {
        H_UNLOCK;
-       return flags;
+       return 0;
     }
     (void)afs_snprintf(tmpStr, sizeof tmpStr,
                       "Host %s:%d down = %d, LastCall %s",
@@ -2786,7 +2771,7 @@ h_PrintClient(struct host *host, int flags, void *rock)
        }
     }
     H_UNLOCK;
-    return flags;
+    return 0;
 
 }                              /*h_PrintClient */
 
@@ -2824,7 +2809,7 @@ h_PrintClients(void)
 
 
 static int
-h_DumpHost(struct host *host, int flags, void *rock)
+h_DumpHost(struct host *host, void *rock)
 {
     StreamHandle_t *file = (StreamHandle_t *)rock;
 
@@ -2862,7 +2847,7 @@ h_DumpHost(struct host *host, int flags, void *rock)
     (void)STREAM_WRITE(tmpStr, strlen(tmpStr), 1, file);
 
     H_UNLOCK;
-    return flags;
+    return 0;
 
 }                              /*h_DumpHost */
 
@@ -2899,10 +2884,10 @@ h_DumpHosts(void)
 static int h_stateFillHeader(struct host_state_header * hdr);
 static int h_stateCheckHeader(struct host_state_header * hdr);
 static int h_stateAllocMap(struct fs_dump_state * state);
-static int h_stateSaveHost(struct host * host, int flags, void *rock);
+static int h_stateSaveHost(struct host * host, void *rock);
 static int h_stateRestoreHost(struct fs_dump_state * state);
-static int h_stateRestoreIndex(struct host * h, int flags, void *rock);
-static int h_stateVerifyHost(struct host * h, int flags, void *rock);
+static int h_stateRestoreIndex(struct host * h, void *rock);
+static int h_stateVerifyHost(struct host * h, void *rock);
 static int h_stateVerifyAddrHash(struct fs_dump_state * state, struct host * h,
                                  afs_uint32 addr, afs_uint16 port, int valid);
 static int h_stateVerifyUuidHash(struct fs_dump_state * state, struct host * h);
@@ -2995,13 +2980,13 @@ h_stateRestoreIndices(struct fs_dump_state * state)
 }
 
 static int
-h_stateRestoreIndex(struct host * h, int flags, void *rock)
+h_stateRestoreIndex(struct host * h, void *rock)
 {
     struct fs_dump_state *state = (struct fs_dump_state *)rock;
     if (cb_OldToNew(state, h->cblist, &h->cblist)) {
-       return H_ENUMERATE_BAIL(flags);
+       return H_ENUMERATE_BAIL(0);
     }
-    return flags;
+    return 0;
 }
 
 int
@@ -3012,14 +2997,14 @@ h_stateVerify(struct fs_dump_state * state)
 }
 
 static int
-h_stateVerifyHost(struct host * h, int flags, void* rock)
+h_stateVerifyHost(struct host * h, void* rock)
 {
     struct fs_dump_state *state = (struct fs_dump_state *)rock;
     int i;
 
     if (h == NULL) {
        ViceLog(0, ("h_stateVerifyHost: error: NULL host pointer in linked list\n"));
-       return H_ENUMERATE_BAIL(flags);
+       return H_ENUMERATE_BAIL(0);
     }
 
     if (h->interface) {
@@ -3041,7 +3026,7 @@ h_stateVerifyHost(struct host * h, int flags, void* rock)
        state->bail = 1;
     }
 
-    return flags;
+    return 0;
 }
 
 /**
@@ -3219,7 +3204,7 @@ h_stateAllocMap(struct fs_dump_state * state)
 
 /* function called by h_Enumerate to save a host to disk */
 static int
-h_stateSaveHost(struct host * host, int flags, void* rock)
+h_stateSaveHost(struct host * host, void* rock)
 {
     struct fs_dump_state *state = (struct fs_dump_state *) rock;
     int if_len=0, hcps_len=0;
@@ -3285,9 +3270,9 @@ h_stateSaveHost(struct host * host, int flags, void* rock)
     if (hcps)
        free(hcps);
     if (state->bail) {
-       return H_ENUMERATE_BAIL(flags);
+       return H_ENUMERATE_BAIL(0);
     }
-    return flags;
+    return 0;
 }
 
 /* restores a host from disk */
@@ -3812,7 +3797,7 @@ CheckHost(struct host *host, int flags, void *rock)
 #endif
 
 int
-CheckHost_r(struct host *host, int flags, void *dummy)
+CheckHost_r(struct host *host, void *dummy)
 {
     struct client *client;
     struct rx_connection *cb_conn = NULL;
@@ -3823,7 +3808,7 @@ CheckHost_r(struct host *host, int flags, void *dummy)
     FS_STATE_RDLOCK;
     if (fs_state.mode == FS_MODE_SHUTDOWN) {
        FS_STATE_UNLOCK;
-       return H_ENUMERATE_BAIL(flags);
+       return H_ENUMERATE_BAIL(0);
     }
     FS_STATE_UNLOCK;
 #endif
@@ -3910,7 +3895,7 @@ CheckHost_r(struct host *host, int flags, void *dummy)
        }
        h_Unlock_r(host);
     }
-    return flags;
+    return 0;
 
 }                              /*CheckHost_r */
 

diff --git a/src/viced/host.h b/src/viced/host.h
index e02e4426bbf29ac76284c12404d17284873cc2b8..e9353535a05b133fa8634d54f72a23987bf73d04 100644 (file)
--- a/src/viced/host.h
+++ b/src/viced/host.h
@@ -220,8 +220,8 @@ extern struct host *h_Alloc_r(struct rx_connection *r_con);
 extern int h_Lookup_r(afs_uint32 hostaddr, afs_uint16 hport,
                      struct host **hostp);
 extern struct host *h_LookupUuid_r(afsUUID * uuidp);
-extern void h_Enumerate(int (*proc) (struct host *, int, void *), void *param);
-extern void h_Enumerate_r(int (*proc) (struct host *, int, void *), struct host *enumstart, void *param);
+extern void h_Enumerate(int (*proc) (struct host *, void *), void *param);
+extern void h_Enumerate_r(int (*proc) (struct host *, void *), struct host *enumstart, void *param);
 extern struct host *h_GetHost_r(struct rx_connection *tcon);
 extern struct client *h_FindClient_r(struct rx_connection *tcon);
 extern int h_ReleaseClient_r(struct client *client);
@@ -265,7 +265,6 @@ extern int h_RestoreState(void);
 
 #define H_ENUMERATE_BAIL(flags)        ((flags)|0x80000000)
 #define H_ENUMERATE_ISSET_BAIL(flags)  ((flags)&0x80000000)
-#define H_ENUMERATE_ISSET_HELD(flags)  ((flags)&0x7FFFFFFF)
 
 struct host *(hosttableptrs[h_MAXHOSTTABLES]); /* Used by h_itoh */
 #define h_htoi(host) ((host)->index)   /* index isn't zeroed, no need to lock */