From ab878f7479245fbb8c98d17e0f963afc0a163be7 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Mon, 6 Oct 2014 21:16:00 -0400 Subject: [PATCH 01/11] libnm: further NULL-vs-empty-array fixes In some cases, code may look at the value of an array-valued property during object initialization, before NMObject has set it to its actual initial value. So ensure that we initialize all such properties to an empty array, rather than leaving them NULL. Also fix another bug in NMClient that could result in priv->active_connections being NULL during certain signal emissions, and fix nm_client_get_active_connections() to not return NULL when NM was not running. --- libnm/nm-active-connection.c | 5 ++++- libnm/nm-device-bond.c | 4 ++++ libnm/nm-device-bridge.c | 4 ++++ libnm/nm-device-team.c | 4 ++++ libnm/nm-device-wifi.c | 4 ++++ libnm/nm-manager.c | 17 +++++++---------- libnm/nm-remote-settings.c | 1 + 7 files changed, 28 insertions(+), 11 deletions(-) diff --git a/libnm/nm-active-connection.c b/libnm/nm-active-connection.c index ae36bf78ee..a64877be23 100644 --- a/libnm/nm-active-connection.c +++ b/libnm/nm-active-connection.c @@ -362,8 +362,11 @@ nm_active_connection_get_master (NMActiveConnection *connection) } static void -nm_active_connection_init (NMActiveConnection *ap) +nm_active_connection_init (NMActiveConnection *connection) { + NMActiveConnectionPrivate *priv = NM_ACTIVE_CONNECTION_GET_PRIVATE (connection); + + priv->devices = g_ptr_array_new (); } static void diff --git a/libnm/nm-device-bond.c b/libnm/nm-device-bond.c index b22d8b30fb..9656de631e 100644 --- a/libnm/nm-device-bond.c +++ b/libnm/nm-device-bond.c @@ -173,7 +173,11 @@ get_hw_address (NMDevice *device) static void nm_device_bond_init (NMDeviceBond *device) { + NMDeviceBondPrivate *priv = NM_DEVICE_BOND_GET_PRIVATE (device); + _nm_device_set_device_type (NM_DEVICE (device), NM_DEVICE_TYPE_BOND); + + priv->slaves = g_ptr_array_new (); } static void diff --git a/libnm/nm-device-bridge.c b/libnm/nm-device-bridge.c index b4ee25b811..63d9040d49 100644 --- a/libnm/nm-device-bridge.c +++ b/libnm/nm-device-bridge.c @@ -173,7 +173,11 @@ get_hw_address (NMDevice *device) static void nm_device_bridge_init (NMDeviceBridge *device) { + NMDeviceBridgePrivate *priv = NM_DEVICE_BRIDGE_GET_PRIVATE (device); + _nm_device_set_device_type (NM_DEVICE (device), NM_DEVICE_TYPE_BRIDGE); + + priv->slaves = g_ptr_array_new (); } static void diff --git a/libnm/nm-device-team.c b/libnm/nm-device-team.c index 06db4cf84a..ce7a82f9cd 100644 --- a/libnm/nm-device-team.c +++ b/libnm/nm-device-team.c @@ -173,7 +173,11 @@ get_setting_type (NMDevice *device) static void nm_device_team_init (NMDeviceTeam *device) { + NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (device); + _nm_device_set_device_type (NM_DEVICE (device), NM_DEVICE_TYPE_TEAM); + + priv->slaves = g_ptr_array_new (); } static void diff --git a/libnm/nm-device-wifi.c b/libnm/nm-device-wifi.c index 1d0cd39aa7..f804875719 100644 --- a/libnm/nm-device-wifi.c +++ b/libnm/nm-device-wifi.c @@ -565,12 +565,16 @@ get_hw_address (NMDevice *device) static void nm_device_wifi_init (NMDeviceWifi *device) { + NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (device); + _nm_device_set_device_type (NM_DEVICE (device), NM_DEVICE_TYPE_WIFI); g_signal_connect (device, "notify::" NM_DEVICE_STATE, G_CALLBACK (state_changed_cb), NULL); + + priv->aps = g_ptr_array_new (); } static void diff --git a/libnm/nm-manager.c b/libnm/nm-manager.c index 7bfab68a86..24d7080b65 100644 --- a/libnm/nm-manager.c +++ b/libnm/nm-manager.c @@ -127,6 +127,8 @@ nm_manager_init (NMManager *manager) priv->connectivity = NM_CONNECTIVITY_UNKNOWN; priv->permissions = g_hash_table_new (g_direct_hash, g_direct_equal); + priv->devices = g_ptr_array_new (); + priv->active_connections = g_ptr_array_new (); } static void @@ -647,9 +649,6 @@ nm_manager_get_device_by_path (NMManager *manager, const char *object_path) g_return_val_if_fail (object_path, NULL); devices = nm_manager_get_devices (manager); - if (!devices) - return NULL; - for (i = 0; i < devices->len; i++) { NMDevice *candidate = g_ptr_array_index (devices, i); if (!strcmp (nm_object_get_path (NM_OBJECT (candidate)), object_path)) { @@ -672,9 +671,6 @@ nm_manager_get_device_by_iface (NMManager *manager, const char *iface) g_return_val_if_fail (iface, NULL); devices = nm_manager_get_devices (manager); - if (!devices) - return NULL; - for (i = 0; i < devices->len; i++) { NMDevice *candidate = g_ptr_array_index (devices, i); if (!strcmp (nm_device_get_iface (candidate), iface)) { @@ -1097,7 +1093,10 @@ free_active_connections (NMManager *manager, gboolean in_dispose) return; active_connections = priv->active_connections; - priv->active_connections = NULL; + if (in_dispose) + priv->active_connections = NULL; + else + priv->active_connections = g_ptr_array_new (); for (i = 0; i < active_connections->len; i++) { active_connection = active_connections->pdata[i]; @@ -1106,10 +1105,8 @@ free_active_connections (NMManager *manager, gboolean in_dispose) } g_ptr_array_unref (active_connections); - if (!in_dispose) { - priv->active_connections = g_ptr_array_new (); + if (!in_dispose) g_object_notify (G_OBJECT (manager), NM_MANAGER_ACTIVE_CONNECTIONS); - } } static void diff --git a/libnm/nm-remote-settings.c b/libnm/nm-remote-settings.c index 2c2dd597e6..e072e9ac1c 100644 --- a/libnm/nm-remote-settings.c +++ b/libnm/nm-remote-settings.c @@ -652,6 +652,7 @@ nm_remote_settings_init (NMRemoteSettings *self) { NMRemoteSettingsPrivate *priv = NM_REMOTE_SETTINGS_GET_PRIVATE (self); + priv->all_connections = g_ptr_array_new (); priv->visible_connections = g_ptr_array_new (); } From 2ac83c0e8b06de9ebfce3038c1afb01061f6e680 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Mon, 6 Oct 2014 21:17:14 -0400 Subject: [PATCH 02/11] libnm: fix async property circular reference handling 3e5b3833 changed various libnm methods to return 0-length arrays rather than NULL, and changed various other places to assume this behavior. The guarantee that they didn't return NULL relied on the assumption that all D-Bus properties would get initialized by NMObject code before anyone could call their get methods. However, this assumption was violated in the case where two objects circularly referenced each other (eg, NMDevice and NMActiveConnection). f9f9d297 attempted to fix this by slightly changing the ordering of NMObjectCache operations, but it actually ended up breaking things and causing infinite loops of object creation in some cases. There's no way to guarantee we never return partially-initialized objects without heavily rewriting the object-creation code, so this reverts most of f9f9d297 (leaving only the new comment in _nm_object_create()). The crashes introduced by 3e5b3833 will no longer happen because we've now fixed the classes to have initialized their object arrays to non-NULL values even before they get the real values. --- libnm/nm-object.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/libnm/nm-object.c b/libnm/nm-object.c index 73dbade4ab..0e82861fc4 100644 --- a/libnm/nm-object.c +++ b/libnm/nm-object.c @@ -736,21 +736,7 @@ create_async_inited (GObject *object, GAsyncResult *result, gpointer user_data) NMObjectTypeAsyncData *async_data = user_data; GError *error = NULL; - if (g_async_initable_init_finish (G_ASYNC_INITABLE (object), result, &error)) { - NMObject *cached; - - /* Unlike in the sync case, in the async case we can't cache the object - * until it is completely initialized. But that means someone else might - * have been creating it at the same time, and they might have finished - * before us. - */ - cached = _nm_object_cache_get (nm_object_get_path (NM_OBJECT (object))); - if (cached) { - g_object_unref (object); - object = G_OBJECT (cached); - } else - _nm_object_cache_add (NM_OBJECT (object)); - } else { + if (!g_async_initable_init_finish (G_ASYNC_INITABLE (object), result, &error)) { dbgmsg ("Could not create object for %s: %s", nm_object_get_path (NM_OBJECT (object)), error->message); @@ -766,6 +752,17 @@ create_async_got_type (NMObjectTypeAsyncData *async_data, GType type) { GObject *object; + /* Ensure we don't have the object already; we may get multiple type + * requests for the same object if there are multiple properties on + * other objects that refer to the object at this path. One of those + * other requests may have already completed. + */ + object = (GObject *) _nm_object_cache_get (async_data->path); + if (object) { + create_async_complete (object, async_data); + return; + } + if (type == G_TYPE_INVALID) { /* Don't know how to create this object */ create_async_complete (NULL, async_data); @@ -776,6 +773,7 @@ create_async_got_type (NMObjectTypeAsyncData *async_data, GType type) NM_OBJECT_PATH, async_data->path, NM_OBJECT_DBUS_CONNECTION, async_data->connection, NULL); + _nm_object_cache_add (NM_OBJECT (object)); g_async_initable_init_async (G_ASYNC_INITABLE (object), G_PRIORITY_DEFAULT, NULL, create_async_inited, async_data); } From a9e906fcbd0fb06a9d435e7a1db21a0f91521669 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Fri, 10 Oct 2014 14:06:44 -0400 Subject: [PATCH 03/11] libnm: add -added and -removed signals for ACs to NMManager --- libnm/nm-manager.c | 21 ++++++++++++++++++++- libnm/nm-manager.h | 2 ++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/libnm/nm-manager.c b/libnm/nm-manager.c index 24d7080b65..f2e6fd0ed1 100644 --- a/libnm/nm-manager.c +++ b/libnm/nm-manager.c @@ -105,6 +105,8 @@ enum { enum { DEVICE_ADDED, DEVICE_REMOVED, + ACTIVE_CONNECTION_ADDED, + ACTIVE_CONNECTION_REMOVED, PERMISSION_CHANGED, LAST_SIGNAL @@ -170,7 +172,7 @@ init_dbus (NMObject *object) { NM_MANAGER_WWAN_HARDWARE_ENABLED, &priv->wwan_hw_enabled }, { NM_MANAGER_WIMAX_ENABLED, &priv->wimax_enabled }, { NM_MANAGER_WIMAX_HARDWARE_ENABLED, &priv->wimax_hw_enabled }, - { NM_MANAGER_ACTIVE_CONNECTIONS, &priv->active_connections, NULL, NM_TYPE_ACTIVE_CONNECTION }, + { NM_MANAGER_ACTIVE_CONNECTIONS, &priv->active_connections, NULL, NM_TYPE_ACTIVE_CONNECTION, "active-connection" }, { NM_MANAGER_CONNECTIVITY, &priv->connectivity }, { NM_MANAGER_PRIMARY_CONNECTION, &priv->primary_connection, NULL, NM_TYPE_ACTIVE_CONNECTION }, { NM_MANAGER_ACTIVATING_CONNECTION, &priv->activating_connection, NULL, NM_TYPE_ACTIVE_CONNECTION }, @@ -1100,6 +1102,7 @@ free_active_connections (NMManager *manager, gboolean in_dispose) for (i = 0; i < active_connections->len; i++) { active_connection = active_connections->pdata[i]; + g_signal_emit (manager, signals[ACTIVE_CONNECTION_REMOVED], 0, active_connection); /* Break circular refs */ g_object_run_dispose (G_OBJECT (active_connection)); } @@ -1571,6 +1574,22 @@ nm_manager_class_init (NMManagerClass *manager_class) NULL, NULL, NULL, G_TYPE_NONE, 1, G_TYPE_OBJECT); + signals[ACTIVE_CONNECTION_ADDED] = + g_signal_new ("active-connection-added", + G_OBJECT_CLASS_TYPE (object_class), + G_SIGNAL_RUN_FIRST, + G_STRUCT_OFFSET (NMManagerClass, active_connection_added), + NULL, NULL, NULL, + G_TYPE_NONE, 1, + G_TYPE_OBJECT); + signals[ACTIVE_CONNECTION_REMOVED] = + g_signal_new ("active-connection-removed", + G_OBJECT_CLASS_TYPE (object_class), + G_SIGNAL_RUN_FIRST, + G_STRUCT_OFFSET (NMManagerClass, active_connection_removed), + NULL, NULL, NULL, + G_TYPE_NONE, 1, + G_TYPE_OBJECT); signals[PERMISSION_CHANGED] = g_signal_new ("permission-changed", G_OBJECT_CLASS_TYPE (object_class), diff --git a/libnm/nm-manager.h b/libnm/nm-manager.h index 430b038e03..ca9f7dd60b 100644 --- a/libnm/nm-manager.h +++ b/libnm/nm-manager.h @@ -61,6 +61,8 @@ typedef struct { /* Signals */ void (*device_added) (NMManager *manager, NMDevice *device); void (*device_removed) (NMManager *manager, NMDevice *device); + void (*active_connection_added) (NMManager *manager, NMActiveConnection *ac); + void (*active_connection_removed) (NMManager *manager, NMActiveConnection *ac); void (*permission_changed) (NMManager *manager, NMClientPermission permission, NMClientPermissionResult result); From d78ea27d36a0ba3039c6c1b1b94e4ae784afa4ab Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Fri, 10 Oct 2014 14:43:55 -0400 Subject: [PATCH 04/11] libnm: postpone activation callback until all objects are ready In some cases, the nm_client_activate_connection() callback could be invoked when either the NMActiveConnection or the NMDevice had not been initialized yet. Fix it to wait for both of them to be fully initialized before invoking the callback. --- libnm/nm-manager.c | 151 +++++++++++++++++++++++++++++++-------------- 1 file changed, 103 insertions(+), 48 deletions(-) diff --git a/libnm/nm-manager.c b/libnm/nm-manager.c index f2e6fd0ed1..4088548344 100644 --- a/libnm/nm-manager.c +++ b/libnm/nm-manager.c @@ -154,8 +154,6 @@ wireless_enabled_cb (GObject *object, GParamSpec *pspec, gpointer user_data) } static void manager_recheck_permissions (NMDBusManager *proxy, gpointer user_data); -static void active_connections_changed_cb (GObject *object, GParamSpec *pspec, gpointer user_data); -static void object_creation_failed_cb (GObject *object, GError *error, char *failed_path); static void init_dbus (NMObject *object) @@ -747,56 +745,56 @@ activate_info_complete (ActivateInfo *info, g_slice_free (ActivateInfo, info); } +static NMActiveConnection * +find_active_connection_by_path (NMManager *self, const char *ac_path) +{ + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); + int i; + + for (i = 0; i < priv->active_connections->len; i++) { + NMActiveConnection *candidate = g_ptr_array_index (priv->active_connections, i); + const char *candidate_path = nm_object_get_path (NM_OBJECT (candidate)); + + if (g_strcmp0 (ac_path, candidate_path) == 0) + return candidate; + } + + return NULL; +} + static void -recheck_pending_activations (NMManager *self, const char *failed_path, GError *error) +recheck_pending_activations (NMManager *self) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); GSList *iter, *next; - const GPtrArray *active_connections; - gboolean found_in_active = FALSE; - gboolean found_in_pending = FALSE; - ActivateInfo *ainfo = NULL; - int i; + NMActiveConnection *candidate; + const GPtrArray *devices; + NMDevice *device; - active_connections = nm_manager_get_active_connections (self); - - /* For each pending activation, look for a active connection that has - * the pending activation's object path, and call pending connection's - * callback. - * If the connection to activate doesn't make it to active_connections, - * due to an error, we have to call the callback for failed_path. + /* For each pending activation, look for an active connection that has the + * pending activation's object path, where the active connection and its + * device have both updated their properties to point to each other, and + * call the pending connection's callback. */ for (iter = priv->pending_activations; iter; iter = next) { ActivateInfo *info = iter->data; next = g_slist_next (iter); - if (!found_in_pending && failed_path && g_strcmp0 (failed_path, info->active_path) == 0) { - found_in_pending = TRUE; - ainfo = info; - } + candidate = find_active_connection_by_path (self, info->active_path); + if (!candidate) + continue; - for (i = 0; i < active_connections->len; i++) { - NMActiveConnection *active = g_ptr_array_index (active_connections, i); - const char *active_path = nm_object_get_path (NM_OBJECT (active)); + /* Check that the AC and device are both ready */ + devices = nm_active_connection_get_devices (candidate); + if (devices->len == 0) + continue; + device = devices->pdata[0]; + if (nm_device_get_active_connection (device) != candidate) + continue; - if (!found_in_active && failed_path && g_strcmp0 (failed_path, active_path) == 0) - found_in_active = TRUE; - - if (g_strcmp0 (info->active_path, active_path) == 0) { - /* Call the pending activation's callback and it all up */ - activate_info_complete (info, active, NULL); - break; - } - } - } - - if (!found_in_active && found_in_pending) { - /* A newly activated connection failed due to some immediate error - * and disappeared from active connection list. Make sure the - * callback gets called. - */ - activate_info_complete (ainfo, NULL, error); + activate_info_complete (info, candidate, NULL); + break; } } @@ -830,7 +828,7 @@ activate_cb (GObject *object, G_CALLBACK (activation_cancelled), info); } - recheck_pending_activations (info->manager, NULL, NULL); + recheck_pending_activations (info->manager); } else { activate_info_complete (info, NULL, error); g_clear_error (&error); @@ -905,7 +903,7 @@ add_activate_cb (GObject *object, G_CALLBACK (activation_cancelled), info); } - recheck_pending_activations (info->manager, NULL, NULL); + recheck_pending_activations (info->manager); } else { activate_info_complete (info, NULL, error); g_clear_error (&error); @@ -969,16 +967,71 @@ nm_manager_add_and_activate_connection_finish (NMManager *manager, } static void -active_connections_changed_cb (GObject *object, GParamSpec *pspec, gpointer user_data) +device_ac_changed (GObject *object, GParamSpec *pspec, gpointer user_data) { - recheck_pending_activations (NM_MANAGER (object), NULL, NULL); + NMManager *self = user_data; + + recheck_pending_activations (self); +} + +static void +device_added (NMManager *self, NMDevice *device) +{ + g_signal_connect (device, "notify::" NM_DEVICE_ACTIVE_CONNECTION, + G_CALLBACK (device_ac_changed), self); +} + +static void +device_removed (NMManager *self, NMDevice *device) +{ + g_signal_handlers_disconnect_by_func (device, G_CALLBACK (device_ac_changed), self); +} + +static void +ac_devices_changed (GObject *object, GParamSpec *pspec, gpointer user_data) +{ + NMManager *self = user_data; + + recheck_pending_activations (self); +} + +static void +active_connection_added (NMManager *self, NMActiveConnection *ac) +{ + g_signal_connect (ac, "notify::" NM_ACTIVE_CONNECTION_DEVICES, + G_CALLBACK (ac_devices_changed), self); + recheck_pending_activations (self); +} + +static void +active_connection_removed (NMManager *self, NMActiveConnection *ac) +{ + g_signal_handlers_disconnect_by_func (ac, G_CALLBACK (ac_devices_changed), self); } static void object_creation_failed_cb (GObject *object, GError *error, char *failed_path) { - if (error) - recheck_pending_activations (NM_MANAGER (object), failed_path, error); + NMManager *self = NM_MANAGER (object); + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); + GSList *iter; + + g_return_if_fail (error != NULL); + g_return_if_fail (find_active_connection_by_path (self, failed_path) == NULL); + + /* A newly activated connection failed due to some immediate error + * and disappeared from active connection list. Make sure the + * callback gets called. + */ + + for (iter = priv->pending_activations; iter; iter = iter->next) { + ActivateInfo *info = iter->data; + + if (g_strcmp0 (failed_path, info->active_path) == 0) { + activate_info_complete (info, NULL, error); + return; + } + } } gboolean @@ -1176,9 +1229,6 @@ constructed (GObject *object) g_signal_connect (object, "notify::" NM_MANAGER_WIRELESS_ENABLED, G_CALLBACK (wireless_enabled_cb), NULL); - g_signal_connect (object, "notify::" NM_MANAGER_ACTIVE_CONNECTIONS, - G_CALLBACK (active_connections_changed_cb), NULL); - g_signal_connect (object, "object-creation-failed", G_CALLBACK (object_creation_failed_cb), NULL); } @@ -1455,6 +1505,11 @@ nm_manager_class_init (NMManagerClass *manager_class) nm_object_class->init_dbus = init_dbus; + manager_class->device_added = device_added; + manager_class->device_removed = device_removed; + manager_class->active_connection_added = active_connection_added; + manager_class->active_connection_removed = active_connection_removed; + /* properties */ g_object_class_install_property From 4779d96685b3e2c295218088db8a3999450da39b Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 16 Oct 2014 18:00:54 -0400 Subject: [PATCH 05/11] core: lie about NMActiveConnection:state in new connections NMActiveConnections start out in state "unknown", but then quickly switch to "activating". Unfortunately, it's sometimes possible for this to be externally visible. Fix this by lying and saying that state is "activating" during the initial "unknown" stage (though not if the state changes to "unknown" later on). (Actually changing the initial state to "activating" breaks things because some code depends on there being a transition into the "activating" state.) --- src/nm-active-connection.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index 3e532e5225..d91f462663 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -51,6 +51,7 @@ typedef struct { gboolean is_default; gboolean is_default6; NMActiveConnectionState state; + gboolean state_set; gboolean vpn; NMAuthSubject *subject; @@ -138,6 +139,7 @@ nm_active_connection_set_state (NMActiveConnection *self, old_state = priv->state; priv->state = new_state; + priv->state_set = TRUE; g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_STATE); check_master_ready (self); @@ -748,7 +750,14 @@ get_property (GObject *object, guint prop_id, g_value_take_boxed (value, devices); break; case PROP_STATE: - g_value_set_uint (value, priv->state); + if (priv->state_set) + g_value_set_uint (value, priv->state); + else { + /* When the AC has just been created, its externally-visible state should + * be "ACTIVATING", even though internally it is "UNKNOWN". + */ + g_value_set_uint (value, NM_ACTIVE_CONNECTION_STATE_ACTIVATING); + } break; case PROP_DEFAULT: g_value_set_boolean (value, priv->is_default); From 08a344d723d1c22a9f0f3b2be85904dab9c3fd61 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 8 Oct 2014 18:14:30 -0400 Subject: [PATCH 06/11] libnm: abstract out duplicated device-creating code in tests --- libnm/tests/common.c | 67 ++++++++++++ libnm/tests/common.h | 6 ++ libnm/tests/test-nm-client.c | 184 +++++--------------------------- libnm/tests/test-secret-agent.c | 46 ++------ 4 files changed, 103 insertions(+), 200 deletions(-) diff --git a/libnm/tests/common.c b/libnm/tests/common.c index 109ab04322..79308329a0 100644 --- a/libnm/tests/common.c +++ b/libnm/tests/common.c @@ -116,3 +116,70 @@ nm_test_service_cleanup (NMTestServiceInfo *info) memset (info, 0, sizeof (*info)); g_free (info); } + +typedef struct { + GMainLoop *loop; + const char *ifname; + char *path; + NMDevice *device; +} AddDeviceInfo; + +static void +device_added_cb (NMClient *client, + NMDevice *device, + gpointer user_data) +{ + AddDeviceInfo *info = user_data; + + g_assert (device); + g_assert_cmpstr (nm_object_get_path (NM_OBJECT (device)), ==, info->path); + g_assert_cmpstr (nm_device_get_iface (device), ==, info->ifname); + + info->device = device; + g_main_loop_quit (info->loop); +} + +static gboolean +timeout (gpointer user_data) +{ + g_assert_not_reached (); + return G_SOURCE_REMOVE; +} + +NMDevice * +nm_test_service_add_device (NMTestServiceInfo *sinfo, NMClient *client, + const char *method, const char *ifname) +{ + AddDeviceInfo info; + GError *error = NULL; + GVariant *ret; + guint timeout_id; + + ret = g_dbus_proxy_call_sync (sinfo->proxy, + method, + g_variant_new ("(s)", ifname), + G_DBUS_CALL_FLAGS_NO_AUTO_START, + 3000, + NULL, + &error); + g_assert_no_error (error); + g_assert (ret); + g_assert_cmpstr (g_variant_get_type_string (ret), ==, "(o)"); + g_variant_get (ret, "(o)", &info.path); + g_variant_unref (ret); + + /* Wait for libnm to find the device */ + info.ifname = ifname; + info.loop = g_main_loop_new (NULL, FALSE); + g_signal_connect (client, "device-added", + G_CALLBACK (device_added_cb), &info); + timeout_id = g_timeout_add_seconds (5, timeout, NULL); + g_main_loop_run (info.loop); + + g_source_remove (timeout_id); + g_signal_handlers_disconnect_by_func (client, device_added_cb, &info); + g_free (info.path); + g_main_loop_unref (info.loop); + + return info.device; +} diff --git a/libnm/tests/common.h b/libnm/tests/common.h index 7c49d2532b..aa15167b85 100644 --- a/libnm/tests/common.h +++ b/libnm/tests/common.h @@ -19,6 +19,7 @@ */ #include +#include typedef struct { GDBusConnection *bus; @@ -29,3 +30,8 @@ typedef struct { NMTestServiceInfo *nm_test_service_init (void); void nm_test_service_cleanup (NMTestServiceInfo *info); + +NMDevice *nm_test_service_add_device (NMTestServiceInfo *info, + NMClient *client, + const char *method, + const char *ifname); diff --git a/libnm/tests/test-nm-client.c b/libnm/tests/test-nm-client.c index e666551f49..82943e588f 100644 --- a/libnm/tests/test-nm-client.c +++ b/libnm/tests/test-nm-client.c @@ -40,65 +40,14 @@ loop_quit (gpointer user_data) return G_SOURCE_REMOVE; } -static gboolean -add_device (const char *method, const char *ifname, char **out_path) -{ - GError *error = NULL; - GVariant *ret; - - ret = g_dbus_proxy_call_sync (sinfo->proxy, - method, - g_variant_new ("(s)", ifname), - G_DBUS_CALL_FLAGS_NO_AUTO_START, - 3000, - NULL, - &error); - g_assert_no_error (error); - g_assert (ret); - g_assert_cmpstr (g_variant_get_type_string (ret), ==, "(o)"); - if (out_path) - g_variant_get (ret, "(o)", out_path); - g_variant_unref (ret); - return TRUE; -} - /*******************************************************************/ -typedef struct { - GMainLoop *loop; - gboolean signaled; - gboolean notified; - guint quit_count; - guint quit_id; -} DeviceAddedInfo; - -static void -device_add_check_quit (DeviceAddedInfo *info) -{ - info->quit_count--; - if (info->quit_count == 0) { - g_source_remove (info->quit_id); - info->quit_id = 0; - g_main_loop_quit (info->loop); - } -} - -static void -device_added_cb (NMClient *c, - NMDevice *device, - DeviceAddedInfo *info) -{ - g_assert (device); - g_assert_cmpstr (nm_device_get_iface (device), ==, "eth0"); - info->signaled = TRUE; - device_add_check_quit (info); -} - static void devices_notify_cb (NMClient *c, GParamSpec *pspec, - DeviceAddedInfo *info) + gpointer user_data) { + gboolean *notified = user_data; const GPtrArray *devices; NMDevice *device; @@ -110,9 +59,7 @@ devices_notify_cb (NMClient *c, g_assert (device); g_assert_cmpstr (nm_device_get_iface (device), ==, "eth0"); - info->notified = TRUE; - - device_add_check_quit (info); + *notified = TRUE; } static void @@ -121,7 +68,7 @@ test_device_added (void) NMClient *client; const GPtrArray *devices; NMDevice *device; - DeviceAddedInfo info = { loop, FALSE, FALSE, 0, 0 }; + gboolean notified = FALSE; GError *error = NULL; sinfo = nm_test_service_init (); @@ -131,30 +78,18 @@ test_device_added (void) devices = nm_client_get_devices (client); g_assert (devices->len == 0); - /* Tell the test service to add a new device */ - add_device ("AddWiredDevice", "eth0", NULL); - - g_signal_connect (client, - "device-added", - (GCallback) device_added_cb, - &info); - info.quit_count++; - g_signal_connect (client, "notify::devices", (GCallback) devices_notify_cb, - &info); - info.quit_count++; + ¬ified); - /* Wait for libnm to find the device */ - info.quit_id = g_timeout_add_seconds (5, loop_quit, loop); - g_main_loop_run (loop); + /* Tell the test service to add a new device */ + nm_test_service_add_device (sinfo, client, "AddWiredDevice", "eth0"); - g_assert (info.signaled); - g_assert (info.notified); + while (!notified) + g_main_context_iteration (NULL, TRUE); - g_signal_handlers_disconnect_by_func (client, device_added_cb, &info); - g_signal_handlers_disconnect_by_func (client, devices_notify_cb, &info); + g_signal_handlers_disconnect_by_func (client, devices_notify_cb, ¬ified); devices = nm_client_get_devices (client); g_assert (devices); @@ -193,16 +128,6 @@ wifi_check_quit (WifiApInfo *info) } } -static void -wifi_device_added_cb (NMClient *c, - NMDevice *device, - WifiApInfo *info) -{ - g_assert_cmpstr (nm_device_get_iface (device), ==, "wlan0"); - info->found = TRUE; - wifi_check_quit (info); -} - static void got_ap_path (WifiApInfo *info, const char *path) { @@ -288,26 +213,11 @@ test_wifi_ap_added_removed (void) /*************************************/ /* Add the wifi device */ - add_device ("AddWifiDevice", "wlan0", NULL); - - g_signal_connect (client, - "device-added", - (GCallback) wifi_device_added_cb, - &info); - info.quit_count = 1; - - /* Wait for libnm to find the device */ - info.quit_id = g_timeout_add_seconds (5, loop_quit, loop); - g_main_loop_run (loop); - - g_assert (info.found); - g_signal_handlers_disconnect_by_func (client, wifi_device_added_cb, &info); - - wifi = (NMDeviceWifi *) nm_client_get_device_by_iface (client, "wlan0"); + wifi = (NMDeviceWifi *) nm_test_service_add_device (sinfo, client, "AddWifiDevice", "wlan0"); g_assert (NM_IS_DEVICE_WIFI (wifi)); /*************************************/ - /* Add the wifi device */ + /* Add the wifi AP */ info.signaled = FALSE; info.notified = FALSE; info.quit_id = 0; @@ -417,16 +327,6 @@ wimax_check_quit (WimaxNspInfo *info) } } -static void -wimax_device_added_cb (NMClient *c, - NMDevice *device, - WimaxNspInfo *info) -{ - g_assert_cmpstr (nm_device_get_iface (device), ==, "wmx0"); - info->found = TRUE; - wimax_check_quit (info); -} - static void got_nsp_path (WimaxNspInfo *info, const char *path) { @@ -512,22 +412,7 @@ test_wimax_nsp_added_removed (void) /*************************************/ /* Add the wimax device */ - add_device ("AddWimaxDevice", "wmx0", NULL); - - g_signal_connect (client, - "device-added", - (GCallback) wimax_device_added_cb, - &info); - info.quit_count = 1; - - /* Wait for libnm to find the device */ - info.quit_id = g_timeout_add_seconds (5, loop_quit, loop); - g_main_loop_run (loop); - - g_assert (info.found); - g_signal_handlers_disconnect_by_func (client, wimax_device_added_cb, &info); - - wimax = (NMDeviceWimax *) nm_client_get_device_by_iface (client, "wmx0"); + wimax = (NMDeviceWimax *) nm_test_service_add_device (sinfo, client, "AddWimaxDevice", "wmx0"); g_assert (NM_IS_DEVICE_WIMAX (wimax)); /*************************************/ @@ -637,14 +522,6 @@ da_check_quit (DaInfo *info) } } -static void -da_device_added_cb (NMClient *c, - NMDevice *device, - DaInfo *info) -{ - da_check_quit (info); -} - static void da_device_removed_cb (NMClient *c, NMDevice *device, @@ -700,8 +577,7 @@ test_devices_array (void) { NMClient *client = NULL; DaInfo info = { loop }; - char *paths[3] = { NULL, NULL, NULL }; - NMDevice *device; + NMDevice *wlan0, *eth0, *eth1, *device; const GPtrArray *devices; GError *error = NULL; GVariant *ret; @@ -715,22 +591,9 @@ test_devices_array (void) /*************************************/ /* Add some devices */ - add_device ("AddWifiDevice", "wlan0", &paths[0]); - add_device ("AddWiredDevice", "eth0", &paths[1]); - add_device ("AddWiredDevice", "eth1", &paths[2]); - info.quit_count = 3; - - g_signal_connect (client, - "device-added", - (GCallback) da_device_added_cb, - &info); - - /* Wait for libnm to find the device */ - info.quit_id = g_timeout_add_seconds (5, loop_quit, loop); - g_main_loop_run (loop); - - g_assert_cmpint (info.quit_count, ==, 0); - g_signal_handlers_disconnect_by_func (client, da_device_added_cb, &info); + wlan0 = nm_test_service_add_device (sinfo, client,"AddWifiDevice", "wlan0"); + eth0 = nm_test_service_add_device (sinfo, client, "AddWiredDevice", "eth0"); + eth1 = nm_test_service_add_device (sinfo, client, "AddWiredDevice", "eth1"); /* Ensure the devices now exist */ devices = nm_client_get_devices (client); @@ -739,18 +602,21 @@ test_devices_array (void) device = nm_client_get_device_by_iface (client, "wlan0"); g_assert (NM_IS_DEVICE_WIFI (device)); + g_assert (device == wlan0); device = nm_client_get_device_by_iface (client, "eth0"); g_assert (NM_IS_DEVICE_ETHERNET (device)); + g_assert (device == eth0); device = nm_client_get_device_by_iface (client, "eth1"); g_assert (NM_IS_DEVICE_ETHERNET (device)); + g_assert (device == eth1); /********************************/ /* Now remove the device in the middle */ ret = g_dbus_proxy_call_sync (sinfo->proxy, "RemoveDevice", - g_variant_new ("(o)", paths[1]), + g_variant_new ("(o)", nm_object_get_path (NM_OBJECT (eth0))), G_DBUS_CALL_FLAGS_NO_AUTO_START, 3000, NULL, @@ -770,7 +636,7 @@ test_devices_array (void) &info); info.quit_count = 2; - /* Wait for libnm to find the device */ + /* Wait for libnm to notice the changes */ info.quit_id = g_timeout_add_seconds (5, loop_quit, loop); g_main_loop_run (loop); @@ -785,13 +651,11 @@ test_devices_array (void) device = nm_client_get_device_by_iface (client, "wlan0"); g_assert (NM_IS_DEVICE_WIFI (device)); + g_assert (device == wlan0); device = nm_client_get_device_by_iface (client, "eth1"); g_assert (NM_IS_DEVICE_ETHERNET (device)); - - g_free (paths[0]); - g_free (paths[1]); - g_free (paths[2]); + g_assert (device == eth1); g_object_unref (client); g_clear_pointer (&sinfo, nm_test_service_cleanup); diff --git a/libnm/tests/test-secret-agent.c b/libnm/tests/test-secret-agent.c index de49893c75..83755450fe 100644 --- a/libnm/tests/test-secret-agent.c +++ b/libnm/tests/test-secret-agent.c @@ -192,7 +192,6 @@ typedef struct { char *ifname; char *con_id; - char *devpath; int secrets_requested; } TestSecretAgentData; @@ -203,20 +202,6 @@ timeout_assert (gpointer user_data) g_assert_not_reached (); } -static void -device_added_cb (NMClient *c, - NMDevice *device, - gpointer user_data) -{ - TestSecretAgentData *sadata = user_data; - - g_assert (device); - g_assert_cmpstr (nm_device_get_iface (device), ==, sadata->ifname); - - sadata->device = device; - g_main_loop_quit (sadata->loop); -} - static void connection_added_cb (GObject *s, GAsyncResult *result, @@ -261,8 +246,6 @@ test_setup (TestSecretAgentData *sadata, gconstpointer test_data) GBytes *ssid; NMSetting *s_wsec; GError *error = NULL; - GVariant *ret; - gulong handler; sadata->sinfo = nm_test_service_init (); sadata->client = nm_client_new (NULL, &error); @@ -276,24 +259,8 @@ test_setup (TestSecretAgentData *sadata, gconstpointer test_data) counter++; /* Create the device */ - ret = g_dbus_proxy_call_sync (sadata->sinfo->proxy, - "AddWifiDevice", - g_variant_new ("(s)", sadata->ifname), - G_DBUS_CALL_FLAGS_NO_AUTO_START, - 3000, - NULL, - &error); - g_assert_no_error (error); - g_assert (ret); - g_assert_cmpstr (g_variant_get_type_string (ret), ==, "(o)"); - g_variant_get (ret, "(o)", &sadata->devpath); - g_variant_unref (ret); - - handler = g_signal_connect (sadata->client, "device-added", - G_CALLBACK (device_added_cb), sadata); - g_main_loop_run (sadata->loop); - g_signal_handler_disconnect (sadata->client, handler); - g_assert (sadata->device); + sadata->device = nm_test_service_add_device (sadata->sinfo, sadata->client, + "AddWifiDevice", sadata->ifname); /* Create the connection */ connection = nmtst_create_minimal_connection (sadata->con_id, NULL, NM_SETTING_WIRELESS_SETTING_NAME, &s_con); @@ -353,12 +320,9 @@ test_cleanup (TestSecretAgentData *sadata, gconstpointer test_data) g_object_unref (sadata->agent); } - g_object_unref (sadata->connection); - g_object_unref (sadata->client); - ret = g_dbus_proxy_call_sync (sadata->sinfo->proxy, "RemoveDevice", - g_variant_new ("(s)", sadata->devpath), + g_variant_new ("(s)", nm_object_get_path (NM_OBJECT (sadata->device))), G_DBUS_CALL_FLAGS_NO_AUTO_START, 3000, NULL, @@ -366,6 +330,9 @@ test_cleanup (TestSecretAgentData *sadata, gconstpointer test_data) g_assert_no_error (error); g_variant_unref (ret); + g_object_unref (sadata->connection); + g_object_unref (sadata->client); + nm_test_service_cleanup (sadata->sinfo); g_source_remove (sadata->timeout_id); @@ -373,7 +340,6 @@ test_cleanup (TestSecretAgentData *sadata, gconstpointer test_data) g_free (sadata->ifname); g_free (sadata->con_id); - g_free (sadata->devpath); } /*******************************************************************/ From b9c09a2b0d991a783724ca6583f5d0c795c56253 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Mon, 6 Oct 2014 13:35:03 -0400 Subject: [PATCH 07/11] tools: add a bit more activation support to test-networkmanager-service.py Now test-networkmanager-service.py can create ActiveConnections, though they don't actually finish activating. --- libnm/tests/test-secret-agent.c | 9 +-- tools/test-networkmanager-service.py | 92 +++++++++++++++++++++++++++- 2 files changed, 91 insertions(+), 10 deletions(-) diff --git a/libnm/tests/test-secret-agent.c b/libnm/tests/test-secret-agent.c index 83755450fe..a1ce5d5324 100644 --- a/libnm/tests/test-secret-agent.c +++ b/libnm/tests/test-secret-agent.c @@ -497,14 +497,9 @@ connection_activated_good_cb (GObject *c, GError *error = NULL; ac = nm_client_activate_connection_finish (sadata->client, result, &error); + g_assert_no_error (error); - /* test-networkmanager-service.py doesn't implement activation, but - * we should at least get as far as the error telling us that (which the - * other tests won't get to). - */ - g_assert (error != NULL); - g_dbus_error_strip_remote_error (error); - g_assert_cmpstr (error->message, ==, "Not yet implemented"); + g_object_unref (ac); g_main_loop_quit (sadata->loop); } diff --git a/tools/test-networkmanager-service.py b/tools/test-networkmanager-service.py index defc320937..c3e251e49b 100755 --- a/tools/test-networkmanager-service.py +++ b/tools/test-networkmanager-service.py @@ -37,6 +37,7 @@ NM_DEVICE_STATE_ACTIVATED = 100 NM_DEVICE_STATE_DEACTIVATING = 110 NM_DEVICE_STATE_FAILED = 120 +# Device type NM_DEVICE_TYPE_UNKNOWN = 0 NM_DEVICE_TYPE_ETHERNET = 1 NM_DEVICE_TYPE_WIFI = 2 @@ -54,6 +55,13 @@ NM_DEVICE_TYPE_BRIDGE = 13 NM_DEVICE_TYPE_GENERIC = 14 NM_DEVICE_TYPE_TEAM = 15 +# AC state +NM_ACTIVE_CONNECTION_STATE_UNKNOWN = 0 +NM_ACTIVE_CONNECTION_STATE_ACTIVATING = 1 +NM_ACTIVE_CONNECTION_STATE_ACTIVATED = 2 +NM_ACTIVE_CONNECTION_STATE_DEACTIVATING = 3 +NM_ACTIVE_CONNECTION_STATE_DEACTIVATED = 4 + ######################################################### IFACE_DBUS = 'org.freedesktop.DBus' @@ -171,6 +179,9 @@ class Device(ExportedObj): def PropertiesChanged(self, changed): pass + def set_active_connection(self, ac): + self.active_connection = ac + self.__notify(PD_ACTIVE_CONNECTION) ################################################################### @@ -507,6 +518,73 @@ class WimaxDevice(Device): return raise NspNotFoundException("NSP %s not found" % path) +################################################################### +IFACE_ACTIVE_CONNECTION = 'org.freedesktop.NetworkManager.Connection.Active' + +PAC_CONNECTION = "Connection" +PAC_SPECIFIC_OBJECT = "SpecificObject" +PAC_ID = "Id" +PAC_UUID = "Uuid" +PAC_TYPE = "Type" +PAC_DEVICES = "Devices" +PAC_STATE = "State" +PAC_DEFAULT = "Default" +PAC_IP4CONFIG = "Ip4Config" +PAC_DHCP4CONFIG = "Dhcp4Config" +PAC_DEFAULT6 = "Default6" +PAC_IP6CONFIG = "Ip6Config" +PAC_DHCP6CONFIG = "Dhcp6Config" +PAC_VPN = "Vpn" +PAC_MASTER = "Master" + +class ActiveConnection(ExportedObj): + counter = 1 + + def __init__(self, bus, device, connection, specific_object): + object_path = "/org/freedesktop/NetworkManager/ActiveConnection/%d" % ActiveConnection.counter + ActiveConnection.counter = ActiveConnection.counter + 1 + ExportedObj.__init__(self, bus, object_path) + self.add_dbus_interface(IFACE_ACTIVE_CONNECTION, self.__get_props) + + self.device = device + self.conn = connection + self.specific_object = specific_object + self.state = NM_ACTIVE_CONNECTION_STATE_UNKNOWN + self.default = False + self.ip4config = None + self.dhcp4config = None + self.default6 = False + self.ip6config = None + self.dhcp6config = None + self.vpn = False + self.master = None + + # Properties interface + def __get_props(self): + props = {} + props[PAC_CONNECTION] = to_path(self.conn) + props[PAC_SPECIFIC_OBJECT] = to_path(self.specific_object) + conn_settings = self.conn.GetSettings() + s_con = conn_settings['connection'] + props[PAC_ID] = s_con['id'] + props[PAC_UUID] = s_con['uuid'] + props[PAC_TYPE] = s_con['type'] + props[PAC_DEVICES] = to_path_array([self.device]) + props[PAC_STATE] = dbus.UInt32(self.state) + props[PAC_DEFAULT] = self.default + props[PAC_IP4CONFIG] = to_path(self.ip4config) + props[PAC_DHCP4CONFIG] = to_path(self.dhcp4config) + props[PAC_DEFAULT6] = self.default6 + props[PAC_IP6CONFIG] = to_path(self.ip6config) + props[PAC_DHCP6CONFIG] = to_path(self.dhcp6config) + props[PAC_VPN] = self.vpn + props[PAC_MASTER] = to_path(self.master) + return props + + @dbus.service.signal(IFACE_ACTIVE_CONNECTION, signature='a{sv}') + def PropertiesChanged(self, changed): + pass + ################################################################### IFACE_TEST = 'org.freedesktop.NetworkManager.LibnmGlibTest' IFACE_NM = 'org.freedesktop.NetworkManager' @@ -536,11 +614,15 @@ PM_STATE = 'State' PM_VERSION = 'Version' PM_CONNECTIVITY = 'Connectivity' +def set_device_ac_cb(device, ac): + device.set_active_connection(ac) + class NetworkManager(ExportedObj): def __init__(self, bus, object_path): ExportedObj.__init__(self, bus, object_path) self.add_dbus_interface(IFACE_NM, self.__get_props) + self._bus = bus; self.devices = [] self.active_connections = [] self.primary_connection = None @@ -598,7 +680,11 @@ class NetworkManager(ExportedObj): if not s_wsec.has_key('psk'): raise NoSecretsException("No secrets provided") - raise PermissionDeniedException("Not yet implemented") + ac = ActiveConnection(self._bus, device, connection, None) + self.active_connections.append(ac) + self.__notify(PM_ACTIVE_CONNECTIONS) + GLib.timeout_add(50, set_device_ac_cb, device, ac) + return to_path(ac) @dbus.service.method(dbus_interface=IFACE_NM, in_signature='a{sa{sv}}oo', out_signature='oo') def AddAndActivateConnection(self, connection, devpath, specific_object): @@ -610,8 +696,8 @@ class NetworkManager(ExportedObj): if not device: raise UnknownDeviceException("No device found for the requested iface.") - conpath = manager.AddConnection(connection) - return self.ActivateConnection(conpath, devpath, specific_object) + conpath = settings.AddConnection(connection) + return (conpath, self.ActivateConnection(conpath, devpath, specific_object)) @dbus.service.method(dbus_interface=IFACE_NM, in_signature='o', out_signature='') def DeactivateConnection(self, active_connection): From e06bd1ef17bacf38d5a1f06ff827971a5969da53 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Mon, 6 Oct 2014 21:20:17 -0400 Subject: [PATCH 08/11] libnm: add an active-connection test to test-nm-client Test NMClient's handling of active connections, and in particular test that we can correctly resolve the circular reference between an NMDevice and an NMActiveConnection, both synchronously and asynchronously. --- libnm/tests/test-nm-client.c | 146 +++++++++++++++++++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/libnm/tests/test-nm-client.c b/libnm/tests/test-nm-client.c index 82943e588f..77d38645f0 100644 --- a/libnm/tests/test-nm-client.c +++ b/libnm/tests/test-nm-client.c @@ -28,6 +28,8 @@ #include "common.h" +#include "nm-test-utils.h" + static GMainLoop *loop = NULL; static NMTestServiceInfo *sinfo; @@ -730,6 +732,149 @@ test_client_nm_running (void) g_object_unref (client2); } +typedef struct { + GMainLoop *loop; + NMActiveConnection *ac; + + int remaining; +} TestACInfo; + +static void +assert_ac_and_device (NMClient *client) +{ + const GPtrArray *devices, *acs, *ac_devices; + NMDevice *device, *ac_device; + NMActiveConnection *ac, *device_ac; + + acs = nm_client_get_active_connections (client); + g_assert (acs != NULL); + g_assert_cmpint (acs->len, ==, 1); + devices = nm_client_get_devices (client); + g_assert (devices != NULL); + g_assert_cmpint (devices->len, ==, 1); + + ac = acs->pdata[0]; + ac_devices = nm_active_connection_get_devices (ac); + g_assert (ac_devices != NULL); + g_assert_cmpint (ac_devices->len, ==, 1); + ac_device = ac_devices->pdata[0]; + g_assert (ac_device != NULL); + + device = devices->pdata[0]; + device_ac = nm_device_get_active_connection (device); + g_assert (device_ac != NULL); + + g_assert_cmpstr (nm_object_get_path (NM_OBJECT (device)), ==, nm_object_get_path (NM_OBJECT (ac_device))); + g_assert (device == ac_device); + g_assert_cmpstr (nm_object_get_path (NM_OBJECT (ac)), ==, nm_object_get_path (NM_OBJECT (device_ac))); + g_assert (ac == device_ac); +} + +static void +add_and_activate_cb (GObject *object, + GAsyncResult *result, + gpointer user_data) +{ + NMClient *client = NM_CLIENT (object); + TestACInfo *info = user_data; + GError *error = NULL; + + info->ac = nm_client_add_and_activate_connection_finish (client, result, &error); + g_assert_no_error (error); + g_assert (info->ac != NULL); + + assert_ac_and_device (client); + + info->remaining--; + if (!info->remaining) + g_main_loop_quit (info->loop); +} + +static void +client_acs_changed_cb (GObject *client, + GParamSpec *pspec, + gpointer user_data) +{ + TestACInfo *info = user_data; + const GPtrArray *acs; + + acs = nm_client_get_active_connections (NM_CLIENT (client)); + g_assert (acs != NULL); + g_assert_cmpint (acs->len, ==, 1); + + info->remaining--; + if (!info->remaining) + g_main_loop_quit (info->loop); +} + +static void +device_ac_changed_cb (GObject *device, + GParamSpec *pspec, + gpointer user_data) +{ + TestACInfo *info = user_data; + + g_assert (nm_device_get_active_connection (NM_DEVICE (device)) != NULL); + + info->remaining--; + if (!info->remaining) + g_main_loop_quit (info->loop); +} + +static void +test_active_connections (void) +{ + NMClient *client; + NMDevice *device; + NMConnection *conn; + TestACInfo info = { loop, NULL, 0 }; + GError *error = NULL; + + sinfo = nm_test_service_init (); + client = nm_client_new (NULL, &error); + g_assert_no_error (error); + + /* Tell the test service to add a new device */ + device = nm_test_service_add_device (sinfo, client, "AddWiredDevice", "eth0"); + + conn = nmtst_create_minimal_connection ("test-ac", NULL, NM_SETTING_WIRED_SETTING_NAME, NULL); + nm_client_add_and_activate_connection_async (client, conn, device, NULL, + NULL, add_and_activate_cb, &info); + g_object_unref (conn); + + g_signal_connect (client, "notify::" NM_CLIENT_ACTIVE_CONNECTIONS, + G_CALLBACK (client_acs_changed_cb), &info); + g_signal_connect (device, "notify::" NM_DEVICE_ACTIVE_CONNECTION, + G_CALLBACK (device_ac_changed_cb), &info); + + /* Two signals plus activate_cb */ + info.remaining = 3; + g_main_loop_run (loop); + g_signal_handlers_disconnect_by_func (client, client_acs_changed_cb, &info); + g_signal_handlers_disconnect_by_func (device, device_ac_changed_cb, &info); + + g_assert (info.ac != NULL); + + g_object_unref (info.ac); + g_object_unref (client); + + /* Ensure that we can correctly resolve the recursive property link between the + * AC and the Device in a newly-created client. + */ + client = nm_client_new (NULL, &error); + g_assert_no_error (error); + assert_ac_and_device (client); + g_object_unref (client); + + client = NULL; + nm_client_new_async (NULL, new_client_cb, &client); + g_main_loop_run (loop); + assert_ac_and_device (client); + g_object_unref (client); + + g_clear_pointer (&sinfo, nm_test_service_cleanup); +} + /*******************************************************************/ int @@ -750,6 +895,7 @@ main (int argc, char **argv) g_test_add_func ("/libnm/wimax-nsp-added-removed", test_wimax_nsp_added_removed); g_test_add_func ("/libnm/devices-array", test_devices_array); g_test_add_func ("/libnm/client-nm-running", test_client_nm_running); + g_test_add_func ("/libnm/active-connections", test_active_connections); return g_test_run (); } From ba900f2a44f978300cac768c75384b54e3be61bc Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 8 Oct 2014 18:15:23 -0400 Subject: [PATCH 09/11] tools: add a bit of support for VLANs to test-networkmanager-service.py --- tools/test-networkmanager-service.py | 52 +++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 9 deletions(-) diff --git a/tools/test-networkmanager-service.py b/tools/test-networkmanager-service.py index c3e251e49b..064336fa39 100755 --- a/tools/test-networkmanager-service.py +++ b/tools/test-networkmanager-service.py @@ -225,6 +225,34 @@ class WiredDevice(Device): def PropertiesChanged(self, changed): pass +################################################################### +IFACE_VLAN = 'org.freedesktop.NetworkManager.Device.Vlan' + +PV_HW_ADDRESS = "HwAddress" +PV_CARRIER = "Carrier" +PV_VLAN_ID = "VlanId" + +class VlanDevice(Device): + def __init__(self, bus, iface): + Device.__init__(self, bus, iface, NM_DEVICE_TYPE_VLAN) + self.add_dbus_interface(IFACE_VLAN, self.__get_props) + + self.mac = random_mac() + self.carrier = False + self.vlan_id = 1 + + # Properties interface + def __get_props(self): + props = {} + props[PV_HW_ADDRESS] = self.mac + props[PV_CARRIER] = self.carrier + props[PV_VLAN_ID] = self.vlan_id + return props + + @dbus.service.signal(IFACE_VLAN, signature='a{sv}') + def PropertiesChanged(self, changed): + pass + ################################################################### IFACE_WIFI_AP = 'org.freedesktop.NetworkManager.AccessPoint' @@ -653,21 +681,27 @@ class NetworkManager(ExportedObj): @dbus.service.method(dbus_interface=IFACE_NM, in_signature='ooo', out_signature='o') def ActivateConnection(self, conpath, devpath, specific_object): - device = None - for d in self.devices: - if d.path == devpath: - device = d - break - if not device: - raise UnknownDeviceException("No device found for the requested iface.") - try: connection = settings.get_connection(conpath) except Exception as e: raise UnknownConnectionException("Connection not found") - # See if we need secrets. For the moment, we only support WPA hash = connection.GetSettings() + s_con = hash['connection'] + + device = None + for d in self.devices: + if d.path == devpath: + device = d + break + if not device and s_con['type'] == 'vlan': + ifname = s_con['interface-name'] + device = VlanDevice(self._bus, ifname) + self.add_device(device) + if not device: + raise UnknownDeviceException("No device found for the requested iface.") + + # See if we need secrets. For the moment, we only support WPA if hash.has_key('802-11-wireless-security'): s_wsec = hash['802-11-wireless-security'] if (s_wsec['key-mgmt'] == 'wpa-psk' and not s_wsec.has_key('psk')): From f96835b83c1a3a8e1f4046ef6899a4ec85e2b25e Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Fri, 10 Oct 2014 16:49:12 -0400 Subject: [PATCH 10/11] libnm: add a virtual device creation-and-activation test Add a test of creating a (virtual) device and activating a connection on it at the same time. --- libnm/tests/test-nm-client.c | 136 ++++++++++++++++++++++++++++++++++- 1 file changed, 135 insertions(+), 1 deletion(-) diff --git a/libnm/tests/test-nm-client.c b/libnm/tests/test-nm-client.c index 77d38645f0..76469aac53 100644 --- a/libnm/tests/test-nm-client.c +++ b/libnm/tests/test-nm-client.c @@ -751,7 +751,7 @@ assert_ac_and_device (NMClient *client) g_assert_cmpint (acs->len, ==, 1); devices = nm_client_get_devices (client); g_assert (devices != NULL); - g_assert_cmpint (devices->len, ==, 1); + g_assert_cmpint (devices->len, >=, 1); ac = acs->pdata[0]; ac_devices = nm_active_connection_get_devices (ac); @@ -761,6 +761,8 @@ assert_ac_and_device (NMClient *client) g_assert (ac_device != NULL); device = devices->pdata[0]; + if (device != ac_device && devices->len > 1) + device = devices->pdata[1]; device_ac = nm_device_get_active_connection (device); g_assert (device_ac != NULL); @@ -875,6 +877,137 @@ test_active_connections (void) g_clear_pointer (&sinfo, nm_test_service_cleanup); } +static void +client_devices_changed_cb (GObject *client, + GParamSpec *pspec, + gpointer user_data) +{ + TestACInfo *info = user_data; + const GPtrArray *devices; + NMDevice *device; + + devices = nm_client_get_devices (NM_CLIENT (client)); + g_assert (devices != NULL); + g_assert_cmpint (devices->len, ==, 2); + + if (NM_IS_DEVICE_VLAN (devices->pdata[0])) + device = devices->pdata[0]; + else if (NM_IS_DEVICE_VLAN (devices->pdata[1])) + device = devices->pdata[1]; + else + g_assert_not_reached (); + + g_assert_cmpstr (nm_device_get_iface (device), ==, "eth0.1"); + + if (nm_device_get_active_connection (device)) + info->remaining--; + else { + g_signal_connect (device, "notify::" NM_DEVICE_ACTIVE_CONNECTION, + G_CALLBACK (device_ac_changed_cb), &info); + } + + info->remaining--; + if (!info->remaining) + g_main_loop_quit (info->loop); +} + +typedef struct { + GMainLoop *loop; + NMRemoteConnection *remote; +} TestConnectionInfo; + +static void +add_connection_cb (GObject *object, + GAsyncResult *result, + gpointer user_data) +{ + TestConnectionInfo *info = user_data; + GError *error = NULL; + + info->remote = nm_client_add_connection_finish (NM_CLIENT (object), result, &error); + g_assert_no_error (error); + g_main_loop_quit (info->loop); +} + +static void +activate_cb (GObject *object, + GAsyncResult *result, + gpointer user_data) +{ + NMClient *client = NM_CLIENT (object); + TestACInfo *info = user_data; + GError *error = NULL; + + info->ac = nm_client_activate_connection_finish (client, result, &error); + g_assert_no_error (error); + g_assert (info->ac != NULL); + + assert_ac_and_device (client); + + info->remaining--; + if (!info->remaining) + g_main_loop_quit (info->loop); +} + +static void +test_activate_virtual (void) +{ + NMClient *client; + NMConnection *conn; + NMSettingConnection *s_con; + NMSettingVlan *s_vlan; + TestACInfo info = { loop, NULL, 0 }; + TestConnectionInfo conn_info = { loop, NULL }; + GError *error = NULL; + + sinfo = nm_test_service_init (); + client = nm_client_new (NULL, &error); + g_assert_no_error (error); + + nm_test_service_add_device (sinfo, client, "AddWiredDevice", "eth0"); + + conn = nmtst_create_minimal_connection ("test-ac", NULL, NM_SETTING_VLAN_SETTING_NAME, &s_con); + g_object_set (s_con, + NM_SETTING_CONNECTION_INTERFACE_NAME, "eth0.1", + NULL); + s_vlan = nm_connection_get_setting_vlan (conn); + g_object_set (s_vlan, + NM_SETTING_VLAN_ID, 1, + NM_SETTING_VLAN_PARENT, "eth0", + NULL); + + nm_client_add_connection_async (client, conn, TRUE, + NULL, add_connection_cb, &conn_info); + g_main_loop_run (loop); + g_object_unref (conn); + conn = NM_CONNECTION (conn_info.remote); + + nm_client_activate_connection_async (client, conn, NULL, NULL, + NULL, activate_cb, &info); + g_object_unref (conn); + + g_signal_connect (client, "notify::" NM_CLIENT_ACTIVE_CONNECTIONS, + G_CALLBACK (client_acs_changed_cb), &info); + g_signal_connect (client, "notify::" NM_CLIENT_DEVICES, + G_CALLBACK (client_devices_changed_cb), &info); + + /* As with test_active_connections() above, except that now we're waiting + * for NMClient:devices to change rather than NMDevice:active-connections. + */ + info.remaining = 3; + + g_main_loop_run (loop); + g_signal_handlers_disconnect_by_func (client, client_acs_changed_cb, &info); + g_signal_handlers_disconnect_by_func (client, client_devices_changed_cb, &info); + + g_assert (info.ac != NULL); + + g_object_unref (info.ac); + g_object_unref (client); + + g_clear_pointer (&sinfo, nm_test_service_cleanup); +} + /*******************************************************************/ int @@ -896,6 +1029,7 @@ main (int argc, char **argv) g_test_add_func ("/libnm/devices-array", test_devices_array); g_test_add_func ("/libnm/client-nm-running", test_client_nm_running); g_test_add_func ("/libnm/active-connections", test_active_connections); + g_test_add_func ("/libnm/activate-virtual", test_activate_virtual); return g_test_run (); } From be8060f42fd3b3c15755e97f0c35886596a4732c Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Fri, 10 Oct 2014 17:10:28 -0400 Subject: [PATCH 11/11] libnm: add an object-creation-failed test --- libnm/tests/test-nm-client.c | 52 +++++++++++++++++++++++++++- tools/test-networkmanager-service.py | 8 ++++- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/libnm/tests/test-nm-client.c b/libnm/tests/test-nm-client.c index 76469aac53..26ffbec4ac 100644 --- a/libnm/tests/test-nm-client.c +++ b/libnm/tests/test-nm-client.c @@ -1008,8 +1008,57 @@ test_activate_virtual (void) g_clear_pointer (&sinfo, nm_test_service_cleanup); } +static void +activate_failed_cb (GObject *object, + GAsyncResult *result, + gpointer user_data) +{ + NMClient *client = NM_CLIENT (object); + NMActiveConnection *ac; + GError *error = NULL; + + ac = nm_client_activate_connection_finish (client, result, &error); + g_assert (ac == NULL); + g_assert_error (error, NM_OBJECT_ERROR, NM_OBJECT_ERROR_OBJECT_CREATION_FAILURE); + g_clear_error (&error); + + g_main_loop_quit (loop); +} + +static void +test_activate_failed (void) +{ + NMClient *client; + NMDevice *device; + NMConnection *conn; + GError *error = NULL; + + sinfo = nm_test_service_init (); + client = nm_client_new (NULL, &error); + g_assert_no_error (error); + + device = nm_test_service_add_device (sinfo, client, "AddWiredDevice", "eth0"); + + /* Note that test-networkmanager-service.py checks for this exact name */ + conn = nmtst_create_minimal_connection ("object-creation-failed-test", NULL, + NM_SETTING_WIRED_SETTING_NAME, NULL); + + nm_client_add_and_activate_connection_async (client, conn, device, NULL, + NULL, activate_failed_cb, NULL); + g_test_expect_message ("libnm", G_LOG_LEVEL_WARNING, "*Method*doesn't exist*"); + g_main_loop_run (loop); + g_test_assert_expected_messages (); + + g_object_unref (conn); + g_object_unref (client); + + g_clear_pointer (&sinfo, nm_test_service_cleanup); +} + /*******************************************************************/ +NMTST_DEFINE (); + int main (int argc, char **argv) { @@ -1019,7 +1068,7 @@ main (int argc, char **argv) g_type_init (); #endif - g_test_init (&argc, &argv, NULL); + nmtst_init (&argc, &argv, TRUE); loop = g_main_loop_new (NULL, FALSE); @@ -1030,6 +1079,7 @@ main (int argc, char **argv) g_test_add_func ("/libnm/client-nm-running", test_client_nm_running); g_test_add_func ("/libnm/active-connections", test_active_connections); g_test_add_func ("/libnm/activate-virtual", test_activate_virtual); + g_test_add_func ("/libnm/activate-failed", test_activate_failed); return g_test_run (); } diff --git a/tools/test-networkmanager-service.py b/tools/test-networkmanager-service.py index 064336fa39..d98174ce20 100755 --- a/tools/test-networkmanager-service.py +++ b/tools/test-networkmanager-service.py @@ -717,7 +717,13 @@ class NetworkManager(ExportedObj): ac = ActiveConnection(self._bus, device, connection, None) self.active_connections.append(ac) self.__notify(PM_ACTIVE_CONNECTIONS) - GLib.timeout_add(50, set_device_ac_cb, device, ac) + + if s_con['id'] == 'object-creation-failed-test': + self.active_connections.remove(ac) + ac.remove_from_connection() + else: + GLib.timeout_add(50, set_device_ac_cb, device, ac) + return to_path(ac) @dbus.service.method(dbus_interface=IFACE_NM, in_signature='a{sa{sv}}oo', out_signature='oo')