client: Improve initialization of client-state structure

- Split ni_fsm_get_matching_workers() and ni_fsm_mark_matching_workers() - Introduce new type ni_ifmarker_t to mark specific attributes of ifworkers (target_range, persistent) - get rid of ni_fsm_set_client_state() and move the functionality to ni_ifworker_start() - possible due to the above split - Introduce macro to carefully set control flags like persistent
openSUSE · Feb 4, 2014 · 427b557 · 427b557
1 parent fb41aee
commit 427b557
Show file tree

Hide file tree

Showing 6 changed files with 97 additions and 83 deletions.
diff --git a/client/ifdown.c b/client/ifdown.c
@@ -53,7 +53,8 @@ ni_do_ifdown(int argc, char **argv)
 		{ NULL }
 	};
 	ni_ifmatcher_t ifmatch;
-	ni_uint_range_t target_range = { .min = NI_FSM_STATE_DEVICE_DOWN, .max = __NI_FSM_STATE_MAX - 2};
+	ni_ifmarker_t ifmarker;
+	ni_ifworker_array_t ifmarked;
 	unsigned int nmarked, max_state = NI_FSM_STATE_DEVICE_DOWN;
 	ni_stringbuf_t sb = NI_STRINGBUF_INIT_DYNAMIC;
 	ni_fsm_t *fsm;
@@ -65,16 +66,22 @@ ni_do_ifdown(int argc, char **argv)
 
 	/* Allow ifdown only on non-persistent interfaces previously configured by ifup */
 	memset(&ifmatch, 0, sizeof(ifmatch));
+	memset(&ifmarker, 0, sizeof(ifmarker));
+	memset(&ifmarked, 0, sizeof(ifmarked));
+
 	ifmatch.require_configured = TRUE;
 	ifmatch.allow_persistent = FALSE;
 	ifmatch.require_config = FALSE;
 
+	ifmarker.target_range.min = NI_FSM_STATE_DEVICE_DOWN;
+	ifmarker.target_range.max = __NI_FSM_STATE_MAX - 2;
+
 	optind = 1;
 	while ((c = getopt_long(argc, argv, "", ifdown_options, NULL)) != EOF) {
 		switch (c) {
 		case OPT_FORCE:
 			if (!ni_ifworker_state_from_name(optarg, &max_state) ||
-			    !ni_ifworker_state_in_range(&target_range, max_state)) {
+			    !ni_ifworker_state_in_range(&ifmarker.target_range, max_state)) {
 				ni_error("ifdown: wrong force option \"%s\"", optarg);
 				goto usage;
 			}
@@ -114,7 +121,7 @@ ni_do_ifdown(int argc, char **argv)
 		default:
 		case OPT_HELP:
 usage:
-			ni_fsm_fill_state_string(&sb, &target_range);
+			ni_fsm_fill_state_string(&sb, &ifmarker.target_range);
 			fprintf(stderr,
 				"wicked [options] ifdown [ifdown-options] <ifname ...>|all\n"
 				"\nSupported ifdown-options:\n"
@@ -141,20 +148,26 @@ ni_do_ifdown(int argc, char **argv)
 		goto usage;
 	}
 
-	target_range.min = NI_FSM_STATE_NONE;
-	target_range.max = max_state;
+	ifmarker.target_range.min = NI_FSM_STATE_NONE;
+	ifmarker.target_range.max = max_state;
 
 	if (!ni_fsm_create_client(fsm))
 		/* Severe error we always explicitly return */
 		return NI_WICKED_RC_ERROR;
 
 	ni_fsm_refresh_state(fsm);
 
+	/* Get workers that match given criteria */
 	nmarked = 0;
 	while (optind < argc) {
 		ifmatch.name = argv[optind++];
-		nmarked += ni_fsm_mark_matching_workers(fsm, &ifmatch, &target_range);
+		ni_fsm_get_matching_workers(fsm, &ifmatch, &ifmarked);
 	}
+
+	/* Mark and start selected workers */
+	if (ifmarked.count)
+		nmarked = ni_fsm_mark_matching_workers(fsm, &ifmarked, &ifmarker);
+
 	if (nmarked == 0) {
 		printf("ifdown: no matching interfaces\n");
 		status = NI_WICKED_RC_SUCCESS;
@@ -174,6 +187,7 @@ ni_do_ifdown(int argc, char **argv)
 			status = NI_LSB_RC_SUCCESS;
 	}
 
+	ni_ifworker_array_destroy(&ifmarked);
 	return status;
 }
 
diff --git a/client/ifreload.c b/client/ifreload.c
@@ -191,6 +191,10 @@ ni_do_ifreload(int argc, char **argv)
 			if (ni_ifcheck_worker_config_matches(w))
 				continue;
 
+			/* Mark persistend when requested */
+			if (opt_persistent)
+				w->client_state.persistent = TRUE;
+
 			/* Remember all changed devices */
 			ni_ifworker_array_append(&marked, w);
 
@@ -247,9 +251,6 @@ ni_do_ifreload(int argc, char **argv)
 			goto cleanup;
 		}
 
