Two followups.

First, though not the problem I'm seeing, as I understand
it, mkstemp isn't guaranteed to work on NFS v2 mounted
drives.  Thus I suggest including the hostname; there's
already tmp_string() which is perfect.  Patch below.

Second, I'm still not sure of the cause, but if I count
bytes copied, and fail if zero bytes are moved this works
around the issue.  Patch below, but this not be the right
solution.  I thought of calling stat() after the copy, and
fail if the size doesn't match what stat says, alas
compression makes that difficult.

One thing I wonder is the unlink() before the rename.  I
presume that is needed to prevent permission issues, but I
wonder if it contributes to the race involving
host1:unlink(), host1:rename(), host2:unlink(),
host2:rename(), host3:open().

-Wilson


diff --git a/util.c b/util.c
index ae48cf0..df522f1 100644
--- a/util.c
+++ b/util.c
@@ -128,7 +128,7 @@ int copy_file(const char *src, const char *dest, int 
compress_dest)
   mode_t mask;
   struct stat st;
 
-       x_asprintf(&tmp_name, "%s.XXXXXX", dest);
+       x_asprintf(&tmp_name, "%s.%s.XXXXXX", dest, tmp_string());
 
        /* open source file */
        fd_in = open(src, O_RDONLY);


diff --git a/util.c b/util.c
index ae48cf0..df522f1 100644
--- a/util.c
+++ b/util.c
@@ -177,7 +177,9 @@ int copy_file(const char *src, const char *dest, int 
compress_dest)
          }
          }
 
+       int bytecnt = 0;
        while ((n = gzread(gz_in, buf, sizeof(buf))) > 0) {
+             bytecnt += n;
                      if (compress_dest) {
                                         ret = gzwrite(gz_out, buf, n);
                                             } else {
@@ -212,14 +214,25 @@ int copy_file(const char *src, const char *dest, int 
compress_dest)
           return -1;
           }
 
+       /* Empty objects are bad - created by race in NFS between 2 writers and 
1+ readers */
+       if (bytecnt == 0) {
+           unlink(tmp_name);
+               free(tmp_name);
+                   cc_log("Refusing to copy zero sized file %s", src);
+                       return -1;
+                       }
+
        unlink(dest);
 
        if (rename(tmp_name, dest) == -1) {
           unlink(tmp_name);
                free(tmp_name);
+                       cc_log("Rename failed");
                                       return -1;
                                       }
 
+       cc_log("Copied %d bytes from %s via %s to %s", bytecnt, src, tmp_name, 
dest);
+
        free(tmp_name);
 
        return 0;
_______________________________________________
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache

Reply via email to