changeset 21238:bfbe1f6b8ab6

lib-index: mail_transaction_log_file_sync(): Don't mix I/O errors and corruption
author Timo Sirainen <timo.sirainen@dovecot.fi>
date Mon, 28 Nov 2016 13:52:40 +0200
parents a0be55f275c5
children 02762bd30721
files src/lib-index/mail-transaction-log-file.c
diffstat 1 files changed, 25 insertions(+), 20 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib-index/mail-transaction-log-file.c	Mon Nov 28 13:04:47 2016 +0200
+++ b/src/lib-index/mail-transaction-log-file.c	Mon Nov 28 13:52:40 2016 +0200
@@ -17,7 +17,8 @@
 #define LOG_NEW_DOTLOCK_SUFFIX ".newlock"
 
 static int
-mail_transaction_log_file_sync(struct mail_transaction_log_file *file);
+mail_transaction_log_file_sync(struct mail_transaction_log_file *file,
+			       bool *retry_r);
 
 static void
 log_file_set_syscall_error(struct mail_transaction_log_file *file,
@@ -189,6 +190,7 @@
 mail_transaction_log_file_add_to_list(struct mail_transaction_log_file *file)
 {
 	struct mail_transaction_log_file **p;
+	bool retry;
 
 	file->sync_offset = file->hdr.hdr_size;
 	file->sync_highest_modseq = file->hdr.initial_modseq;
@@ -207,7 +209,7 @@
 	if (file->buffer != NULL) {
 		/* if we read any unfinished data, make sure the buffer gets
 		   truncated. */
-		(void)mail_transaction_log_file_sync(file);
+		(void)mail_transaction_log_file_sync(file, &retry);
 		buffer_set_used_size(file->buffer,
 				     file->sync_offset - file->buffer_offset);
 	}
@@ -1356,7 +1358,8 @@
 }
 
 static int
-mail_transaction_log_file_sync(struct mail_transaction_log_file *file)
+mail_transaction_log_file_sync(struct mail_transaction_log_file *file,
+			       bool *retry_r)
 {
         const struct mail_transaction_header *hdr;
 	const void *data;
@@ -1367,6 +1370,8 @@
 
 	i_assert(file->sync_offset >= file->buffer_offset);
 
+	*retry_r = FALSE;
+
 	data = buffer_get_data(file->buffer, &size);
 	if (file->buffer_offset + size < file->sync_offset) {
 		mail_transaction_log_file_set_corrupted(file,
@@ -1374,7 +1379,7 @@
 			file->buffer_offset + (uoff_t)size, file->sync_offset);
 		/* fix the sync_offset to avoid crashes later on */
 		file->sync_offset = file->buffer_offset + size;
-		return -1;
+		return 0;
 	}
 	while (file->sync_offset - file->buffer_offset + sizeof(*hdr) <= size) {
 		hdr = CONST_PTR_OFFSET(data, file->sync_offset -
@@ -1387,7 +1392,7 @@
 		if (trans_size < sizeof(*hdr)) {
 			mail_transaction_log_file_set_corrupted(file,
 				"hdr.size too small (%u)", trans_size);
-			return -1;
+			return 0;
 		}
 
 		if (file->sync_offset - file->buffer_offset + trans_size > size)
@@ -1396,7 +1401,7 @@
 		/* transaction has been fully written */
 		if ((ret = log_file_track_sync(file, hdr, trans_size)) <= 0) {
 			if (ret < 0)
-				return -1;
+				return 0;
 			break;
 		}
 
@@ -1419,6 +1424,7 @@
 		}
 		if ((uoff_t)st.st_size != file->last_size) {
 			file->last_size = st.st_size;
+			*retry_r = TRUE;
 			return 0;
 		}
 	}
@@ -1431,7 +1437,7 @@
 		if (file->locked) {
 			mail_transaction_log_file_set_corrupted(file,
 				"Unexpected garbage at EOF");
-			return -1;
+			return 0;
 		}
 		/* The size field will be updated soon */
 		mail_index_flush_read_cache(file->log->index, file->filepath,
@@ -1445,7 +1451,7 @@
 			"Invalid transaction log size "
 			"(%"PRIuUOFF_T" vs %u): %s", file->sync_offset,
 			file->log->head->hdr.prev_file_offset, file->filepath);
-		return -1;
+		return 0;
 	}
 
 	return 1;
@@ -1544,6 +1550,7 @@
 mail_transaction_log_file_read(struct mail_transaction_log_file *file,
 			       uoff_t start_offset, bool nfs_flush)
 {
+	bool retry;
 	int ret;
 
 	i_assert(file->mmap_base == NULL);
@@ -1579,11 +1586,10 @@
 		 mail_transaction_log_file_need_nfs_flush(file)) {
 		/* we didn't read enough data. flush and try again. */
 		return mail_transaction_log_file_read(file, start_offset, TRUE);
-	} else if ((ret = mail_transaction_log_file_sync(file)) <= 0) {
-		i_assert(ret != 0); /* ret=0 happens only with mmap */
-	} else {
-		i_assert(file->sync_offset >= file->buffer_offset);
+	} else if ((ret = mail_transaction_log_file_sync(file, &retry)) == 0) {
+		i_assert(!retry); /* retry happens only with mmap */
 	}
+	i_assert(file->sync_offset >= file->buffer_offset);
 	buffer_set_used_size(file->buffer,
 			     file->sync_offset - file->buffer_offset);
 	return ret;
@@ -1665,6 +1671,7 @@
 				   uoff_t start_offset)
 {
 	struct stat st;
+	bool retry;
 	int ret;
 
 	/* we are going to mmap() this file, but it's not necessarily
@@ -1688,10 +1695,9 @@
 	if (file->buffer != NULL && file->buffer_offset <= start_offset &&
 	    (uoff_t)st.st_size == file->buffer_offset + file->buffer->used) {
 		/* we already have the whole file mapped */
-		if ((ret = mail_transaction_log_file_sync(file)) < 0)
-			return 0;
-		if (ret > 0)
-			return 1;
+		if ((ret = mail_transaction_log_file_sync(file, &retry)) != 0 ||
+		    !retry)
+			return ret;
 		/* size changed, re-mmap */
 	}
 
@@ -1707,11 +1713,10 @@
 
 		if (mail_transaction_log_file_mmap(file) < 0)
 			return -1;
-		if ((ret = mail_transaction_log_file_sync(file)) < 0)
-			return 0;
-	} while (ret == 0);
+		ret = mail_transaction_log_file_sync(file, &retry);
+	} while (retry);
 
-	return 1;
+	return ret;
 }
 
 int mail_transaction_log_file_map(struct mail_transaction_log_file *file,