Timo Sirainen wrote:
On 27.2.2007, at 17.21, Nicolas Boullis wrote:
It is configured in the plugin section of the configuration file: quota_warning = 80:0:/usr/local/bin/quota_warning.sh
I've tried to make it possible to support more than message size and message count limits, although I don't know if there ever is going to be more than those two. But I think the configuration should be more like:
quota_warning = storage=80% /usr/local... quota_warning = storage=80%:messages=80% /usr/local...
That would even allow implementing eg. storage=512k or something to give a warning when there is only 512kB of quota left.
OK, that sounds sane, I'll try to fix this (at least the first part that is only a matter of parsing).
As for the code, like you said the quota implementation is rewritten in CVS HEAD so getting this patch to work there would require some changes. And I don't want to add any more new features to v1.0.
I understand your point. On the other hand, I will need warnings with dovecot 1.0, so I think I will first try to implement something for 1.0, and possibly port it afterwards, if you are intersted by the feature.
Some things about the code:
- You're creating a warnings array, but quota_warning_init() supports only a single warning. I guess it could be useful to have multiple ones, so you could setup the rest in eg. quota_warning2, quota_warning3, etc. environments.
Of course. That was planned for future improvements of my patch... ;-) Any way, I guess the warnings should perhaps be rule-specific and/or root-specific, but I urrently get somewhat lost in the code...
- storage_limit should be uint64_t. int holds only 2GB which isn't all that much anymore. :)
In my version, storage_limit is a percentage, so I think an int is large enough for a number between 0 and 100... ;-) Anyway, it might become a size in bytes if something like "storage=512k" is to be supported...
- If quota_warning_init() fails, it could just do i_fatal("quota: Invalid value in quota_warning") or something instead of silently ignoring it.
Sounds sane.
- system() waits for the command execution to finish. I think fork() +exec() would be better.
I tend to disagree here: it might be interesting to catch potential errors while running the command, which requires to wait. On the other hand, if the admin does not care and does not want to wait, a single & at the end of the command would do it.
Cheers,
Nicolas