MESSAGE quota resource implemention
gnb at fastmail.fm
Thu Sep 1 04:15:12 EDT 2011
Julien Coloos wrote:
> As discussed on IRC last week, I worked on implementing MESSAGE quota
> resource in cyrus (see RFC 2087; STORAGE resource already being handled).
> I created a branch based on Greg's 'annotate' one on github, since his
> work on annotation storage management made mine a lot easier.
> Details on the changes I made:
> - added MESSAGE quota resource management
> -> updated cunit test
> -> added 'quotawarnmsg' option which behaviour is similar to
> 'quotawarnkb'; warning message shown when selecting folder in IMAP
> tells which resource limit was triggered
> -> added 'autocreatequotamsg' option, to set MESSAGE limit
> (unlimited by default) when user auto-creates its mailbox
> - xfer transfers all non-unlimited resources
> - added helper function to compute annotation storage usage for a
> given mailbox
> - changed the way quota entries are read/written
> -> resources presence in fetched entry is remembered
> -> when writing entry, only present resources are written
> -> when setting resources limits, entries which were not present
> upon fetching are now marked as present and their current usage is
> - quota utility lists and computes (-f) all resources associated to
> The branch 'quotamessage/gnb/annotate' is available here:
> git://github.com/worldline-messaging/cyrus-imapd.git. It is based on
> Greg's 'annotate' branch on github.
commit 2d4fb0 "Added quota MESSAGE resource (RFC 2087) management."
Why #include <stdint.h>? It's not like we're using any of the typedefs
there. There's a good argument to be made that we *should* do so, but
until then the #include doesn't seem useful.
I'm unconvinced about the need for quota.sets. After a lot of reading
it seems to me that (quota.sets[x]) and (quota.limits[x] !=
QUOTA_UNLIMITED) are identical or should be. Also, are you setting
sets anywhere except when loading from the db?
I like quota_update_useds(), it does seem necessary to be able to update
several usages together.
Otherwise, looks good.
In quota_write(), your sanity check should goto the buf_free() at the
end of the function rather than return early. Otherwise, looks good.
In the 1st hunk in cmd_append(), at this point in the code I believe
totalsize = 0, so you could easily pass 0 for the new last argument as well.
Otherwise looks good.
BTW, reading your code I realised a number of places I failed to account
for X-ANNOTATION-STORAGE quota properly :( I think I'll have to change
a number of those new pairs of quota_t arguments to a quota_t
I'm not sure it's a good idea to set mailbox->i.exists = 0 in
mailbox_delete(). Perhaps it would be better to check for
(mailbox->i.options & OPT_MAILBOX_DELETED) when sampling
mailbox->i.exists in mailbox_commit_quota().
Otherwise, looks good.
commit 0e613c "Removed unused code."
commit c8aeef "Compute each quota resource upon setting it for the first
commit b30c10 "List and compute all set quota resources."
Oh, here's why you include stdint.h: you're using PRIdMAX and not
QUOTA_REPORT_FMT, which is used only in imapd.c.
I'm thinking that there's now three places in the code which take a
mailbox* and fill out an array of quota diffs, interpreting the contents
of the struct mailbox. That should really be centralised.
More information about the Cyrus-devel