On 07/04/15 15:00, Jan Friesse wrote:
> Chrissie,
>
> Christine Caulfield napsal(a):
>> On 20/03/15 10:26, Jan Friesse wrote:
>>> Chrissie,
>>> nice idea but I have two comments.
>>> 1. Nodes without nodeid (so auto generated nodeid) are not checked. It
>>> can happen that user enters nodeid which collides with auto generated
>>> nodeid. In practice not very important, but still make sense to make it
>>> right.
>>
>> I've had a look at this and I'm not sure it's feasible. There's no
>> reasonable way I can think of defining a node that's in the nodelist
>> that shouldn't be there because it clashes with the active list.
>>
>
> I was actually thinking about following scenario:
>
> nodelist {
> node {
> ring0_addr: 192.168.1.1
> }
> node {
> ring0_addr: 192.168.1.2
> nodeid: decimal form of 192.168.1.1
> }
> }
>
> So there are two nodes with same nodeid.
>
> Does this fall into one of three scenarios you've wrote?
>
> Because scenario I've described is not properly detected in corosync
> (and it's one of big todo) and corosync will crash. I believe your patch
> will be more complete if it would be able to detect this case.
Attached :)
Chrissie
diff --git a/exec/totemconfig.c b/exec/totemconfig.c
index b678752..f232ea8 100644
--- a/exec/totemconfig.c
+++ b/exec/totemconfig.c
@@ -481,6 +481,120 @@ static int get_cluster_mcast_addr (
return (err);
}
+static unsigned int generate_nodeid_for_duplicate_test(
+ struct totem_config *totem_config,
+ char *addr)
+{
+ unsigned int nodeid;
+ struct totem_ip_address totemip;
+
+ /* AF_INET hard-coded here because auto-generated nodeids
+ are only for IPv4 */
+ if (totemip_parse(&totemip, addr, AF_INET) != 0)
+ return -1;
+
+ memcpy (&nodeid, &totemip.addr, sizeof (unsigned int));
+
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+ nodeid = swab32 (nodeid);
+#endif
+
+ if (totem_config->clear_node_high_bit) {
+ nodeid &= 0x7FFFFFFF;
+ }
+ return nodeid;
+}
+
+static int check_for_duplicate_nodeids(
+ struct totem_config *totem_config,
+ const char **error_string)
+{
+ icmap_iter_t iter;
+ icmap_iter_t subiter;
+ const char *iter_key;
+ int res = 0;
+ int retval = 0;
+ char tmp_key[ICMAP_KEYNAME_MAXLEN];
+ char *ring0_addr=NULL;
+ char *ring0_addr1=NULL;
+ unsigned int node_pos;
+ unsigned int node_pos1;
+ unsigned int nodeid;
+ unsigned int nodeid1;
+ int autogenerated;
+
+ iter = icmap_iter_init("nodelist.node.");
+ while ((iter_key = icmap_iter_next(iter, NULL, NULL)) != NULL) {
+ res = sscanf(iter_key, "nodelist.node.%u.%s", &node_pos, tmp_key);
+ if (res != 2) {
+ continue;
+ }
+
+ if (strcmp(tmp_key, "ring0_addr") != 0) {
+ continue;
+ }
+
+ snprintf(tmp_key, ICMAP_KEYNAME_MAXLEN, "nodelist.node.%u.nodeid", node_pos);
+ autogenerated = 0;
+ if (icmap_get_uint32(tmp_key, &nodeid) != CS_OK) {
+
+ snprintf(tmp_key, ICMAP_KEYNAME_MAXLEN, "nodelist.node.%u.ring0_addr", node_pos);
+ if (icmap_get_string(tmp_key, &ring0_addr) != CS_OK) {
+ continue;
+ }
+
+ /* Generate nodeid so we can check that auto-generated nodeids don't clash either */
+ nodeid = generate_nodeid_for_duplicate_test(totem_config, ring0_addr);
+ if (nodeid == -1) {
+ continue;
+ }
+ autogenerated = 1;
+ }
+
+ node_pos1 = 0;
+ subiter = icmap_iter_init("nodelist.node.");
+ while (((iter_key = icmap_iter_next(subiter, NULL, NULL)) != NULL) && (node_pos1 < node_pos)) {
+ res = sscanf(iter_key, "nodelist.node.%u.%s", &node_pos1, tmp_key);
+ if ((res != 2) || (node_pos1 >= node_pos)) {
+ continue;
+ }
+
+ if (strcmp(tmp_key, "ring0_addr") != 0) {
+ continue;
+ }
+
+ snprintf(tmp_key, ICMAP_KEYNAME_MAXLEN, "nodelist.node.%u.nodeid", node_pos1);
+ if (icmap_get_uint32(tmp_key, &nodeid1) != CS_OK) {
+
+ snprintf(tmp_key, ICMAP_KEYNAME_MAXLEN, "nodelist.node.%u.ring0_addr", node_pos1);
+ if (icmap_get_string(tmp_key, &ring0_addr1) != CS_OK) {
+ continue;
+ }
+ nodeid1 = generate_nodeid_for_duplicate_test(totem_config, ring0_addr1);
+ if (nodeid1 == -1) {
+ continue;
+ }
+ }
+
+ if (nodeid == nodeid1) {
+ retval = -1;
+ snprintf (error_string_response, sizeof(error_string_response),
+ "Nodeid %u%s%s%s appears twice in corosync.conf", nodeid,
+ autogenerated?"(autogenerated from ":"",
+ autogenerated?ring0_addr:"",
+ autogenerated?")":"");
+ log_printf (LOGSYS_LEVEL_ERROR, error_string_response);
+ *error_string = error_string_response;
+ break;
+ }
+ }
+ icmap_iter_finalize(subiter);
+ }
+ icmap_iter_finalize(iter);
+ return retval;
+}
+
+
static int find_local_node_in_nodelist(struct totem_config *totem_config)
{
icmap_iter_t iter;
@@ -1236,6 +1350,10 @@ int totem_config_validate (
return (-1);
}
+ if (check_for_duplicate_nodeids(totem_config, error_string) == -1) {
+ return (-1);
+ }
+
/*
* RRP values validation
*/
_______________________________________________
discuss mailing list
discuss@xxxxxxxxxxxx
http://lists.corosync.org/mailman/listinfo/discuss
