From 49ed2079abd629d516e3227ce2aa5fffd5b46fc8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 4 May 2019 13:43:19 +0200 Subject: [PATCH] core/dbus: let NMDBusManager create a D-Bus connection in "configure-and-quit=true" mode Note that various components (NMFirewallManager, NMAuthManager, NMConnectivity, etc.pp) all request their own GDBusConnection from glib's G_BUS_TYPE_SYSTEM singleton. In the future, let them instead use the D-Bus connection that NMDBusManager already has. - NMDBusManager also uses g_bus_get(G_BUS_TYPE_SYSTEM), so in practice this is just the same GDBusConnection instance. - if it would not be the same GDBusConnection instance, it would be more correct/logical to use the one that NMDBusManager uses. - NMDBusManager already aquired the GDBusConnection synchronously and it's ready for use. On the other hand, g_bus_get()/g_bus_get_sync() has the notion that getting the singleton cannot be done without waiting/blocking. So at least it involves locking or even dispatching the async reply on D-Bus. - in "configure-and-quit=initrd" we really don't have D-Bus available. NMDBusManager should control whether the other components use D-Bus or not. For example, NMFirewallManager should not ask glib for a G_BUS_TYPE_SYSTEM singleton only to later find out that it doesn't work. So if these components would reuse NMDBusManager's GDBusConnection, then it must have the connection also in regular "configure-and-quit=true" mode. In this case, we are in late boot and want do connectivity checking and talk to firewalld. --- src/main.c | 39 +++++++++++++++++---- src/nm-connectivity.c | 2 +- src/nm-dbus-manager.c | 79 ++++++++++++++++++++++++++----------------- src/nm-dbus-manager.h | 3 +- 4 files changed, 83 insertions(+), 40 deletions(-) diff --git a/src/main.c b/src/main.c index 02a4210532..6b356e16c2 100644 --- a/src/main.c +++ b/src/main.c @@ -213,6 +213,35 @@ do_early_setup (int *argc, char **argv[], NMConfigCmdLineOptions *config_cli) global_opt.pidfile = global_opt.pidfile ?: g_strdup(NM_DEFAULT_PID_FILE); } +static gboolean +_dbus_manager_init (NMConfig *config) +{ + NMDBusManager *busmgr; + NMConfigConfigureAndQuitType c_a_q_type; + + busmgr = nm_dbus_manager_get (); + + c_a_q_type = nm_config_get_configure_and_quit (config); + + if (c_a_q_type == NM_CONFIG_CONFIGURE_AND_QUIT_DISABLED) + return nm_dbus_manager_acquire_bus (busmgr, TRUE); + + if (c_a_q_type == NM_CONFIG_CONFIGURE_AND_QUIT_ENABLED) { + /* D-Bus is useless in configure and quit mode -- we're eventually dropping + * off and potential clients would have no way of knowing whether we're + * finished already or didn't start yet. + * + * But we still create a nm_dbus_manager_get_dbus_connection() D-Bus connection + * so that we can talk to other services like firewalld. */ + return nm_dbus_manager_acquire_bus (busmgr, FALSE); + } + + nm_assert (c_a_q_type == NM_CONFIG_CONFIGURE_AND_QUIT_INITRD); + /* in initrd we don't have D-Bus at all. Don't even try to get the G_BUS_TYPE_SYSTEM + * connection. And of course don't claim the D-Bus name. */ + return TRUE; +} + /* * main * @@ -399,15 +428,11 @@ main (int argc, char *argv[]) NM_CONFIG_KEYFILE_KEY_MAIN_AUTH_POLKIT, NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT_BOOL)); - if (!nm_config_get_configure_and_quit (config)) { - /* D-Bus is useless in configure and quit mode -- we're eventually dropping - * off and potential clients would have no way of knowing whether we're - * finished already or didn't start yet. */ - if (!nm_dbus_manager_acquire_bus (nm_dbus_manager_get ())) - goto done_no_manager; - } + if (!_dbus_manager_init (config)) + goto done_no_manager; manager = nm_manager_setup (); + nm_dbus_manager_start (nm_dbus_manager_get(), nm_manager_dbus_set_property_handle, manager); diff --git a/src/nm-connectivity.c b/src/nm-connectivity.c index 2816e76afc..de3d8b4dff 100644 --- a/src/nm-connectivity.c +++ b/src/nm-connectivity.c @@ -876,7 +876,7 @@ nm_connectivity_check_start (NMConnectivity *self, cb_data->concheck.resolve_cancellable = g_cancellable_new (); - g_dbus_connection_call (nm_dbus_manager_get_dbus_connection (nm_dbus_manager_get ()), + g_dbus_connection_call (dbus_connection, "org.freedesktop.resolve1", "/org/freedesktop/resolve1", "org.freedesktop.resolve1.Manager", diff --git a/src/nm-dbus-manager.c b/src/nm-dbus-manager.c index 1a455c4280..6a61079d6a 100644 --- a/src/nm-dbus-manager.c +++ b/src/nm-dbus-manager.c @@ -471,6 +471,9 @@ _bus_get_unix_pid (NMDBusManager *self, guint32 unix_pid = G_MAXUINT32; gs_unref_variant GVariant *ret = NULL; + if (!priv->main_dbus_connection) + return FALSE; + ret = g_dbus_connection_call_sync (priv->main_dbus_connection, DBUS_SERVICE_DBUS, DBUS_PATH_DBUS, @@ -500,6 +503,9 @@ _bus_get_unix_user (NMDBusManager *self, guint32 unix_uid = G_MAXUINT32; gs_unref_variant GVariant *ret = NULL; + if (!priv->main_dbus_connection) + return FALSE; + ret = g_dbus_connection_call_sync (priv->main_dbus_connection, DBUS_SERVICE_DBUS, DBUS_PATH_DBUS, @@ -1024,6 +1030,7 @@ _obj_register (NMDBusManager *self, nm_assert (c_list_is_empty (&obj->internal.registration_lst_head)); nm_assert (priv->main_dbus_connection); + nm_assert (priv->objmgr_registration_id != 0); nm_assert (priv->started); n_klasses = 0; @@ -1119,15 +1126,10 @@ _obj_unregister (NMDBusManager *self, GVariantBuilder builder; nm_assert (NM_IS_DBUS_OBJECT (obj)); - - if (!priv->main_dbus_connection) { - /* nothing to do for the moment. */ - nm_assert (c_list_is_empty (&obj->internal.registration_lst_head)); - return; - } - + nm_assert (priv->main_dbus_connection); + nm_assert (priv->objmgr_registration_id != 0); + nm_assert (priv->started); nm_assert (!c_list_is_empty (&obj->internal.registration_lst_head)); - nm_assert (priv->objmgr_registration_id); g_variant_builder_init (&builder, G_VARIANT_TYPE ("as")); @@ -1202,7 +1204,7 @@ _nm_dbus_manager_obj_export (NMDBusObject *obj) nm_assert_not_reached (); c_list_link_tail (&priv->objects_lst_head, &obj->internal.objects_lst); - if (priv->main_dbus_connection && priv->started) + if (priv->started) _obj_register (self, obj); } @@ -1223,7 +1225,10 @@ _nm_dbus_manager_obj_unexport (NMDBusObject *obj) nm_assert (&obj->internal == g_hash_table_lookup (priv->objects_by_path, &obj->internal)); nm_assert (c_list_contains (&priv->objects_lst_head, &obj->internal.objects_lst)); - _obj_unregister (self, obj); + if (priv->started) + _obj_unregister (self, obj); + else + nm_assert (c_list_is_empty (&obj->internal.registration_lst_head)); if (!g_hash_table_remove (priv->objects_by_path, &obj->internal)) nm_assert_not_reached (); @@ -1249,6 +1254,16 @@ _nm_dbus_manager_obj_notify (NMDBusObject *obj, nm_assert (NM_IS_DBUS_MANAGER (obj->internal.bus_manager)); nm_assert (!c_list_is_empty (&obj->internal.objects_lst)); + self = obj->internal.bus_manager; + priv = NM_DBUS_MANAGER_GET_PRIVATE (self); + + nm_assert (!priv->started || priv->objmgr_registration_id != 0); + nm_assert (priv->objmgr_registration_id == 0 || priv->main_dbus_connection); + nm_assert (c_list_is_empty (&obj->internal.registration_lst_head) != priv->started); + + if (G_UNLIKELY (!priv->started)) + return; + c_list_for_each_entry (reg_data, &obj->internal.registration_lst_head, registration_lst) { if (_reg_data_get_interface_info (reg_data)->legacy_property_changed) { any_legacy_signals = TRUE; @@ -1256,9 +1271,6 @@ _nm_dbus_manager_obj_notify (NMDBusObject *obj, } } - self = obj->internal.bus_manager; - priv = NM_DBUS_MANAGER_GET_PRIVATE (self); - /* do a naive search for the matching NMDBusPropertyInfoExtended infos. Since the number of * (interfaces x properties) is static and possibly small, this naive search is effectively * O(1). We might wanna introduce some index to lookup the properties in question faster. @@ -1401,7 +1413,7 @@ _nm_dbus_manager_obj_emit_signal (NMDBusObject *obj, self = obj->internal.bus_manager; priv = NM_DBUS_MANAGER_GET_PRIVATE (self); - if (!priv->main_dbus_connection || !priv->started) { + if (!priv->started) { nm_g_variant_unref_floating (args); return; } @@ -1565,9 +1577,12 @@ nm_dbus_manager_start (NMDBusManager *self, NMDBusObject *obj; g_return_if_fail (NM_IS_DBUS_MANAGER (self)); + priv = NM_DBUS_MANAGER_GET_PRIVATE (self); - if (!priv->main_dbus_connection) { + nm_assert (!priv->started); + + if (priv->objmgr_registration_id == 0) { /* Do nothing. We're presumably in the configure-and-quit mode. */ return; } @@ -1581,12 +1596,12 @@ nm_dbus_manager_start (NMDBusManager *self, } gboolean -nm_dbus_manager_acquire_bus (NMDBusManager *self) +nm_dbus_manager_acquire_bus (NMDBusManager *self, + gboolean request_name) { NMDBusManagerPrivate *priv; gs_free_error GError *error = NULL; gs_unref_variant GVariant *ret = NULL; - gs_unref_object GDBusConnection *connection = NULL; guint32 result; guint registration_id; @@ -1599,17 +1614,22 @@ nm_dbus_manager_acquire_bus (NMDBusManager *self) * acquire the name despite connecting to the bus successfully. * It means that something is gravely broken -- such as another NetworkManager * instance running. */ - connection = g_bus_get_sync (G_BUS_TYPE_SYSTEM, - NULL, - &error); - if (!connection) { - _LOGI ("cannot connect to D-Bus: %s", error->message); + priv->main_dbus_connection = g_bus_get_sync (G_BUS_TYPE_SYSTEM, + NULL, + &error); + if (!priv->main_dbus_connection) { + _LOGE ("cannot connect to D-Bus: %s", error->message); return FALSE; } - g_dbus_connection_set_exit_on_close (connection, FALSE); + g_dbus_connection_set_exit_on_close (priv->main_dbus_connection, FALSE); - registration_id = g_dbus_connection_register_object (connection, + if (!request_name) { + _LOGD ("D-Bus connection created"); + return TRUE; + } + + registration_id = g_dbus_connection_register_object (priv->main_dbus_connection, OBJECT_MANAGER_SERVER_BASE_PATH, NM_UNCONST_PTR (GDBusInterfaceInfo, &interface_info_objmgr), &dbus_vtable_objmgr, @@ -1621,7 +1641,7 @@ nm_dbus_manager_acquire_bus (NMDBusManager *self) return FALSE; } - ret = g_dbus_connection_call_sync (connection, + ret = g_dbus_connection_call_sync (priv->main_dbus_connection, DBUS_SERVICE_DBUS, DBUS_PATH_DBUS, DBUS_INTERFACE_DBUS, @@ -1634,13 +1654,10 @@ nm_dbus_manager_acquire_bus (NMDBusManager *self) -1, NULL, &error); - if (!ret) - return FALSE; - if (!ret) { _LOGE ("fatal failure to acquire D-Bus service \"%s"": %s", NM_DBUS_SERVICE, error->message); - g_dbus_connection_unregister_object(connection, registration_id); + g_dbus_connection_unregister_object (priv->main_dbus_connection, registration_id); return FALSE; } @@ -1648,12 +1665,11 @@ nm_dbus_manager_acquire_bus (NMDBusManager *self) if (result != DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER) { _LOGE ("fatal failure to acquire D-Bus service \"%s\" (%u). Service already taken", NM_DBUS_SERVICE, (guint) result); - g_dbus_connection_unregister_object(connection, registration_id); + g_dbus_connection_unregister_object (priv->main_dbus_connection, registration_id); return FALSE; } priv->objmgr_registration_id = registration_id; - priv->main_dbus_connection = g_steal_pointer (&connection); _LOGI ("acquired D-Bus service \"%s\"", NM_DBUS_SERVICE); @@ -1690,6 +1706,7 @@ nm_dbus_manager_init (NMDBusManager *self) c_list_init (&priv->private_servers_lst_head); c_list_init (&priv->objects_lst_head); + priv->objects_by_path = g_hash_table_new ((GHashFunc) _objects_by_path_hash, (GEqualFunc) _objects_by_path_equal); c_list_init (&priv->caller_info_lst_head); diff --git a/src/nm-dbus-manager.h b/src/nm-dbus-manager.h index 2de6ff0ba7..7a510e6f40 100644 --- a/src/nm-dbus-manager.h +++ b/src/nm-dbus-manager.h @@ -49,7 +49,8 @@ typedef void (*NMDBusManagerSetPropertyHandler) (NMDBusObject *obj, GVariant *value, gpointer user_data); -gboolean nm_dbus_manager_acquire_bus (NMDBusManager *self); +gboolean nm_dbus_manager_acquire_bus (NMDBusManager *self, + gboolean request_name); GDBusConnection *nm_dbus_manager_get_dbus_connection (NMDBusManager *self);