dovecot-2.2: lib-fs: Always keep the error in the parentmost fs.

dovecot at dovecot.org dovecot at dovecot.org
Wed Mar 27 14:45:30 EET 2013


details:   http://hg.dovecot.org/dovecot-2.2/rev/d4014211c154
changeset: 16124:d4014211c154
user:      Timo Sirainen <tss at iki.fi>
date:      Wed Mar 27 14:45:14 2013 +0200
description:
lib-fs: Always keep the error in the parentmost fs.
This fixes problems with getting the correct error message when wrapper
filesystems are used.

diffstat:

 src/lib-fs/fs-api-private.h |    1 +
 src/lib-fs/fs-api.c         |   25 +++++++-
 src/lib-fs/fs-metawrap.c    |  111 +++++++++----------------------------------
 3 files changed, 44 insertions(+), 93 deletions(-)

diffs (truncated from 315 to 300 lines):

diff -r b4040180f30c -r d4014211c154 src/lib-fs/fs-api-private.h
--- a/src/lib-fs/fs-api-private.h	Wed Mar 27 12:53:35 2013 +0200
+++ b/src/lib-fs/fs-api-private.h	Wed Mar 27 14:45:14 2013 +0200
@@ -53,6 +53,7 @@
 };
 
 struct fs {
+	struct fs *parent; /* for wrapper filesystems */
 	const char *name;
 	struct fs_vfuncs v;
 	char *temp_path_prefix;
diff -r b4040180f30c -r d4014211c154 src/lib-fs/fs-api.c
--- a/src/lib-fs/fs-api.c	Wed Mar 27 12:53:35 2013 +0200
+++ b/src/lib-fs/fs-api.c	Wed Mar 27 14:45:14 2013 +0200
@@ -227,8 +227,24 @@
 		file->fs->v.get_path(file);
 }
 
+static void ATTR_FORMAT(2, 0)
+fs_set_verror(struct fs *fs, const char *fmt, va_list args)
+{
+	/* the error is always kept in the parentmost fs */
+	if (fs->parent != NULL)
+		fs_set_verror(fs->parent, fmt, args);
+	else {
+		str_truncate(fs->last_error, 0);
+		str_vprintfa(fs->last_error, fmt, args);
+	}
+}
+
 const char *fs_last_error(struct fs *fs)
 {
+	/* the error is always kept in the parentmost fs */
+	if (fs->parent != NULL)
+		return fs_last_error(fs->parent);
+
 	if (str_len(fs->last_error) == 0)
 		return "BUG: Unknown fs error";
 	return str_c(fs->last_error);
@@ -529,8 +545,7 @@
 	va_list args;
 
 	va_start(args, fmt);
-	str_truncate(fs->last_error, 0);
-	str_vprintfa(fs->last_error, fmt, args);
+	fs_set_verror(fs, fmt, args);
 	va_end(args);
 }
 
@@ -539,9 +554,9 @@
 	va_list args;
 
 	va_start(args, fmt);
-	str_truncate(fs->last_error, 0);
-	str_vprintfa(fs->last_error, fmt, args);
-	i_error("fs-%s: %s", fs->name, str_c(fs->last_error));
+	fs_set_verror(fs, fmt, args);
+
+	i_error("fs-%s: %s", fs->name, fs_last_error(fs));
 	va_end(args);
 }
 
diff -r b4040180f30c -r d4014211c154 src/lib-fs/fs-metawrap.c
--- a/src/lib-fs/fs-metawrap.c	Wed Mar 27 12:53:35 2013 +0200
+++ b/src/lib-fs/fs-metawrap.c	Wed Mar 27 14:45:14 2013 +0200
@@ -15,7 +15,6 @@
 
 struct metawrap_fs {
 	struct fs fs;
-	struct fs *super;
 	bool wrap_metadata;
 };
 
@@ -29,18 +28,6 @@
 	bool metadata_read;
 };
 
