colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

libnm: advise using D-Bus instead of deprecated synchronous methods

Browse Source 
With 1.22, various synchronous functions for invoking D-Bus methods were
deprecated. The reason was that D-Bus is fundamentally asynchronous, and
providing synchronous API in NMClient is inherently wrong. That is
because NMClient essentially is a cache of the D-Bus API, and invoking
g_dbus_connection_call_sync() messes up the order of events from D-Bus.
In particular, when the synchronous function completes, the content of
the cache does not yet reflect the change.

Since they got deprecated, the question is with what to replace them.

Instead of adding a (e.g.) nm_client_networking_set_enabled_async()
for nm_client_networking_set_enabled(), just expect the user to call
D-Bus directly.

D-Bus itself defines a reasonable API, and with GDBusConnection it
is fine (and convenient) to just call D-Bus operations directly.
Often libraries try to abstract D-Bus by providing convenience
wrappers around D-Bus API. I think that often is wrong and unnecessary.

Note that libnm's NMClient does a lot more than just wrapping simple
D-Bus calls. It provides a complete client-side cache of the D-Bus
interface. As such, what libnm's NMClient does is more than simple
wrappers around D-Bus. NMClient is a reasonable thing to do.

However, it is unnecessary to add API like nm_client_networking_set_enabled_async()
that only calls g_dbus_connection_call(). Don't pretend that we would need such
trivial wrappers in libnm.

Instead, recommend to use g_dbus_connection_call(). Or alternatively,
the convenience wrappers nm_client_dbus_call() and
nm_client_dbus_set_property().
This commit is contained in:
Thomas Haller
2020-03-13 18:24:37 +01:00
parent a7b8c82ee2
commit 06da903bb6
2 changed files with 20 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

31
libnm/nm-client.c
View File

