diff options
author | Thomas Haller <thaller@redhat.com> | 2019-09-04 13:58:43 +0200 |
---|---|---|
committer | Thomas Haller <thaller@redhat.com> | 2019-09-27 16:28:15 +0200 |
commit | 1a3006fd7440c4a1b0d131992b07e17de1cba834 (patch) | |
tree | 5000e09008367d36f3c128b1c4654021bfe4fc96 | |
parent | 5f284e1574e0c3ab319b504671d263ce89ca12f9 (diff) | |
download | NetworkManager-1a3006fd7440c4a1b0d131992b07e17de1cba834.tar.gz |
libnm: deprecate all synchronous/blocking API in libnm
Note that D-Bus is fundamentally asynchronous. Doing blocking calls
on top of that is wrong, especially for libnm. That is because libnm
also provides a client-side cache of the objects from the D-Bus
interface. This cache must be filled by asynchronous events (of course).
So, making a blocking D-Bus call means to wait for a response and
return it, while queuing everything that happens in between. That
violates the strict ordering that we want to guarantee. It also means,
when the blocking call returns, then the cache is still frozen in the
state as it was before the call, and it will stay so until you iterate
the main context.
Read also [1], for why blocking calls are wrong.
[1] https://smcv.pseudorandom.co.uk/2008/11/nonblocking/
Mark all this API as deprecated. Also, this serves the purpose of
identifying blocking D-Bus calls in libnm.
Note that API like nm_device_wifi_request_scan_options() indeed
does not really need to be in sync with the platform cache. I mean,
it makes a blocking request to start scanning, which does not change
the content of the cache and there wouldn't be a severe problem with
this API. However, it requires as source-argument a NMDeviceWifi, instead
of a plain D-Bus path. If that API would be just a convenience wrapper around
g_dbus_connection_call_sync(), and accept as target a plan D-Bus path,
it would be fine. But having synchronous API that operates on a
NMDeviceWifi is wrong. Hence deprecate it too. Also, it is still wrong
to process any messages from D-Bus (the reply in this case) before other
signals as it destroys the ordering.
-rw-r--r-- | libnm-core/nm-version.h | 26 | ||||
-rw-r--r-- | libnm/nm-client.c | 22 | ||||
-rw-r--r-- | libnm/nm-client.h | 27 | ||||
-rw-r--r-- | libnm/nm-device-wifi.c | 4 | ||||
-rw-r--r-- | libnm/nm-device-wifi.h | 2 | ||||
-rw-r--r-- | libnm/nm-device.c | 14 | ||||
-rw-r--r-- | libnm/nm-device.h | 13 | ||||
-rw-r--r-- | libnm/nm-libnm-utils.h | 4 | ||||
-rw-r--r-- | libnm/nm-manager.h | 22 |
9 files changed, 126 insertions, 8 deletions
diff --git a/libnm-core/nm-version.h b/libnm-core/nm-version.h index be402760b2..00badcf7f0 100644 --- a/libnm-core/nm-version.h +++ b/libnm-core/nm-version.h @@ -215,4 +215,30 @@ # define NM_AVAILABLE_IN_1_22 #endif +/* libnm's NMClient maintains a cache of NetworkManager's D-Bus interface. + * Issuing blocking calls from libnm API will only invoke the D-Bus method + * and return it's result, without updating the cache (of course, otherwise + * if it would emit signals and change the cache content while waiting, it + * wouldn't be very blocking). + * + * When a blocking call returns (from g_dbus_connection_call_sync()), the + * response is processed out of order from other events that populate the + * object cache. That is bad. + * + * Even worse, the cache is no longer up-to-date, when the blocking call + * returns. It will only get sync'ed when you iterate the main context again. + * At this point, why did you call the blocking method? It doesn't make sense. + * + * For that reason, blocking API is deprecated. They are odd to use. You cannot + * glue a synchronous API on top of D-Bus (which is inherrently asynchronous). + * At least not, if you also have other state (the object cache), that should stay + * in sync. + * + * These methods are effectively deprecated since 1.22. However, at this point + * we don't yet mark them as such, because it might just cause unnecessary compiler + * warnings. Let's first deprecate them for a long time, before we enable the + * compiler warning. */ +#define _NM_DEPRECATED_SYNC_METHOD /*NM_DEPRECATED_IN_1_22*/ +#define _NM_DEPRECATED_SYNC_WRITABLE_PROPERTY /*NM_DEPRECATED_IN_1_22*/ + #endif /* NM_VERSION_H */ diff --git a/libnm/nm-client.c b/libnm/nm-client.c index 26bf91f5fa..1e81221537 100644 --- a/libnm/nm-client.c +++ b/libnm/nm-client.c @@ -291,6 +291,8 @@ nm_client_networking_get_enabled (NMClient *client) * all controlled interfaces are available for activation. * * Returns: %TRUE on success, %FALSE otherwise + * + * Deprecated: 1.22 **/ gboolean nm_client_networking_set_enabled (NMClient *client, gboolean enable, GError **error) @@ -329,7 +331,9 @@ nm_client_wireless_get_enabled (NMClient *client) * @enabled: %TRUE to enable wireless * * Enables or disables wireless devices. - **/ + * + * Deprecated: 1.22 + */ void nm_client_wireless_set_enabled (NMClient *client, gboolean enabled) { @@ -3661,7 +3665,9 @@ nm_client_class_init (NMClientClass *client_class) * NMClient:networking-enabled: * * Whether networking is enabled. - **/ + * + * The property setter is a synchronous D-Bus call. This is deprecated since 1.22. + */ g_object_class_install_property (object_class, PROP_NETWORKING_ENABLED, g_param_spec_boolean (NM_CLIENT_NETWORKING_ENABLED, "", "", @@ -3673,6 +3679,8 @@ nm_client_class_init (NMClientClass *client_class) * NMClient:wireless-enabled: * * Whether wireless is enabled. + * + * The property setter is a synchronous D-Bus call. This is deprecated since 1.22. **/ g_object_class_install_property (object_class, PROP_WIRELESS_ENABLED, @@ -3697,7 +3705,9 @@ nm_client_class_init (NMClientClass *client_class) * NMClient:wwan-enabled: * * Whether WWAN functionality is enabled. - **/ + * + * The property setter is a synchronous D-Bus call. This is deprecated since 1.22. + */ g_object_class_install_property (object_class, PROP_WWAN_ENABLED, g_param_spec_boolean (NM_CLIENT_WWAN_ENABLED, "", "", @@ -3721,7 +3731,9 @@ nm_client_class_init (NMClientClass *client_class) * NMClient:wimax-enabled: * * Whether WiMAX functionality is enabled. - **/ + * + * The property setter is a synchronous D-Bus call. This is deprecated since 1.22. + */ g_object_class_install_property (object_class, PROP_WIMAX_ENABLED, g_param_spec_boolean (NM_CLIENT_WIMAX_ENABLED, "", "", @@ -3786,6 +3798,8 @@ nm_client_class_init (NMClientClass *client_class) * Whether a connectivity checking service has been enabled. * * Since: 1.10 + * + * The property setter is a synchronous D-Bus call. This is deprecated since 1.22. */ g_object_class_install_property (object_class, PROP_CONNECTIVITY_CHECK_ENABLED, diff --git a/libnm/nm-client.h b/libnm/nm-client.h index f8ffd52508..44b71ee094 100644 --- a/libnm/nm-client.h +++ b/libnm/nm-client.h @@ -26,17 +26,28 @@ G_BEGIN_DECLS #define NM_CLIENT_STATE "state" #define NM_CLIENT_STARTUP "startup" #define NM_CLIENT_NM_RUNNING "nm-running" + +_NM_DEPRECATED_SYNC_WRITABLE_PROPERTY #define NM_CLIENT_NETWORKING_ENABLED "networking-enabled" + +_NM_DEPRECATED_SYNC_WRITABLE_PROPERTY #define NM_CLIENT_WIRELESS_ENABLED "wireless-enabled" -#define NM_CLIENT_WIRELESS_HARDWARE_ENABLED "wireless-hardware-enabled" +_NM_DEPRECATED_SYNC_WRITABLE_PROPERTY #define NM_CLIENT_WWAN_ENABLED "wwan-enabled" -#define NM_CLIENT_WWAN_HARDWARE_ENABLED "wwan-hardware-enabled" +_NM_DEPRECATED_SYNC_WRITABLE_PROPERTY #define NM_CLIENT_WIMAX_ENABLED "wimax-enabled" + +#define NM_CLIENT_WIRELESS_HARDWARE_ENABLED "wireless-hardware-enabled" +#define NM_CLIENT_WWAN_HARDWARE_ENABLED "wwan-hardware-enabled" #define NM_CLIENT_WIMAX_HARDWARE_ENABLED "wimax-hardware-enabled" + #define NM_CLIENT_ACTIVE_CONNECTIONS "active-connections" #define NM_CLIENT_CONNECTIVITY "connectivity" #define NM_CLIENT_CONNECTIVITY_CHECK_AVAILABLE "connectivity-check-available" + +_NM_DEPRECATED_SYNC_WRITABLE_PROPERTY #define NM_CLIENT_CONNECTIVITY_CHECK_ENABLED "connectivity-check-enabled" + #define NM_CLIENT_PRIMARY_CONNECTION "primary-connection" #define NM_CLIENT_ACTIVATING_CONNECTION "activating-connection" #define NM_CLIENT_DEVICES "devices" @@ -225,20 +236,31 @@ gboolean nm_client_get_startup (NMClient *client); gboolean nm_client_get_nm_running (NMClient *client); gboolean nm_client_networking_get_enabled (NMClient *client); + +_NM_DEPRECATED_SYNC_METHOD gboolean nm_client_networking_set_enabled (NMClient *client, gboolean enabled, GError **error); gboolean nm_client_wireless_get_enabled (NMClient *client); + +_NM_DEPRECATED_SYNC_METHOD void nm_client_wireless_set_enabled (NMClient *client, gboolean enabled); + gboolean nm_client_wireless_hardware_get_enabled (NMClient *client); gboolean nm_client_wwan_get_enabled (NMClient *client); + +_NM_DEPRECATED_SYNC_METHOD void nm_client_wwan_set_enabled (NMClient *client, gboolean enabled); + gboolean nm_client_wwan_hardware_get_enabled (NMClient *client); gboolean nm_client_wimax_get_enabled (NMClient *client); + +_NM_DEPRECATED_SYNC_METHOD void nm_client_wimax_set_enabled (NMClient *client, gboolean enabled); + gboolean nm_client_wimax_hardware_get_enabled (NMClient *client); NM_AVAILABLE_IN_1_10 @@ -248,6 +270,7 @@ NM_AVAILABLE_IN_1_10 gboolean nm_client_connectivity_check_get_enabled (NMClient *client); NM_AVAILABLE_IN_1_10 +_NM_DEPRECATED_SYNC_METHOD void nm_client_connectivity_check_set_enabled (NMClient *client, gboolean enabled); diff --git a/libnm/nm-device-wifi.c b/libnm/nm-device-wifi.c index 165f535e45..e1d966af6d 100644 --- a/libnm/nm-device-wifi.c +++ b/libnm/nm-device-wifi.c @@ -331,6 +331,8 @@ _device_wifi_request_scan (NMDeviceWifi *device, * * Returns: %TRUE on success, %FALSE on error, in which case @error will be * set. + * + * Deprecated: 1.22 **/ gboolean nm_device_wifi_request_scan (NMDeviceWifi *device, @@ -359,6 +361,8 @@ nm_device_wifi_request_scan (NMDeviceWifi *device, * set. * * Since: 1.2 + * + * Deprecated: 1.22 **/ gboolean nm_device_wifi_request_scan_options (NMDeviceWifi *device, diff --git a/libnm/nm-device-wifi.h b/libnm/nm-device-wifi.h index 847f701460..3f86e7b847 100644 --- a/libnm/nm-device-wifi.h +++ b/libnm/nm-device-wifi.h @@ -66,10 +66,12 @@ const GPtrArray * nm_device_wifi_get_access_points (NMDeviceWifi * NM_AVAILABLE_IN_1_12 gint64 nm_device_wifi_get_last_scan (NMDeviceWifi *device); +_NM_DEPRECATED_SYNC_METHOD gboolean nm_device_wifi_request_scan (NMDeviceWifi *device, GCancellable *cancellable, GError **error); NM_AVAILABLE_IN_1_2 +_NM_DEPRECATED_SYNC_METHOD gboolean nm_device_wifi_request_scan_options (NMDeviceWifi *device, GVariant *options, GCancellable *cancellable, diff --git a/libnm/nm-device.c b/libnm/nm-device.c index f6663fbd23..df26c272ca 100644 --- a/libnm/nm-device.c +++ b/libnm/nm-device.c @@ -610,6 +610,8 @@ nm_device_class_init (NMDeviceClass *device_class) * NMDevice:autoconnect: * * Whether the device can auto-activate a connection. + * + * The property setter is a synchronous D-Bus call. This is deprecated since 1.22. **/ g_object_class_install_property (object_class, PROP_AUTOCONNECT, @@ -1087,6 +1089,8 @@ nm_device_get_managed (NMDevice *device) * Enables or disables management of #NMDevice by NetworkManager. * * Since: 1.2 + * + * Deprecated: 1.22 **/ void nm_device_set_managed (NMDevice *device, gboolean managed) @@ -1125,6 +1129,8 @@ nm_device_get_autoconnect (NMDevice *device) * @autoconnect: %TRUE to enable autoconnecting * * Enables or disables automatic activation of the #NMDevice. + * + * Deprecated: 1.22 **/ void nm_device_set_autoconnect (NMDevice *device, gboolean autoconnect) @@ -1988,6 +1994,8 @@ nm_device_is_software (NMDevice *device) * Returns: %TRUE on success, %FALSE on error, in which case @error will be set. * * Since: 1.2 + * + * Deprecated: 1.22 **/ gboolean nm_device_reapply (NMDevice *device, @@ -2130,6 +2138,8 @@ nm_device_reapply_finish (NMDevice *device, * to nm_connection_verify(). * * Since: 1.2 + * + * Deprecated: 1.22 **/ NMConnection * nm_device_get_applied_connection (NMDevice *device, @@ -2302,6 +2312,8 @@ nm_device_get_applied_connection_finish (NMDevice *device, * request. * * Returns: %TRUE on success, %FALSE on error, in which case @error will be set. + * + * Deprecated: 1.22 **/ gboolean nm_device_disconnect (NMDevice *device, @@ -2406,6 +2418,8 @@ nm_device_disconnect_finish (NMDevice *device, * * Returns: %TRUE on success, %FALSE on error, in which case @error * will be set. + * + * Deprecated: 1.22 **/ gboolean nm_device_delete (NMDevice *device, diff --git a/libnm/nm-device.h b/libnm/nm-device.h index ba9cee8c79..5c2f5d3b7c 100644 --- a/libnm/nm-device.h +++ b/libnm/nm-device.h @@ -32,7 +32,10 @@ G_BEGIN_DECLS #define NM_DEVICE_CAPABILITIES "capabilities" #define NM_DEVICE_REAL "real" #define NM_DEVICE_MANAGED "managed" + +_NM_DEPRECATED_SYNC_WRITABLE_PROPERTY #define NM_DEVICE_AUTOCONNECT "autoconnect" + #define NM_DEVICE_FIRMWARE_MISSING "firmware-missing" #define NM_DEVICE_NM_PLUGIN_MISSING "nm-plugin-missing" #define NM_DEVICE_IP4_CONFIG "ip4-config" @@ -97,10 +100,16 @@ const char * nm_device_get_type_description (NMDevice *device); const char * nm_device_get_hw_address (NMDevice *device); NMDeviceCapabilities nm_device_get_capabilities (NMDevice *device); gboolean nm_device_get_managed (NMDevice *device); + NM_AVAILABLE_IN_1_2 +_NM_DEPRECATED_SYNC_METHOD void nm_device_set_managed (NMDevice *device, gboolean managed); + gboolean nm_device_get_autoconnect (NMDevice *device); + +_NM_DEPRECATED_SYNC_METHOD void nm_device_set_autoconnect (NMDevice *device, gboolean autoconnect); + gboolean nm_device_get_firmware_missing (NMDevice *device); NM_AVAILABLE_IN_1_2 gboolean nm_device_get_nm_plugin_missing (NMDevice *device); @@ -130,6 +139,7 @@ GPtrArray * nm_device_get_lldp_neighbors (NMDevice *device); char ** nm_device_disambiguate_names (NMDevice **devices, int num_devices); NM_AVAILABLE_IN_1_2 +_NM_DEPRECATED_SYNC_METHOD gboolean nm_device_reapply (NMDevice *device, NMConnection *connection, guint64 version_id, @@ -150,6 +160,7 @@ gboolean nm_device_reapply_finish (NMDevice *device, GError **error); NM_AVAILABLE_IN_1_2 +_NM_DEPRECATED_SYNC_METHOD NMConnection *nm_device_get_applied_connection (NMDevice *device, guint32 flags, guint64 *version_id, @@ -167,6 +178,7 @@ NMConnection *nm_device_get_applied_connection_finish (NMDevice *device, guint64 *version_id, GError **error); +_NM_DEPRECATED_SYNC_METHOD gboolean nm_device_disconnect (NMDevice *device, GCancellable *cancellable, GError **error); @@ -178,6 +190,7 @@ gboolean nm_device_disconnect_finish (NMDevice *device, GAsyncResult *result, GError **error); +_NM_DEPRECATED_SYNC_METHOD gboolean nm_device_delete (NMDevice *device, GCancellable *cancellable, GError **error); diff --git a/libnm/nm-libnm-utils.h b/libnm/nm-libnm-utils.h index 20c0161a18..6d5e146515 100644 --- a/libnm/nm-libnm-utils.h +++ b/libnm/nm-libnm-utils.h @@ -10,6 +10,10 @@ #error Cannot use this header. #endif +/* Markers for deprecated sync code in internal API. */ +#define _NM_DEPRECATED_SYNC_METHOD_INTERNAL NM_DEPRECATED_IN_1_22 +#define _NM_DEPRECATED_SYNC_WRITABLE_PROPERTY_INTERNAL NM_DEPRECATED_IN_1_22 + char *nm_utils_fixup_vendor_string (const char *desc); char *nm_utils_fixup_product_string (const char *desc); diff --git a/libnm/nm-manager.h b/libnm/nm-manager.h index 9bbbe53af4..eb71574d5c 100644 --- a/libnm/nm-manager.h +++ b/libnm/nm-manager.h @@ -13,6 +13,7 @@ #include "nm-object.h" #include "nm-client.h" +#include "nm-libnm-utils.h" #define NM_TYPE_MANAGER (nm_manager_get_type ()) #define NM_MANAGER(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_MANAGER, NMManager)) @@ -25,16 +26,26 @@ #define NM_MANAGER_STATE "state" #define NM_MANAGER_STARTUP "startup" #define NM_MANAGER_NETWORKING_ENABLED "networking-enabled" + +_NM_DEPRECATED_SYNC_WRITABLE_PROPERTY_INTERNAL #define NM_MANAGER_WIRELESS_ENABLED "wireless-enabled" -#define NM_MANAGER_WIRELESS_HARDWARE_ENABLED "wireless-hardware-enabled" + +_NM_DEPRECATED_SYNC_WRITABLE_PROPERTY_INTERNAL #define NM_MANAGER_WWAN_ENABLED "wwan-enabled" -#define NM_MANAGER_WWAN_HARDWARE_ENABLED "wwan-hardware-enabled" + +_NM_DEPRECATED_SYNC_WRITABLE_PROPERTY_INTERNAL #define NM_MANAGER_WIMAX_ENABLED "wimax-enabled" + +#define NM_MANAGER_WIRELESS_HARDWARE_ENABLED "wireless-hardware-enabled" +#define NM_MANAGER_WWAN_HARDWARE_ENABLED "wwan-hardware-enabled" #define NM_MANAGER_WIMAX_HARDWARE_ENABLED "wimax-hardware-enabled" #define NM_MANAGER_ACTIVE_CONNECTIONS "active-connections" #define NM_MANAGER_CONNECTIVITY "connectivity" #define NM_MANAGER_CONNECTIVITY_CHECK_AVAILABLE "connectivity-check-available" + +_NM_DEPRECATED_SYNC_WRITABLE_PROPERTY_INTERNAL #define NM_MANAGER_CONNECTIVITY_CHECK_ENABLED "connectivity-check-enabled" + #define NM_MANAGER_PRIMARY_CONNECTION "primary-connection" #define NM_MANAGER_ACTIVATING_CONNECTION "activating-connection" #define NM_MANAGER_DEVICES "devices" @@ -71,12 +82,17 @@ NMState nm_manager_get_state (NMManager *manager); gboolean nm_manager_get_startup (NMManager *manager); gboolean nm_manager_networking_get_enabled (NMManager *manager); + +_NM_DEPRECATED_SYNC_METHOD_INTERNAL gboolean nm_manager_networking_set_enabled (NMManager *manager, gboolean enabled, GError **error); gboolean nm_manager_wireless_get_enabled (NMManager *manager); + +_NM_DEPRECATED_SYNC_METHOD_INTERNAL void nm_manager_wireless_set_enabled (NMManager *manager, gboolean enabled); + gboolean nm_manager_wireless_hardware_get_enabled (NMManager *manager); gboolean nm_manager_wwan_get_enabled (NMManager *manager); @@ -110,6 +126,7 @@ NMClientPermissionResult nm_manager_get_permission_result (NMManager *manager, NMConnectivityState nm_manager_get_connectivity (NMManager *manager); +_NM_DEPRECATED_SYNC_METHOD_INTERNAL NMConnectivityState nm_manager_check_connectivity (NMManager *manager, GCancellable *cancellable, GError **error); @@ -160,6 +177,7 @@ NMActiveConnection *nm_manager_add_and_activate_connection_finish (NMManager *ma GVariant **out_result, GError **error); +_NM_DEPRECATED_SYNC_METHOD_INTERNAL gboolean nm_manager_deactivate_connection (NMManager *manager, NMActiveConnection *active, GCancellable *cancellable, |