config: Fail to start if ping timers are invalid

This required adding a lot of return values to two previously
'void' functions. I did two rather than just the one that was
needed because it seemed to make sense to do them both together.

Although these functions now return errors, they are probably
still ignored higher up. this really needs a comprehensive audit.

Signed-off-by: Christine Caulfield <ccaulfie@redhat.com>
Reviewed-by: Jan Friesse <jfriesse@redhat.com>

exec/totemconfig.c

@@ -1157,11 +1157,12 @@ static void calc_knet_ping_timers(struct totem_config *totem_config)
  * (set to 0), and ips which are only in set1 or set2 remains untouched.
  * totempg_node_add/remove is called.
  */
-static void compute_and_set_totempg_interfaces(struct totem_interface *set1,
+static int compute_and_set_totempg_interfaces(struct totem_interface *set1,
 	struct totem_interface *set2)
 {
 	int ring_no, set1_pos, set2_pos;
 	struct totem_ip_address empty_ip_address;
+	int res = 0;
 
 	memset(&empty_ip_address, 0, sizeof(empty_ip_address));
 
@@ -1217,10 +1218,13 @@ static void compute_and_set_totempg_interfaces(struct totem_interface *set1,
 					totemip_print(&set2[ring_no].member_list[set2_pos]),
 					ring_no);
 
-				totempg_member_add(&set2[ring_no].member_list[set2_pos], ring_no);
+				if (totempg_member_add(&set2[ring_no].member_list[set2_pos], ring_no)) {
+					res = -1;
+				}
 			}
 		}
 	}
+	return res;
 }
 
 /*
@@ -2414,10 +2418,11 @@ int totemconfig_configure_new_params(
 	return 0;
 }
 
-void totemconfig_commit_new_params(
+int totemconfig_commit_new_params(
 	struct totem_config *totem_config,
 	icmap_map_t map)
 {
+	int res;
 	struct totem_interface *new_interfaces = NULL;
 
 	new_interfaces = malloc (sizeof (struct totem_interface) * INTERFACE_MAX);
@@ -2427,13 +2432,14 @@ void totemconfig_commit_new_params(
 	/* Set link parameters including local_ip */
 	configure_totem_links(totem_config, map);
 
-	/* Add & remove nodes */
-	compute_and_set_totempg_interfaces(totem_config->orig_interfaces, new_interfaces);
+	/* Add & remove nodes & link properties */
+	res = compute_and_set_totempg_interfaces(totem_config->orig_interfaces, new_interfaces);
 
 	/* Does basic global params (like compression) */
 	totempg_reconfigure();
 
 	free(new_interfaces);
+	return res; /* On a reload this is ignored */
 }
 
 static void add_totem_config_notification(struct totem_config *totem_config)

exec/totemconfig.h

@@ -86,7 +86,7 @@ extern int totemconfig_configure_new_params(
 	icmap_map_t map,
 	const char **error_string);
 
