Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix no link0 startup #484

Closed

Conversation

ThomasLamprecht
Copy link
Contributor

No description provided.

As totem_config->interfaces entries are _all_ possible links and not
only the configured ones we cannot trust that interface[0] is
configured at the time of checking, and thus has a valid
member_count. So set the members variable to the member_count entry
from an actually configured interface and loop over that one.

This fixes a case where the check for the name property on all nodes
for multi links was skipped if link 0 was not configured, as then its
member_count was 0.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
configured links may not come in order in the interfaces array, which
holds an entry for _all_ possible links, not just configured ones.

So iterate through all interfaces, but skip those which are not
configured. This allows to start corosync with a configuration where
link 0 is currently not mentioned, as else it was checked but had
member_count = 0 from it's default initialization, which then made
this code report a false positive for the "Not all nodes have the
same number of links" check even on a correct config.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
@knet-ci-bot
Copy link

Can one of the admins verify this patch?

@jfriesse
Copy link
Member

@knet-ci-bot ok to test

@jfriesse
Copy link
Member

Looks fine so ACK by me, but I would like also ask @chrissie-c for her opinion, because she has more experience in this part of code.

@jfriesse jfriesse requested a review from chrissie-c June 17, 2019 07:26
Copy link
Contributor

@chrissie-c chrissie-c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of excellent catches there. Thank you!

@jfriesse
Copy link
Member

@chrissie-c Thank you for additional ack.

@GamerSource Thank you for the patch (and braveness for making totemconfig a bit better) . I've merged patches as a 7ada508 and 816324c

@jfriesse jfriesse closed this Jun 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants