fs-setserverprefs-bug-20040320

FIXES 3555

Rodney Dyer (UNCC) reported that the "fs setserverprefs" command if
issued multiple times would result in the AFSD server becoming unresponsive.
This appears to have been caused by improper use of writeLocks, a failure
to use writeLocks when necessary, and improper reference counting on the
cm_server_t data structures placed into the cm_allServersp list.
This commit is contained in:
Jeffrey Altman 2004-03-20 17:23:48 +00:00 committed by Jeffrey Altman
parent c763e4b643
commit 804a85a487
3 changed files with 62 additions and 58 deletions

View File

@ -1060,7 +1060,8 @@ long cm_IoctlSetSPrefs(struct smb_ioctl *ioctlp, struct cm_user *userp)
vlonly = spin->flags;
if ( vlonly )
type = CM_SERVER_VLDB;
else type = CM_SERVER_FILE;
else
type = CM_SERVER_FILE;
for ( i=0; i < noServers; i++)
{
@ -1070,7 +1071,7 @@ long cm_IoctlSetSPrefs(struct smb_ioctl *ioctlp, struct cm_user *userp)
tmp.sin_family = AF_INET;
tsp = cm_FindServer(&tmp, type);
if ( tsp ) /* an existing server */
if ( tsp ) /* an existing server - ref count increased */
{
tsp->ipRank = rank; /* no need to protect by mutex*/
@ -1086,13 +1087,13 @@ long cm_IoctlSetSPrefs(struct smb_ioctl *ioctlp, struct cm_user *userp)
/* set preferences for an existing vlserver */
cm_ChangeRankCellVLServer(tsp);
}
cm_PutServer(tsp); /* decrease refcount */
}
else /* add a new server without a cell*/
else /* add a new server without a cell */
{
tsp = cm_NewServer(&tmp, type, NULL);
tsp = cm_NewServer(&tmp, type, NULL); /* refcount = 1 */
tsp->ipRank = rank;
}
cm_PutServer(tsp);
}
return 0;
}

View File

