[OpenAFS-devel] Proposal for capabilities support in Unix client 1.4.x

Felix Frank Felix.Frank@Desy.de
Wed, 17 Jun 2009 14:47:45 +0200


This is a multi-part message in MIME format.
--------------020509070307010601030309
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hi,

the attached patch against 1.4.10 is basically a back-port of 
separate-capabilities-by-source-20051217 to the 1.4.x. Unix client, plus 
some more state for keeping things coherent/up-to-date, minus move and 
rename of constants to fsint/common.xg etc. (I wanted to keep the change 
functional yet minimal). Thanks go to Simon Wilkinson and Jeffrey Altman 
for pointers and comments.

The idea in CheckServers() is that
  - if setTime is set, both GetCapabilities and GetTime are called on 
all servers
  - if setTime is not set, GetTime is only called for servers that do 
not implement GetCapabilities

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.

Any criticism is appreciated.

Cheers
  - Felix

--------------020509070307010601030309
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..0dc070e 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,63 @@ 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->flags |= SCAPS_KNOWN;
+	if ( results[i] == RXGEN_OPCODE )
+	    continue;
+	if ( results[i] >= 0 ) {
+	    printf("Seen server with caps 0x%x\n", caps[i].Capabilities_val[0]);
+	    /* we currently handle 32-bits of capabilities */
+	    if (caps[i].Capabilities_len > 0) {
+		XDR xdr;
+		ts->capabilities = caps[i].Capabilities_val[0];
+		xdrrx_create(&xdr, NULL, XDR_FREE);
+		if ( !xdr_Capabilities(&xdr, &caps[i]) )
+		    afs_warn("afs_GetServer: failed to free Capabilities\n");
+		caps[i].Capabilities_len = 0;
+		caps[i].Capabilities_val = NULL;
+	    }
+	    else
+		ts->capabilities = 0;
+	}
+    }
+
+    safe = nconns;
+
+    if ( afs_setTime == 0 ) {	/* prune the servers that sent caps */
+	for ( i=0, k=0; i<nconns; i++) {
+            if (results[i] == RXGEN_OPCODE) {
+                if (i != k) { 	/*swap incapable servers to beginning of list*/
+		    void *bucket;
+		    bucket = (void*)conns[k];
+                    conns[k] = conns[i];
+		    conns[i] = bucket;
+
+		    bucket = (void*)rxconns[k];
+                    rxconns[k] = rxconns[i];
+		    rxconns[i] = bucket;
+                }
+                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 +709,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 +730,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 +815,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 +1682,51 @@ 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 ) {
+	XDR xdr;
+	printf("Seen server with caps 0x%x\n", caps.Capabilities_val[0]);
+	ts->capabilities = caps.Capabilities_val[0];
+	xdrrx_create(&xdr, NULL, XDR_FREE);
+	if ( !xdr_Capabilities(&xdr, &caps) )
+	    afs_warn("afs_GetServer: failed to free Capabilities\n");
+	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 +1736,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 +1931,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);

--------------020509070307010601030309--