Issue #69: Copy-up fix

src/cow.c

@@ -122,6 +122,46 @@ int path_create_cutlast(const char *path, int nbranch_ro, int nbranch_rw) {
 	RETURN(ret);
 }
 
+/**
+ * copy file in locked state (to avoid competing processes
+ * stepping on each other).
+ */
+static int copy_file_locked(struct cow *cow, const char *path,
+	int branch_rw) {
+	DBG("%s %s %s %d\n", cow->from_path, cow->to_path,
+		path, branch_rw);
+
+	/* lock copy-up metadata */
+	int lockfd = lock_file_copyup(path, branch_rw);
+	if (lockfd < 0) RETURN(1);
+
+	int res = 0;
+
+	struct stat stbuf;
+	if (lstat(cow->to_path, &stbuf) == 0)  {
+		/* after obtaining the metadata lock, if the file
+		 * already exists, it means another process already
+		 * copied the file. so we do nothing.
+		 */
+		DBG("File %s already copied up. %s, %d\n", cow->to_path,
+			path, branch_rw);
+		goto out;
+	}
+
+	if (errno != ENOENT) {
+		USYSLOG(LOG_ERR, "stat(%s) failed. %s\n",
+			cow->to_path, strerror(errno));
+		res = 1;
+		goto out;
+	}
+
+	res = copy_file(cow);
+out:
+	/* unlock copy-up metadata */
+	unlock_file_copyup(path, branch_rw, lockfd);
+	RETURN(res);
+}
+
 /**
  * initiate the cow-copy action
  */
@@ -177,7 +217,7 @@ int cow_cp(const char *path, int branch_ro, int branch_rw, bool copy_dir) {
 			USYSLOG(LOG_WARNING, "COW of sockets not supported: %s\n", cow.from_path);
 			RETURN(1);
 		default:
-			res = copy_file(&cow);
+			res = copy_file_locked(&cow, path, branch_rw);
 	}
 
 	RETURN(res);

src/findbranch.c