-static void fs_metawrap_copy_error(struct metawrap_fs *fs)
-{
-	fs_set_error(&fs->fs, "%s", fs_last_error(fs->super));
-}
-
-static void fs_metawrap_file_copy_error(struct metawrap_fs_file *file)
-{
-	struct metawrap_fs *fs = (struct metawrap_fs *)file->file.fs;
-
-	fs_metawrap_copy_error(fs);
-}
-
 static struct fs *fs_metawrap_alloc(void)
 {
 	struct metawrap_fs *fs;
@@ -70,11 +57,11 @@
 		parent_name = t_strdup_until(args, parent_args);
 		parent_args++;
 	}
-	if (fs_init(parent_name, parent_args, set, &fs->super, &error) < 0) {
+	if (fs_init(parent_name, parent_args, set, &_fs->parent, &error) < 0) {
 		fs_set_error(_fs, "%s: %s", parent_name, error);
 		return -1;
 	}
-	if ((fs_get_properties(fs->super) & FS_PROPERTY_METADATA) == 0)
+	if ((fs_get_properties(_fs->parent) & FS_PROPERTY_METADATA) == 0)
 		fs->wrap_metadata = TRUE;
 	return 0;
 }
@@ -83,8 +70,8 @@
 {
 	struct metawrap_fs *fs = (struct metawrap_fs *)_fs;
 
-	if (fs->super != NULL)
-		fs_deinit(&fs->super);
+	if (_fs->parent != NULL)
+		fs_deinit(&_fs->parent);
 	i_free(fs);
 }
 
@@ -93,7 +80,7 @@
 	const struct metawrap_fs *fs = (const struct metawrap_fs *)_fs;
 	enum fs_properties props;
 
