Merge pull request #208 from mtomaschewski/testing

kmroz · kmroz · commit 31632ab29603 · 2014-04-10T23:36:36.000-07:00
do not fail on device create/delete loops
diff --git a/client/ifcheck.c b/client/ifcheck.c
@@ -325,11 +325,17 @@ ni_do_ifcheck(int argc, char **argv)
 	}
 
 	if (!ni_fsm_create_client(fsm)) {
+		/* Severe error we always explicitly return */
+		status = NI_WICKED_RC_ERROR;
+		goto cleanup;
+	}
+
+	if (!ni_fsm_refresh_state(fsm)) {
+		/* Severe error we always explicitly return */
 		status = NI_WICKED_RC_ERROR;
 		goto cleanup;
 	}
 
-	ni_fsm_refresh_state(fsm);
 	status = NI_WICKED_ST_OK;
 
 	if (0 == checks.count)
diff --git a/client/ifdown.c b/client/ifdown.c
@@ -151,11 +151,15 @@ ni_do_ifdown(int argc, char **argv)
 	ifmarker.target_range.min = NI_FSM_STATE_NONE;
 	ifmarker.target_range.max = max_state;
 
-	if (!ni_fsm_create_client(fsm))
+	if (!ni_fsm_create_client(fsm)) {
 		/* Severe error we always explicitly return */
 		return NI_WICKED_RC_ERROR;
+	}
 
-	ni_fsm_refresh_state(fsm);
+	if (!ni_fsm_refresh_state(fsm)) {
+		/* Severe error we always explicitly return */
+		return NI_WICKED_RC_ERROR;
+	}
 
 	/* Get workers that match given criteria */
 	nmarked = 0;
diff --git a/client/ifreload.c b/client/ifreload.c
@@ -147,7 +147,11 @@ ni_do_ifreload(int argc, char **argv)
 		goto cleanup;
 	}
 