-extern void totemconfig_commit_new_params(
+extern int totemconfig_commit_new_params(
 	struct totem_config *totem_config,
 	icmap_map_t map);
 

exec/totemknet.c

@@ -101,13 +101,13 @@ struct totemknet_instance {
 
 	void *context;
 
-	void (*totemknet_deliver_fn) (
+	int (*totemknet_deliver_fn) (
 		void *context,
 		const void *msg,
 		unsigned int msg_len,
 		const struct sockaddr_storage *system_from);
 
-	void (*totemknet_iface_change_fn) (
+	int (*totemknet_iface_change_fn) (
 		void *context,
 		const struct totem_ip_address *iface_address,
 		unsigned int link_no);
@@ -865,14 +865,20 @@ static void timer_function_netif_check_timeout (
 {
 	struct totemknet_instance *instance = (struct totemknet_instance *)data;
 	int i;
+	int res = 0;
 
 	for (i=0; i < INTERFACE_MAX; i++) {
 		if (!instance->totem_config->interfaces[i].configured) {
 			continue;
 		}
-		instance->totemknet_iface_change_fn (instance->context,
-						     &instance->my_ids[i],
-						     i);
+		res = instance->totemknet_iface_change_fn (instance->context,
+							   &instance->my_ids[i],
+							   i);
+	}
+	if (res != 0) {
+		/* This is only called at startup, so we can quit here.
+		   Refresh takes a different path */
+		corosync_exit_error(COROSYNC_DONE_MAINCONFIGREAD);
 	}
 }
 
@@ -1121,13 +1127,13 @@ int totemknet_initialize (
 	totemsrp_stats_t *stats,
 	void *context,
 
-	void (*deliver_fn) (
+	int (*deliver_fn) (
 		void *context,
 		const void *msg,
 		unsigned int msg_len,
 		const struct sockaddr_storage *system_from),
 
-	void (*iface_change_fn) (
+	int (*iface_change_fn) (
 		void *context,
 		const struct totem_ip_address *iface_address,
 		unsigned int link_no),
@@ -1634,20 +1640,33 @@ int totemknet_member_add (
 		KNET_LOGSYS_PERROR(errno, LOGSYS_LEVEL_ERROR, "knet_link_set_priority for nodeid " CS_PRI_NODE_ID ", link %d failed", member->nodeid, link_no);
 	}
 
-	/* ping timeouts maybe 0 here for a newly added interface so we leave this till later, it will
-	   get done in totemknet_refresh_config */
+	/*
+	 * Ping timeouts may be 0 here for a newly added interface (on a reload),
+	 * so we leave this till later, it will get done in totemknet_refresh_config.
+	 * For the initial startup, we are all preset and ready to go from here.
+	 */
 	if (instance->totem_config->interfaces[link_no].knet_ping_interval != 0) {
 		err = knet_link_set_ping_timers(instance->knet_handle, member->nodeid, link_no,
 						instance->totem_config->interfaces[link_no].knet_ping_interval,
 						instance->totem_config->interfaces[link_no].knet_ping_timeout,
 						instance->totem_config->interfaces[link_no].knet_ping_precision);
 		if (err) {
+			/* Flush logs before reporting this error so that the knet message prints before ours */
+			int saved_errno = errno;
+			log_flush_messages(instance);
+			errno = saved_errno;
 			KNET_LOGSYS_PERROR(errno, LOGSYS_LEVEL_ERROR, "knet_link_set_ping_timers for nodeid " CS_PRI_NODE_ID ", link %d failed", member->nodeid, link_no);
+			return -1;
 		}
 		err = knet_link_set_pong_count(instance->knet_handle, member->nodeid, link_no,
 					       instance->totem_config->interfaces[link_no].knet_pong_count);
 		if (err) {
+			/* Flush logs before reporting this error so that the knet message prints before ours */
+			int saved_errno = errno;
+			log_flush_messages(instance);
+			errno = saved_errno;
 			KNET_LOGSYS_PERROR(errno, LOGSYS_LEVEL_ERROR, "knet_link_set_pong_count for nodeid " CS_PRI_NODE_ID ", link %d failed", member->nodeid, link_no);
+			return -1;
 		}
 	}
 

exec/totemknet.h

@@ -51,13 +51,13 @@ extern int totemknet_initialize (
 	totemsrp_stats_t *stats,
 	void *context,
 
-	void (*deliver_fn) (
+	int (*deliver_fn) (
 		void *context,
 		const void *msg,
 		unsigned int msg_len,
 		const struct sockaddr_storage *system_from),
 
-	void (*iface_change_fn) (
+	int (*iface_change_fn) (
 		void *context,
 		const struct totem_ip_address *iface_address,
 		unsigned int ring_no),

exec/totemnet.c

@@ -56,13 +56,13 @@ struct transport {
 		totemsrp_stats_t *stats,
 		void *context,
 
-		void (*deliver_fn) (
+		int (*deliver_fn) (
 			void *context,
 			const void *msg,
 			unsigned int msg_len,
 			const struct sockaddr_storage *system_from),
 
-		void (*iface_change_fn) (
+		int (*iface_change_fn) (
 			void *context,
 			const struct totem_ip_address *iface_address,
 			unsigned int ring_no),
@@ -321,13 +321,13 @@ int totemnet_initialize (
 	totemsrp_stats_t *stats,
 	void *context,
 
-	void (*deliver_fn) (
+	int (*deliver_fn) (
 		void *context,
 		const void *msg,
 		unsigned int msg_len,
 		const struct sockaddr_storage *system_from),
 
-	void (*iface_change_fn) (
+	int (*iface_change_fn) (
 		void *context,
 		const struct totem_ip_address *iface_address,
 		unsigned int ring_no),

exec/totemnet.h

@@ -61,13 +61,13 @@ extern int totemnet_initialize (
 	totemsrp_stats_t *stats,
 	void *context,
 
-	void (*deliver_fn) (
+	int (*deliver_fn) (
 		void *context,
 		const void *msg,
 		unsigned int msg_len,
 		const struct sockaddr_storage *system_from),
 
-	void (*iface_change_fn) (
+	int (*iface_change_fn) (
 		void *context,
 		const struct totem_ip_address *iface_address,
 		unsigned int iface_no),

exec/totemsrp.c

@@ -664,13 +664,13 @@ static void *totemsrp_buffer_alloc (struct totemsrp_instance *instance);
 static void totemsrp_buffer_release (struct totemsrp_instance *instance, void *ptr);
 static const char* gsfrom_to_msg(enum gather_state_from gsfrom);
 
-void main_deliver_fn (
+int main_deliver_fn (
 	void *context,
 	const void *msg,
 	unsigned int msg_len,
 	const struct sockaddr_storage *system_from);
 
-void main_iface_change_fn (
+int main_iface_change_fn (
 	void *context,
 	const struct totem_ip_address *iface_address,
 	unsigned int iface_no);
@@ -5032,7 +5032,7 @@ static int check_message_header_validity(
 }
 
 
-void main_deliver_fn (
+int main_deliver_fn (
 	void *context,
 	const void *msg,
 	unsigned int msg_len,
@@ -5042,7 +5042,7 @@ void main_deliver_fn (
 	const struct totem_message_header *message_header = msg;
 
 	if (check_message_header_validity(context, msg, msg_len, system_from) == -1) {
-		return ;
+		return -1;
 	}
 
 	switch (message_header->type) {
@@ -5071,12 +5071,12 @@ void main_deliver_fn (
 		    (int)message_header->type);
 
 		instance->stats.rx_msg_dropped++;
-		return;
+		return 0;
 	}
 	/*
 	 * Handle incoming message
 	 */
-	totemsrp_message_handlers.handler_functions[(int)message_header->type] (
+	return totemsrp_message_handlers.handler_functions[(int)message_header->type] (
 		instance,
 		msg,
 		msg_len,
@@ -5104,7 +5104,7 @@ int totemsrp_iface_set (
 }
 
 /* Contrary to its name, this only gets called when the interface is enabled */
-void main_iface_change_fn (
+int main_iface_change_fn (
 	void *context,
 	const struct totem_ip_address *iface_addr,
 	unsigned int iface_no)
@@ -5112,6 +5112,7 @@ void main_iface_change_fn (
 	struct totemsrp_instance *instance = context;
 	int num_interfaces;
 	int i;
+	int res = 0;
 
 	if (!instance->my_id.nodeid) {
 		instance->my_id.nodeid = iface_addr->nodeid;
@@ -5154,11 +5155,12 @@ void main_iface_change_fn (
 		assert(instance->totem_config->orig_interfaces != NULL);
 		memset(instance->totem_config->orig_interfaces, 0, sizeof (struct totem_interface) * INTERFACE_MAX);
 
-		totemconfig_commit_new_params(instance->totem_config, icmap_get_global_map());
+		res = totemconfig_commit_new_params(instance->totem_config, icmap_get_global_map());
 
 		memb_state_gather_enter (instance, TOTEMSRP_GSFROM_INTERFACE_CHANGE);
 		free(instance->totem_config->orig_interfaces);
 	}
+	return res;
 }
 
 void totemsrp_net_mtu_adjust (struct totem_config *totem_config) {

exec/totemudp.c

@@ -110,13 +110,13 @@ struct totemudp_instance {
 
 	void *context;
 
-	void (*totemudp_deliver_fn) (
+	int (*totemudp_deliver_fn) (
 		void *context,
 		const void *msg,
 		unsigned int msg_len,
 		const struct sockaddr_storage *system_from);
 
-	void (*totemudp_iface_change_fn) (
+	int (*totemudp_iface_change_fn) (
 		void *context,
 		const struct totem_ip_address *iface_address,
 		unsigned int ring_no);
@@ -1136,13 +1136,13 @@ int totemudp_initialize (
 
 	void *context,
 
-	void (*deliver_fn) (
+	int (*deliver_fn) (
 		void *context,
 		const void *msg,
 		unsigned int msg_len,
 		const struct sockaddr_storage *system_from),
 
-	void (*iface_change_fn) (
+	int (*iface_change_fn) (
 		void *context,
 		const struct totem_ip_address *iface_address,
 		unsigned int ring_no),

exec/totemudp.h

@@ -51,13 +51,13 @@ extern int totemudp_initialize (
 	totemsrp_stats_t *stats,
 	void *context,
 
-	void (*deliver_fn) (
+	int (*deliver_fn) (
 		void *context,
 		const void *msg,
 		unsigned int msg_len,
 		const struct sockaddr_storage *system_from),
 
-	void (*iface_change_fn) (
+	int (*iface_change_fn) (
 		void *context,
 		const struct totem_ip_address *iface_address,
 		unsigned int ring_no),

exec/totemudpu.c

@@ -100,13 +100,13 @@ struct totemudpu_instance {
 
 	void *context;
 
-	void (*totemudpu_deliver_fn) (
+	int (*totemudpu_deliver_fn) (
 		void *context,
 		const void *msg,
 		unsigned int msg_len,
 		const struct sockaddr_storage *system_from);
 
-	void (*totemudpu_iface_change_fn) (
+	int (*totemudpu_iface_change_fn) (
 		void *context,
 		const struct totem_ip_address *iface_address,
 		unsigned int ring_no);
@@ -957,13 +957,13 @@ int totemudpu_initialize (
 	totemsrp_stats_t *stats,
 	void *context,
 
-	void (*deliver_fn) (
+	int (*deliver_fn) (
 		void *context,
 		const void *msg,
 		unsigned int msg_len,
 		const struct sockaddr_storage *system_from),
 
-	void (*iface_change_fn) (
+        int (*iface_change_fn) (
 		void *context,
 		const struct totem_ip_address *iface_address,
 		unsigned int ring_no),

exec/totemudpu.h

@@ -51,13 +51,13 @@ extern int totemudpu_initialize (
 	totemsrp_stats_t *stats,
 	void *context,
 
-	void (*deliver_fn) (
+	int (*deliver_fn) (
 		void *context,
 		const void *msg,
 		unsigned int msg_len,
 		const struct sockaddr_storage *system_from),
 
-	void (*iface_change_fn) (
+	int (*iface_change_fn) (
 		void *context,
 		const struct totem_ip_address *iface_address,
 		unsigned int ring_no),