-	props = fs_get_properties(fs->super);
+	props = fs_get_properties(_fs->parent);
 	if (fs->wrap_metadata) {
 		/* we don't have a quick stat() to see the file's size,
 		   because of the metadata header */
@@ -118,12 +105,12 @@
 	/* avoid unnecessarily creating two seekable streams */
 	flags &= ~FS_OPEN_FLAG_SEEKABLE;
 
-	file->super = fs_file_init(fs->super, path, mode | flags);
+	file->super = fs_file_init(_fs->parent, path, mode | flags);
 	if (file->fs->wrap_metadata && mode == FS_OPEN_MODE_READONLY &&
 	    (flags & FS_OPEN_FLAG_ASYNC) == 0) {
 		/* use async stream for super, so fs_read_stream() won't create
 		   another seekable stream unneededly */
-		file->super_read = fs_file_init(fs->super, path, mode | flags |
+		file->super_read = fs_file_init(_fs->parent, path, mode | flags |
 						FS_OPEN_FLAG_ASYNC);
 	} else {
 		file->super_read = file->super;
@@ -164,13 +151,7 @@
 
 static int fs_metawrap_wait_async(struct fs *_fs)
 {
-	struct metawrap_fs *fs = (struct metawrap_fs *)_fs;
-
-	if (fs_wait_async(fs->super) < 0) {
-		fs_metawrap_copy_error(fs);
-		return -1;
-	}
-	return 0;
+	return fs_wait_async(_fs->parent);
 }
 
 static void
@@ -192,13 +173,8 @@
 	struct metawrap_fs_file *file = (struct metawrap_fs_file *)_file;
 	char c;
 
-	if (!file->fs->wrap_metadata) {
-		if (fs_get_metadata(file->super, metadata_r) < 0) {
-			fs_metawrap_file_copy_error(file);
-			return -1;
-		}
-		return 0;
-	}
+	if (!file->fs->wrap_metadata)
+		return fs_get_metadata(file->super, metadata_r);
 
 	if (!file->metadata_read) {
 		if (fs_read(_file, &c, 1) < 0)
@@ -221,13 +197,9 @@
 static ssize_t fs_metawrap_read(struct fs_file *_file, void *buf, size_t size)
 {
 	struct metawrap_fs_file *file = (struct metawrap_fs_file *)_file;
-	ssize_t ret;
 
-	if (!file->fs->wrap_metadata) {
-		if ((ret = fs_read(file->super, buf, size)) < 0)
-			fs_metawrap_file_copy_error(file);
-		return ret;
-	}
+	if (!file->fs->wrap_metadata)
+		return fs_read(file->super, buf, size);
 	return fs_read_via_stream(_file, buf, size);
 }
 
@@ -275,13 +247,8 @@
 {
 	struct metawrap_fs_file *file = (struct metawrap_fs_file *)_file;
 
-	if (!file->fs->wrap_metadata) {
-		if (fs_write(file->super, data, size) < 0) {
-			fs_metawrap_file_copy_error(file);
-			return -1;
-		}
-		return 0;
-	}
+	if (!file->fs->wrap_metadata)
+		return fs_write(file->super, data, size);
 	return fs_write_via_stream(_file, data, size);
 }
 
@@ -343,9 +310,6 @@
 	} else {
 		ret = fs_write_stream_finish(file->super, &file->super_output);
 	}
-
-	if (ret < 0)
-		fs_metawrap_file_copy_error(file);
 	return ret;
 }
 
@@ -354,11 +318,7 @@
 {
 	struct metawrap_fs_file *file = (struct metawrap_fs_file *)_file;
 
-	if (fs_lock(file->super, secs, lock_r) < 0) {
-		fs_metawrap_file_copy_error(file);
-		return -1;
-	}
-	return 0;
+	return fs_lock(file->super, secs, lock_r);
 }
 
 static void fs_metawrap_unlock(struct fs_lock *_lock ATTR_UNUSED)
@@ -370,11 +330,7 @@
 {
 	struct metawrap_fs_file *file = (struct metawrap_fs_file *)_file;
 
-	if (fs_exists(file->super) < 0) {
-		fs_metawrap_copy_error(file->fs);
-		return -1;
-	}
-	return 0;
+	return fs_exists(file->super);
 }
 
 static int fs_metawrap_stat(struct fs_file *_file, struct stat *st_r)
@@ -384,13 +340,8 @@
 	uoff_t input_size;
 	ssize_t ret;
 
-	if (!file->fs->wrap_metadata) {
-		if (fs_stat(file->super, st_r) < 0) {
-			fs_metawrap_copy_error(file->fs);
-			return -1;
-		}
-		return 0;
-	}
+	if (!file->fs->wrap_metadata)
+		return fs_stat(file->super, st_r);
 	input = fs_read_stream(_file, IO_BLOCK_SIZE);
 	if ((ret = i_stream_get_size(input, TRUE, &input_size)) < 0) {
 		fs_set_error(_file->fs, "i_stream_get_size(%s) failed: %m",
@@ -406,7 +357,6 @@
 
 	if (fs_stat(file->super, st_r) < 0) {
 		i_assert(errno != EAGAIN); /* read should have caught this */
-		fs_metawrap_copy_error(file->fs);
 		return -1;
 	}
 	st_r->st_size = input_size;
@@ -418,13 +368,8 @@
 	struct metawrap_fs_file *src = (struct metawrap_fs_file *)_src;
 	struct metawrap_fs_file *dest = (struct metawrap_fs_file *)_dest;
 
-	if (!dest->fs->wrap_metadata) {
-		if (fs_copy(src->super, dest->super) < 0) {
-			fs_metawrap_copy_error(src->fs);
-			return -1;
-		}
-		return 0;
-	}
+	if (!dest->fs->wrap_metadata)
+		return fs_copy(src->super, dest->super);
 	return fs_default_copy(_src, _dest);
 }
 
@@ -433,31 +378,21 @@
 	struct metawrap_fs_file *src = (struct metawrap_fs_file *)_src;
 	struct metawrap_fs_file *dest = (struct metawrap_fs_file *)_dest;
 
-	if (fs_rename(src->super, dest->super) < 0) {
-		fs_metawrap_copy_error(src->fs);
-		return -1;
-	}
-	return 0;
+	return fs_rename(src->super, dest->super);
 }
 
 static int fs_metawrap_delete(struct fs_file *_file)
 {
 	struct metawrap_fs_file *file = (struct metawrap_fs_file *)_file;
 
-	if (fs_delete(file->super) < 0) {
-		fs_metawrap_copy_error(file->fs);
-		return -1;
-	}


More information about the dovecot-cvs mailing list