Changeset

--- a/plugins/mod_posix.lua	Mon Mar 27 18:30:11 2017 +0200
+++ b/plugins/mod_posix.lua	Tue Mar 28 20:14:35 2017 +0200
@@ -7,7 +7,7 @@
 --
 
 
-local want_pposix_version = "0.3.6";
+local want_pposix_version = "0.4.0";
 
 local pposix = assert(require "util.pposix");
 if pposix._VERSION ~= want_pposix_version then

--- a/prosodyctl	Mon Mar 27 18:30:11 2017 +0200
+++ b/prosodyctl	Tue Mar 28 20:14:35 2017 +0200
@@ -135,7 +135,7 @@
 -- Switch away from root and into the prosody user --
 local switched_user, current_uid;
 
-local want_pposix_version = "0.3.6";
+local want_pposix_version = "0.4.0";
 local ok, pposix = pcall(require, "util.pposix");
 
 if ok and pposix then

--- a/util-src/pposix.c	Mon Mar 27 18:30:11 2017 +0200
+++ b/util-src/pposix.c	Tue Mar 28 20:14:35 2017 +0200
@@ -13,7 +13,7 @@
 * POSIX support functions for Lua
 */
 
-#define MODULE_VERSION "0.3.6"
+#define MODULE_VERSION "0.4.0"
 
 
 #if defined(__linux__)
@@ -730,62 +730,66 @@
 }
 #endif
 
