From a33ed5ad820242adcfa87d30f2a7dd540f1d32e8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 5 Dec 2019 15:53:51 +0100 Subject: [PATCH] libnm: allow to enable/disable fetching of permissions in NMClient Currently, NMClient by default always fetches the permissions ("GetPermissions()") and refreshes them on "CheckPermissions" signal. Fetching permissions is relatively expensive, while they are not used most of the time. Allow the user to opt out of this. For that, have a NMClientInstanceFlags to enable/disable automatic fetching. Also add a "permissions-state" property that allows the user to understand whether the cached permissions are up to date or not. This is a bit an awkward API for handling this. E.g. you cannot explicitly request permissions, you can just enable/disable fetching permissions. And then you can watch the permission-state to know whether you are ready. It's done this way because it fits the previous model and extends the API with a (relative) small amount of new functions and properties. --- libnm/libnm.ver | 1 + libnm/nm-client.c | 146 ++++++++++++++++++++++++++++++++++------- libnm/nm-client.h | 13 +++- libnm/nm-libnm-utils.h | 2 +- 4 files changed, 138 insertions(+), 24 deletions(-) diff --git a/libnm/libnm.ver b/libnm/libnm.ver index ffd6e3dbd5..d2af078d8d 100644 --- a/libnm/libnm.ver +++ b/libnm/libnm.ver @@ -1656,5 +1656,6 @@ global: libnm_1_24_0 { global: nm_client_get_instance_flags; + nm_client_get_permissions_state; nm_client_instance_flags_get_type; } libnm_1_22_0; diff --git a/libnm/nm-client.c b/libnm/nm-client.c index 6b2844a0d7..4dd0c222e1 100644 --- a/libnm/nm-client.c +++ b/libnm/nm-client.c @@ -223,6 +223,7 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMClient, PROP_DNS_RC_MANAGER, PROP_DNS_CONFIGURATION, PROP_CHECKPOINTS, + PROP_PERMISSIONS_STATE, ); enum { @@ -293,6 +294,8 @@ typedef struct { NMClientInstanceFlags instance_flags:3; + NMTernary permissions_state:3; + bool instance_flags_constructed:1; bool udev_inited:1; @@ -3373,18 +3376,17 @@ _dbus_check_permissions_start_cb (GObject *source, GAsyncResult *result, gpointe g_clear_object (&priv->permissions_cancellable); + old_permissions = g_steal_pointer (&priv->permissions); + if (!ret) { + /* when the call completes, we always pretend success. Even a failure means + * that we fetched the permissions, however they are all unknown. */ NML_NMCLIENT_LOG_T (self, "GetPermissions call failed: %s", error->message); - return; + goto out; } NML_NMCLIENT_LOG_T (self, "GetPermissions call finished with success"); - old_permissions = g_steal_pointer (&priv->permissions); - priv->permissions = g_new (guint8, G_N_ELEMENTS (nm_auth_permission_sorted)); - for (i = 0; i < (int) G_N_ELEMENTS (nm_auth_permission_sorted); i++) - priv->permissions[i] = NM_CLIENT_PERMISSION_NONE; - g_variant_get (ret, "(a{ss})", &v_permissions); while (g_variant_iter_next (v_permissions, "{&s&s}", &pkey, &pvalue)) { NMClientPermission perm; @@ -3395,34 +3397,52 @@ _dbus_check_permissions_start_cb (GObject *source, GAsyncResult *result, gpointe continue; perm_result = nm_client_permission_result_from_string (pvalue); + + if (!priv->permissions) { + if (perm_result == NM_CLIENT_PERMISSION_RESULT_UNKNOWN) + continue; + priv->permissions = g_new (guint8, G_N_ELEMENTS (nm_auth_permission_sorted)); + for (i = 0; i < (int) G_N_ELEMENTS (nm_auth_permission_sorted); i++) + priv->permissions[i] = NM_CLIENT_PERMISSION_RESULT_UNKNOWN; + } priv->permissions[perm - 1] = perm_result; } +out: + priv->permissions_state = NM_TERNARY_TRUE; + dbus_context = nm_g_main_context_push_thread_default_if_necessary (priv->main_context); _emit_permissions_changed (self, old_permissions, priv->permissions); + _notify (self, PROP_PERMISSIONS_STATE); } static void _dbus_check_permissions_start (NMClient *self) { NMClientPrivate *priv = NM_CLIENT_GET_PRIVATE (self); + gboolean fetch; + + fetch = !NM_FLAGS_HAS ((NMClientInstanceFlags) priv->instance_flags, + NM_CLIENT_INSTANCE_FLAGS_NO_AUTO_FETCH_PERMISSIONS); nm_clear_g_cancellable (&priv->permissions_cancellable); - priv->permissions_cancellable = g_cancellable_new (); - NML_NMCLIENT_LOG_T (self, "GetPermissions() call started..."); + if (fetch) { + NML_NMCLIENT_LOG_T (self, "GetPermissions() call started..."); - _nm_client_dbus_call_simple (self, - priv->permissions_cancellable, - NM_DBUS_PATH, - NM_DBUS_INTERFACE, - "GetPermissions", - g_variant_new ("()"), - G_VARIANT_TYPE ("(a{ss})"), - G_DBUS_CALL_FLAGS_NONE, - NM_DBUS_DEFAULT_TIMEOUT_MSEC, - _dbus_check_permissions_start_cb, - self); + priv->permissions_cancellable = g_cancellable_new (); + _nm_client_dbus_call_simple (self, + priv->permissions_cancellable, + NM_DBUS_PATH, + NM_DBUS_INTERFACE, + "GetPermissions", + g_variant_new ("()"), + G_VARIANT_TYPE ("(a{ss})"), + G_DBUS_CALL_FLAGS_NONE, + NM_DBUS_DEFAULT_TIMEOUT_MSEC, + _dbus_check_permissions_start_cb, + self); + } } static void @@ -3435,6 +3455,7 @@ _dbus_nm_check_permissions_cb (GDBusConnection *connection, gpointer user_data) { NMClient *self = user_data; + NMClientPrivate *priv = NM_CLIENT_GET_PRIVATE (self); if (!g_variant_is_of_type (parameters, G_VARIANT_TYPE ("()"))) { NML_NMCLIENT_LOG_E (self, "ignore CheckPermissions signal with unexpected signature %s", @@ -3443,6 +3464,10 @@ _dbus_nm_check_permissions_cb (GDBusConnection *connection, } _dbus_check_permissions_start (self); + + if (priv->permissions_state == NM_TERNARY_TRUE) + priv->permissions_state = NM_TERNARY_FALSE; + _notify (self, PROP_PERMISSIONS_STATE); } /*****************************************************************************/ @@ -4308,6 +4333,27 @@ nm_client_get_permission_result (NMClient *client, NMClientPermission permission return result; } +/** + * nm_client_get_permissions_state: + * @self: the #NMClient instance + * + * Returns: the state of the cached permissions. %NM_TERNARY_DEFAULT + * means that no permissions result was yet received. All permissions + * are unknown. %NM_TERNARY_TRUE means that the permissions got received + * and are cached. %%NM_TERNARY_FALSE means that permissions are cached, + * but they are invalided as as "CheckPermissions" signal was received + * in the meantime. + * + * Since: 1.24 + */ +NMTernary +nm_client_get_permissions_state (NMClient *self) +{ + g_return_val_if_fail (NM_IS_CLIENT (self), NM_TERNARY_DEFAULT); + + return NM_CLIENT_GET_PRIVATE (self)->permissions_state; +} + /** * nm_client_get_connectivity: * @client: an #NMClient @@ -6556,6 +6602,7 @@ _init_release_all (NMClient *self) CList **dbus_objects_lst_heads; NMLDBusObject *dbobj; int i; + gboolean permissions_state_changed = FALSE; NML_NMCLIENT_LOG_D (self, "release all"); @@ -6575,12 +6622,20 @@ _init_release_all (NMClient *self) nm_clear_g_dbus_connection_signal (priv->dbus_connection, &priv->dbsid_nm_check_permissions); + if (priv->permissions_state != NM_TERNARY_DEFAULT) { + priv->permissions_state = NM_TERNARY_DEFAULT; + permissions_state_changed = TRUE; + } + if (priv->permissions) { gs_free guint8 *old_permissions = g_steal_pointer (&priv->permissions); _emit_permissions_changed (self, old_permissions, NULL); } + if (permissions_state_changed) + _notify (self, PROP_PERMISSIONS_STATE); + nm_assert (c_list_is_empty (&priv->obj_changed_lst_head)); dbus_objects_lst_heads = ((CList *[]) { @@ -7027,6 +7082,9 @@ get_property (GObject *object, guint prop_id, case PROP_CHECKPOINTS: g_value_take_boxed (value, _nm_utils_copy_object_array (nm_client_get_checkpoints (self))); break; + case PROP_PERMISSIONS_STATE: + g_value_set_enum (value, priv->permissions_state); + break; /* Settings properties. */ case PROP_CONNECTIONS: @@ -7081,8 +7139,19 @@ set_property (GObject *object, guint prop_id, priv->instance_flags = v_uint; nm_assert ((guint) priv->instance_flags == v_uint); } else { - /* Later, we may want to implement setting some flags not only at construct time. - * For now, that is not implemented and resetting flags afterwards has no effect. */ + NMClientInstanceFlags flags = v_uint; + + /* After object construction, we only allow to toggle certain flags and + * ignore all other flags. */ + + if ((priv->instance_flags ^ flags) & NM_CLIENT_INSTANCE_FLAGS_NO_AUTO_FETCH_PERMISSIONS) { + if (NM_FLAGS_HAS (flags, NM_CLIENT_INSTANCE_FLAGS_NO_AUTO_FETCH_PERMISSIONS)) + priv->instance_flags |= NM_CLIENT_INSTANCE_FLAGS_NO_AUTO_FETCH_PERMISSIONS; + else + priv->instance_flags &= ~NM_CLIENT_INSTANCE_FLAGS_NO_AUTO_FETCH_PERMISSIONS; + if (priv->dbsid_nm_check_permissions != 0) + _dbus_check_permissions_start (self); + } } break; @@ -7268,6 +7337,8 @@ nm_client_init (NMClient *self) { NMClientPrivate *priv = NM_CLIENT_GET_PRIVATE (self); + priv->permissions_state = NM_TERNARY_DEFAULT; + priv->context_busy_watcher = g_object_new (G_TYPE_OBJECT, NULL); c_list_init (&self->obj_base.queue_notify_lst); @@ -7522,9 +7593,14 @@ nm_client_class_init (NMClientClass *client_class) * NMClient:instance-flags: * * #NMClientInstanceFlags for the instance. These affect behavior of #NMClient. - * This is a construct property and you may only set the flags during + * This is a construct property and you may only set most flags only during * construction. * + * The flag %NM_CLIENT_INSTANCE_FLAGS_NO_AUTO_FETCH_PERMISSIONS can be toggled any time, + * even after constructing the instance. Note that you may want to watch NMClient:permissions-state + * property to know whether permissions are ready. Note that permissions are only fetched + * when NMClient has a D-Bus name owner. + * * Since: 1.24 */ obj_properties[PROP_INSTANCE_FLAGS] = @@ -7896,6 +7972,32 @@ nm_client_class_init (NMClientClass *client_class) G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); + /** + * NMClient:permissions-state: + * + * The state of the cached permissions. The value %NM_TERNARY_DEFAULT + * means that no permissions are yet received (or not yet requested). + * %NM_TERNARY_TRUE means that permissions are received, cached and up + * to date. %NM_TERNARY_FALSE means that permissions were received and are + * cached, but in the meantime a "CheckPermissions" signal was received + * that invalidated the cached permissions. + * Note that NMClient will always emit a notify::permissions-state signal + * when a "CheckPermissions" signal got received or after new permissions + * got received (that is regardless whether the value of the permission state + * actually changed). With this you can watch the permissions-state property + * to know whether the permissions are ready. Note that while NMClient has + * no D-Bus name owner, no permissions are fetched (and this property won't + * change). + * + * Since: 1.24 + */ + obj_properties[PROP_PERMISSIONS_STATE] = + g_param_spec_enum (NM_CLIENT_PERMISSIONS_STATE, "", "", + NM_TYPE_TERNARY, + NM_TERNARY_DEFAULT, + G_PARAM_READABLE | + G_PARAM_STATIC_STRINGS); + _nml_dbus_meta_class_init_with_properties (object_class, &_nml_dbus_meta_iface_nm, &_nml_dbus_meta_iface_nm_settings, &_nml_dbus_meta_iface_nm_dnsmanager); diff --git a/libnm/nm-client.h b/libnm/nm-client.h index 4dc1a16791..37bac38b8b 100644 --- a/libnm/nm-client.h +++ b/libnm/nm-client.h @@ -18,11 +18,18 @@ G_BEGIN_DECLS /** * NMClientInstanceFlags: * @NM_CLIENT_INSTANCE_FLAGS_NONE: special value to indicate no flags. + * @NM_CLIENT_INSTANCE_FLAGS_NO_AUTO_FETCH_PERMISSIONS: by default, NMClient + * will fetch the permissions via "GetPermissions" and refetch them when + * "CheckPermissions" signal gets received. By setting this flag, this behavior + * can be disabled. You can toggle this flag to enable and disable automatic + * fetching of the permissions. Watch also nm_client_get_permissions_state() + * to know whether the permissions are up to date. * * Since: 1.24 */ typedef enum { /*< flags >*/ - NM_CLIENT_INSTANCE_FLAGS_NONE = 0, + NM_CLIENT_INSTANCE_FLAGS_NONE = 0, + NM_CLIENT_INSTANCE_FLAGS_NO_AUTO_FETCH_PERMISSIONS = 1, } NMClientInstanceFlags; #define NM_TYPE_CLIENT (nm_client_get_type ()) @@ -74,6 +81,7 @@ _NM_DEPRECATED_SYNC_WRITABLE_PROPERTY #define NM_CLIENT_DNS_RC_MANAGER "dns-rc-manager" #define NM_CLIENT_DNS_CONFIGURATION "dns-configuration" #define NM_CLIENT_CHECKPOINTS "checkpoints" +#define NM_CLIENT_PERMISSIONS_STATE "permissions-state" #define NM_CLIENT_DEVICE_ADDED "device-added" #define NM_CLIENT_DEVICE_REMOVED "device-removed" @@ -228,6 +236,9 @@ gboolean nm_client_set_logging (NMClient *client, NMClientPermissionResult nm_client_get_permission_result (NMClient *client, NMClientPermission permission); +NM_AVAILABLE_IN_1_24 +NMTernary nm_client_get_permissions_state (NMClient *self); + NMConnectivityState nm_client_get_connectivity (NMClient *client); _NM_DEPRECATED_SYNC_METHOD diff --git a/libnm/nm-libnm-utils.h b/libnm/nm-libnm-utils.h index 5dea53c682..7f984de2a5 100644 --- a/libnm/nm-libnm-utils.h +++ b/libnm/nm-libnm-utils.h @@ -166,7 +166,7 @@ typedef enum { /*****************************************************************************/ -#define NM_CLIENT_INSTANCE_FLAGS_ALL ((NMClientInstanceFlags) 0x0) +#define NM_CLIENT_INSTANCE_FLAGS_ALL ((NMClientInstanceFlags) 0x1) typedef struct { GType (*get_o_type_fcn) (void);