@@ -59,6 +59,16 @@ static int find_branch(const char *path, searchflag_t flag) {
 
 	int i = 0;
 	for (i = 0; i < uopt.nbranches; i++) {
+
+		if (ongoing_copyup(path, i)) {
+			/* copy-up is in progress. this indicates that
+			 * the write() operation which triggered copy-up
+			 * is not complete yet. in such a case
+			 * search next branch.
+			 */
+			continue;
+		}
+
 		char p[PATHLEN_MAX];
 		if (BUILD_PATH(p, uopt.branches[i].path, path)) {
 			errno = ENAMETOOLONG;

src/general.c

@@ -22,6 +22,7 @@
 #include <pwd.h>
 #include <grp.h>
 #include <pthread.h>
+#include <sys/file.h>
 
 #include "unionfs.h"
 #include "opts.h"
@@ -213,3 +214,91 @@ int set_owner(const char *path) {
 	}
 	RETURN(0);
 }
+
+static int build_copyup_path(const char *path, int branch,
+	bool do_create_path, char *path_out) {
+
+	char mp[PATHLEN_MAX];
+	if (BUILD_PATH(mp, METADIR, path))  RETURN(-1);
+
+	if (do_create_path) {
+		path_create_cutlast(mp, branch, branch);
+	}
+
+	if (BUILD_PATH(path_out, uopt.branches[branch].path, mp)) RETURN(-1);
+	strcat(path_out, COPYUPTAG);
+
+	RETURN(0);
+}
+
+/**
+ * Creates copy-up meta file to indicate ongoing copy-up. It also locks the
+ * metafile. Lock is meant for blocking competing processes trying to copy-up.
+ * @return locked file descriptor.
+ */
+int lock_file_copyup(const char *path, int branch_rw) {
+	DBG("%s\n", path);
+
+	char p[PATHLEN_MAX];
+	if (build_copyup_path(path, branch_rw, true, p) != 0) RETURN(-1);
+
+	int lockfd = open(p, O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR);
+	if (lockfd  == -1) {
+		USYSLOG(LOG_ERR, "open(%s) failed. %s\n",
+			p, strerror(errno));
+		RETURN(-1);
+	}
+
+	if (flock(lockfd, LOCK_EX) != 0) {
+		USYSLOG(LOG_ERR, "flock(%d) failed to lock. %s\n",
+			lockfd, strerror(errno));
+		close(lockfd);
+		RETURN(-1);
+	}
+
+	RETURN(lockfd);
+}
+
+/**
+ * Removes copy-up meta file to indicate copy-up is complete.
+ * Also unlocks the copy-up meta file to unlock other competing
+ * processes.
+ */
+int unlock_file_copyup(const char *path, int branch_rw, int lockfd) {
+	DBG("%s\n", path);
+
+	char p[PATHLEN_MAX];
+	if (build_copyup_path(path, branch_rw, false, p) != 0) RETURN(-1);
+
+	/* we do not check the return status of unlink here, because
+	 * only one competing process succeeds and rest of the
+	 * processes fail.
+	 */
+	unlink(p);
+
+	if (flock(lockfd, LOCK_UN) != 0) {
+		USYSLOG(LOG_ERR, "flock(%d) failed to unlock. %s\n",
+			lockfd, strerror(errno));
+		RETURN(-1);
+	}
+
+	int res = close(lockfd);
+
+	RETURN(res);
+}
+
+/**
+ * Returns true when a copy-up is in progerss.
+ * Otherwise false.
+ */
+bool ongoing_copyup(const char *path, int branch_rw) {
+
+	char p[PATHLEN_MAX];
+	if (build_copyup_path(path, branch_rw, false, p) != 0) RETURN(false);
+
+	struct stat stbuf;
+	if (lstat(p, &stbuf) == 0) RETURN(true);
+	if (errno != ENOENT) RETURN(true);
+
+	RETURN(false);
+}

src/general.h

@@ -27,6 +27,9 @@ int hide_dir(const char *path, int branch_rw);
 filetype_t path_is_dir (const char *path);
 int maybe_whiteout(const char *path, int branch_rw, enum whiteout mode);
 int set_owner(const char *path);
+int lock_file_copyup(const char *path, int branch_rw);
+int unlock_file_copyup(const char *path, int branch_rw, int lockfd);
+bool ongoing_copyup(const char *path, int branch_rw);
 
 
 #endif

src/unionfs.h

@@ -9,6 +9,7 @@
 
 #define PATHLEN_MAX 1024
 #define HIDETAG "_HIDDEN~"
+#define COPYUPTAG "_COPYUP~"
 
 #define METANAME ".unionfs"
 #define METADIR (METANAME  "/") // string concetanation!

test_copyup.py

@@ -0,0 +1,122 @@
+#!/usr/bin/python
+
+# This script forks three parallel processes - two for writing (to trigger
+# copy-up) and another for checking file size.
+# File size checking process sleeps 5 secs to give writer process
+# sufficient time to trigger copy-up.
+# But this also means, copy-up needs to slowed down artificially to
+# reproduce this problem reliably. And this can be done by adding
+# some sleep in cow_utils.c:copy_file() function.
+
+import os
+import signal
+import hashlib
+import subprocess
+import inspect
+import multiprocessing
+import time
+
+# global stuffs
+TestDir = "testdir"
+LoDir = TestDir + "/lower"
+UpDir = TestDir + "/upper"
+MntDir = TestDir + "/mnt"
+LoFile = LoDir + "/file1"
+UpFile = UpDir + "/file1"
+MntFile = MntDir + "/file1"
+OrigSize = 1048576
+DbgFile = TestDir + "/log"
+
+# path to unionfs executable
+UfsBin = "./src/unionfs"
+
+def create_file(fname, size):
+	fh = open(fname, 'w+')
+	fh.truncate(size)
+	fh.close()
+
+def write_file(fname, data, offset = 0):
+	fh = open(fname, 'r+')
+	fh.seek(offset)
+	fh.write(data)
+	fh.close()
+
+def create_testbed():
+	os.system("rm -rf " + TestDir)
+	os.mkdir(TestDir)
+	os.mkdir(LoDir)
+	os.mkdir(UpDir)
+	os.mkdir(MntDir)
+	create_file(LoFile, OrigSize)
+
+def do_setup():
+	print "Creating test setup..."
+	create_testbed()
+	os.system(UfsBin
+		+ " -o cow -o auto_unmount,debug_file=" + DbgFile
+		+ " "
+		+ UpDir + "=RW:" + LoDir + "=RO "
+		+ MntDir)
+
+def undo_setup():
+	print "Destroying test setup..."
+	res = subprocess.check_output(["pgrep", "-f", UfsBin, "-u",  str(os.getuid())])
+	os.kill(int(res), signal.SIGTERM)
+
+# test code
+def test_func_checker(fc):
+
+	time.sleep(5)
+	sz = os.path.getsize(MntFile)
+	if sz == OrigSize:
+		fc.value = 0
+	else:
+		print "Size mismatch " + str(sz) + " vs " + str(OrigSize) + ". line no: " + str(inspect.currentframe().f_lineno)
+
+# test code
+def test_func_writer(fc):
+
+	smalldata = bytearray(['X']*100)
+	write_file(MntFile, smalldata, 2000)
+
+	sz = os.path.getsize(MntFile)
+	if sz == OrigSize:
+		fc.value = 0
+	else:
+		print "Size mismatch " + str(sz) + " vs " + str(OrigSize) + ". line no: " + str(inspect.currentframe().f_lineno)
+
+# set up function: do setup and fork a process to execute the tests
+def main():
+	do_setup()
+
+	tw1 = multiprocessing.Value('i', -1)
+	tw2 = multiprocessing.Value('i', -1)
+	tc = multiprocessing.Value('i', -1)
+
+	# two writers and one checker
+	pw1 = multiprocessing.Process(target=test_func_writer, args=(tw1,))
+	pw2 = multiprocessing.Process(target=test_func_writer, args=(tw2,))
+	pc = multiprocessing.Process(target=test_func_checker, args=(tc,))
+
+	pw1.start()
+	pw2.start()
+	pc.start()
+
+	pc.join()
+	pw1.join()
+	pw2.join()
+
+	undo_setup()
+
+	if tw1.value < 0:
+		print "***** Writer1 failed. *****"
+	elif tw2.value < 0:
+		print "***** Writer2 failed. *****"
+	elif tc.value < 0:
+		print "***** Checker failed. *****"
+	else:
+		print "***** Test passed *******"
+
+if __name__ == "__main__":
+	main()
+