Mercurial > dovecot > core-2.2
changeset 4023:b19ccbaa3802 HEAD
Try to handle ESTALE NFS errors the best way we can.
| author | Timo Sirainen <timo.sirainen@movial.fi> |
|---|---|
| date | Thu, 16 Feb 2006 17:23:00 +0200 |
| parents | 2f26567685ff |
| children | 7d7b0f427d68 |
| files | src/lib-index/mail-cache.c src/lib-index/mail-index-private.h src/lib-index/mail-index-sync-keywords.c src/lib-index/mail-index-view.c src/lib-index/mail-index.c src/lib-index/mail-transaction-log.c src/lib-storage/index/maildir/maildir-keywords.c src/lib-storage/index/maildir/maildir-uidlist.c src/lib-storage/subscription-file/subscription-file.c src/lib/Makefile.am src/lib/file-cache.c src/lib/safe-open.c src/lib/safe-open.h |
| diffstat | 13 files changed, 442 insertions(+), 180 deletions(-) [+] |
line wrap: on
line diff
--- a/src/lib-index/mail-cache.c Thu Feb 16 16:58:31 2006 +0200 +++ b/src/lib-index/mail-cache.c Thu Feb 16 17:23:00 2006 +0200 @@ -180,8 +180,15 @@ ret = file_cache_read(cache->file_cache, offset, size); if (ret < 0) { - // FIXME: ESTALE - mail_cache_set_syscall_error(cache, "read()"); + /* In case of ESTALE we'll simply fail without error + messages. The caller will then just have to + fallback to generating the value itself. + + We can't simply reopen the cache flie, because + using it requires also having updated file + offsets. */ + if (errno != ESTALE) + mail_cache_set_syscall_error(cache, "read()"); return -1; }
--- a/src/lib-index/mail-index-private.h Thu Feb 16 16:58:31 2006 +0200 +++ b/src/lib-index/mail-index-private.h Thu Feb 16 17:23:00 2006 +0200 @@ -246,8 +246,8 @@ const struct mail_index_ext * mail_index_view_get_ext(struct mail_index_view *view, uint32_t ext_id); -int mail_index_map_read_keywords(struct mail_index *index, - struct mail_index_map *map); +int mail_index_map_parse_keywords(struct mail_index *index, + struct mail_index_map *map); int mail_index_fix_header(struct mail_index *index, struct mail_index_map *map, struct mail_index_header *hdr, const char **error_r);
--- a/src/lib-index/mail-index-sync-keywords.c Thu Feb 16 16:58:31 2006 +0200 +++ b/src/lib-index/mail-index-sync-keywords.c Thu Feb 16 17:23:00 2006 +0200 @@ -16,7 +16,7 @@ unsigned int i, count, keyword_idx; if (!ctx->keywords_read) { - if (mail_index_map_read_keywords(ctx->view->index, map) < 0) + if (mail_index_map_parse_keywords(ctx->view->index, map) < 0) return -1; ctx->keywords_read = TRUE; } @@ -298,8 +298,8 @@ } if (!ctx->keywords_read) { - if (mail_index_map_read_keywords(ctx->view->index, - ctx->view->map) < 0) + if (mail_index_map_parse_keywords(ctx->view->index, + ctx->view->map) < 0) return -1; ctx->keywords_read = TRUE; }
--- a/src/lib-index/mail-index-view.c Thu Feb 16 16:58:31 2006 +0200 +++ b/src/lib-index/mail-index-view.c Thu Feb 16 17:23:00 2006 +0200 @@ -526,11 +526,10 @@ continue; if (idx >= keyword_count) { - /* keyword header is updated, re-read - it so we know what this one is - called */ - if (mail_index_map_read_keywords(view->index, - map) < 0) + /* keyword header was updated, parse it again + it so we know what this keyword is called */ + if (mail_index_map_parse_keywords(view->index, + map) < 0) return -1; if (!array_is_created(&map->keyword_idx_map))
--- a/src/lib-index/mail-index.c Thu Feb 16 16:58:31 2006 +0200 +++ b/src/lib-index/mail-index.c Thu Feb 16 17:23:00 2006 +0200 @@ -5,6 +5,7 @@ #include "buffer.h" #include "hash.h" #include "mmap-util.h" +#include "safe-open.h" #include "read-full.h" #include "write-full.h" #include "mail-index-private.h" @@ -266,8 +267,8 @@ return MAIL_INDEX_HEADER_SIZE_ALIGN(size) - size; } -static int mail_index_read_extensions(struct mail_index *index, - struct mail_index_map *map) +static int mail_index_parse_extensions(struct mail_index *index, + struct mail_index_map *map) { const struct mail_index_ext_header *ext_hdr; unsigned int i, old_count; @@ -377,8 +378,8 @@ return TRUE; } -int mail_index_map_read_keywords(struct mail_index *index, - struct mail_index_map *map) +int mail_index_map_parse_keywords(struct mail_index *index, + struct mail_index_map *map) { const struct mail_index_ext *ext; const struct mail_index_keyword_header *kw_hdr; @@ -486,7 +487,7 @@ { /* Make sure all the keywords are in index->keywords. It's quick to do if nothing has changed. */ - (void)mail_index_map_read_keywords(index, index->map); + (void)mail_index_map_parse_keywords(index, index->map); return &index->keywords; } @@ -546,7 +547,7 @@ hdr->first_deleted_uid_lowwater > hdr->next_uid) return 0; - return mail_index_read_extensions(index, map); + return mail_index_parse_extensions(index, map); } static void mail_index_map_clear(struct mail_index *index, @@ -662,20 +663,25 @@ } static int mail_index_read_header(struct mail_index *index, - struct mail_index_header *hdr, size_t *pos_r) + void *buf, size_t buf_size, size_t *pos_r) { size_t pos; int ret; - memset(hdr, 0, sizeof(*hdr)); + memset(buf, 0, sizeof(struct mail_index_header)); - ret = 1; - for (pos = 0; ret > 0 && pos < sizeof(*hdr); ) { - ret = pread(index->fd, PTR_OFFSET(hdr, pos), - sizeof(*hdr) - pos, pos); + /* try to read the whole header, but it's not necessarily an error to + read less since the older versions of the index format could be + smaller. Request reading up to buf_size, but accept if we only got + the header. */ + pos = 0; + do { + ret = pread(index->fd, PTR_OFFSET(buf, pos), + buf_size - pos, pos); if (ret > 0) pos += ret; - } + } while (ret > 0 && pos < sizeof(struct mail_index_header)); + *pos_r = pos; return ret; } @@ -683,7 +689,8 @@ static int mail_index_read_map(struct mail_index *index, struct mail_index_map *map, bool *retry_r) { - struct mail_index_header hdr; + const struct mail_index_header *hdr; + unsigned char buf[512]; void *data = NULL; ssize_t ret; size_t pos, records_size; @@ -691,39 +698,46 @@ i_assert(map->mmap_base == NULL); *retry_r = FALSE; - ret = mail_index_read_header(index, &hdr, &pos); + ret = mail_index_read_header(index, buf, sizeof(buf), &pos); + hdr = (const struct mail_index_header *)buf; if (pos > (ssize_t)offsetof(struct mail_index_header, major_version) && - hdr.major_version != MAIL_INDEX_MAJOR_VERSION) { + hdr->major_version != MAIL_INDEX_MAJOR_VERSION) { /* major version change - handle silently */ return 0; } if (ret >= 0 && pos >= MAIL_INDEX_HEADER_MIN_SIZE && - (ret > 0 || pos >= hdr.base_header_size)) { - if (hdr.base_header_size < MAIL_INDEX_HEADER_MIN_SIZE || - hdr.header_size < hdr.base_header_size) { + (ret > 0 || pos >= hdr->base_header_size)) { + if (hdr->base_header_size < MAIL_INDEX_HEADER_MIN_SIZE || + hdr->header_size < hdr->base_header_size) { mail_index_set_error(index, "Corrupted index file %s: " "Corrupted header sizes (base %u, full %u)", - index->filepath, hdr.base_header_size, - hdr.header_size); + index->filepath, hdr->base_header_size, + hdr->header_size); return 0; } + if (pos > hdr->header_size) + pos = hdr->header_size; + + /* place the base header into memory. */ buffer_reset(map->hdr_copy_buf); - buffer_append(map->hdr_copy_buf, &hdr, hdr.base_header_size); + buffer_append(map->hdr_copy_buf, buf, pos); - /* @UNSAFE */ - data = buffer_append_space_unsafe(map->hdr_copy_buf, - hdr.header_size - - hdr.base_header_size); - ret = pread_full(index->fd, data, - hdr.header_size - hdr.base_header_size, - hdr.base_header_size); + if (pos != hdr->header_size) { + /* @UNSAFE: read the rest of the header into memory */ + data = buffer_append_space_unsafe(map->hdr_copy_buf, + hdr->header_size - + pos); + ret = pread_full(index->fd, data, + hdr->header_size - pos, pos); + } } if (ret > 0) { - records_size = hdr.messages_count * hdr.record_size; + /* header read, read the records now. */ + records_size = hdr->messages_count * hdr->record_size; if (map->buffer == NULL) { map->buffer = buffer_create_dynamic(default_pool, @@ -735,11 +749,12 @@ data = buffer_append_space_unsafe(map->buffer, records_size); ret = pread_full(index->fd, data, records_size, - hdr.header_size); + hdr->header_size); } if (ret < 0) { if (errno == ESTALE) { + /* a new index file was renamed over this one. */ *retry_r = TRUE; return 0; } @@ -754,13 +769,13 @@ } map->records = data; - map->records_count = hdr.messages_count; + map->records_count = hdr->messages_count; - mail_index_map_copy_hdr(map, &hdr); + mail_index_map_copy_hdr(map, hdr); map->hdr_base = map->hdr_copy_buf->data; - index->sync_log_file_seq = hdr.log_file_seq; - index->sync_log_file_offset = hdr.log_file_int_offset; + index->sync_log_file_seq = hdr->log_file_seq; + index->sync_log_file_offset = hdr->log_file_int_offset; return 1; } @@ -784,7 +799,7 @@ if (sync_to_index) { /* read the real log position where we are supposed to be synced */ - ret = mail_index_read_header(index, &hdr, &pos); + ret = mail_index_read_header(index, &hdr, sizeof(hdr), &pos); if (ret < 0 && errno != ESTALE) { mail_index_set_syscall_error(index, "pread()"); return -1; @@ -904,17 +919,17 @@ for (i = 0; i < count; i++) (*handlers[i])(index); - for (i = 0; i < MAIL_INDEX_ESTALE_RETRY_COUNT; i++) { + for (i = 0;; i++) { ret = mail_index_read_map(index, *map, &retry); - if (ret != 0 || !retry) + if (ret != 0 || !retry || i == MAIL_INDEX_ESTALE_RETRY_COUNT) return ret; /* ESTALE - reopen index file */ - if (close(index->fd) < 0) + if (close(index->fd) < 0) mail_index_set_syscall_error(index, "close()"); index->fd = -1; - ret = mail_index_try_open_only(index); + ret = mail_index_try_open_only(index); if (ret <= 0) { if (ret == 0) { /* the file was lost */ @@ -1053,6 +1068,7 @@ struct mail_index_header *hdr_r) { size_t pos; + unsigned int i; int ret; if (!index->mmap_disable) { @@ -1061,8 +1077,30 @@ *hdr_r = *index->hdr; else memset(hdr_r, 0, sizeof(*hdr_r)); - } else { - ret = mail_index_read_header(index, hdr_r, &pos); + return ret; + } + + for (i = 0;; i++) { + ret = mail_index_read_header(index, hdr_r, sizeof(*hdr_r), + &pos); + if (ret >= 0 || errno != ESTALE || + i == MAIL_INDEX_ESTALE_RETRY_COUNT) + break; + + /* ESTALE - reopen index file */ + if (close(index->fd) < 0) + mail_index_set_syscall_error(index, "close()"); + index->fd = -1; + + ret = mail_index_try_open_only(index); + if (ret <= 0) { + if (ret == 0) { + /* the file was lost */ + errno = ENOENT; + mail_index_set_syscall_error(index, "open()"); + } + return -1; + } } return ret; } @@ -1151,25 +1189,19 @@ static int mail_index_try_open_only(struct mail_index *index) { - int i; - i_assert(!MAIL_INDEX_IS_IN_MEMORY(index)); - for (i = 0; i < 3; i++) { - index->fd = open(index->filepath, O_RDWR); - if (index->fd == -1 && errno == EACCES) { - index->fd = open(index->filepath, O_RDONLY); - index->readonly = TRUE; - } - if (index->fd != -1 || errno != ESTALE) - break; + /* Note that our caller must close index->fd by itself. + mail_index_reopen() for example wants to revert back to old + index file if opening the new one fails. */ + index->fd = safe_open(index->filepath, O_RDWR); + index->readonly = FALSE; - /* May happen with some OSes with NFS. Try again, although - there's still a race condition with another computer - creating the index file again. However, we can't try forever - as ESTALE happens also if index directory has been deleted - from server.. */ + if (index->fd == -1 && errno == EACCES) { + index->fd = open(index->filepath, O_RDONLY); + index->readonly = TRUE; } + if (index->fd == -1) { if (errno != ENOENT) return mail_index_set_syscall_error(index, "open()"); @@ -1186,6 +1218,8 @@ unsigned int lock_id; int ret; + i_assert(index->fd == -1); + if (lock_id_r != NULL) *lock_id_r = 0; @@ -1612,8 +1646,15 @@ if (MAIL_INDEX_IS_IN_MEMORY(index)) return 0; - if (fstat(index->fd, &st1) < 0) + if (fstat(index->fd, &st1) < 0) { + if (errno == ESTALE) { + /* deleted, reopen */ + if (mail_index_reopen(index, -1) < 0) + return -1; + return 1; + } return mail_index_set_syscall_error(index, "fstat()"); + } if (stat(index->filepath, &st2) < 0) { mail_index_set_syscall_error(index, "stat()"); if (errno != ENOENT)
--- a/src/lib-index/mail-transaction-log.c Thu Feb 16 16:58:31 2006 +0200 +++ b/src/lib-index/mail-transaction-log.c Thu Feb 16 17:23:00 2006 +0200 @@ -4,6 +4,8 @@ #include "ioloop.h" #include "buffer.h" #include "file-dotlock.h" +#include "safe-open.h" +#include "close-keep-errno.h" #include "read-full.h" #include "write-full.h" #include "mmap-util.h" @@ -278,6 +280,8 @@ static void mail_transaction_log_file_free(struct mail_transaction_log_file *file) { + int old_errno = errno; + mail_transaction_log_file_unlock(file); if (file == file->log->head) @@ -305,7 +309,9 @@ } i_free(file->filepath); - i_free(file); + i_free(file); + + errno = old_errno; } int mail_transaction_log_move_to_memory(struct mail_transaction_log *log) @@ -357,10 +363,11 @@ ret = pread_full(file->fd, &file->hdr, sizeof(file->hdr), 0); if (ret < 0) { - // FIXME: handle ESTALE - mail_index_file_set_syscall_error(file->log->index, - file->filepath, - "pread_full()"); + if (errno != ESTALE) { + mail_index_file_set_syscall_error(file->log->index, + file->filepath, + "pread_full()"); + } return -1; } if (ret == 0) { @@ -478,7 +485,7 @@ static int mail_transaction_log_file_create2(struct mail_transaction_log *log, - const char *path, int fd, + const char *path, int new_fd, struct dotlock **dotlock, dev_t dev, ino_t ino, uoff_t file_size) { @@ -486,28 +493,31 @@ struct mail_transaction_log_header hdr; struct stat st; const char *path2; - int fd2, ret; + int old_fd, ret; bool found; /* log creation is locked now - see if someone already created it */ - fd2 = open(path, O_RDWR); - if (fd2 != -1) { - if ((ret = fstat(fd2, &st)) < 0) { - mail_index_file_set_syscall_error(index, path, - "fstat()"); + old_fd = safe_open(path, O_RDWR); + if (old_fd != -1) { + if ((ret = fstat(old_fd, &st)) < 0) { + mail_index_file_set_syscall_error(index, path, + "fstat()"); } else if (st.st_ino == ino && CMP_DEV_T(st.st_dev, dev) && (uoff_t)st.st_size == file_size) { /* same file, still broken */ } else { - (void)file_dotlock_delete(dotlock); - return fd2; + /* file changed, use the new file */ + (void)file_dotlock_delete(dotlock); + return old_fd; } - (void)close(fd2); - fd2 = -1; + (void)close(old_fd); + old_fd = -1; - if (ret < 0) - return -1; + if (ret < 0) { + /* fstat() failure, return after closing fd.. */ + return -1; + } found = TRUE; } else if (errno != ENOENT) { mail_index_file_set_syscall_error(index, path, "open()"); @@ -519,7 +529,7 @@ if (mail_transaction_log_init_hdr(log, &hdr) < 0) return -1; - if (write_full(fd, &hdr, sizeof(hdr)) < 0) { + if (write_full(new_fd, &hdr, sizeof(hdr)) < 0) { mail_index_file_set_syscall_error(index, path, "write_full()"); return -1; @@ -529,7 +539,8 @@ if (found) { path2 = t_strconcat(path, ".2", NULL); if (rename(path, path2) < 0) { - i_error("rename(%s, %s) failed: %m", path, path2); + mail_index_set_error(index, + "rename(%s, %s) failed: %m", path, path2); /* ignore the error. we don't care that much about the second log file and we're going to overwrite this first one. */ @@ -541,7 +552,7 @@ return -1; /* success */ - return fd; + return new_fd; } static int @@ -551,7 +562,7 @@ { struct dotlock *dotlock; mode_t old_mask; - int fd, fd2; + int fd; i_assert(!MAIL_INDEX_IS_IN_MEMORY(log->index)); @@ -574,14 +585,16 @@ return -1; } - fd2 = mail_transaction_log_file_create2(log, path, fd, &dotlock, - dev, ino, file_size); - if (fd2 < 0) { + /* either fd gets used or the dotlock gets deleted and returned fd + is for the existing file */ + fd = mail_transaction_log_file_create2(log, path, fd, &dotlock, + dev, ino, file_size); + if (fd < 0) { if (dotlock != NULL) (void)file_dotlock_delete(&dotlock); return -1; } - return fd2; + return fd; } static void @@ -640,8 +653,11 @@ *file_r = NULL; if (fstat(fd, &st) < 0) { - mail_index_file_set_syscall_error(log->index, path, "fstat()"); - (void)close(fd); + if (errno != ESTALE) { + mail_index_file_set_syscall_error(log->index, path, + "fstat()"); + } + close_keep_errno(fd); return -1; } @@ -668,15 +684,20 @@ static struct mail_transaction_log_file * mail_transaction_log_file_fd_open_or_create(struct mail_transaction_log *log, - const char *path, int fd) + const char *path, int fd, + bool *retry_r) { struct mail_transaction_log_file *file; struct stat st; int ret; + *retry_r = FALSE; + ret = mail_transaction_log_file_fd_open(log, &file, path, fd, TRUE); - if (ret < 0) - return NULL; + if (ret < 0) { + *retry_r = errno == ESTALE; + return NULL; + } if (ret == 0) { /* corrupted header */ @@ -686,7 +707,7 @@ if (fd == -1) ret = -1; else if (fstat(fd, &st) < 0) { - mail_index_file_set_syscall_error(log->index, path, + mail_index_file_set_syscall_error(log->index, path, "fstat()"); (void)close(fd); fd = -1; @@ -707,6 +728,7 @@ } } if (ret <= 0) { + *retry_r = errno == ESTALE; mail_transaction_log_file_free(file); return NULL; } @@ -742,25 +764,39 @@ mail_transaction_log_file_open_or_create(struct mail_transaction_log *log, const char *path) { - int fd; + struct mail_transaction_log_file *file; + unsigned int i; + bool retry; + int fd; if (MAIL_INDEX_IS_IN_MEMORY(log->index)) return mail_transaction_log_file_alloc_in_memory(log); - fd = open(path, O_RDWR); - if (fd == -1) { - if (errno != ENOENT) { - mail_index_file_set_syscall_error(log->index, path, - "open()"); - return NULL; - } + for (i = 0; ; i++) { + fd = safe_open(path, O_RDWR); + if (fd == -1) { + if (errno != ENOENT) { + mail_index_file_set_syscall_error(log->index, + path, + "open()"); + return NULL; + } - fd = mail_transaction_log_file_create(log, path, 0, 0, 0); - if (fd == -1) - return NULL; - } + /* doesn't exist, try creating it */ + fd = mail_transaction_log_file_create(log, path, + 0, 0, 0); + if (fd == -1) + return NULL; + } - return mail_transaction_log_file_fd_open_or_create(log, path, fd); + file = mail_transaction_log_file_fd_open_or_create(log, path, + fd, &retry); + if (file != NULL || !retry || + i == MAIL_INDEX_ESTALE_RETRY_COUNT) + return file; + + /* ESTALE - retry */ + } } static struct mail_transaction_log_file * @@ -768,21 +804,34 @@ const char *path) { struct mail_transaction_log_file *file; - int fd, ret; + unsigned int i; + int fd, ret; - fd = open(path, O_RDWR); - if (fd == -1) { - mail_index_file_set_syscall_error(log->index, path, "open()"); - return NULL; - } + for (i = 0;; i++) { + fd = safe_open(path, O_RDWR); + if (fd == -1) { + mail_index_file_set_syscall_error(log->index, path, + "open()"); + return NULL; + } - ret = mail_transaction_log_file_fd_open(log, &file, path, fd, TRUE); - if (ret <= 0) { - /* error / corrupted */ - if (ret == 0) - mail_transaction_log_file_free(file); - return NULL; - } + ret = mail_transaction_log_file_fd_open(log, &file, path, + fd, TRUE); + if (ret > 0) + break; + + /* error / corrupted */ + if (ret == 0) + mail_transaction_log_file_free(file); + else { + if (errno == ESTALE && + i < MAIL_INDEX_ESTALE_RETRY_COUNT) { + /* try again */ + continue; + } + } + return NULL; + } mail_transaction_log_file_add_to_head(file); return file; @@ -807,7 +856,8 @@ { struct mail_transaction_log_file *file; const char *path = log->head->filepath; - struct stat st; + struct stat st; + bool retry; int fd; i_assert(log->head->locked); @@ -815,6 +865,8 @@ if (MAIL_INDEX_IS_IN_MEMORY(log->index)) file = mail_transaction_log_file_alloc_in_memory(log); else { + /* we're locked, we shouldn't need to worry about ESTALE + problems in here. */ if (fstat(log->head->fd, &st) < 0) { mail_index_file_set_syscall_error(log->index, path, "fstat()"); @@ -827,10 +879,16 @@ if (fd == -1) return -1; - file = mail_transaction_log_file_fd_open_or_create(log, - path, fd); - if (file == NULL) - return -1; + file = mail_transaction_log_file_fd_open_or_create(log, path, + fd, &retry); + if (file == NULL) { + if (retry) { + mail_index_set_error(log->index, + "%s: ESTALE unexpected while locked", + path); + } + return -1; + } } if (lock) { @@ -920,7 +978,7 @@ /* see if we have it in log.2 file */ path = t_strconcat(log->index->filepath, MAIL_TRANSACTION_LOG_SUFFIX".2", NULL); - fd = open(path, O_RDWR); + fd = safe_open(path, O_RDWR); if (fd == -1) { if (errno == ENOENT) return 0; @@ -930,7 +988,12 @@ } if (fstat(fd, &st) < 0) { - mail_index_file_set_syscall_error(log->index, path, "fstat()"); + if (errno == ESTALE) { + /* treat as "doesn't exist" */ + (void)close(fd); + return 0; + } + mail_index_file_set_syscall_error(log->index, path, "fstat()"); return -1; } @@ -954,7 +1017,11 @@ } mail_transaction_log_file_free(file); return 0; - } + } + if (errno == ESTALE) { + /* treat as "doesn't exist" */ + return 0; + } return -1; }
--- a/src/lib-storage/index/maildir/maildir-keywords.c Thu Feb 16 16:58:31 2006 +0200 +++ b/src/lib-storage/index/maildir/maildir-keywords.c Thu Feb 16 17:23:00 2006 +0200 @@ -83,6 +83,10 @@ const char **strp; int fd, idx; + /* Remember that we rely on uidlist file locking in here. That's why + we rely on stat()'s timestamp and don't bother handling ESTALE + errors. */ + if (stat(mk->path, &st) < 0) { if (errno == ENOENT) { maildir_keywords_clear(mk);
--- a/src/lib-storage/index/maildir/maildir-uidlist.c Thu Feb 16 16:58:31 2006 +0200 +++ b/src/lib-storage/index/maildir/maildir-uidlist.c Thu Feb 16 17:23:00 2006 +0200 @@ -7,6 +7,8 @@ #include "istream.h" #include "str.h" #include "file-dotlock.h" +#include "close-keep-errno.h" +#include "safe-open.h" #include "write-full.h" #include "maildir-storage.h" #include "maildir-uidlist.h" @@ -16,6 +18,10 @@ #include <sys/stat.h> #include <utime.h> +/* NFS: How many times to retry reading dovecot-uidlist file if ESTALE + error occurs in the middle of reading it */ +#define UIDLIST_ESTALE_RETRY_COUNT 10 + /* how many seconds to wait before overriding uidlist.lock */ #define UIDLIST_LOCK_STALE_TIMEOUT (60*2) @@ -240,7 +246,8 @@ return 1; } -int maildir_uidlist_update(struct maildir_uidlist *uidlist) +static int +maildir_uidlist_update_read(struct maildir_uidlist *uidlist, bool *retry_r) { struct mail_storage *storage = STORAGE(uidlist->mbox->storage); const char *line; @@ -249,35 +256,25 @@ struct stat st; int fd, ret; - if (uidlist->last_mtime != 0) { - if (stat(uidlist->fname, &st) < 0) { - if (errno != ENOENT) { - mail_storage_set_critical(storage, - "stat(%s) failed: %m", uidlist->fname); - return -1; - } - return 0; - } + *retry_r = FALSE; - if (st.st_mtime == uidlist->last_mtime) { - /* unchanged */ - return 1; - } - } - - fd = open(uidlist->fname, O_RDONLY); + fd = safe_open(uidlist->fname, O_RDONLY); if (fd == -1) { if (errno != ENOENT) { mail_storage_set_critical(storage, "open(%s) failed: %m", uidlist->fname); return -1; } - uidlist->initial_read = TRUE; return 0; } if (fstat(fd, &st) < 0) { - mail_storage_set_critical(storage, + close_keep_errno(fd); + if (errno == ESTALE) { + *retry_r = TRUE; + return -1; + } + mail_storage_set_critical(storage, "fstat(%s) failed: %m", uidlist->fname); return -1; } @@ -295,10 +292,13 @@ /* get header */ line = i_stream_read_next_line(input); - if (line == NULL || sscanf(line, "%u %u %u", &uidlist->version, - &uid_validity, &next_uid) != 3 || - uidlist->version < 1 || uidlist->version > 2) { - /* broken file */ + if (line == NULL) { + /* I/O error / empty file */ + ret = input->stream_errno == 0 ? 0 : -1; + } else if (sscanf(line, "%u %u %u", &uidlist->version, + &uid_validity, &next_uid) != 3 || + uidlist->version < 1 || uidlist->version > 2) { + /* broken file */ mail_storage_set_critical(storage, "Corrupted header in file %s (version = %u)", uidlist->fname, uidlist->version); @@ -320,19 +320,62 @@ ret = 0; break; } + } + if (input->stream_errno != 0) + ret = -1; + } + + if (ret == 0) { + /* file is broken */ + (void)unlink(uidlist->fname); + uidlist->last_mtime = 0; + } else if (ret > 0) { + /* success */ + uidlist->last_mtime = st.st_mtime; + } else { + /* I/O error */ + if (input->stream_errno == ESTALE) + *retry_r = TRUE; + } + + i_stream_unref(&input); + return ret; +} + +int maildir_uidlist_update(struct maildir_uidlist *uidlist) +{ + struct mail_storage *storage = STORAGE(uidlist->mbox->storage); + struct stat st; + unsigned int i; + bool retry; + int ret; + + if (uidlist->last_mtime != 0) { + if (stat(uidlist->fname, &st) < 0) { + if (errno != ENOENT) { + mail_storage_set_critical(storage, + "stat(%s) failed: %m", uidlist->fname); + return -1; + } + return 0; + } + + if (st.st_mtime == uidlist->last_mtime) { + /* unchanged */ + return 1; } } - if (ret != 0) - uidlist->last_mtime = st.st_mtime; - else { - (void)unlink(uidlist->fname); - uidlist->last_mtime = 0; - } - - i_stream_unref(&input); - uidlist->initial_read = TRUE; - return ret; + for (i = 0; ; i++) { + ret = maildir_uidlist_update_read(uidlist, &retry); + if (!retry || i == UIDLIST_ESTALE_RETRY_COUNT) { + if (ret >= 0) + uidlist->initial_read = TRUE; + break; + } + /* ESTALE - try reopening and rereading */ + } + return ret; } static const struct maildir_uidlist_rec *
--- a/src/lib-storage/subscription-file/subscription-file.c Thu Feb 16 16:58:31 2006 +0200 +++ b/src/lib-storage/subscription-file/subscription-file.c Thu Feb 16 17:23:00 2006 +0200 @@ -3,6 +3,7 @@ #include "lib.h" #include "istream.h" #include "ostream.h" +#include "safe-open.h" #include "file-dotlock.h" #include "mail-storage-private.h" #include "subscription-file.h" @@ -12,6 +13,7 @@ #define MAX_MAILBOX_LENGTH PATH_MAX +#define SUBSCRIPTION_FILE_ESTALE_RETRY_COUNT 10 #define SUBSCRIPTION_FILE_LOCK_TIMEOUT 120 #define SUBSCRIPTION_FILE_CHANGE_TIMEOUT 30 @@ -40,17 +42,23 @@ } static const char *next_line(struct mail_storage *storage, const char *path, - struct istream *input, bool *failed) + struct istream *input, bool *failed_r) { const char *line; - *failed = FALSE; + *failed_r = FALSE; if (input == NULL) return NULL; while ((line = i_stream_next_line(input)) == NULL) { - switch (i_stream_read(input)) { + switch (i_stream_read(input)) { case -1: + if (input->stream_errno != 0 && + input->stream_errno != ESTALE) { + subsfile_set_syscall_error(storage, + "read()", path); + *failed_r = TRUE; + } return NULL; case -2: /* mailbox name too large */ @@ -58,7 +66,7 @@ "Subscription file %s contains lines longer " "than %u characters", path, MAX_MAILBOX_LENGTH); - *failed = TRUE; + *failed_r = TRUE; return NULL; } } @@ -85,7 +93,6 @@ dotlock_set.timeout = SUBSCRIPTION_FILE_LOCK_TIMEOUT; dotlock_set.stale_timeout = SUBSCRIPTION_FILE_CHANGE_TIMEOUT; - /* FIXME: set lock notification callback */ fd_out = file_dotlock_open(&dotlock_set, path, 0, &dotlock); if (fd_out == -1) { if (errno == EAGAIN) { @@ -98,7 +105,7 @@ return -1; } - fd_in = open(path, O_RDONLY); + fd_in = safe_open(path, O_RDONLY); if (fd_in == -1 && errno != ENOENT) { subsfile_set_syscall_error(storage, "open()", path); (void)file_dotlock_delete(&dotlock); @@ -165,7 +172,7 @@ pool_t pool; int fd; - fd = open(path, O_RDONLY); + fd = safe_open(path, O_RDONLY); if (fd == -1 && errno != ENOENT) { subsfile_set_syscall_error(storage, "open()", path); return NULL; @@ -196,8 +203,42 @@ const char *subsfile_list_next(struct subsfile_list_context *ctx) { - if (ctx->failed || ctx->input == NULL) + const char *line; + unsigned int i; + int fd; + + if (ctx->failed || ctx->input == NULL) return NULL; - return next_line(ctx->storage, ctx->path, ctx->input, &ctx->failed); + for (i = 0;; i++) { + line = next_line(ctx->storage, ctx->path, ctx->input, + &ctx->failed); + if (ctx->input->stream_errno != ESTALE || + i == SUBSCRIPTION_FILE_ESTALE_RETRY_COUNT) + break; + + /* Reopen the subscription file and re-send everything. + this isn't the optimal behavior, but it's allowed by + IMAP and this way we don't have to read everything into + memory or try to play any guessing games. */ + i_stream_unref(&ctx->input); + + fd = safe_open(ctx->path, O_RDONLY); + if (fd == -1) { + /* In case of ENOENT all the subscriptions got lost. + Just return end of subscriptions list in that + case. */ + if (errno != ENOENT) { + subsfile_set_syscall_error(ctx->storage, + "open()", + ctx->path); + ctx->failed = TRUE; + } + return NULL; + } + + ctx->input = i_stream_create_file(fd, ctx->pool, + MAX_MAILBOX_LENGTH, TRUE); + } + return line; }
--- a/src/lib/Makefile.am Thu Feb 16 16:58:31 2006 +0200 +++ b/src/lib/Makefile.am Thu Feb 16 17:23:00 2006 +0200 @@ -65,6 +65,7 @@ restrict-process-size.c \ safe-memset.c \ safe-mkdir.c \ + safe-open.c \ sendfile-util.c \ seq-range-array.c \ sha1.c \ @@ -133,6 +134,7 @@ restrict-process-size.h \ safe-memset.h \ safe-mkdir.h \ + safe-open.h \ sendfile-util.h \ seq-range-array.h \ sha1.h \
--- a/src/lib/file-cache.c Thu Feb 16 16:58:31 2006 +0200 +++ b/src/lib/file-cache.c Thu Feb 16 17:23:00 2006 +0200 @@ -107,8 +107,9 @@ more than needed */ struct stat st; - if (fstat(cache->fd, &st) < 0) { - i_error("fstat(file_cache) failed: %m"); + if (fstat(cache->fd, &st) < 0) { + if (errno != ESTALE) + i_error("fstat(file_cache) failed: %m"); return -1; }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/src/lib/safe-open.c Thu Feb 16 17:23:00 2006 +0200 @@ -0,0 +1,50 @@ +/* Copyright (c) 2006 Timo Sirainen */ + +#include "lib.h" +#include "safe-open.h" + +#include <fcntl.h> +#include <unistd.h> +#include <sys/stat.h> + +#define NFS_OPEN_RETRY_COUNT 10 + +int safe_open(const char *path, int flags) +{ + const char *dir = NULL; + struct stat st; + unsigned int i; + int fd; + + i_assert((flags & O_CREAT) == 0); + + t_push(); + for (i = 1;; i++) { + fd = open(path, flags); + if (fd != -1 || errno != ESTALE || i == NFS_OPEN_RETRY_COUNT) + break; + + /* ESTALE: Some operating systems may fail with this if they + can't internally revalidating the NFS handle. It may also + happen if the parent directory has been deleted. If the + directory still exists, try reopening the file. */ + if (dir == NULL) { + dir = strrchr(path, '/'); + if (dir == NULL) + break; + dir = t_strdup_until(path, dir); + } + if (stat(dir, &st) < 0) { + /* maybe it's gone or something else bad happened to + it. in any case we can't open the file, so fail + with the original ESTALE error and let our caller + handle it. */ + errno = ESTALE; + break; + } + + /* directory still exists, try reopening */ + } + t_pop(); + return fd; +}