@@ -4039,15 +4039,14 @@ nm_client_networking_get_enabled (NMClient *client)
*
* Returns: %TRUE on success, %FALSE otherwise
*
* Deprecated: 1.22, use nm_client_networking_set_enabled_async() or GDBusConnection
* Deprecated: 1.22: Use the async command nm_client_dbus_call() on %NM_DBUS_PATH,
* %NM_DBUS_INTERFACE to call "Enable" with "(b)" arguments and no return value.
**/
gboolean
nm_client_networking_set_enabled (NMClient *client, gboolean enable, GError **error)
{
g_return_val_if_fail (NM_IS_CLIENT (client), FALSE);
/* FIXME(libnm-async-api): add nm_client_networking_set_enabled_async(). */
return _nm_client_dbus_call_sync_void (client,
NULL,
NM_DBUS_PATH,
@@ -4083,15 +4082,14 @@ nm_client_wireless_get_enabled (NMClient *client)
*
* Enables or disables wireless devices.
*
* Deprecated: 1.22, use nm_client_wireless_set_enabled_async() or GDBusConnection
* Deprecated: 1.22: Use the async command nm_client_dbus_set_property() on %NM_DBUS_PATH,
* %NM_DBUS_INTERFACE to set "WirelessEnabled" property to a "(b)" value.
*/
void
nm_client_wireless_set_enabled (NMClient *client, gboolean enabled)
{
g_return_if_fail (NM_IS_CLIENT (client));
/* FIXME(libnm-async-api): add nm_client_wireless_set_enabled_async(). */
_nm_client_set_property_sync_legacy (client,
NM_DBUS_PATH,
NM_DBUS_INTERFACE,
@@ -4138,14 +4136,15 @@ nm_client_wwan_get_enabled (NMClient *client)
* @enabled: %TRUE to enable WWAN
*
* Enables or disables WWAN devices.
*
* Deprecated: 1.22: Use the async command nm_client_dbus_set_property() on %NM_DBUS_PATH,
* %NM_DBUS_INTERFACE to set "WwanEnabled" property to a "(b)" value.
**/
void
nm_client_wwan_set_enabled (NMClient *client, gboolean enabled)
{
g_return_if_fail (NM_IS_CLIENT (client));
/* FIXME(libnm-async-api): add nm_client_wwan_set_enabled_async(). */
_nm_client_set_property_sync_legacy (client,
NM_DBUS_PATH,
NM_DBUS_INTERFACE,
@@ -4269,14 +4268,15 @@ nm_client_connectivity_check_get_enabled (NMClient *client)
* have any effect.
*
* Since: 1.10
*
* Deprecated: 1.22: Use the async command nm_client_dbus_set_property() on %NM_DBUS_PATH,
* %NM_DBUS_INTERFACE to set "ConnectivityCheckEnabled" property to a "(b)" value.
*/
void
nm_client_connectivity_check_set_enabled (NMClient *client, gboolean enabled)
{
g_return_if_fail (NM_IS_CLIENT (client));
/* FIXME(libnm-async-api): add nm_client_wireless_set_enabled_async(). */
_nm_client_set_property_sync_legacy (client,
NM_DBUS_PATH,
NM_DBUS_INTERFACE,
@@ -4316,7 +4316,9 @@ nm_client_connectivity_check_get_uri (NMClient *client)
*
* Returns: %TRUE on success, %FALSE otherwise
*
* Deprecated: 1.22, use nm_client_get_logging_async() or GDBusConnection
* Deprecated: 1.22: Use the async command nm_client_dbus_call() on %NM_DBUS_PATH,
* %NM_DBUS_INTERFACE to call "GetLogging" with no arguments to get "(ss)" for level
* and domains.
**/
gboolean
nm_client_get_logging (NMClient *client,
@@ -4331,8 +4333,6 @@ nm_client_get_logging (NMClient *client,
g_return_val_if_fail (domains == NULL || *domains == NULL, FALSE);
g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
/* FIXME(libnm-async-api): add nm_client_get_logging_async(). */
ret = _nm_client_dbus_call_sync (client,
NULL,
NM_DBUS_PATH,
@@ -4366,7 +4366,8 @@ nm_client_get_logging (NMClient *client,
*
* Returns: %TRUE on success, %FALSE otherwise
*
* Deprecated: 1.22, use nm_client_set_logging_async() or GDBusConnection
* Deprecated: 1.22: Use the async command nm_client_dbus_call() on %NM_DBUS_PATH,
* %NM_DBUS_INTERFACE to call "SetLogging" with "(ss)" arguments for level and domains.
**/
gboolean
nm_client_set_logging (NMClient *client, const char *level, const char *domains, GError **error)
@@ -4374,8 +4375,6 @@ nm_client_set_logging (NMClient *client, const char *level, const char *domains,
g_return_val_if_fail (NM_IS_CLIENT (client), FALSE);
g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
/* FIXME(libnm-async-api): add nm_client_set_logging_async(). */
return _nm_client_dbus_call_sync_void (client,
NULL,
NM_DBUS_PATH,

13
libnm/nm-device.c
View File

@@ -1211,8 +1211,9 @@ nm_device_get_managed (NMDevice *device)
*
* Since: 1.2
*
* Deprecated: 1.22, use nm_device_set_managed_async() or GDBusConnection
*
* Deprecated: 1.22: Use the async command nm_client_dbus_set_property() on
* nm_object_get_path(), interface %NM_DBUS_INTERFACE_DEVICE to set the
* "Managed" property to a "(b)" boolean value.
* This function is deprecated because it calls a synchronous D-Bus method
* and modifies the content of the NMClient cache client side. Also, it does
* not emit a property changed signal.
@@ -1222,8 +1223,6 @@ nm_device_set_managed (NMDevice *device, gboolean managed)
{
g_return_if_fail (NM_IS_DEVICE (device));
/* FIXME(libnm-async-api): add nm_device_set_managed_async(). */
managed = !!managed;
NM_DEVICE_GET_PRIVATE (device)->managed = managed;
@@ -1259,8 +1258,8 @@ nm_device_get_autoconnect (NMDevice *device)
*
* Enables or disables automatic activation of the #NMDevice.
*
* Deprecated: 1.22: Use nm_device_set_autoconnect_async() or GDBusConnection.
*
* Deprecated: 1.22: Use the async command nm_client_dbus_set_property() on
* nm_object_get_path(), %NM_DBUS_INTERFACE_DEVICE to set "AutoConnect" property to a "(b)" value.
* This function is deprecated because it calls a synchronous D-Bus method
* and modifies the content of the NMClient cache client side.
**/
@@ -1269,8 +1268,6 @@ nm_device_set_autoconnect (NMDevice *device, gboolean autoconnect)
{
g_return_if_fail (NM_IS_DEVICE (device));
/* FIXME(libnm-async-api): add nm_device_set_autoconnect_async(). */
NM_DEVICE_GET_PRIVATE (device)->autoconnect = autoconnect;
_nm_client_set_property_sync_legacy (_nm_object_get_client (device),