-/* File handle extraction blatantly stolen from
- * https://github.com/rrthomas/luaposix/blob/master/lposix.c#L631
- * */
+/*
+ * Append some data to a file handle
+ * Attempt to allocate space first
+ * Truncate to original size on failure
+ */
+int lc_atomic_append(lua_State *L) {
+	int err;
+	size_t len;
 
-int lc_fallocate(lua_State *L) {
-	int ret;
-	off_t offset, len;
 	FILE *f = *(FILE **) luaL_checkudata(L, 1, LUA_FILEHANDLE);
+	const char *data = luaL_checklstring(L, 2, &len);
 
-	if(f == NULL) {
-		return luaL_error(L, "attempt to use a closed file");
-	}
-
-	offset = luaL_checkinteger(L, 2);
-	len = luaL_checkinteger(L, 3);
+	off_t offset = ftell(f);
 
 #if defined(__linux__)
-	errno = 0;
-	ret = fallocate(fileno(f), FALLOC_FL_KEEP_SIZE, offset, len);
+	/* Try to allocate space without changing the file size. */
+	if((err = fallocate(fileno(f), FALLOC_FL_KEEP_SIZE, offset, len))) {
+		if(errno != 0) {
+			/* Some old versions of Linux apparently use the return value instead of errno */
+			err = errno;
+		}
+		switch(err) {
+			case ENOSYS: /* Kernel doesn't implement fallocate */
+			case EOPNOTSUPP: /* Filesystem doesn't support it */
+				/* Ignore and proceed to try to write */
+				break;
 
-	if(ret == 0) {
-		lua_pushboolean(L, 1);
-		return 1;
+			case ENOSPC: /* No space left */
+			default: /* Other issues */
+				lua_pushnil(L);
+				lua_pushstring(L, strerror(err));
+				lua_pushinteger(L, err);
+				return 3;
+		}
+	}
+#endif
+
+	if(fwrite(data, sizeof(char), len, f) == len) {
+		if(fflush(f) == 0) {
+			lua_pushboolean(L, 1); /* Great success! */
+			return 1;
+		} else {
+			err = errno;
+		}
+	} else {
+		err = ferror(f);
 	}
 
-	/* Some old versions of Linux apparently use the return value instead of errno */
-	if(errno == 0) {
-		errno = ret;
-	}
+	fseek(f, offset, SEEK_SET);
 
-	if(errno != ENOSYS && errno != EOPNOTSUPP) {
-		lua_pushnil(L);
-		lua_pushstring(L, strerror(errno));
-		return 2;
+	/* Cut partially written data */
+	if(ftruncate(fileno(f), offset)) {
+		/* The file is now most likely corrupted, throw hard error */
+		return luaL_error(L, "atomic_append() failed in ftruncate(): %s", strerror(errno));
 	}
 
-#endif
-
-	ret = posix_fallocate(fileno(f), offset, len);
-
-	if(ret == 0) {
-		lua_pushboolean(L, 1);
-		return 1;
-	} else {
-		lua_pushnil(L);
-		lua_pushstring(L, strerror(ret));
-
-		/* posix_fallocate() can leave a bunch of NULs at the end, so we cut that
-		 * this assumes that offset == length of the file */
-		if(ftruncate(fileno(f), offset) != 0) {
-			lua_pushstring(L, strerror(errno));
-			return 3;
-		}
-
-		return 2;
-	}
+	lua_pushnil(L);
+	lua_pushstring(L, strerror(err));
+	lua_pushinteger(L, err);
+	return 3;
 }
 
 /* Register functions */
@@ -827,7 +831,7 @@
 		{ "meminfo", lc_meminfo },
 #endif
 
-		{ "fallocate", lc_fallocate },
+		{ "atomic_append", lc_atomic_append },
 
 		{ NULL, NULL }
 	};

--- a/util/datamanager.lua	Mon Mar 27 18:30:11 2017 +0200
+++ b/util/datamanager.lua	Tue Mar 28 20:14:35 2017 +0200
@@ -17,7 +17,6 @@
 local os_remove = os.remove;
 local os_rename = os.rename;
 local tonumber = tonumber;
-local tostring = tostring;
 local next = next;
 local type = type;
 local t_insert = table.insert;
@@ -31,21 +30,12 @@
 local prosody = prosody;
 
 local raw_mkdir = lfs.mkdir;
-local function fallocate(f, offset, len)
-	-- This assumes that current position == offset
-	local fake_data = (" "):rep(len);
-	local ok, msg = f:write(fake_data);
-	if not ok then
-		return ok, msg;
-	end
-	f:seek("set", offset);
-	return true;
-end;
+local atomic_append;
 local ENOENT = 2;
 pcall(function()
 	local pposix = require "util.pposix";
 	raw_mkdir = pposix.mkdir or raw_mkdir; -- Doesn't trample on umask
-	fallocate = pposix.fallocate or fallocate;
+	atomic_append = pposix.atomic_append;
 	ENOENT = pposix.ENOENT or ENOENT;
 end);
 
@@ -65,6 +55,19 @@
 	end
 end
 
+if not atomic_append then
+	function atomic_append(f, data)
+		local pos = f:seek();
+		if not f:write(data) or not f:flush() then
+			f:seek("set", pos);
+			f:write((" "):rep(#data));
+			f:flush();
+			return nil, "write-failed";
+		end
+		return true;
+	end
+end
+
 local _mkdir = {};
 local function mkdir(path)
 	path = path:gsub("/", path_separator); -- TODO as an optimization, do this during path creation rather than here
@@ -220,26 +223,16 @@
 	if type(data) ~= "string" then return; end
 	local filename = getpath(username, host, datastore, ext, true);
 
-	local ok;
-	local f, msg = io_open(filename, "r+");
+	local f = io_open(filename, "r+");
 	if not f then
 		return atomic_store(filename, data);
 		-- File did probably not exist, let's create it
 	end
 
 	local pos = f:seek("end");
-	ok, msg = fallocate(f, pos, #data);
-	if not ok then
-		log("warn", "fallocate() failed: %s", tostring(msg));
-		-- This doesn't work on every file system
-	end
 
-	if f:seek() ~= pos then
-		log("debug", "fallocate() changed file position");
-		f:seek("set", pos);
-	end
+	local ok, msg = atomic_append(f, data);
 
-	ok, msg = f:write(data);
 	if not ok then
 		f:close();
 		return ok, msg, "write";
@@ -247,7 +240,7 @@
 
 	ok, msg = f:close();
 	if not ok then
-		return ok, msg;
+		return ok, msg, "close";
 	end
 
 	return true, pos;
@@ -259,9 +252,10 @@
 	-- save the datastore
 
 	data = "item(" ..  serialize(data) .. ");\n";
-	local ok, msg = append(username, host, datastore, "list", data);
+	local ok, msg, where = append(username, host, datastore, "list", data);
 	if not ok then
-		log("error", "Unable to write to %s storage ('%s') for user: %s@%s", datastore, msg, username or "nil", host or "nil");
+		log("error", "Unable to write to %s storage ('%s' in %s) for user: %s@%s",
+			datastore, msg, where, username or "nil", host or "nil");
 		return ok, msg;
 	end
 	return true;