[Bug-tar] [PATCH] tar: Ignore certain file status changes on create

[Robert Morell]

Currently when reading a file tar will first stat the file, read all of
its data, and then stat the file again.  It compares the ctime (last
file status update) between the two stat invocations, and if they don't
match it fails with "file changed as we read it".

Unfortunately, some systems' file caching behavior interacts poorly with
this.  In particular, the Linux kernel may allocate filesystem blocks
asynchronously to file writes, including allocating the blocks after the
process doing the write has terminated.  This changes st_blocks and thus
triggers a ctime update, causing tar to fail.

Since tar really doesn't care about filesystem blocks (since it has no
way of restoring them when untarring anyway), this updates the "file
changed" logic to explicitly check the file status fields that tar
actually cares about, and ignore the problematic ones that it doesn't.
---

This fixes problems I was having with intermittent tar failures.  I also ran
this through "make check" and verified no failures, and did some directed
testing of my own with the --remove-files option and directories.

For what it's worth, I first tried bringing this up with the kernel, but they
don't think it's a bug (and POSIX seems to agree with them).  For more
information, see this thread:
https://lkml.org/lkml/2011/5/11/225

I also considered zeroing out parts of struct stat that aren't relevant and
using memcmp() instead, but that won't work in general because of padding in
the struct that may be uninitialized.

 src/create.c |   27 ++++++++++++++++++++-------
 1 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/create.c b/src/create.c
index 43b5a4c..a43a5fc 100644
--- a/src/create.c
+++ b/src/create.c
@@ -1613,7 +1613,6 @@ dump_file0 (struct tar_stat_info *st, char const *name, 
char const *p)
   union block *header;
   char type;
   off_t original_size;
-  struct timespec original_ctime;
   off_t block_ordinal = -1;
   int fd = 0;
   bool is_dir;
@@ -1659,7 +1658,7 @@ dump_file0 (struct tar_stat_info *st, char const *name, 
char const *p)
   st->archive_file_size = original_size = st->stat.st_size;
   st->atime = get_stat_atime (&st->stat);
   st->mtime = get_stat_mtime (&st->stat);
-  st->ctime = original_ctime = get_stat_ctime (&st->stat);
+  st->ctime = get_stat_ctime (&st->stat);
 
 #ifdef S_ISHIDDEN
   if (S_ISHIDDEN (st->stat.st_mode))
@@ -1785,11 +1784,25 @@ dump_file0 (struct tar_stat_info *st, char const *name, 
char const *p)
 
       if (ok)
        {
-         if ((timespec_cmp (get_stat_ctime (&final_stat), original_ctime) != 0
-              /* Original ctime will change if the file is a directory and
-                 --remove-files is given */
-              && !(remove_files_option && is_dir))
-             || original_size < final_stat.st_size)
+         /* st->stat's size forced to 0 for directories, match that */
+         if (is_dir) final_stat.st_size = 0;
+
+         /* Note: the following fields are intentionally ignored, since they
+            aren't archived.  st_blocks and st_ctime in particular may change
+            asynchronously on some systems.
+            st_blksize
+            st_blocks
+            st_atime
+            st_ctime  */
+         if (st->stat.st_dev      != final_stat.st_dev
+             || st->stat.st_ino   != final_stat.st_ino
+             || st->stat.st_mode  != final_stat.st_mode
+             || st->stat.st_nlink != final_stat.st_nlink
+             || st->stat.st_uid   != final_stat.st_uid
+             || st->stat.st_gid   != final_stat.st_gid
+             || st->stat.st_rdev  != final_stat.st_rdev
+             || st->stat.st_size  != final_stat.st_size
+             || st->stat.st_mtime != final_stat.st_mtime)
            {
              WARNOPT (WARN_FILE_CHANGED,
                       (0, 0, _("%s: file changed as we read it"),
-- 
1.7.3.4