@ -199,17 +199,21 @@ cm_server_t *cm_NewServer(struct sockaddr_in *socketp, int type, cm_cell_t *cell
osi_assert(socketp->sin_family == AF_INET);
tsp = malloc(sizeof(*tsp));
memset(tsp, 0, sizeof(*tsp));
memset(tsp, 0, sizeof(*tsp));
tsp->type = type;
tsp->cellp = cellp;
tsp->refCount = 1;
tsp->allNextp = cm_allServersp;
cm_allServersp = tsp;
tsp->cellp = cellp;
tsp->refCount = 1;
lock_InitializeMutex(&tsp->mx, "cm_server_t mutex");
tsp->addr = *socketp;
cm_SetServerPrefs(tsp);
return tsp;
lock_ObtainWrite(&cm_serverLock); /* get server lock */
tsp->allNextp = cm_allServersp;
cm_allServersp = tsp;
lock_ReleaseWrite(&cm_serverLock); /* release server lock */
return tsp;
}
/* find a server based on its properties */
@ -326,15 +330,14 @@ long cm_ChangeRankServer(cm_serverRef_t** list, cm_server_t* server)
}
current = & ( (*current)->next);
}
/* if this volume is not replicated on this server */
if (!element) {
lock_ReleaseWrite(&cm_serverLock);
lock_ReleaseWrite(&cm_serverLock);
/* if this volume is not replicated on this server */
if (!element)
return 1; /* server is not on list */
}
/* re-insert deleted element into the list with modified rank*/
cm_InsertServerList(list, element);
lock_ReleaseWrite(&cm_serverLock);
return 0;
}
/*

View File

@ -43,14 +43,14 @@ void cm_InitVolume(void)
long cm_UpdateVolume(struct cm_cell *cellp, cm_user_t *userp, cm_req_t *reqp,
cm_volume_t *volp)
{
cm_conn_t *connp;
int i;
cm_conn_t *connp;
int i;
cm_serverRef_t *tsrp;
cm_server_t *tsp;
struct sockaddr_in tsockAddr;
long tflags;
u_long tempAddr;
struct vldbentry vldbEntry; /* don't use NVLDB yet; they're not common */
cm_server_t *tsp;
struct sockaddr_in tsockAddr;
long tflags;
u_long tempAddr;
struct vldbentry vldbEntry; /* don't use NVLDB yet; they're not common */
int ROcount = 0;
long code;
@ -71,34 +71,34 @@ long cm_UpdateVolume(struct cm_cell *cellp, cm_user_t *userp, cm_req_t *reqp,
free(tsrp);
}
/* now we have volume structure locked and held; make RPC to fill it */
do {
/* now we have volume structure locked and held; make RPC to fill it */
do {
code = cm_ConnByMServers(cellp->vlServersp, userp, reqp,
&connp);
if (code) continue;
&connp);
if (code) continue;
osi_Log1(afsd_logp, "CALL VL_GetEntryByNameO name %s",
volp->namep);
code = VL_GetEntryByNameO(connp->callp, volp->namep, &vldbEntry);
volp->namep);
code = VL_GetEntryByNameO(connp->callp, volp->namep, &vldbEntry);
} while (cm_Analyze(connp, userp, reqp, NULL, NULL, NULL, code));
code = cm_MapVLRPCError(code, reqp);
code = cm_MapVLRPCError(code, reqp);
if (code == 0) {
if (code == 0) {
/* decode the response */
lock_ObtainWrite(&cm_volumeLock);
if (vldbEntry.flags & VLF_RWEXISTS)
volp->rwID = vldbEntry.volumeId[0];
if (vldbEntry.flags & VLF_RWEXISTS)
volp->rwID = vldbEntry.volumeId[0];
else
volp->rwID = 0;
if (vldbEntry.flags & VLF_ROEXISTS)
volp->roID = vldbEntry.volumeId[1];
else
volp->roID = 0;
if (vldbEntry.flags & VLF_BACKEXISTS)
volp->bkID = vldbEntry.volumeId[2];
volp->rwID = 0;
if (vldbEntry.flags & VLF_ROEXISTS)
volp->roID = vldbEntry.volumeId[1];
else
volp->roID = 0;
if (vldbEntry.flags & VLF_BACKEXISTS)
volp->bkID = vldbEntry.volumeId[2];
else
volp->bkID = 0;
volp->bkID = 0;
lock_ReleaseWrite(&cm_volumeLock);
for(i=0; i<vldbEntry.nServers; i++) {
for(i=0; i<vldbEntry.nServers; i++) {
/* create a server entry */
tflags = vldbEntry.serverFlags[i];
if (tflags & VLSF_DONTUSE) continue;
@ -107,44 +107,44 @@ long cm_UpdateVolume(struct cm_cell *cellp, cm_user_t *userp, cm_req_t *reqp,
tsockAddr.sin_addr.s_addr = tempAddr;
tsp = cm_FindServer(&tsockAddr, CM_SERVER_FILE);
if (!tsp)
tsp = cm_NewServer(&tsockAddr, CM_SERVER_FILE,
cellp);
tsp = cm_NewServer(&tsockAddr, CM_SERVER_FILE,
cellp);
/* if this server was created by fs setserverprefs */
if ( !tsp->cellp )
tsp->cellp = cellp;
osi_assert(tsp != NULL);
osi_assert(tsp != NULL);
/* and add it to the list(s). */
/* and add it to the list(s). */
/*
* Each call to cm_NewServerRef() increments the
* ref count of tsp. These reference will be dropped,
* Each call to cm_NewServerRef() increments the
* ref count of tsp. These reference will be dropped,
* if and when the volume is reset; see reset code
* earlier in this function.
*/
if ((tflags & VLSF_RWVOL)
&& (vldbEntry.flags & VLF_RWEXISTS)) {
&& (vldbEntry.flags & VLF_RWEXISTS)) {
tsrp = cm_NewServerRef(tsp);
tsrp->next = volp->rwServersp;
volp->rwServersp = tsrp;
volp->rwServersp = tsrp;
}
if ((tflags & VLSF_ROVOL)
&& (vldbEntry.flags & VLF_ROEXISTS)) {
if ((tflags & VLSF_ROVOL)
&& (vldbEntry.flags & VLF_ROEXISTS)) {
tsrp = cm_NewServerRef(tsp);
cm_InsertServerList(&volp->roServersp, tsrp);
ROcount++;
}
}
/* We don't use VLSF_BACKVOL !?! */
if ((tflags & VLSF_RWVOL)
&& (vldbEntry.flags & VLF_BACKEXISTS)) {
if ((tflags & VLSF_RWVOL)
&& (vldbEntry.flags & VLF_BACKEXISTS)) {
tsrp = cm_NewServerRef(tsp);
tsrp->next = volp->bkServersp;
volp->bkServersp = tsrp;
tsrp->next = volp->bkServersp;
volp->bkServersp = tsrp;
}
/* Drop the reference obtained by cm_FindServer() */
cm_PutServer(tsp);
}
}
/*
* Randomize RO list