From: Timo Sirainen Date: Fri, 20 Aug 2021 14:43:09 +0000 (+0300) Subject: lib-storage: Return reason string in mailbox_vfuncs.list_index_has_changed() X-Git-Tag: 2.3.18~79 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=15f25bf7ad4331249dfc1564a13e3762ed29e616;p=thirdparty%2Fdovecot%2Fcore.git lib-storage: Return reason string in mailbox_vfuncs.list_index_has_changed() The callers can use it to log why the list index had changed. --- diff --git a/src/lib-storage/index/index-storage.h b/src/lib-storage/index/index-storage.h index 43e7d24d6f..55cf459b34 100644 --- a/src/lib-storage/index/index-storage.h +++ b/src/lib-storage/index/index-storage.h @@ -168,11 +168,12 @@ bool index_keyword_array_cmp(const ARRAY_TYPE(keyword_indexes) *k1, int index_storage_list_index_has_changed(struct mailbox *box, struct mail_index_view *list_view, - uint32_t seq, bool quick); + uint32_t seq, bool quick, + const char **reason_r); enum index_storage_list_change index_storage_list_index_has_changed_full(struct mailbox *box, struct mail_index_view *list_view, - uint32_t seq); + uint32_t seq, const char **reason_r); void index_storage_list_index_update_sync(struct mailbox *box, struct mail_index_transaction *trans, uint32_t seq); diff --git a/src/lib-storage/index/index-sync.c b/src/lib-storage/index/index-sync.c index 16707d24cf..73ea99c03b 100644 --- a/src/lib-storage/index/index-sync.c +++ b/src/lib-storage/index/index-sync.c @@ -432,7 +432,7 @@ index_list_get_ext_id(struct mailbox *box, struct mail_index_view *view) enum index_storage_list_change index_storage_list_index_has_changed_full(struct mailbox *box, struct mail_index_view *list_view, - uint32_t seq) + uint32_t seq, const char **reason_r) { const struct index_storage_list_index_record *rec; const void *data; @@ -442,15 +442,28 @@ index_storage_list_index_has_changed_full(struct mailbox *box, bool expunged; int ret; - if (mail_index_is_in_memory(mail_index_view_get_index(list_view))) + *reason_r = NULL; + + if (mail_index_is_in_memory(mail_index_view_get_index(list_view))) { + *reason_r = "List index is in memory"; return INDEX_STORAGE_LIST_CHANGE_INMEMORY; + } ext_id = index_list_get_ext_id(box, list_view); mail_index_lookup_ext(list_view, seq, ext_id, &data, &expunged); rec = data; - if (rec == NULL || expunged || rec->size == 0 || rec->mtime == 0) { - /* doesn't exist / not synced */ + if (rec == NULL) { + *reason_r = "Storage record is missing"; + return INDEX_STORAGE_LIST_CHANGE_NORECORD; + } else if (expunged) { + *reason_r = "Storage record is expunged"; + return INDEX_STORAGE_LIST_CHANGE_NORECORD; + } else if (rec->size == 0) { + *reason_r = "Storage record size=0"; + return INDEX_STORAGE_LIST_CHANGE_NORECORD; + } else if (rec->mtime == 0) { + *reason_r = "Storage record mtime=0"; return INDEX_STORAGE_LIST_CHANGE_NORECORD; } if (box->storage->set->mailbox_list_index_very_dirty_syncs) @@ -463,23 +476,35 @@ index_storage_list_index_has_changed_full(struct mailbox *box, path = t_strconcat(dir, "/", box->index_prefix, ".log", NULL); if (stat(path, &st) < 0) { - if (errno == ENOENT) + if (errno == ENOENT) { + *reason_r = t_strdup_printf("%s not found", path); return INDEX_STORAGE_LIST_CHANGE_NOT_IN_FS; + } mailbox_set_critical(box, "stat(%s) failed: %m", path); return INDEX_STORAGE_LIST_CHANGE_ERROR; } - if (rec->size != (st.st_size & 0xffffffffU)) + uint32_t new_size = st.st_size & 0xffffffffU; + if (rec->size != new_size) { + *reason_r = t_strdup_printf("Storage size changed %u != %u", + rec->size, new_size); return INDEX_STORAGE_LIST_CHANGE_SIZE_CHANGED; - if (rec->mtime != (st.st_mtime & 0xffffffffU)) + } + uint32_t new_mtime = st.st_mtime & 0xffffffffU; + if (rec->mtime != new_mtime) { + *reason_r = t_strdup_printf("Storage mtime changed %u != %u", + rec->mtime, new_mtime); return INDEX_STORAGE_LIST_CHANGE_MTIME_CHANGED; + } return INDEX_STORAGE_LIST_CHANGE_NONE; } int index_storage_list_index_has_changed(struct mailbox *box, struct mail_index_view *list_view, - uint32_t seq, bool quick ATTR_UNUSED) + uint32_t seq, bool quick ATTR_UNUSED, + const char **reason_r) { - switch (index_storage_list_index_has_changed_full(box, list_view, seq)) { + switch (index_storage_list_index_has_changed_full(box, list_view, seq, + reason_r)) { case INDEX_STORAGE_LIST_CHANGE_ERROR: return -1; case INDEX_STORAGE_LIST_CHANGE_NONE: diff --git a/src/lib-storage/index/maildir/maildir-sync-index.c b/src/lib-storage/index/maildir/maildir-sync-index.c index 492d71cd62..da0e40c262 100644 --- a/src/lib-storage/index/maildir/maildir-sync-index.c +++ b/src/lib-storage/index/maildir/maildir-sync-index.c @@ -693,7 +693,8 @@ maildir_list_get_ext_id(struct maildir_mailbox *mbox, int maildir_list_index_has_changed(struct mailbox *box, struct mail_index_view *list_view, - uint32_t seq, bool quick) + uint32_t seq, bool quick, + const char **reason_r) { struct maildir_mailbox *mbox = MAILDIR_MAILBOX(box); const struct maildir_list_index_record *rec; @@ -704,7 +705,8 @@ int maildir_list_index_has_changed(struct mailbox *box, bool expunged; int ret; - ret = index_storage_list_index_has_changed(box, list_view, seq, quick); + ret = index_storage_list_index_has_changed(box, list_view, seq, + quick, reason_r); if (ret != 0 || box->storage->set->mailbox_list_index_very_dirty_syncs) return ret; if (mbox->storage->set->maildir_very_dirty_syncs) { @@ -716,9 +718,19 @@ int maildir_list_index_has_changed(struct mailbox *box, mail_index_lookup_ext(list_view, seq, ext_id, &data, &expunged); rec = data; - if (rec == NULL || expunged || - rec->new_mtime == 0 || rec->cur_mtime == 0) { - /* doesn't exist, not synced or dirty-synced */ + if (rec == NULL) { + *reason_r = "Maildir record is missing"; + return 1; + } else if (expunged) { + *reason_r = "Maildir record is expunged"; + return 1; + } else if (rec->new_mtime == 0) { + /* not synced */ + *reason_r = "Maildir record new_mtime=0"; + return 1; + } else if (rec->cur_mtime == 0) { + /* dirty-synced */ + *reason_r = "Maildir record cur_mtime=0"; return 1; } @@ -734,8 +746,12 @@ int maildir_list_index_has_changed(struct mailbox *box, mailbox_set_critical(box, "stat(%s) failed: %m", new_dir); return -1; } - if ((time_t)rec->new_mtime != st.st_mtime) + if ((time_t)rec->new_mtime != st.st_mtime) { + *reason_r = t_strdup_printf( + "Maildir new_mtime changed %u != %"PRIdTIME_T, + rec->new_mtime, st.st_mtime); return 1; + } /* check if cur/ changed */ cur_dir = t_strconcat(root_dir, "/cur", NULL); @@ -743,8 +759,12 @@ int maildir_list_index_has_changed(struct mailbox *box, mailbox_set_critical(box, "stat(%s) failed: %m", cur_dir); return -1; } - if ((time_t)rec->cur_mtime != st.st_mtime) + if ((time_t)rec->cur_mtime != st.st_mtime) { + *reason_r = t_strdup_printf( + "Maildir cur_mtime changed %u != %"PRIdTIME_T, + rec->cur_mtime, st.st_mtime); return 1; + } return 0; } diff --git a/src/lib-storage/index/maildir/maildir-sync.h b/src/lib-storage/index/maildir/maildir-sync.h index b059fbd659..9bc6db4ac9 100644 --- a/src/lib-storage/index/maildir/maildir-sync.h +++ b/src/lib-storage/index/maildir/maildir-sync.h @@ -50,7 +50,8 @@ int maildir_sync_lookup(struct maildir_mailbox *mbox, uint32_t uid, int maildir_list_index_has_changed(struct mailbox *box, struct mail_index_view *list_view, - uint32_t seq, bool quick); + uint32_t seq, bool quick, + const char **reason_r); void maildir_list_index_update_sync(struct mailbox *box, struct mail_index_transaction *trans, uint32_t seq); diff --git a/src/lib-storage/index/mbox/mbox-sync-list-index.c b/src/lib-storage/index/mbox/mbox-sync-list-index.c index ae0dcd832b..934b7ea5d4 100644 --- a/src/lib-storage/index/mbox/mbox-sync-list-index.c +++ b/src/lib-storage/index/mbox/mbox-sync-list-index.c @@ -20,7 +20,7 @@ mbox_list_get_ext_id(struct mbox_mailbox *mbox, int mbox_list_index_has_changed(struct mailbox *box, struct mail_index_view *list_view, - uint32_t seq, bool quick) + uint32_t seq, bool quick, const char **reason_r) { struct mbox_mailbox *mbox = MBOX_MAILBOX(box); const struct mbox_list_index_record *rec; @@ -31,7 +31,8 @@ int mbox_list_index_has_changed(struct mailbox *box, bool expunged; int ret; - ret = index_storage_list_index_has_changed(box, list_view, seq, quick); + ret = index_storage_list_index_has_changed(box, list_view, seq, + quick, reason_r); if (ret != 0 || box->storage->set->mailbox_list_index_very_dirty_syncs) return ret; @@ -39,8 +40,15 @@ int mbox_list_index_has_changed(struct mailbox *box, mail_index_lookup_ext(list_view, seq, ext_id, &data, &expunged); rec = data; - if (rec == NULL || expunged || rec->mtime == 0) { - /* doesn't exist or not synced */ + if (rec == NULL) { + *reason_r = "mbox record is missing"; + return 1; + } else if (expunged) { + *reason_r = "mbox record is expunged"; + return 1; + } else if (rec->mtime == 0) { + /* not synced */ + *reason_r = "mbox record mtime=0"; return 1; } @@ -53,9 +61,18 @@ int mbox_list_index_has_changed(struct mailbox *box, mailbox_set_critical(box, "stat(%s) failed: %m", path); return -1; } - if ((time_t)rec->mtime != st.st_mtime || - rec->size != (uint32_t)(st.st_size & 0xffffffffU)) + if ((time_t)rec->mtime != st.st_mtime) { + *reason_r = t_strdup_printf( + "mbox record mtime changed %u != %"PRIdTIME_T, + rec->mtime, st.st_mtime); return 1; + } + uint32_t new_size = (uint32_t)(st.st_size & 0xffffffffU); + if (rec->size != new_size) { + *reason_r = t_strdup_printf("mbox record size changed %u != %u", + rec->size, new_size); + return 1; + } return 0; } diff --git a/src/lib-storage/index/mbox/mbox-sync-private.h b/src/lib-storage/index/mbox/mbox-sync-private.h index 60fb952466..7585e4df41 100644 --- a/src/lib-storage/index/mbox/mbox-sync-private.h +++ b/src/lib-storage/index/mbox/mbox-sync-private.h @@ -183,7 +183,8 @@ int mbox_sync_get_guid(struct mbox_mailbox *mbox); int mbox_list_index_has_changed(struct mailbox *box, struct mail_index_view *list_view, - uint32_t seq, bool quick); + uint32_t seq, bool quick, + const char **reason_r); void mbox_list_index_update_sync(struct mailbox *box, struct mail_index_transaction *trans, uint32_t seq); diff --git a/src/lib-storage/list/mailbox-list-index-status.c b/src/lib-storage/list/mailbox-list-index-status.c index c4d6d3c85e..4218ae78cf 100644 --- a/src/lib-storage/list/mailbox-list-index-status.c +++ b/src/lib-storage/list/mailbox-list-index-status.c @@ -813,7 +813,8 @@ void mailbox_list_index_status_set_info_flags(struct mailbox *box, uint32_t uid, ret = 1; } else { ret = box->v.list_index_has_changed == NULL ? 0 : - box->v.list_index_has_changed(box, view, seq, TRUE); + box->v.list_index_has_changed(box, view, seq, TRUE, + &reason); } if (ret != 0) { diff --git a/src/lib-storage/list/mailbox-list-index.c b/src/lib-storage/list/mailbox-list-index.c index b5af436dd2..2ba8861d1c 100644 --- a/src/lib-storage/list/mailbox-list-index.c +++ b/src/lib-storage/list/mailbox-list-index.c @@ -775,8 +775,9 @@ int mailbox_list_index_view_open(struct mailbox *box, bool require_refreshed, ret = 0; } else { ret = box->v.list_index_has_changed == NULL ? 0 : - box->v.list_index_has_changed(box, view, seq, FALSE); - reason = "Mailbox has changed"; + box->v.list_index_has_changed(box, view, seq, FALSE, + &reason); + i_assert(ret <= 0 || reason != NULL); } if (ret != 0) { diff --git a/src/lib-storage/mail-storage-private.h b/src/lib-storage/mail-storage-private.h index eca4b5f113..846e2b155b 100644 --- a/src/lib-storage/mail-storage-private.h +++ b/src/lib-storage/mail-storage-private.h @@ -261,10 +261,12 @@ struct mailbox_vfuncs { /* Lookup sync extension record and figure out if it mailbox has changed since. Returns 1 = yes, 0 = no, -1 = error. if quick==TRUE, - return 1 if it's too costly to find out exactly. */ + return 1 if it's too costly to find out exactly. The reason_r is + set if 1 is returned. */ int (*list_index_has_changed)(struct mailbox *box, struct mail_index_view *list_view, - uint32_t seq, bool quick); + uint32_t seq, bool quick, + const char **reason_r); /* Update the sync extension record. */ void (*list_index_update_sync)(struct mailbox *box, struct mail_index_transaction *trans, diff --git a/src/plugins/virtual/virtual-storage.c b/src/plugins/virtual/virtual-storage.c index a57f4f4151..f88a279752 100644 --- a/src/plugins/virtual/virtual-storage.c +++ b/src/plugins/virtual/virtual-storage.c @@ -857,11 +857,13 @@ static bool virtual_is_inconsistent(struct mailbox *box) static int virtual_list_index_has_changed(struct mailbox *box ATTR_UNUSED, struct mail_index_view *list_view ATTR_UNUSED, - uint32_t seq ATTR_UNUSED, bool quick ATTR_UNUSED) + uint32_t seq ATTR_UNUSED, bool quick ATTR_UNUSED, + const char **reason_r) { /* we don't have any quick and easy optimizations for tracking virtual folders. ideally we'd completely disable mailbox list indexes for them, but this is the easiest way to do it for now. */ + *reason_r = "Virtual indexes always change"; return 1; }