[OpenAFS-devel] Proposal for capabilities support in Unix client
1.4.x
Felix Frank
Felix.Frank@Desy.de
Wed, 17 Jun 2009 18:07:43 +0200
This is a multi-part message in MIME format.
--------------070309080608050308010708
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Jeffrey Hutzelman wrote (Wed Jun 17 2009 17:25:00 GMT+0200 (CEST))
> --On Wednesday, June 17, 2009 09:29:02 AM -0400 Jeffrey Altman
> <jaltman@secure-endpoints.com> wrote:
>
>> Felix Frank wrote:
>>
>>> The way I'm currently freeing Capabilities as allocated by the client
>>> stub strikes me as (while conceptionally sound) spectacularly wrong -
>>> how is this supposed to be done?
>>> For a simple case as this (one lone pointer that needs freeing),
>>> osi_free() appears to be the most sensible thing to use after all, as
>>> that is what xdr ultimately does.
>>
>> Frank:
>>
>> Call xdr_free() to deallocate memory allocated by the XDR package.
>
> Yes, or better, use XDR in "free" mode, which saves you from having to
> know what is supposed to be freed and what isn't. But yeah, just
> calling xdr_free() on the ->val thing here should be fine.
I believe that is what the original patch proposal did. However
a) I found no precedent for such behaviour anywhere in any existing code
b) it was real ugly ;/
>> xdr_free(caps, j * sizeof(Capabilities));
>
> This is a bad example; in the patch in question, caps is the array of
> Capabilities structures allocated for rx_multi. It was allocated using
> afs_osi_Alloc(), not by XDR, and so should be freed using
> afs_osi_Free(), as Frank's patch does.
As announced, it now uses xdr_free for individual Capability_vals and
afs_osi_Free() for the list (the latter being necessary only in
CheckServers(), not afs_GetCapabilities()).
>> As for the afs_setTime piece. As you and Simon discussed, there is only
>> one host 'afs_setTimeHost' whose time is used to set the local time.
>> I believe it is a waste of effort to call GetTime on every host. You
>> just need to make sure that GetTime is called on 'afs_setTimeHost' if
>> 'afs_setTimeHost' is non-NULL or at least one of the servers such that
>> afs_IsPrimaryCell(sa->server->cell) is TRUE.
>
> Agree. GetTime should be called only against the set-time host or if
> GetCapabilities is unsupported (and really, you can punt the latter case
> -- getting RXGEN_OPCODE from GetCapabilities is enough to tell you the
> server is up, which is all you really care about).
The idea was to introduce minimum behavior change for the time being,
but you're right of course - if we forego calling GetTime to some
servers, why not go all the way?
Note that all servers are GetTime'd now in the case of a NULL
setTimeHost. Also, server pruning should be correct now (but if GetTime
ends up being completely replaced with GetCapabilities in the setTime ==
0 case, the pruning won't be necessary anyway - only three cases will
remain:
- setTime == 0 -> no GetTime whatsoever
- setTime == 1 && setTimeHost == NULL -> GetTime from all
- else GetTime for setTimeHost only)
Cheers
- Felix
--------------070309080608050308010708
Content-Type: text/x-patch;
name="unix-cm-fs-caps.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="unix-cm-fs-caps.patch"
diff --git a/src/afs/afs.h b/src/afs/afs.h
index 4cefbc4..4e21df7 100644
--- a/src/afs/afs.h
+++ b/src/afs/afs.h
@@ -376,6 +376,10 @@ struct srvAddr {
#define SRVR_ISGONE 0x80
#define SNO_INLINEBULK 0x100
#define SNO_64BIT 0x200
+#define SCAPS_KNOWN 0x400
+
+#define SRV_CAPABILITIES(ts) \
+{ if ( ! ts->flags & SCAPS_KNOWN ) afs_GetCapabilities(ts); ts->capabilities; }
#define afs_serverSetNo64Bit(s) ((s)->srvr->server->flags |= SNO_64BIT)
#define afs_serverHasNo64Bit(s) ((s)->srvr->server->flags & SNO_64BIT)
@@ -407,6 +411,7 @@ struct server {
afs_int32 sumOfDowntimes; /* Total downtime experienced, in seconds */
struct srvAddr *addr;
afs_uint32 flags; /* Misc flags */
+ afs_int32 capabilities;
};
#define afs_PutServer(servp, locktype)
diff --git a/src/afs/afs_callback.c b/src/afs/afs_callback.c
index dd003f8..40c2d32 100644
--- a/src/afs/afs_callback.c
+++ b/src/afs/afs_callback.c
@@ -768,9 +768,10 @@ SRXAFSCB_InitCallBackState(struct rx_call *a_call)
ReleaseWriteLock(&afs_xcbhash);
}
}
- }
-
+ /* capabilities need be requested again */
+ ts->flags &= ~SCAPS_KNOWN;
+ }
/* find any volumes residing on this server and flush their state */
{
diff --git a/src/afs/afs_prototypes.h b/src/afs/afs_prototypes.h
index 0f8ceff..f6c17a1 100644
--- a/src/afs/afs_prototypes.h
+++ b/src/afs/afs_prototypes.h
@@ -698,6 +698,7 @@ extern struct server *afs_GetServer(afs_uint32 * aserver, afs_int32 nservers,
afs_int32 acell, u_short aport,
afs_int32 locktype, afsUUID * uuidp,
afs_int32 addr_uniquifier);
+extern void afs_GetCapabilities(struct server *ts);
extern void ForceAllNewConnections(void);
extern void afs_MarkServerUpOrDown(struct srvAddr *sa, int a_isDown);
extern afs_int32 afs_ServerDown(struct srvAddr *sa);
diff --git a/src/afs/afs_server.c b/src/afs/afs_server.c
index 3e683c3..8097038 100644
--- a/src/afs/afs_server.c
+++ b/src/afs/afs_server.c
@@ -532,7 +532,7 @@ afs_CheckServers(int adown, struct cell *acellp)
struct server *ts;
struct srvAddr *sa;
struct conn *tc;
- afs_int32 i, j;
+ afs_int32 i, j, k;
afs_int32 code;
afs_int32 start, end = 0, delta;
osi_timeval_t tv;
@@ -541,9 +541,10 @@ afs_CheckServers(int adown, struct cell *acellp)
int srvAddrCount;
struct srvAddr **addrs;
struct conn **conns;
- int nconns;
+ int nconns, safe;
struct rx_connection **rxconns;
afs_int32 *conntimer, *deltas, *results;
+ Capabilities *caps = NULL;
AFS_STATCNT(afs_CheckServers);
@@ -583,6 +584,9 @@ afs_CheckServers(int adown, struct cell *acellp)
deltas = (afs_int32 *)afs_osi_Alloc(j * sizeof (afs_int32));
results = (afs_int32 *)afs_osi_Alloc(j * sizeof (afs_int32));
+ caps = (Capabilities *)afs_osi_Alloc(j * sizeof (Capabilities));
+ memset(caps, 0, j * sizeof(Capabilities));
+
for (i = 0; i < j; i++) {
deltas[i] = 0;
sa = addrs[i];
@@ -631,6 +635,60 @@ afs_CheckServers(int adown, struct cell *acellp)
}
} /* Outer loop over addrs */
+ AFS_GUNLOCK();
+ multi_Rx(rxconns,nconns)
+ {
+ multi_RXAFS_GetCapabilities(&caps[multi_i]);
+ results[multi_i] = multi_error;
+ } multi_End;
+ AFS_GLOCK();
+
+ for ( i = 0 ; i < nconns ; i++ ) {
+ ts = addrs[i]->server;
+ if ( !ts )
+ continue;
+ ts->capabilities = 0;
+ ts->flags |= SCAPS_KNOWN;
+ if ( results[i] == RXGEN_OPCODE )
+ continue;
+ if ( results[i] >= 0 )
+ /* we currently handle 32-bits of capabilities */
+ if (caps[i].Capabilities_len > 0) {
+ ts->capabilities = caps[i].Capabilities_val[0];
+ xdr_free(caps[i].Capabilities_val,
+ caps[i].Capabilities_len * sizeof(afs_int32));
+ caps[i].Capabilities_val = NULL;
+ caps[i].Capabilities_len = 0;
+ }
+ }
+
+ safe = nconns;
+
+ /* prune the servers that sent capabilities */
+ for ( i=0, k=0; i<nconns; i++) {
+ /* be sure to include the setTimeHost if necessary */
+ if (results[i] == RXGEN_OPCODE ||
+ (afs_setTime && afs_setTimeHost == NULL) ||
+ (afs_setTime && conns[i]->srvr->server == afs_setTimeHost)) {
+ if (i != k) { /*swap incapable servers to beginning of list*/
+#define SWAP(array,tmp) tmp=array[k]; array[k]=array[i]; array[i]=tmp;
+ void *pointer_bucket;
+ afs_int32 int_bucket;
+ SWAP(conns, pointer_bucket);
+ SWAP(rxconns, pointer_bucket);
+ SWAP(conntimer, int_bucket);
+ SWAP(deltas, int_bucket);
+#undef SWAP
+ }
+ k++;
+ }
+ else if ( !results[i] ) {
+ afs_PutConn(conns[i], SHARED_LOCK);
+ }
+ }
+ nconns = k; /* next, only use servers that couldn't GetCapabilities */
+
+ if ( nconns ) {
start = osi_Time(); /* time the gettimeofday call */
AFS_GUNLOCK();
multi_Rx(rxconns,nconns)
@@ -648,6 +706,9 @@ afs_CheckServers(int adown, struct cell *acellp)
} multi_End;
AFS_GLOCK();
+ }
+
+ nconns = safe; /* restore in case of GetTime limiting */
for(i=0;i<nconns;i++){
tc = conns[i];
@@ -666,7 +727,8 @@ afs_CheckServers(int adown, struct cell *acellp)
if (afs_waitForeverCount) {
afs_osi_Wakeup(&afs_waitForever);
}
- } else {
+ }
+ else {
if (results[i] < 0) {
/* server crashed */
afs_ServerDown(sa);
@@ -750,6 +812,7 @@ afs_CheckServers(int adown, struct cell *acellp)
afs_osi_Free(conntimer, j * sizeof(afs_int32));
afs_osi_Free(deltas, j * sizeof(afs_int32));
afs_osi_Free(results, j * sizeof(afs_int32));
+ afs_osi_Free(caps, j * sizeof(Capabilities));
} /*afs_CheckServers*/
@@ -1616,6 +1679,48 @@ void afs_RemoveSrvAddr(struct srvAddr *sap) {
}
}
+/* afs_GetCapabilities
+ * Try and retrieve capabilities of a given file server. Carps on actual
+ * failure. Servers are not expected to support this RPC. */
+void afs_GetCapabilities(struct server *ts)
+{
+ Capabilities caps = {0, NULL};
+ struct vrequest treq;
+ struct conn *tc;
+ struct unixuser *tu;
+ afs_int32 code;
+
+ if ( !ts )
+ return;
+ if ( !afs_osi_credp )
+ return;
+
+ if ((code = afs_InitReq(&treq, afs_osi_credp)))
+ return;
+ tu = afs_GetUser(treq.uid, ts->cell->cellNum, SHARED_LOCK);
+ if ( !tu )
+ return;
+ tc = afs_ConnBySA(ts->addr, ts->cell->fsport, ts->cell->cellNum, tu, 0, 1,
+ SHARED_LOCK);
+ if ( !tc )
+ return;
+ code = RXAFS_GetCapabilities(tc->id, &caps);
+ if ( code && code != RXGEN_OPCODE )
+ afs_warn("RXAFS_GetCapabilities failed with code %d\n", code);
+ else
+ ts->flags |= SCAPS_KNOWN;
+ afs_PutConn(tc, SHARED_LOCK);
+
+ if ( caps.Capabilities_len > 0 ) {
+ ts->capabilities = caps.Capabilities_val[0];
+ xdr_free(caps.Capabilities_val,
+ caps.Capabilities_len * sizeof(afs_int32));
+ caps.Capabilities_len = 0;
+ caps.Capabilities_val = NULL;
+ }
+
+}
+
/* afs_GetServer()
* Return an updated and properly initialized server structure
* corresponding to the server ID, cell, and port specified.
@@ -1625,7 +1730,8 @@ void afs_RemoveSrvAddr(struct srvAddr *sap) {
struct server *afs_GetServer(afs_uint32 * aserverp, afs_int32 nservers,
afs_int32 acell, u_short aport,
afs_int32 locktype, afsUUID * uuidp,
- afs_int32 addr_uniquifier) {
+ afs_int32 addr_uniquifier)
+{
struct server *oldts = 0, *ts, *newts, *orphts = 0;
struct srvAddr *oldsa, *newsa, *nextsa, *orphsa;
u_short fsport;
@@ -1819,6 +1925,9 @@ struct server *afs_GetServer(afs_uint32 * aserverp, afs_int32 nservers,
afs_stats_cmperf.srvRecordsHWM = afs_stats_cmperf.srvRecords;
}
+ if ( aport == AFS_FSPORT && !(newts->flags & SCAPS_KNOWN))
+ afs_GetCapabilities(newts);
+
ReleaseWriteLock(&afs_xsrvAddr);
ReleaseWriteLock(&afs_xserver);
return (newts);
diff --git a/src/libafsrpc/afsrpc.def b/src/libafsrpc/afsrpc.def
index cd75567..3c0f173 100644
--- a/src/libafsrpc/afsrpc.def
+++ b/src/libafsrpc/afsrpc.def
@@ -217,5 +217,6 @@ EXPORTS
rxkad_stats_key @222 DATA
rx_InitHost @224
rx_NewServiceHost @225
+ xdr_free @252
diff --git a/src/rx/xdr.c b/src/rx/xdr.c
index 1a01bab..3de1467 100644
--- a/src/rx/xdr.c
+++ b/src/rx/xdr.c
@@ -583,4 +583,10 @@ xdr_wrapstring(register XDR * xdrs, char **cpp)
return (FALSE);
}
#endif
+
+void
+xdr_free(void *x, afs_int32 size)
+{
+ osi_free(x, size);
+}
#endif /* NeXT */
diff --git a/src/rx/xdr_prototypes.h b/src/rx/xdr_prototypes.h
index 5c1898e..e36ddd4 100644
--- a/src/rx/xdr_prototypes.h
+++ b/src/rx/xdr_prototypes.h
@@ -58,6 +58,7 @@ extern bool_t xdr_union(register XDR * xdrs, enum_t * dscmp, caddr_t unp,
struct xdr_discrim *choices, xdrproc_t dfault);
extern bool_t xdr_string(register XDR * xdrs, char **cpp, u_int maxsize);
extern bool_t xdr_wrapstring(register XDR * xdrs, char **cpp);
+extern void xdr_free(void *x, afs_int32 size);
/* xdr_float.c */
--------------070309080608050308010708--