Re: [PATCH] : Inversion for options

On Wed, 5 Dec 2007, Willy Tarreau wrote:

> Hi Krzysztof,
>
> On Tue, Dec 04, 2007 at 11:18:49PM +0100, Krzysztof Oledzki wrote:
>> [RFC]: Inversion for options
>>
>> This RFC-quality patch adds a possibility to invert most of
>> available options. It allows to use all options from a current
>> defaults section except selected ones, for example:
>>
>> --- cut here ---
>> defaults
>> contimeout 4200
>> clitimeout 50000
>> srvtimeout 40000
>> option contstats
>>
>> listen stats 1.2.3.4:80
>> option !contstats
>
> I've already felt the need for such a negation. However, I was
> thinking about a different syntax, prefixing the statement with
> the "no" keyword, just like you have on cisco for instance. It
> will have a lot of sense when we'll be able to access the conf
> via a CLI, and it will even be possible to disable/remove other
> statements that way (eg: no log, no server xxx, ...).

So, something like "no option contstats" will satisfy you? OK, this can be done. ;)

>> --- cut here ---
>>
>> The patch splits val into val (actual binary option id) and ival (implied option id)
>> and moves ival and last_checks calculation at the and of the readcfgfile() function
>> so it is possible to invert forceclose without breaking httpclose (and vice versa) and
>> to invert tcpsplice in one proxy but to keep a proper last_checks value when tcpsplice
>> is used in another proxy.
>
> At first glance, that makes sense. But since it's used only for forceclose,
> I think it should be processed in a simpler way: let's only set one bit per
> option in the cfg_opts[], and check for PR_O_FORCE_CLO too everywhere we check
> for PR_O_HTTP_CLOSE (only 4 places). This would make a lot more sense given
> the fact that options are just booleans, and it would definitely remove all
> those complex calculations.

But we still need this complex calculations for last_checks. If you wish I can remove ival and fix PR_O_FORCE_CLO but I think we will need such functionality some day anyway...

>> I also decided to depreciate "redisp" and "redispatch"
>> keywords as it is IMHO better to use "option redispatch" which can be inverted.
>
> Agreed. I think we will need some general purpose function to deprecate such
> keywords and provide hints for their replacement.

Right. Are there more keywords we need to mark deprecated?

>> Some useful documentation were added and at the same time I sorted (alfabetically)
>> all valid options both in the code and documentation.
>
> OK.
>
> I'm releasing 1.3.14 today, but will probably merge your (updated) work in
> 1.3.14.1 as it's non-intrusive and almost riskless.

OK. Thank you.

Best regards,

                                 Krzysztof Olêdzki Received on 2007/12/05 23:06