[Dovecot] quota warning

[Nicolas Boullis]

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