[Cluster-devel] [PATCH] config: drastically improve cman RRP configuration handling

[Fabio M. Di Nitto <fdinitto@redhat.com>]

On 12/1/2011 8:51 AM, Jan Friesse wrote:
> Patch looks good with one question.
> 
> Fabio M. Di Nitto wrote:
>> From: "Fabio M. Di Nitto" <fdinitto@redhat.com>
>>
>> - don't allow configuration of more than 2 rings
>> - allow overrided of alternate mcast address and port via
>>   envars
>> - when using broadcast, set different ports on second ring.
>>   this also required a substantial change in transport handling
>> ...
>> +		if (!strcmp(altmcast_name, mcast_name) &&
>> +		    ((altportnum == portnum) || (altportnum == portnum - 1) || (portnum == altportnum - 1))) {
>> +			sprintf(error_reason, "Alternate communication channel (mcast: %s ports: %d,%d) cannot use\n"
>> +					      "same address and ports of primary channel (mcast: %s ports: %d,%d)",
>> +					      altmcast_name, altportnum, altportnum - 1,
>> +					      mcast_name, portnum, portnum - 1);
> 
> (altportnum == portnum - 1) || (portnum == altportnum - 1)) ->
> (altportnum == portnum - 1) || (altportnum == portnum + 1)), but
> corosync uses only port and port -1, so second condition seems to be
> useless (on the other hand, it doesn't hurt anything).

You have a total of 4 ports to check and that can be resolved with 3 tests.

Example:

ring0 port0 portnum
ring0 port1 portnum - 1

ring1 port0 altportnum
ring1 port1 altportnum - 1

if r0p0 == r1p0 (altportnum == portnum)
(this also catches r0p1 == r1p1 so no need to repeat the test)

if r0p1 == r1p0 (altportnum == portnum - 1)

if r0p0 == r1p1 (portnum == altportnum - 1)

So effectively with 3 tests you catch all conditions, unless of course I
made a mistake in the coding or in the thinking that is entirely possible :)

Thanks for the review!

Fabio