[OpenAFS-devel] CopyOnWrite optimisation misses second call for the "tail"

[Rainer Toebbicke]

--------------080404040100040602080402
Content-Type: text/plain; charset="ISO-8859-15"; format=flowed
Content-Transfer-Encoding: 7bit

Many thanks to Hartmut Reuter for spotting this: my patch optimizing the 
CopyOnWrite in the file server only copied the "tail" when an error occurred, 
I had forgotten the "normal" case. The consequence would be a corrupted file 
in case of a partial file update.

Here's the fix, to be added to the previous patch. 1.4.9 should rather not go 
out without this!

Embarrassed... Rainer (sigh)

Bcc'ed to openafs-bugs

-- 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
Rainer Toebbicke
European Laboratory for Particle Physics(CERN) - Geneva, Switzerland
Phone: +41 22 767 8985       Fax: +41 22 767 7155

--------------080404040100040602080402
Content-Type: text/plain; name="p_copyOnWrite2"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="p_copyOnWrite2"

--- openafs/src/viced/afsfileprocs.c.~	2009-03-24 10:00:45.000000000 +0100
+++ openafs/src/viced/afsfileprocs.c	2009-03-24 10:10:16.000000000 +0100
@@ -1483,6 +1483,7 @@
     FDH_SEEK(targFdP, off, SEEK_SET);
     FDH_SEEK(newFdP, off, SEEK_SET);
 
+    if (size > FDH_SIZE(targFdP) - off) size = FDH_SIZE(targFdP) - off;
     while (size > 0) {
 	if (size > COPYBUFFSIZE) {	/* more than a buffer */
 	    length = COPYBUFFSIZE;
@@ -7567,7 +7568,7 @@
     afs_fsize_t NewLength;	/* size after this store completes */
     afs_sfsize_t adjustSize;	/* bytes to call VAdjust... with */
     int linkCount;		/* link count on inode */
-    afs_fsize_t CoW_off = 0, CoW_len = 0;
+    afs_fsize_t CoW_off, CoW_len;
     FdHandle_t *fdP, *origfdP = NULL;
     struct in_addr logHostAddr;	/* host ip holder for inet_ntoa */
 
@@ -7638,18 +7639,15 @@
 		return (errorCode);
 	    }
 
-	    if (Pos == 0) 
-		CoW_off = Length;	/* only copy remaining parts of file */
-	    if (Length <= FileLength)
-		CoW_len = FileLength - Length;
+	    CoW_len = (FileLength >= (Length + Pos)) ? FileLength - Length : Pos;
 	    CopyOnWrite_calls++;
 	    if (CoW_len == 0) CopyOnWrite_size0++;
 	    if (CoW_off == 0) CopyOnWrite_off0++;
 	    if (CoW_len > CopyOnWrite_maxsize) CopyOnWrite_maxsize = CoW_len;
 
 	    ViceLog(1, ("StoreData : calling CopyOnWrite on vnode %lu.%lu (%s) off 0x%llx size 0x%llx\n",
-			V_id(volptr), targetptr->vnodeNumber, V_name(volptr), CoW_off, CoW_len));
-	    if ((errorCode = CopyOnWrite(targetptr, volptr, CoW_off, CoW_len))) {
+			V_id(volptr), targetptr->vnodeNumber, V_name(volptr), 0, Pos));
+	    if ((errorCode = CopyOnWrite(targetptr, volptr, 0, Pos))) {
 		ViceLog(25, ("StoreData : CopyOnWrite failed\n"));
 		volptr->partition->flags &= ~PART_DONTUPDATE;
 		FDH_CLOSE(origfdP);
@@ -7790,7 +7788,7 @@
 	 */
 	targetptr->changed_newTime = 1;
 	if (origfdP && (bytesTransfered < Length))	/* Need to "finish" CopyOnWrite still */
-	    CopyOnWrite2(origfdP, fdP, bytesTransfered, Length - bytesTransfered);
+	    CopyOnWrite2(origfdP, fdP, Pos + bytesTransfered, NewLength - Pos - bytesTransfered);
 	if (origfdP) FDH_CLOSE(origfdP);
 	FDH_CLOSE(fdP);
 	/* set disk usage to be correct */
@@ -7799,7 +7797,14 @@
 					 nBlocks(NewLength)), 0);
 	return errorCode;
     }
-    if (origfdP) FDH_CLOSE(origfdP);
+    if (origfdP) {					/* finish CopyOnWrite */
+	if ( (CoW_off = Pos + Length) < NewLength) {
+	    errorCode = CopyOnWrite2(origfdP, fdP, CoW_off, CoW_len = NewLength - CoW_off);
+	    ViceLog(1, ("StoreData : CopyOnWrite2 on vnode %lu.%lu (%s) off 0x%llx size 0x%llx returns %d\n",
+                        V_id(volptr), targetptr->vnodeNumber, V_name(volptr), CoW_off, CoW_len, errorCode));
+	}
+	FDH_CLOSE(origfdP);
+    }
     FDH_CLOSE(fdP);
 
     TM_GetTimeOfDay(&StopTime, 0);

--------------080404040100040602080402--