From: Andrew Deason Date: Sat, 23 Apr 2011 21:25:00 +0000 (-0500) Subject: viced: Fix host enumeration flags X-Git-Tag: openafs-devel-1_7_1~577 X-Git-Url: https://git.openafs.org/?p=openafs.git;a=commitdiff_plain;h=5b9d427141f0a6fd0e83de9564e70ef2cfebf656 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 Change-Id: Ibb01ce62411602a9f83f437125fb87a7a84160b4 Reviewed-on: http://gerrit.openafs.org/4527 Tested-by: BuildBot Reviewed-by: Derrick Brashear --- diff --git a/src/viced/callback.c b/src/viced/callback.c index 3322b0e..90e2f71 100644 --- a/src/viced/callback.c +++ b/src/viced/callback.c @@ -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 */ diff --git a/src/viced/host.c b/src/viced/host.c index 9e250c3..212c6c7 100644 --- a/src/viced/host.c +++ b/src/viced/host.c @@ -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 */ diff --git a/src/viced/host.h b/src/viced/host.h index e02e442..e935353 100644 --- 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 */