viced: Fix host enumeration flags
authorAndrew Deason <adeason@sinenomine.net>
Sat, 23 Apr 2011 21:25:00 +0000 (16:25 -0500)
committerDerrick Brashear <shadow@dementia.org>
Mon, 25 Apr 2011 20:38:28 +0000 (13:38 -0700)
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

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

src/viced/callback.c
src/viced/host.c
src/viced/host.h

index 3322b0e..90e2f71 100644 (file)
@@ -183,10 +183,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);
@@ -1111,18 +1110,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 */
@@ -1143,7 +1138,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);
 
@@ -1164,20 +1159,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;
 }
@@ -1444,7 +1440,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;
 
@@ -1462,12 +1458,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;
 
@@ -1483,7 +1479,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 */
index 9e250c3..212c6c7 100644 (file)
@@ -952,23 +952,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;
 
@@ -981,10 +976,6 @@ h_Enumerate(int (*proc) (struct host*, int, void *), void *param)
     if (!list) {
        ViceLogThenPanic(0, ("Failed malloc in h_Enumerate (list)\n"));
     }
-    flags = (int *)malloc(hostCount * sizeof(int));
-    if (!flags) {
-       ViceLogThenPanic(0, ("Failed malloc in h_Enumerate (flags)\n"));
-    }
     for (totalCount = count = 0, host = hostList;
          host && totalCount < hostCount;
         host = host->next, totalCount++) {
@@ -1003,43 +994,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;
 
@@ -1078,7 +1063,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 */
@@ -1090,11 +1075,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;
@@ -2713,7 +2699,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;
@@ -2728,7 +2714,7 @@ h_PrintClient(struct host *host, int flags, void *rock)
     LastCall = host->LastCall;
     if (host->hostFlags & HOSTDELETED) {
        H_UNLOCK;
-       return flags;
+       return 0;
     }
     strftime(tbuffer, sizeof(tbuffer), "%a %b %d %T %Y",
             localtime_r(&LastCall, &tm));
@@ -2764,7 +2750,7 @@ h_PrintClient(struct host *host, int flags, void *rock)
        }
     }
     H_UNLOCK;
-    return flags;
+    return 0;
 
 }                              /*h_PrintClient */
 
@@ -2805,7 +2791,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;
 
@@ -2844,7 +2830,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 */
 
@@ -2883,10 +2869,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);
@@ -2979,13 +2965,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
@@ -2996,14 +2982,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) {
@@ -3025,7 +3011,7 @@ h_stateVerifyHost(struct host * h, int flags, void* rock)
        state->bail = 1;
     }
 
-    return flags;
+    return 0;
 }
 
 /**
@@ -3203,7 +3189,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;
@@ -3269,9 +3255,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 */
@@ -3796,7 +3782,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;
@@ -3807,7 +3793,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
@@ -3894,7 +3880,7 @@ CheckHost_r(struct host *host, int flags, void *dummy)
        }
        h_Unlock_r(host);
     }
-    return flags;
+    return 0;
 
 }                              /*CheckHost_r */
 
index e02e442..e935353 100644 (file)
@@ -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 */