-	ni_fsm_refresh_state(fsm);
+	if (!ni_fsm_refresh_state(fsm)) {
+		/* Severe error we always explicitly return */
+		status = NI_WICKED_RC_ERROR;
+		goto cleanup;
+	}
 
 	if (opt_ifconfig.count == 0) {
 		const ni_string_array_t *sources = ni_config_sources("ifconfig");
diff --git a/client/ifstatus.c b/client/ifstatus.c
@@ -215,12 +215,12 @@ if_printf(const char *dev, const char *tag, const char *fmt, ...)
 	va_list ap;
 
 	if (!ni_string_empty(dev)) {
-		printf("%-15s", dev);
+		printf("%-16s", dev);
 	} else {
 		printf("%-6s", "");
 	}
 	if (!ni_string_empty(tag)) {
-		printf("%-8s ", tag);
+		printf("%-9s ", tag);
 	}
 	if (!ni_string_empty(fmt)) {
 		va_start(ap, fmt);
@@ -563,14 +563,16 @@ ni_do_ifstatus(int argc, char **argv)
 	}
 
 	if (!ni_fsm_create_client(fsm)) {
+		/* Severe error we always explicitly return */
 		status = NI_WICKED_ST_ERROR;
 		goto cleanup;
 	}
 
-	/* TODO: we connect to wickedd here. currently, it
-	 * may exit(1), that is with NI_WICKED_ST_ERROR...
-	 */
-	ni_fsm_refresh_state(fsm);
+	if (!ni_fsm_refresh_state(fsm)) {
+		/* Severe error we always explicitly return */
+		status = NI_WICKED_ST_ERROR;
+		goto cleanup;
+	}
 
 	if (check_config && opt_ifconfig.count == 0) {
 		const ni_string_array_t *sources = ni_config_sources("ifconfig");
diff --git a/client/ifup.c b/client/ifup.c
@@ -206,7 +206,11 @@ ni_do_ifup(int argc, char **argv)
 		goto cleanup;
 	}
 
-	ni_fsm_refresh_state(fsm);
+	if (!ni_fsm_refresh_state(fsm)) {
+		/* Severe error we always explicitly return */
+		status = NI_WICKED_RC_ERROR;
+		goto cleanup;
+	}
 
 	if (opt_ifconfig.count == 0) {
 		const ni_string_array_t *sources = ni_config_sources("ifconfig");
diff --git a/include/wicked/fsm.h b/include/wicked/fsm.h
@@ -251,7 +251,7 @@ 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 ni_dbus_client_t *	ni_fsm_create_client(ni_fsm_t *);
-extern void			ni_fsm_refresh_state(ni_fsm_t *);
+extern ni_bool_t		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 *);
diff --git a/nanny/device.c b/nanny/device.c
@@ -429,8 +429,6 @@ ni_nanny_get_device(ni_nanny_t *mgr, ni_ifworker_t *w)
 {
 	ni_managed_device_t *mdev;
 
-	ni_assert(w);
-
 	for (mdev = mgr->device_list; mdev; mdev = mdev->next) {
 		if (mdev->worker == w)
 			return mdev;
diff --git a/nanny/main.c b/nanny/main.c
@@ -300,9 +300,10 @@ ni_nanny_netif_state_change_signal_receive(ni_dbus_connection_t *conn, ni_dbus_m
 		// A new device was added. Could be a virtual device like
 		// a VLAN or vif, or a hotplug device
 		// Create a worker and a managed_netif for this device.
-		w = ni_fsm_recv_new_netif_path(mgr->fsm, object_path);
-		ni_nanny_register_device(mgr, w);
-		ni_nanny_schedule_recheck(mgr, w);
+		if ((w = ni_fsm_recv_new_netif_path(mgr->fsm, object_path))) {
+			ni_nanny_register_device(mgr, w);
+			ni_nanny_schedule_recheck(mgr, w);
+		}
 		return;
 	}
 
@@ -311,19 +312,22 @@ ni_nanny_netif_state_change_signal_receive(ni_dbus_connection_t *conn, ni_dbus_m
 				signal_name, object_path);
 		return;
 	}
-
-	ni_assert(w->type == NI_IFWORKER_TYPE_NETDEV);
-	ni_assert(w->device);
+	if (w->type != NI_IFWORKER_TYPE_NETDEV || w->device == NULL) {
+		ni_error("%s: received signal \"%s\" from \"%s\" (not a managed network device)",
+				w->name, signal_name, object_path);
+		return;
+	}
 
 	if (event == NI_EVENT_DEVICE_DELETE) {
-		ni_debug_nanny("%s: received signal %s from %s", w->name, signal_name, object_path);
+		ni_debug_nanny("%s: received signal \"%s\" from \"%s\"",
+				w->name, signal_name, object_path);
 		// delete the worker and the managed netif
 		ni_nanny_unregister_device(mgr, w);
 		return;
 	}
 
 	if ((mdev = ni_nanny_get_device(mgr, w)) == NULL) {
-		ni_debug_nanny("%s: received signal %s from %s (not a managed device)",
+		ni_debug_nanny("%s: received signal \"%s\" from \"%s\" (not a managed device)",
 				w->name, signal_name, object_path);
 		return;
 	}
@@ -388,9 +392,10 @@ ni_nanny_modem_state_change_signal_receive(ni_dbus_connection_t *conn, ni_dbus_m
 
 	// We receive a deviceCreate signal when a modem was plugged in
 	if (event == NI_EVENT_DEVICE_CREATE) {
-		w = ni_fsm_recv_new_modem_path(mgr->fsm, object_path);
-		ni_nanny_register_device(mgr, w);
-		ni_nanny_schedule_recheck(mgr, w);
+		if ((w = ni_fsm_recv_new_modem_path(mgr->fsm, object_path))) {
+			ni_nanny_register_device(mgr, w);
+			ni_nanny_schedule_recheck(mgr, w);
+		}
 		return;
 	}
 
@@ -400,10 +405,13 @@ ni_nanny_modem_state_change_signal_receive(ni_dbus_connection_t *conn, ni_dbus_m
 		return;
 	}
 
-	ni_debug_nanny("%s: received signal %s from %s", w->name, signal_name, object_path);
-	ni_assert(w->type == NI_IFWORKER_TYPE_MODEM);
-	ni_assert(w->modem);
+	if (w->type != NI_IFWORKER_TYPE_MODEM || w->modem == NULL) {
+		ni_error("%s: received signal \"%s\" from \"%s\" (not a managed modem device)",
+				w->name, signal_name, object_path);
+		return;
+	}
 
+	ni_debug_nanny("%s: received signal %s from %s", w->name, signal_name, object_path);
 	if (event == NI_EVENT_DEVICE_DELETE) {
 		// delete the worker and the managed modem
 		ni_nanny_unregister_device(mgr, w);
diff --git a/server/main.c b/server/main.c
@@ -347,10 +347,14 @@ handle_interface_event(ni_netdev_t *dev, ni_event_t event)
 			/* A new netif was discovered; create a dbus server object
 			 * enacpsulating it. */
 			object = ni_objectmodel_register_netif(dbus_server, dev, NULL);
-		} else
-		if (!(object = ni_objectmodel_get_netif_object(dbus_server, dev))) {
-			ni_error("cannot send %s event for model \"%s\" - no dbus device",
-				ni_event_type_to_name(event), dev->name);
+		} else {
+			object = ni_objectmodel_get_netif_object(dbus_server, dev);
+		}
+		if (!object) {
+			/* usually a "bad event", e.g. when the underlying netdev
+			 * does not exists any more, but events still arrive ... */
+			ni_debug_events("cannot handle %s event for model \"%s\" - no dbus object",
+					ni_event_type_to_name(event), dev->name);
 			return;
 		}
 
diff --git a/src/dbus-object.c b/src/dbus-object.c
@@ -301,7 +301,7 @@ ni_dbus_object_get_service(const ni_dbus_object_t *object, const char *interface
 	const ni_dbus_service_t *svc;
 	unsigned int i;
 
-	if (object->interfaces == NULL)
+	if (object == NULL || object->interfaces == NULL)
 		return NULL;
 
 	for (i = 0; (svc = object->interfaces[i]) != NULL; ++i) {
@@ -364,7 +364,7 @@ ni_dbus_object_get_all_services_for_method(const ni_dbus_object_t *object, const
 	const ni_dbus_service_t *svc;
 	unsigned int i, found = 0;
 
-	if (object->interfaces == NULL || method == NULL)
+	if (object == NULL || object->interfaces == NULL || method == NULL)
 		return 0;
 
 	for (i = 0; (svc = object->interfaces[i]) != NULL; ++i) {
@@ -383,7 +383,7 @@ ni_dbus_object_get_service_for_signal(const ni_dbus_object_t *object, const char
 	const ni_dbus_service_t *svc, *best = NULL;
 	unsigned int i;
 
-	if (object->interfaces == NULL)
+	if (object == NULL || object->interfaces == NULL)
 		return NULL;
 
 	for (i = 0; (svc = object->interfaces[i]) != NULL; ++i) {
@@ -404,7 +404,7 @@ ni_dbus_object_get_service_for_property(const ni_dbus_object_t *object, const ch
 	const ni_dbus_service_t *svc;
 	unsigned int i;
 
-	if (object->interfaces == NULL)
+	if (object == NULL || object->interfaces == NULL)
 		return NULL;
 
 	for (i = 0; (svc = object->interfaces[i]) != NULL; ++i) {
diff --git a/src/dbus-objects/interface.c b/src/dbus-objects/interface.c
@@ -374,8 +374,10 @@ __ni_objectmodel_build_netif_object(ni_dbus_server_t *server, ni_netdev_t *dev,
 		object = ni_dbus_object_new(class, NULL, ni_netdev_get(dev));
 	}
 
-	if (object == NULL)
-		ni_fatal("Unable to create dbus object for network interface %s", dev->name);
+	if (object == NULL) {
+		ni_error("Unable to create dbus object for network interface %s", dev->name);
+		return NULL;
+	}
 
 	ni_objectmodel_bind_compatible_interfaces(object);
 	return object;
@@ -504,8 +506,16 @@ ni_objectmodel_wrap_netif_request(ni_netdev_req_t *req)
 ni_netdev_t *
 ni_objectmodel_unwrap_netif(const ni_dbus_object_t *object, DBusError *error)
 {
-	ni_netdev_t *dev = object->handle;
+	ni_netdev_t *dev;
+
+	if (!object) {
+		if (error)
+			dbus_set_error(error, DBUS_ERROR_FAILED,
+				"Cannot unwrap network interface from a NULL dbus object");
+		return NULL;
+	}
 
+	dev = object->handle;
 	if (ni_dbus_object_isa(object, &ni_objectmodel_netif_class))
 		return dev;
 	if (error)
diff --git a/src/dbus-objects/modem.c b/src/dbus-objects/modem.c
@@ -198,8 +198,11 @@ __ni_objectmodel_build_modem_object(ni_dbus_server_t *server, ni_modem_t *modem)
 		object = ni_dbus_object_new(class, NULL, ni_modem_hold(modem));
 	}
 
-	if (object == NULL)
-		ni_fatal("Unable to create proxy object for modem %s (%s)", modem->device, modem->real_path);
+	if (object == NULL) {
+		ni_error("Unable to create proxy object for modem %s (%s)",
+				modem->device, modem->real_path);
+		return NULL;
+	}
 
 	ni_objectmodel_bind_compatible_interfaces(object);
 	return object;
@@ -262,8 +265,16 @@ ni_objectmodel_modem_full_path(const ni_modem_t *modem)
 ni_modem_t *
 ni_objectmodel_unwrap_modem(const ni_dbus_object_t *object, DBusError *error)
 {
-	ni_modem_t *modem = object->handle;
+	ni_modem_t *modem;
+
+	if (!object) {
+		if (error)
+			dbus_set_error(error, DBUS_ERROR_FAILED,
+				"Cannot unwrap modem from a NULL dbus object");
+		return NULL;
+	}
 
+	modem = object->handle;
 	if (ni_dbus_object_isa(object, &ni_objectmodel_mm_modem_class))
 		return modem;
 	if (ni_dbus_object_isa(object, &ni_objectmodel_modem_class))
diff --git a/src/fsm.c b/src/fsm.c
diff --git a/src/ifevent.c b/src/ifevent.c

Original file line number	Diff line number	Diff line change
`@@ -429,8 +429,6 @@ ni_nanny_get_device(ni_nanny_t mgr, ni_ifworker_t w)`
`429`	`429`	`{`
`430`	`430`	`ni_managed_device_t *mdev;`
`431`	`431`
`432`		`- ni_assert(w);`
`433`		`-`
`434`	`432`	`for (mdev = mgr->device_list; mdev; mdev = mdev->next) {`
`435`	`433`	`if (mdev->worker == w)`
`436`	`434`	`return mdev;`