-		/* Mark persistend when requested */
-		ni_fsm_set_client_state(fsm, opt_persistent);
-
 		/* Execute the up run */
 		if (ni_fsm_schedule(fsm) != 0)
 			ni_fsm_mainloop(fsm);

diff --git a/client/ifup.c b/client/ifup.c
@@ -62,6 +62,7 @@ ni_do_ifup(int argc, char **argv)
 {
 	enum  { OPT_HELP, OPT_IFCONFIG, OPT_IFPOLICY, OPT_CONTROL_MODE, OPT_STAGE,
 		OPT_TIMEOUT, OPT_SKIP_ACTIVE, OPT_SKIP_ORIGIN, OPT_FORCE, OPT_PERSISTENT };
+
 	static struct option ifup_options[] = {
 		{ "help",	no_argument,       NULL,	OPT_HELP },
 		{ "ifconfig",	required_argument, NULL,	OPT_IFCONFIG },
@@ -75,15 +76,13 @@ ni_do_ifup(int argc, char **argv)
 		{ "persistent",	no_argument, NULL,	OPT_PERSISTENT },
 		{ NULL }
 	};
-	ni_uint_range_t state_range = { .min = NI_FSM_STATE_ADDRCONF_UP, .max = __NI_FSM_STATE_MAX };
+
+	ni_ifmatcher_t ifmatch;
+	ni_ifmarker_t ifmarker;
+	ni_ifworker_array_t ifmarked;
 	ni_string_array_t opt_ifconfig = NI_STRING_ARRAY_INIT;
 	const char *opt_ifpolicy = NULL;
-	const char *opt_control_mode = NULL;
-	const char *opt_boot_stage = NULL;
-	const char *opt_skip_origin = NULL;
 	ni_bool_t opt_force = FALSE;
-	ni_bool_t opt_skip_active = FALSE;
-	ni_bool_t opt_persistent = FALSE;
 	unsigned int nmarked, i;
 	ni_fsm_t *fsm;
 	int c, status = NI_WICKED_RC_USAGE;
@@ -92,6 +91,18 @@ ni_do_ifup(int argc, char **argv)
 	ni_assert(fsm);
 	ni_fsm_require_register_type("reachable", ni_ifworker_reachability_check_new);
 
+	memset(&ifmatch, 0, sizeof(ifmatch));
+	memset(&ifmarker, 0, sizeof(ifmarker));
+	memset(&ifmarked, 0, sizeof(ifmarked));
+
+	/* Allow ifup on all interfaces we have config for */
+	ifmatch.require_configured = FALSE;
+	ifmatch.allow_persistent = TRUE;
+	ifmatch.require_config = TRUE;
+
+	ifmarker.target_range.min = NI_FSM_STATE_ADDRCONF_UP;
+	ifmarker.target_range.max = __NI_FSM_STATE_MAX;
+
 	optind = 1;
 	while ((c = getopt_long(argc, argv, "", ifup_options, NULL)) != EOF) {
 		switch (c) {
@@ -104,11 +115,11 @@ ni_do_ifup(int argc, char **argv)
 			break;
 
 		case OPT_CONTROL_MODE:
-			opt_control_mode = optarg;
+			ifmatch.mode = optarg;
 			break;
 
 		case OPT_STAGE:
-			opt_boot_stage = optarg;
+			ifmatch.boot_stage= optarg;
 			break;
 
 		case OPT_TIMEOUT:
@@ -123,19 +134,19 @@ ni_do_ifup(int argc, char **argv)
 			break;
 
 		case OPT_SKIP_ORIGIN:
-			opt_skip_origin = optarg;
+			ifmatch.skip_origin = optarg;
 			break;
 
 		case OPT_SKIP_ACTIVE:
-			opt_skip_active = 1;
+			ifmatch.skip_active = TRUE;
 			break;
 
 		case OPT_FORCE:
 			opt_force = TRUE;
 			break;
 
 		case OPT_PERSISTENT:
-			opt_persistent = TRUE;
+			ifmarker.persistent = TRUE;
 			break;
 
 		default:
@@ -216,31 +227,23 @@ ni_do_ifup(int argc, char **argv)
 		goto cleanup;
 	}
 
-	ni_fsm_set_client_state(fsm, opt_persistent);
-
+	/* Get workers that match given criteria */
 	nmarked = 0;
 	while (optind < argc) {
-		static ni_ifmatcher_t ifmatch;
-
-		memset(&ifmatch, 0, sizeof(ifmatch));
 		ifmatch.name = argv[optind++];
-		/* Allow ifup on all interfaces we have config for */
-		ifmatch.require_configured = FALSE;
-		ifmatch.allow_persistent = TRUE;
-		ifmatch.require_config = TRUE;
-		ifmatch.skip_active = opt_skip_active;
-		ifmatch.skip_origin = opt_skip_origin;
 
 		if (!strcmp(ifmatch.name, "boot")) {
 			ifmatch.name = "all";
 			ifmatch.mode = "boot";
-		} else {
-			ifmatch.mode = opt_control_mode;
-			ifmatch.boot_stage = opt_boot_stage;
 		}
 
-		nmarked += ni_fsm_mark_matching_workers(fsm, &ifmatch, &state_range);
+		ni_fsm_get_matching_workers(fsm, &ifmatch, &ifmarked);
 	}
+
+	/* Mark and start selected workers */
+	if (ifmarked.count)
+		nmarked = ni_fsm_mark_matching_workers(fsm, &ifmarked, &ifmarker);
+
 	if (nmarked == 0) {
 		printf("ifup: no matching interfaces\n");
 		status = NI_WICKED_RC_SUCCESS;
@@ -261,6 +264,7 @@ ni_do_ifup(int argc, char **argv)
 	}
 
 cleanup:
+	ni_ifworker_array_destroy(&ifmarked);
 	ni_string_array_destroy(&opt_ifconfig);
 	return status;
 }

diff --git a/include/wicked/fsm.h b/include/wicked/fsm.h
@@ -231,6 +231,10 @@ typedef struct ni_ifmatcher {
 				skip_active        : 1;
 } ni_ifmatcher_t;
 
+typedef struct ni_ifmarker {
+	ni_uint_range_t	target_range;
+	unsigned int	persistent	: 1;
+} ni_ifmarker_t;
 
 extern ni_fsm_t *		ni_fsm_new(void);
 extern void			ni_fsm_free(ni_fsm_t *);
@@ -245,14 +249,13 @@ extern const char *		ni_fsm_policy_name(const ni_fsm_policy_t *);
 extern xml_location_t *	ni_fsm_policy_location(const ni_fsm_policy_t *);
 extern ni_bool_t		ni_fsm_policies_changed_since(const ni_fsm_t *, unsigned int *tstamp);
 
-extern void			ni_fsm_set_client_state(ni_fsm_t *, ni_bool_t);
 extern ni_dbus_client_t *	ni_fsm_create_client(ni_fsm_t *);
 extern void			ni_fsm_refresh_state(ni_fsm_t *);
 extern unsigned int		ni_fsm_schedule(ni_fsm_t *);
 extern ni_bool_t		ni_fsm_do(ni_fsm_t *fsm, long *timeout_p);
 extern void			ni_fsm_mainloop(ni_fsm_t *);
 extern unsigned int		ni_fsm_get_matching_workers(ni_fsm_t *, ni_ifmatcher_t *, ni_ifworker_array_t *);
-extern unsigned int		ni_fsm_mark_matching_workers(ni_fsm_t *, ni_ifmatcher_t *, const ni_uint_range_t *);
+extern unsigned int		ni_fsm_mark_matching_workers(ni_fsm_t *, ni_ifworker_array_t *, const ni_ifmarker_t *);
 extern unsigned int		ni_fsm_start_matching_workers(ni_fsm_t *, ni_ifworker_array_t *);
 extern void			ni_fsm_reset_matching_workers(ni_fsm_t *, ni_ifworker_array_t *, const ni_uint_range_t *, ni_bool_t);
 extern int			ni_fsm_build_hierarchy(ni_fsm_t *);

diff --git a/src/client/client_state.h b/src/client/client_state.h
@@ -67,4 +67,19 @@ ni_client_state_print_timeval(const struct timeval *tv, char **str)
 			(unsigned long)tv->tv_usec);
 }
 
+#define NI_CLIENT_STATE_SET_CONTROL_FLAG(flag, cond, value) \
+	do { \
+		if (!(cond)) \
+			break; \
+		if (value) {\
+			flag = TRUE; \
+			ni_debug_application("%s: set %s control flag to TRUE", \
+				__func__, #flag); \
+		} \
+		else if (flag) {\
+			ni_fatal("%s: attempt to switch off %s control flag", \
+				__func__, #flag); \
+		} \
+	} while(0)
+
 #endif