On Tue, Aug 2, 2016, at 02:14 PM, Kenneth Marshall via Info-cyrus wrote:
> On Tue, Aug 02, 2016 at 12:34:13PM +1000, Bron Gondwana wrote:
> > 2.3.2 wasn't version 10, it was version 7! It would have upgraded through 8, 9, 10 - and maybe you needed to reconstruct to get GUIDs for those versions - that sounds familiar.
> >
> > Bron.
> >
>
> Hi Bron,
>
> Is there a way to identify mailboxes with no GUID? Then I could target
> them
> for reconstruction first.
>
> Regards,
> Ken
Hi Ken,
I don't suppose you still have mailboxes with no GUID? Or have you
already found and "reconstruct -G"ed everything?
I've attached a patch that does two things, which are kind of the same
thing:
* If the mailbox on either end of the replication is of a version < 10,
then the operation will fail cleanly and early with an "Operation is not
supported on mailbox" error (rather than trying to do the replication,
then crashing out like it currently does) -- though I don't believe this
case will affect you, as I believe your mailboxes are all at least
version 10.
* If the mailbox on either end of the replication contains any index
records that do not have a GUID set, then the same will occur, and (in
addition to the above error) a warning will be logged to syslog like:
<mailbox>: missing guid for record <number> -- needs 'reconstruct
-G'?
So with this patch, you should be able to avoid these crashes, and also
identify mailboxes that need "reconstruct -G" just by checking for the
warning in syslog.
Are you able to try this out?
Cheers,
ellie
diff --git a/imap/sync_client.c b/imap/sync_client.c
index 2663301..8408380 100644
--- a/imap/sync_client.c
+++ b/imap/sync_client.c
@@ -212,6 +212,7 @@ static int find_reserve_all(struct sync_name_list *mboxname_list,
* USE the value... the whole "add to master folders" actually
* looks a bit pointless... */
r = mailbox_open_iwl(mbox->name, &mailbox);
+ if (!r) r = sync_mailbox_version_check(&mailbox);
/* Quietly skip over folders which have been deleted since we
started working (but record fact in case caller cares) */
@@ -1298,6 +1299,7 @@ static int mailbox_full_update(const char *mboxname)
/* we'll be updating it! */
r = mailbox_open_iwl(mboxname, &mailbox);
+ if (!r) r = sync_mailbox_version_check(&mailbox);
if (r) goto done;
/* if local UIDVALIDITY is lower, copy from remote, otherwise
@@ -1468,6 +1470,7 @@ static int update_mailbox_once(struct sync_folder *local,
annotate_state_t *astate = NULL;
r = mailbox_open_iwl(local->name, &mailbox);
+ if (!r) r = sync_mailbox_version_check(&mailbox);
if (r == IMAP_MAILBOX_NONEXISTENT) {
/* been deleted in the meanwhile... it will get picked up by the
* delete call later */
@@ -1946,6 +1949,7 @@ static int do_mailbox_info(void *rock,
/* XXX - check for deleted? */
r = mailbox_open_irl(name, &mailbox);
+ if (!r) r = sync_mailbox_version_check(&mailbox);
/* doesn't exist? Probably not finished creating or removing yet */
if (r == IMAP_MAILBOX_NONEXISTENT) {
r = 0;
diff --git a/imap/sync_server.c b/imap/sync_server.c
index c49df9f..51daf94 100644
--- a/imap/sync_server.c
+++ b/imap/sync_server.c
@@ -1052,7 +1052,7 @@ static void reserve_folder(const char *part, const char *mboxname,
/* Open and lock mailbox */
r = mailbox_open_irl(mboxname, &mailbox);
-
+ if (!r) r = sync_mailbox_version_check(&mailbox);
if (r) return;
for (recno = 1; recno <= mailbox->i.num_records; recno++) {
@@ -1437,6 +1437,7 @@ static int do_mailbox(struct dlist *kin)
mbtype = mboxlist_string_to_mbtype(mboxtype);
r = mailbox_open_iwl(mboxname, &mailbox);
+ if (!r) r = sync_mailbox_version_check(&mailbox);
if (r == IMAP_MAILBOX_NONEXISTENT) {
r = mboxlist_createsync(mboxname, mbtype, partition,
sync_userid, sync_authstate,
@@ -1668,6 +1669,7 @@ static int mailbox_cb(char *name,
* to safely get read-only access to the annotation and
* other "side" databases here */
r = mailbox_open_iwl(name, &mailbox);
+ if (!r) r = sync_mailbox_version_check(&mailbox);
/* doesn't exist? Probably not finished creating or removing yet */
if (r == IMAP_MAILBOX_NONEXISTENT ||
r == IMAP_MAILBOX_RESERVED) {
@@ -1705,6 +1707,7 @@ static int do_getfullmailbox(struct dlist *kin)
* don't have a good way to express that, so we use
* write locks anyway */
r = mailbox_open_iwl(kin->sval, &mailbox);
+ if (!r) r = sync_mailbox_version_check(&mailbox);
if (r) goto out;
r = sync_mailbox(mailbox, NULL, NULL, kl, NULL, 1);
@@ -1955,6 +1958,7 @@ static int do_annotation(struct dlist *kin)
mboxname_hiersep_toexternal(sync_namespacep, name, 0);
r = mailbox_open_iwl(name, &mailbox);
+ if (!r) r = sync_mailbox_version_check(&mailbox);
if (r) goto done;
appendattvalue(&attvalues,
@@ -2010,6 +2014,8 @@ static int do_unannotation(struct dlist *kin)
mboxname_hiersep_toexternal(sync_namespacep, name, 0);
r = mailbox_open_iwl(name, &mailbox);
+ if (!r)
+ r = sync_mailbox_version_check(&mailbox);
if (r)
goto done;
@@ -2267,6 +2273,7 @@ static int do_expunge(struct dlist *kin)
return IMAP_PROTOCOL_BAD_PARAMETERS;
r = mailbox_open_iwl(mboxname, &mailbox);
+ if (!r) r = sync_mailbox_version_check(&mailbox);
if (r) goto done;
/* don't want to expunge the wrong mailbox! */
@@ -2342,6 +2349,9 @@ static void print_response(int r)
case IMAP_PROTOCOL_BAD_PARAMETERS:
prot_printf(sync_out, "NO IMAP_PROTOCOL_BAD_PARAMETERS near %s\r\n", dlist_lastkey());
break;
+ case IMAP_MAILBOX_NOTSUPPORTED:
+ prot_printf(sync_out, "NO IMAP_MAILBOX_NOTSUPPORTED Operation is not supported on mailbox\r\n");
+ break;
default:
prot_printf(sync_out, "NO %s\r\n", error_message(r));
}
diff --git a/imap/sync_support.c b/imap/sync_support.c
index f85357c..82e93a1 100644
--- a/imap/sync_support.c
+++ b/imap/sync_support.c
@@ -1497,6 +1497,9 @@ int sync_parse_response(const char *cmd, struct protstream *in,
else if (!strncmp(errmsg.s, "IMAP_PROTOCOL_BAD_PARAMETERS ",
strlen("IMAP_PROTOCOL_BAD_PARAMETERS ")))
return IMAP_PROTOCOL_BAD_PARAMETERS;
+ else if (!strncmp(errmsg.s, "IMAP_MAILBOX_NOTSUPPORTED ",
+ strlen("IMAP_MAILBOX_NOTSUPPORTED ")))
+ return IMAP_MAILBOX_NOTSUPPORTED;
else
return IMAP_REMOTE_DENIED;
}
@@ -1802,3 +1805,39 @@ out:
}
/* ====================================================================== */
+
+int sync_mailbox_version_check(struct mailbox **mailboxp)
+{
+ struct index_record record;
+ uint32_t recno;
+ int r = 0;
+
+ if ((*mailboxp)->i.minor_version < 10) {
+ /* index records will definitely not have guids! */
+ r = IMAP_MAILBOX_NOTSUPPORTED;
+ goto done;
+ }
+
+ /* scan index records to ensure they have guids. version 10 index records
+ * have this field, but it might have never been initialised.
+ * XXX this might be overkill for versions > 10, but let's be cautious */
+ for (recno = 1; recno < (*mailboxp)->i.num_records; recno++) {
+ r = mailbox_read_index_record(*mailboxp, recno, &record);
+ if (r) goto done;
+
+ if (message_guid_isnull(&record.guid)) {
+ syslog(LOG_WARNING, "%s: missing guid for record %u -- needs 'reconstruct -G'?",
+ (*mailboxp)->name, recno);
+ r = IMAP_MAILBOX_NOTSUPPORTED;
+ goto done;
+ }
+ }
+
+done:
+ if (r) {
+ syslog(LOG_DEBUG, "%s: %s failed version check: %s",
+ __func__, (*mailboxp)->name, error_message(r));
+ mailbox_close(mailboxp);
+ }
+ return r;
+}
diff --git a/imap/sync_support.h b/imap/sync_support.h
index ebb81e6..200a9ce 100644
--- a/imap/sync_support.h
+++ b/imap/sync_support.h
@@ -433,4 +433,6 @@ int diff_annotations(const struct sync_annot_list *local_annots,
/* ====================================================================== */
+int sync_mailbox_version_check(struct mailbox **mailboxp);
+
#endif /* INCLUDED_SYNC_SUPPORT_H */
----
Cyrus Home Page: http://www.cyrusimap.org/
List Archives/Info: http://lists.andrew.cmu.edu/pipermail/info-cyrus/
To Unsubscribe:
https://lists.andrew.cmu.edu/mailman/listinfo/info-cyrus
