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
--- 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);

Reply via email to