FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Cluster Development

 
 
LinkBack Thread Tools
 
Old 12-01-2011, 06:51 AM
Jan Friesse
 
Default config: drastically improve cman RRP configuration handling

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
"
> + "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).

> + write_cman_pipe(error_reason);
> + return -1;
 
Old 12-01-2011, 07:43 AM
"Fabio M. Di Nitto"
 
Default config: drastically improve cman RRP configuration handling

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
"
>> + "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
 

Thread Tools




All times are GMT. The time now is 12:48 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org