From ae5cfba05c8bc9a6654534ca09309ea520be6430 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Nov 2015 14:49:15 +0100 Subject: [PATCH 1/8] ifcfg-rh: fix chaining constructed() method for SettingsPluingIfcfg --- src/settings/plugins/ifcfg-rh/plugin.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/settings/plugins/ifcfg-rh/plugin.c b/src/settings/plugins/ifcfg-rh/plugin.c index 70e06db9f7..e7ce97a28d 100644 --- a/src/settings/plugins/ifcfg-rh/plugin.c +++ b/src/settings/plugins/ifcfg-rh/plugin.c @@ -761,6 +761,8 @@ constructed (GObject *object) GVariant *ret; guint32 result; + G_OBJECT_CLASS (settings_plugin_ifcfg_parent_class)->constructed (object); + bus = g_bus_get_sync (G_BUS_TYPE_SYSTEM, NULL, &error); if (!bus) { _LOGW ("Couldn't connect to D-Bus: %s", error->message); From 8a8ecc46cad07be209f202d36eba871653d79ede Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Nov 2015 14:44:29 +0100 Subject: [PATCH 2/8] core: fix wrongly exporting object before instance is fully constructed Exporting the object already in the *_init() function will later break because the object is not yet fully initialized at that point. Add a convenient flag so that the NMExportedObject parent implementation automatically can export itself. This saves the derived class from overwriting the constructed() method. Also add an assertion to catch such bugs. --- src/nm-dhcp4-config.c | 3 +-- src/nm-dhcp6-config.c | 3 +-- src/nm-exported-object.c | 30 ++++++++++++++++++++++++++++++ src/nm-exported-object.h | 1 + 4 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/nm-dhcp4-config.c b/src/nm-dhcp4-config.c index 1aed32d9b6..00b9949b36 100644 --- a/src/nm-dhcp4-config.c +++ b/src/nm-dhcp4-config.c @@ -104,8 +104,6 @@ nm_dhcp4_config_init (NMDhcp4Config *self) { NMDhcp4ConfigPrivate *priv = NM_DHCP4_CONFIG_GET_PRIVATE (self); - nm_exported_object_export (NM_EXPORTED_OBJECT (self)); - priv->options = g_variant_new_array (G_VARIANT_TYPE ("{sv}"), NULL, 0); g_variant_ref_sink (priv->options); } @@ -145,6 +143,7 @@ nm_dhcp4_config_class_init (NMDhcp4ConfigClass *config_class) g_type_class_add_private (config_class, sizeof (NMDhcp4ConfigPrivate)); exported_object_class->export_path = NM_DBUS_PATH "/DHCP4Config/%u"; + exported_object_class->export_on_construction = TRUE; /* virtual methods */ object_class->get_property = get_property; diff --git a/src/nm-dhcp6-config.c b/src/nm-dhcp6-config.c index 4855839275..8aa8c2e07a 100644 --- a/src/nm-dhcp6-config.c +++ b/src/nm-dhcp6-config.c @@ -104,8 +104,6 @@ nm_dhcp6_config_init (NMDhcp6Config *self) { NMDhcp6ConfigPrivate *priv = NM_DHCP6_CONFIG_GET_PRIVATE (self); - nm_exported_object_export (NM_EXPORTED_OBJECT (self)); - priv->options = g_variant_new_array (G_VARIANT_TYPE ("{sv}"), NULL, 0); g_variant_ref_sink (priv->options); } @@ -145,6 +143,7 @@ nm_dhcp6_config_class_init (NMDhcp6ConfigClass *config_class) g_type_class_add_private (config_class, sizeof (NMDhcp6ConfigPrivate)); exported_object_class->export_path = NM_DBUS_PATH "/DHCP6Config/%u"; + exported_object_class->export_on_construction = TRUE; /* virtual methods */ object_class->get_property = get_property; diff --git a/src/nm-exported-object.c b/src/nm-exported-object.c index bdf409ad17..07caa3a20b 100644 --- a/src/nm-exported-object.c +++ b/src/nm-exported-object.c @@ -29,6 +29,10 @@ static GHashTable *prefix_counters; +#if NM_MORE_ASSERTS >= 2 +#define _ASSERT_NO_EARLY_EXPORT +#endif + G_DEFINE_ABSTRACT_TYPE_WITH_CODE (NMExportedObject, nm_exported_object, G_TYPE_OBJECT, prefix_counters = g_hash_table_new (g_str_hash, g_str_equal); ) @@ -41,6 +45,10 @@ typedef struct { GVariantBuilder pending_notifies; guint notify_idle_id; + +#ifdef _ASSERT_NO_EARLY_EXPORT + gboolean _constructed; +#endif } NMExportedObjectPrivate; #define NM_EXPORTED_OBJECT_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_EXPORTED_OBJECT, NMExportedObjectPrivate)) @@ -490,6 +498,10 @@ nm_exported_object_export (NMExportedObject *self) g_return_val_if_fail (!priv->path, priv->path); g_return_val_if_fail (!priv->bus_mgr, NULL); +#ifdef _ASSERT_NO_EARLY_EXPORT + nm_assert (priv->_constructed); +#endif + class_export_path = NM_EXPORTED_OBJECT_GET_CLASS (self)->export_path; p = strchr (class_export_path, '%'); if (p) { @@ -731,6 +743,23 @@ nm_exported_object_notify (GObject *object, GParamSpec *pspec) priv->notify_idle_id = g_idle_add (idle_emit_properties_changed, object); } +static void +constructed (GObject *object) +{ + NMExportedObjectClass *klass; + + G_OBJECT_CLASS (nm_exported_object_parent_class)->constructed (object); + +#ifdef _ASSERT_NO_EARLY_EXPORT + NM_EXPORTED_OBJECT_GET_PRIVATE (object)->_constructed = TRUE; +#endif + + klass = NM_EXPORTED_OBJECT_GET_CLASS (object); + + if (klass->export_on_construction) + nm_exported_object_export ((NMExportedObject *) object); +} + static void nm_exported_object_dispose (GObject *object) { @@ -752,6 +781,7 @@ nm_exported_object_class_init (NMExportedObjectClass *klass) g_type_class_add_private (object_class, sizeof (NMExportedObjectPrivate)); + object_class->constructed = constructed; object_class->notify = nm_exported_object_notify; object_class->dispose = nm_exported_object_dispose; } diff --git a/src/nm-exported-object.h b/src/nm-exported-object.h index 65f0eb6563..0b952b837a 100644 --- a/src/nm-exported-object.h +++ b/src/nm-exported-object.h @@ -40,6 +40,7 @@ typedef struct { GObjectClass parent; const char *export_path; + char export_on_construction; } NMExportedObjectClass; GType nm_exported_object_get_type (void); From 0c1e290c97555c3ed14bed9d0ab55a5dab2c7a2e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 7 Nov 2015 10:00:06 +0100 Subject: [PATCH 3/8] wifi/ap: set the current-ap after adding the new AP First add the new AP, before setting it as current. Also set the AP *after* thawing the notifications. Otherwise it is not clear which notification gets raised first as their order is undefined. But we want that the client first sees the new AP and later gets a notification about having a new current. --- src/devices/wifi/nm-device-wifi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 99e35a9651..68c6745295 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -2346,9 +2346,9 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *reason) ap_path = nm_exported_object_export (NM_EXPORTED_OBJECT (ap)); g_hash_table_insert (priv->aps, (gpointer) ap_path, ap); g_object_freeze_notify (G_OBJECT (self)); - set_current_ap (self, ap, FALSE, FALSE); emit_ap_added_removed (self, ACCESS_POINT_ADDED, ap, TRUE); g_object_thaw_notify (G_OBJECT (self)); + set_current_ap (self, ap, FALSE, FALSE); nm_active_connection_set_specific_object (NM_ACTIVE_CONNECTION (req), ap_path); return NM_ACT_STAGE_RETURN_SUCCESS; From 96f40dcdcd8b2df204d64026f0315ff6370048fa Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Nov 2015 12:43:56 +0100 Subject: [PATCH 4/8] wifi/ap: explicitly unexport AP and refactor add/remove AP For future use of ObjectManager, we must explicitly unexport the AP and no longer depend on having it unexported during deconstruction (because object manager keeps the instance alive). Also refactor adding/removal of APs and move the export/unexport calls to the place where we emit the signal. --- src/devices/wifi/nm-device-wifi.c | 76 ++++++++++++++++++------------- src/nm-exported-object.c | 2 +- 2 files changed, 45 insertions(+), 33 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 68c6745295..f076983d33 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -162,10 +162,10 @@ static void supplicant_iface_notify_current_bss (NMSupplicantInterface *iface, static void request_wireless_scan (NMDeviceWifi *self, GVariant *scan_options); -static void emit_ap_added_removed (NMDeviceWifi *self, - guint signum, - NMAccessPoint *ap, - gboolean recheck_available_connections); +static void ap_add_remove (NMDeviceWifi *self, + guint signum, + NMAccessPoint *ap, + gboolean recheck_available_connections); static void remove_supplicant_interface_error_handler (NMDeviceWifi *self); @@ -358,12 +358,8 @@ set_current_ap (NMDeviceWifi *self, NMAccessPoint *new_ap, gboolean recheck_avai if (old_ap) { NM80211Mode mode = nm_ap_get_mode (old_ap); - if (force_remove_old_ap || mode == NM_802_11_MODE_ADHOC || mode == NM_802_11_MODE_AP || nm_ap_get_fake (old_ap)) { - emit_ap_added_removed (self, ACCESS_POINT_REMOVED, old_ap, FALSE); - g_hash_table_remove (priv->aps, nm_exported_object_get_path (NM_EXPORTED_OBJECT (old_ap))); - if (recheck_available_connections) - nm_device_recheck_available_connections (NM_DEVICE (self)); - } + if (force_remove_old_ap || mode == NM_802_11_MODE_ADHOC || mode == NM_802_11_MODE_AP || nm_ap_get_fake (old_ap)) + ap_add_remove (self, ACCESS_POINT_REMOVED, old_ap, recheck_available_connections); g_object_unref (old_ap); } @@ -442,13 +438,30 @@ bring_up (NMDevice *device, gboolean *no_firmware) } static void -emit_ap_added_removed (NMDeviceWifi *self, - guint signum, - NMAccessPoint *ap, - gboolean recheck_available_connections) +ap_add_remove (NMDeviceWifi *self, + guint signum, + NMAccessPoint *ap, + gboolean recheck_available_connections) { + NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); + + nm_assert (NM_IN_SET (signum, ACCESS_POINT_ADDED, ACCESS_POINT_REMOVED)); + + if (signum == ACCESS_POINT_ADDED) { + g_hash_table_insert (priv->aps, + (gpointer) nm_exported_object_export ((NMExportedObject *) ap), + g_object_ref (ap)); + } + g_signal_emit (self, signals[signum], 0, ap); g_object_notify (G_OBJECT (self), NM_DEVICE_WIFI_ACCESS_POINTS); + + if (signum == ACCESS_POINT_REMOVED) { + g_hash_table_steal (priv->aps, nm_exported_object_get_path ((NMExportedObject *) ap)); + nm_exported_object_unexport ((NMExportedObject *) ap); + g_object_unref (ap); + } + nm_device_emit_recheck_auto_activate (NM_DEVICE (self)); if (recheck_available_connections) nm_device_recheck_available_connections (NM_DEVICE (self)); @@ -461,16 +474,19 @@ remove_all_aps (NMDeviceWifi *self) GHashTableIter iter; NMAccessPoint *ap; - if (g_hash_table_size (priv->aps)) { - set_current_ap (self, NULL, FALSE, FALSE); + if (!g_hash_table_size (priv->aps)) + return; - g_hash_table_iter_init (&iter, priv->aps); - while (g_hash_table_iter_next (&iter, NULL, (gpointer) &ap)) { - emit_ap_added_removed (self, ACCESS_POINT_REMOVED, ap, FALSE); - g_hash_table_iter_remove (&iter); - } - nm_device_recheck_available_connections (NM_DEVICE (self)); + set_current_ap (self, NULL, FALSE, FALSE); + +again: + g_hash_table_iter_init (&iter, priv->aps); + if (g_hash_table_iter_next (&iter, NULL, (gpointer) &ap)) { + ap_add_remove (self, ACCESS_POINT_REMOVED, ap, FALSE); + goto again; } + + nm_device_recheck_available_connections (NM_DEVICE (self)); } static void @@ -1520,7 +1536,7 @@ supplicant_iface_new_bss_cb (NMSupplicantInterface *iface, NMAccessPoint *ap; NMAccessPoint *found_ap = NULL; const GByteArray *ssid; - const char *bssid, *ap_path; + const char *bssid; g_return_if_fail (self != NULL); g_return_if_fail (properties != NULL); @@ -1564,9 +1580,7 @@ supplicant_iface_new_bss_cb (NMSupplicantInterface *iface, nm_ap_update_from_properties (found_ap, object_path, properties); } else { nm_ap_dump (ap, "added ", nm_device_get_iface (NM_DEVICE (self))); - ap_path = nm_exported_object_export (NM_EXPORTED_OBJECT (ap)); - g_hash_table_insert (priv->aps, (gpointer) ap_path, g_object_ref (ap)); - emit_ap_added_removed (self, ACCESS_POINT_ADDED, ap, TRUE); + ap_add_remove (self, ACCESS_POINT_ADDED, ap, TRUE); } g_object_unref (ap); @@ -1629,8 +1643,7 @@ supplicant_iface_bss_removed_cb (NMSupplicantInterface *iface, nm_ap_set_fake (ap, TRUE); } else { nm_ap_dump (ap, "removed ", nm_device_get_iface (NM_DEVICE (self))); - emit_ap_added_removed (self, ACCESS_POINT_REMOVED, ap, TRUE); - g_hash_table_remove (priv->aps, nm_exported_object_get_path (NM_EXPORTED_OBJECT (ap))); + ap_add_remove (self, ACCESS_POINT_REMOVED, ap, TRUE); schedule_ap_list_dump (self); } } @@ -2343,13 +2356,12 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *reason) if (nm_ap_is_hotspot (ap)) nm_ap_set_address (ap, nm_device_get_hw_address (device)); - ap_path = nm_exported_object_export (NM_EXPORTED_OBJECT (ap)); - g_hash_table_insert (priv->aps, (gpointer) ap_path, ap); g_object_freeze_notify (G_OBJECT (self)); - emit_ap_added_removed (self, ACCESS_POINT_ADDED, ap, TRUE); + ap_add_remove (self, ACCESS_POINT_ADDED, ap, TRUE); g_object_thaw_notify (G_OBJECT (self)); set_current_ap (self, ap, FALSE, FALSE); - nm_active_connection_set_specific_object (NM_ACTIVE_CONNECTION (req), ap_path); + nm_active_connection_set_specific_object (NM_ACTIVE_CONNECTION (req), + nm_exported_object_get_path (NM_EXPORTED_OBJECT (ap))); return NM_ACT_STAGE_RETURN_SUCCESS; done: diff --git a/src/nm-exported-object.c b/src/nm-exported-object.c index 07caa3a20b..845eaa4190 100644 --- a/src/nm-exported-object.c +++ b/src/nm-exported-object.c @@ -496,7 +496,7 @@ nm_exported_object_export (NMExportedObject *self) priv = NM_EXPORTED_OBJECT_GET_PRIVATE (self); g_return_val_if_fail (!priv->path, priv->path); - g_return_val_if_fail (!priv->bus_mgr, NULL); + g_return_val_if_fail (!priv->bus_mgr, priv->path); #ifdef _ASSERT_NO_EARLY_EXPORT nm_assert (priv->_constructed); From d51827801128564c31dc5f9cc31db18f1bf7ae79 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Nov 2015 12:38:34 +0100 Subject: [PATCH 5/8] wifi/ap: use direct-hashing for aps hash The @aps hash has the D-Bus path of the exported object as key. It already rightly saved to additionally copy the string and relied on the path being stable. When doing that, we can just go one step further and use direct-hashing instead of string-hashing. Note that NMExportedObject already promises that the path will not change as long as the object is exported. See code comments in the export/unexport functions. --- src/devices/wifi/nm-device-wifi.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index f076983d33..c3e5657599 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -457,7 +457,7 @@ ap_add_remove (NMDeviceWifi *self, g_object_notify (G_OBJECT (self), NM_DEVICE_WIFI_ACCESS_POINTS); if (signum == ACCESS_POINT_REMOVED) { - g_hash_table_steal (priv->aps, nm_exported_object_get_path ((NMExportedObject *) ap)); + g_hash_table_remove (priv->aps, nm_exported_object_get_path ((NMExportedObject *) ap)); nm_exported_object_unexport ((NMExportedObject *) ap); g_object_unref (ap); } @@ -2897,7 +2897,7 @@ nm_device_wifi_init (NMDeviceWifi *self) NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); priv->mode = NM_802_11_MODE_INFRA; - priv->aps = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, g_object_unref); + priv->aps = g_hash_table_new (g_direct_hash, g_direct_equal); } static void @@ -2924,7 +2924,12 @@ dispose (GObject *object) static void finalize (GObject *object) { - g_clear_pointer (&NM_DEVICE_WIFI_GET_PRIVATE (object)->aps, g_hash_table_unref); + NMDeviceWifi *self = NM_DEVICE_WIFI (object); + NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); + + nm_assert (g_hash_table_size (priv->aps) == 0); + + g_hash_table_unref (priv->aps); G_OBJECT_CLASS (nm_device_wifi_parent_class)->finalize (object); } From f9ee20a7b2bd364384af2487da357344fd61a6b9 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 23 Jul 2015 10:03:21 -0500 Subject: [PATCH 6/8] core: explicitly unexport objects when we're done with them Previously most objects were implicitly unexported when they were destroyed, but since refcounts may make the object live longer than intended, we should explicitly unexport them when they should no longer be present on the bus. This means we can assume that objects will always be un-exported already when they are destroyed, *except* when quitting where most objects will live until exit because NM leaves interfaces up and running on quit. --- src/devices/adsl/nm-device-adsl.c | 8 ++--- src/devices/nm-device-ethernet.c | 8 ++--- src/devices/nm-device.c | 17 +++++------ src/devices/wwan/nm-modem.c | 8 ++--- src/main.c | 3 ++ src/nm-exported-object.c | 43 +++++++++++++++++++++++++-- src/nm-exported-object.h | 5 ++++ src/nm-manager.c | 5 ++-- src/settings/nm-settings-connection.c | 7 ----- src/settings/nm-settings.c | 2 ++ src/vpn-manager/nm-vpn-connection.c | 16 +++++----- 11 files changed, 75 insertions(+), 47 deletions(-) diff --git a/src/devices/adsl/nm-device-adsl.c b/src/devices/adsl/nm-device-adsl.c index 019cccfa59..08a7511abe 100644 --- a/src/devices/adsl/nm-device-adsl.c +++ b/src/devices/adsl/nm-device-adsl.c @@ -441,8 +441,7 @@ act_stage3_ip4_config_start (NMDevice *device, _LOGW (LOGD_ADSL, "PPP failed to start: %s", err->message); g_error_free (err); - g_object_unref (priv->ppp_manager); - priv->ppp_manager = NULL; + nm_exported_object_clear_and_unexport (&priv->ppp_manager); *reason = NM_DEVICE_STATE_REASON_PPP_START_FAILED; } @@ -456,10 +455,7 @@ deactivate (NMDevice *device) NMDeviceAdsl *self = NM_DEVICE_ADSL (device); NMDeviceAdslPrivate *priv = NM_DEVICE_ADSL_GET_PRIVATE (self); - if (priv->ppp_manager) { - g_object_unref (priv->ppp_manager); - priv->ppp_manager = NULL; - } + nm_exported_object_clear_and_unexport (&priv->ppp_manager); g_signal_handlers_disconnect_by_func (nm_platform_get (), G_CALLBACK (link_changed_cb), device); diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index d1202dc72f..560057ad38 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -1010,8 +1010,7 @@ pppoe_stage3_ip4_config_start (NMDeviceEthernet *self, NMDeviceStateReason *reas _LOGW (LOGD_DEVICE, "PPPoE failed to start: %s", err->message); g_error_free (err); - g_object_unref (priv->ppp_manager); - priv->ppp_manager = NULL; + nm_exported_object_clear_and_unexport (&priv->ppp_manager); *reason = NM_DEVICE_STATE_REASON_PPP_START_FAILED; } @@ -1399,10 +1398,7 @@ deactivate (NMDevice *device) priv->pending_ip4_config = NULL; } - if (priv->ppp_manager) { - g_object_unref (priv->ppp_manager); - priv->ppp_manager = NULL; - } + nm_exported_object_clear_and_unexport (&priv->ppp_manager); supplicant_interface_release (self); diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 89b7854dbc..9e7243705d 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -3581,7 +3581,7 @@ dhcp4_cleanup (NMDevice *self, CleanupType cleanup_type, gboolean release) } if (priv->dhcp4_config) { - g_clear_object (&priv->dhcp4_config); + nm_exported_object_clear_and_unexport (&priv->dhcp4_config); g_object_notify (G_OBJECT (self), NM_DEVICE_DHCP4_CONFIG); } } @@ -3938,8 +3938,7 @@ dhcp4_start (NMDevice *self, s_ip4 = nm_connection_get_setting_ip4_config (connection); /* Clear old exported DHCP options */ - if (priv->dhcp4_config) - g_object_unref (priv->dhcp4_config); + nm_exported_object_clear_and_unexport (&priv->dhcp4_config); priv->dhcp4_config = nm_dhcp4_config_new (); hw_addr = nm_platform_link_get_address (NM_PLATFORM_GET, nm_device_get_ip_ifindex (self), &hw_addr_len); @@ -4275,7 +4274,7 @@ dhcp6_cleanup (NMDevice *self, CleanupType cleanup_type, gboolean release) nm_device_remove_pending_action (self, PENDING_ACTION_DHCP6, FALSE); if (priv->dhcp6_config) { - g_clear_object (&priv->dhcp6_config); + nm_exported_object_clear_and_unexport (&priv->dhcp6_config); g_object_notify (G_OBJECT (self), NM_DEVICE_DHCP6_CONFIG); } } @@ -4700,7 +4699,7 @@ dhcp6_start (NMDevice *self, gboolean wait_for_ll, NMDeviceStateReason *reason) NMConnection *connection; NMSettingIPConfig *s_ip6; - g_clear_object (&priv->dhcp6_config); + nm_exported_object_clear_and_unexport (&priv->dhcp6_config); priv->dhcp6_config = nm_dhcp6_config_new (); g_warn_if_fail (priv->dhcp6_ip6_config == NULL); @@ -6864,8 +6863,8 @@ nm_device_set_ip4_config (NMDevice *self, g_object_notify (G_OBJECT (self), NM_DEVICE_IP4_CONFIG); g_signal_emit (self, signals[IP4_CONFIG_CHANGED], 0, priv->ip4_config, old_config); - if (old_config != priv->ip4_config && old_config) - g_object_unref (old_config); + if (old_config != priv->ip4_config) + nm_exported_object_clear_and_unexport (&old_config); if (nm_device_uses_generated_assumed_connection (self)) { NMConnection *connection = nm_device_get_applied_connection (self); @@ -7031,8 +7030,8 @@ nm_device_set_ip6_config (NMDevice *self, g_object_notify (G_OBJECT (self), NM_DEVICE_IP6_CONFIG); g_signal_emit (self, signals[IP6_CONFIG_CHANGED], 0, priv->ip6_config, old_config); - if (old_config != priv->ip6_config && old_config) - g_object_unref (old_config); + if (old_config != priv->ip6_config) + nm_exported_object_clear_and_unexport (&old_config); if (nm_device_uses_generated_assumed_connection (self)) { NMConnection *connection = nm_device_get_applied_connection (self); diff --git a/src/devices/wwan/nm-modem.c b/src/devices/wwan/nm-modem.c index 7e70ba5608..5889aa7805 100644 --- a/src/devices/wwan/nm-modem.c +++ b/src/devices/wwan/nm-modem.c @@ -530,8 +530,7 @@ ppp_stage3_ip_config_start (NMModem *self, error && error->message ? error->message : "(unknown)"); g_error_free (error); - g_object_unref (priv->ppp_manager); - priv->ppp_manager = NULL; + nm_exported_object_clear_and_unexport (&priv->ppp_manager); *reason = NM_DEVICE_STATE_REASON_PPP_START_FAILED; ret = NM_ACT_STAGE_RETURN_FAILURE; @@ -885,10 +884,7 @@ deactivate_cleanup (NMModem *self, NMDevice *device) priv->in_bytes = priv->out_bytes = 0; - if (priv->ppp_manager) { - g_object_unref (priv->ppp_manager); - priv->ppp_manager = NULL; - } + nm_exported_object_clear_and_unexport (&priv->ppp_manager); if (device) { g_return_if_fail (NM_IS_DEVICE (device)); diff --git a/src/main.c b/src/main.c index 7e36a3821c..a0c5c048b7 100644 --- a/src/main.c +++ b/src/main.c @@ -50,6 +50,7 @@ #include "nm-settings.h" #include "nm-auth-manager.h" #include "nm-core-internal.h" +#include "nm-exported-object.h" #if !defined(NM_DIST_VERSION) # define NM_DIST_VERSION VERSION @@ -454,6 +455,8 @@ main (int argc, char *argv[]) if (configure_and_quit == FALSE) g_main_loop_run (main_loop); + nm_exported_object_class_set_quitting (); + nm_manager_stop (nm_manager_get ()); done: diff --git a/src/nm-exported-object.c b/src/nm-exported-object.c index 845eaa4190..3a22b5c24b 100644 --- a/src/nm-exported-object.c +++ b/src/nm-exported-object.c @@ -28,6 +28,8 @@ #include "nm-default.h" static GHashTable *prefix_counters; +static gboolean quitting = FALSE; + #if NM_MORE_ASSERTS >= 2 #define _ASSERT_NO_EARLY_EXPORT @@ -632,6 +634,28 @@ nm_exported_object_get_interface_by_type (NMExportedObject *self, GType interfac return NULL; } +void +_nm_exported_object_clear_and_unexport (NMExportedObject **location) +{ + NMExportedObject *self; + NMExportedObjectPrivate *priv; + + if (!location || !*location) + return; + + self = *location; + *location = NULL; + + g_return_if_fail (NM_IS_EXPORTED_OBJECT (self)); + + priv = NM_EXPORTED_OBJECT_GET_PRIVATE (self); + + if (priv->path) + nm_exported_object_unexport (self); + + g_object_unref (self); +} + static void nm_exported_object_init (NMExportedObject *self) { @@ -765,8 +789,16 @@ nm_exported_object_dispose (GObject *object) { NMExportedObjectPrivate *priv = NM_EXPORTED_OBJECT_GET_PRIVATE (object); - if (priv->path) - nm_exported_object_unexport (NM_EXPORTED_OBJECT (object)); + /* Objects should have already been unexported by their owner, unless + * we are quitting, where many objects stick around until exit. + */ + if (!quitting) { + if (priv->path) { + g_warn_if_reached (); + nm_exported_object_unexport (NM_EXPORTED_OBJECT (object)); + } + } else + g_clear_pointer (&priv->path, g_free); g_variant_builder_clear (&priv->pending_notifies); nm_clear_g_source (&priv->notify_idle_id); @@ -785,3 +817,10 @@ nm_exported_object_class_init (NMExportedObjectClass *klass) object_class->notify = nm_exported_object_notify; object_class->dispose = nm_exported_object_dispose; } + +void +nm_exported_object_class_set_quitting (void) +{ + quitting = TRUE; +} + diff --git a/src/nm-exported-object.h b/src/nm-exported-object.h index 0b952b837a..cf2cacd1cb 100644 --- a/src/nm-exported-object.h +++ b/src/nm-exported-object.h @@ -45,6 +45,8 @@ typedef struct { GType nm_exported_object_get_type (void); +void nm_exported_object_class_set_quitting (void); + void nm_exported_object_class_add_interface (NMExportedObjectClass *object_class, GType dbus_skeleton_type, ...) G_GNUC_NULL_TERMINATED; @@ -56,6 +58,9 @@ void nm_exported_object_unexport (NMExportedObject *self); GSList * nm_exported_object_get_interfaces (NMExportedObject *self); GDBusInterfaceSkeleton *nm_exported_object_get_interface_by_type (NMExportedObject *self, GType interface_type); +void _nm_exported_object_clear_and_unexport (NMExportedObject **location); +#define nm_exported_object_clear_and_unexport(location) _nm_exported_object_clear_and_unexport ((NMExportedObject **) (location)) + G_END_DECLS #endif /* NM_EXPORTED_OBJECT_H */ diff --git a/src/nm-manager.c b/src/nm-manager.c index a3d4117008..5cba730fec 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -225,7 +225,7 @@ active_connection_remove (NMManager *self, NMActiveConnection *active) else connection = NULL; - g_object_unref (active); + nm_exported_object_clear_and_unexport (&active); if ( connection && nm_settings_has_connection (priv->settings, connection)) { @@ -801,8 +801,7 @@ remove_device (NMManager *manager, g_object_notify (G_OBJECT (manager), NM_MANAGER_DEVICES); nm_device_removed (device); - nm_exported_object_unexport (NM_EXPORTED_OBJECT (device)); - g_object_unref (device); + nm_exported_object_clear_and_unexport (&device); check_if_startup_complete (manager); } diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 29162c7984..20f4578da3 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -2053,14 +2053,7 @@ nm_settings_connection_signal_remove (NMSettingsConnection *self) if (priv->removed) g_return_if_reached (); priv->removed = TRUE; - - /* Emit removed first */ g_signal_emit_by_name (self, NM_SETTINGS_CONNECTION_REMOVED); - - /* And unregister last to ensure the removed signal goes out before - * we take the connection off the bus. - */ - nm_exported_object_unexport (NM_EXPORTED_OBJECT (self)); } gboolean diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 951f417646..bdea933249 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -905,6 +905,8 @@ connection_removed (NMSettingsConnection *connection, gpointer user_data) /* Re-emit for listeners like NMPolicy */ g_signal_emit_by_name (self, NM_CP_SIGNAL_CONNECTION_REMOVED, connection); g_object_notify (G_OBJECT (self), NM_SETTINGS_CONNECTIONS); + if (nm_exported_object_is_exported (NM_EXPORTED_OBJECT (connection))) + nm_exported_object_unexport (NM_EXPORTED_OBJECT (connection)); check_startup_complete (self); diff --git a/src/vpn-manager/nm-vpn-connection.c b/src/vpn-manager/nm-vpn-connection.c index 0c0ac1c740..5f41050c11 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -1117,8 +1117,8 @@ _cleanup_failed_config (NMVpnConnection *self) { NMVpnConnectionPrivate *priv = NM_VPN_CONNECTION_GET_PRIVATE (self); - g_clear_object (&priv->ip4_config); - g_clear_object (&priv->ip6_config); + nm_exported_object_clear_and_unexport (&priv->ip4_config); + nm_exported_object_clear_and_unexport (&priv->ip6_config); _LOGW ("VPN connection: did not receive valid IP config information"); _set_vpn_state (self, STATE_FAILED, NM_VPN_CONNECTION_STATE_REASON_IP_CONFIG_INVALID, FALSE); @@ -1316,12 +1316,12 @@ nm_vpn_connection_config_get (NMVpnConnection *self, GVariant *dict) priv->has_ip4 = FALSE; if (g_variant_lookup (dict, NM_VPN_PLUGIN_CONFIG_HAS_IP4, "b", &b)) priv->has_ip4 = b; - g_clear_object (&priv->ip4_config); + nm_exported_object_clear_and_unexport (&priv->ip4_config); priv->has_ip6 = FALSE; if (g_variant_lookup (dict, NM_VPN_PLUGIN_CONFIG_HAS_IP6, "b", &b)) priv->has_ip6 = b; - g_clear_object (&priv->ip6_config); + nm_exported_object_clear_and_unexport (&priv->ip6_config); } guint32 @@ -1491,7 +1491,7 @@ nm_vpn_connection_ip4_config_get (NMVpnConnection *self, GVariant *dict) nm_connection_get_setting_ip4_config (_get_applied_connection (self)), route_metric); - g_clear_object (&priv->ip4_config); + nm_exported_object_clear_and_unexport (&priv->ip4_config); priv->ip4_config = config; nm_exported_object_export (NM_EXPORTED_OBJECT (config)); g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_IP4_CONFIG); @@ -1626,7 +1626,7 @@ next: nm_connection_get_setting_ip6_config (_get_applied_connection (self)), route_metric); - g_clear_object (&priv->ip6_config); + nm_exported_object_clear_and_unexport (&priv->ip6_config); priv->ip6_config = config; nm_exported_object_export (NM_EXPORTED_OBJECT (config)); g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_IP6_CONFIG); @@ -2444,8 +2444,8 @@ dispose (GObject *object) g_cancellable_cancel (priv->cancellable); g_clear_object (&priv->cancellable); } - g_clear_object (&priv->ip4_config); - g_clear_object (&priv->ip6_config); + nm_exported_object_clear_and_unexport (&priv->ip4_config); + nm_exported_object_clear_and_unexport (&priv->ip6_config); g_clear_object (&priv->proxy); g_clear_object (&priv->plugin_info); From 57128494e09534645002a1bd8436d014e91b25f4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Nov 2015 17:29:46 +0100 Subject: [PATCH 7/8] exported-object: split out the creation of interface skeletons Will be reused for ifcfg-rh plugin, which also has a skeleton, but will not implement NMExportedObject. --- src/nm-exported-object.c | 191 ++++++++++++++++++++++----------------- src/nm-exported-object.h | 19 ++++ 2 files changed, 129 insertions(+), 81 deletions(-) diff --git a/src/nm-exported-object.c b/src/nm-exported-object.c index 3a22b5c24b..0746fec0d4 100644 --- a/src/nm-exported-object.c +++ b/src/nm-exported-object.c @@ -55,12 +55,6 @@ typedef struct { #define NM_EXPORTED_OBJECT_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_EXPORTED_OBJECT, NMExportedObjectPrivate)) -typedef struct { - GType dbus_skeleton_type; - char *method_name; - GCallback impl; -} NMExportedObjectDBusMethodImpl; - typedef struct { GHashTable *properties; GSList *skeleton_types; @@ -71,8 +65,8 @@ GQuark nm_exported_object_class_info_quark (void); G_DEFINE_QUARK (NMExportedObjectClassInfo, nm_exported_object_class_info) /* "AddConnectionUnsaved" -> "handle-add-connection-unsaved" */ -static char * -skeletonify_method_name (const char *dbus_method_name) +char * +nm_exported_object_skeletonify_method_name (const char *dbus_method_name) { GString *out; const char *p; @@ -265,7 +259,7 @@ nm_exported_object_class_add_interface (NMExportedObjectClass *object_class, va_start (ap, dbus_skeleton_type); while ((method_name = va_arg (ap, const char *)) && (impl = va_arg (ap, GCallback))) { method.dbus_skeleton_type = dbus_skeleton_type; - method.method_name = skeletonify_method_name (method_name); + method.method_name = nm_exported_object_skeletonify_method_name (method_name); g_assert (g_signal_lookup (method.method_name, dbus_skeleton_type) != 0); method.impl = impl; @@ -371,81 +365,128 @@ typedef struct { gulong *method_signals; } SkeletonData; +GDBusInterfaceSkeleton * +nm_exported_object_skeleton_create (GType dbus_skeleton_type, + GObjectClass *object_class, + const NMExportedObjectDBusMethodImpl *methods, + guint methods_len, + GObject *target) +{ + GDBusInterfaceSkeleton *interface; + gs_free GParamSpec **properties = NULL; + SkeletonData *skeleton_data; + guint n_properties; + guint i, j; + + interface = G_DBUS_INTERFACE_SKELETON (g_object_new (dbus_skeleton_type, NULL)); + + skeleton_data = g_slice_new (SkeletonData); + + /* Bind properties */ + properties = g_object_class_list_properties (G_OBJECT_GET_CLASS (interface), &n_properties); + skeleton_data->prop_bindings = g_new (GBinding *, n_properties + 1); + for (i = 0, j = 0; i < n_properties; i++) { + GParamSpec *nm_property; + GBindingFlags flags; + GBinding *prop_binding; + + nm_property = g_object_class_find_property (object_class, properties[i]->name); + if (!nm_property) + continue; + + flags = G_BINDING_SYNC_CREATE; + if ( (nm_property->flags & G_PARAM_WRITABLE) + && !(nm_property->flags & G_PARAM_CONSTRUCT_ONLY)) + flags |= G_BINDING_BIDIRECTIONAL; + prop_binding = g_object_bind_property (target, properties[i]->name, + interface, properties[i]->name, + flags); + if (prop_binding) + skeleton_data->prop_bindings[j++] = prop_binding; + } + skeleton_data->prop_bindings[j++] = NULL; + + /* Bind methods */ + skeleton_data->method_signals = g_new (gulong, methods_len + 1); + for (i = 0, j = 0; i < methods_len; i++) { + const NMExportedObjectDBusMethodImpl *method = &methods[i]; + GClosure *closure; + gulong method_signal; + + /* ignore methods that are for a different skeleton-type. */ + if ( method->dbus_skeleton_type + && method->dbus_skeleton_type != dbus_skeleton_type) + continue; + + closure = g_cclosure_new_swap (method->impl, target, NULL); + g_closure_set_meta_marshal (closure, NULL, nm_exported_object_meta_marshal); + method_signal = g_signal_connect_closure (interface, method->method_name, closure, FALSE); + + if (method_signal != 0) + skeleton_data->method_signals[j++] = method_signal; + } + skeleton_data->method_signals[j++] = 0; + + g_object_set_qdata ((GObject *) interface, _skeleton_data_quark (), skeleton_data); + + return interface; +} + static void nm_exported_object_create_skeletons (NMExportedObject *self, GType object_type) { - NMExportedObjectPrivate *priv = NM_EXPORTED_OBJECT_GET_PRIVATE (self); - GObjectClass *object_class = g_type_class_peek (object_type); + NMExportedObjectPrivate *priv; + GObjectClass *object_class; NMExportedObjectClassInfo *classinfo; GSList *iter; GDBusInterfaceSkeleton *interface; - guint n_properties; - int i; + const NMExportedObjectDBusMethodImpl *methods; + guint methods_len; classinfo = g_type_get_qdata (object_type, nm_exported_object_class_info_quark ()); if (!classinfo) return; - for (iter = classinfo->skeleton_types; iter; iter = iter->next) { - GType dbus_skeleton_type = GPOINTER_TO_SIZE (iter->data); - gs_free GParamSpec **properties = NULL; - SkeletonData *skeleton_data; - guint j; + object_class = g_type_class_peek (object_type); + priv = NM_EXPORTED_OBJECT_GET_PRIVATE (self); - interface = G_DBUS_INTERFACE_SKELETON (g_object_new (dbus_skeleton_type, NULL)); + methods = classinfo->methods->len ? &g_array_index (classinfo->methods, NMExportedObjectDBusMethodImpl, 0) : NULL; + methods_len = classinfo->methods->len; + + for (iter = classinfo->skeleton_types; iter; iter = iter->next) { + interface = nm_exported_object_skeleton_create (GPOINTER_TO_SIZE (iter->data), + object_class, + methods, + methods_len, + (GObject *) self); priv->interfaces = g_slist_prepend (priv->interfaces, interface); - - skeleton_data = g_slice_new (SkeletonData); - - /* Bind properties */ - properties = g_object_class_list_properties (G_OBJECT_GET_CLASS (interface), &n_properties); - skeleton_data->prop_bindings = g_new (GBinding *, n_properties + 1); - for (i = 0, j = 0; i < n_properties; i++) { - GParamSpec *nm_property; - GBindingFlags flags; - GBinding *prop_binding; - - nm_property = g_object_class_find_property (object_class, properties[i]->name); - if (!nm_property) - continue; - - flags = G_BINDING_SYNC_CREATE; - if ( (nm_property->flags & G_PARAM_WRITABLE) - && !(nm_property->flags & G_PARAM_CONSTRUCT_ONLY)) - flags |= G_BINDING_BIDIRECTIONAL; - prop_binding = g_object_bind_property (self, properties[i]->name, - interface, properties[i]->name, - flags); - if (prop_binding) - skeleton_data->prop_bindings[j++] = prop_binding; - } - skeleton_data->prop_bindings[j++] = NULL; - - /* Bind methods */ - skeleton_data->method_signals = g_new (gulong, classinfo->methods->len + 1); - for (i = 0, j = 0; i < classinfo->methods->len; i++) { - NMExportedObjectDBusMethodImpl *method = &g_array_index (classinfo->methods, NMExportedObjectDBusMethodImpl, i); - GClosure *closure; - gulong method_signal; - - if (method->dbus_skeleton_type != dbus_skeleton_type) - continue; - - closure = g_cclosure_new_swap (method->impl, self, NULL); - g_closure_set_meta_marshal (closure, NULL, nm_exported_object_meta_marshal); - method_signal = g_signal_connect_closure (interface, method->method_name, closure, FALSE); - - if (method_signal != 0) - skeleton_data->method_signals[j++] = method_signal; - } - skeleton_data->method_signals[j++] = 0; - - g_object_set_qdata ((GObject *) interface, _skeleton_data_quark (), skeleton_data); } } +void +nm_exported_object_skeleton_release (GDBusInterfaceSkeleton *interface) +{ + SkeletonData *skeleton_data; + guint j; + + g_return_if_fail (G_IS_DBUS_INTERFACE_SKELETON (interface)); + + skeleton_data = g_object_steal_qdata ((GObject *) interface, _skeleton_data_quark ()); + + for (j = 0; skeleton_data->prop_bindings[j]; j++) + g_object_unref (skeleton_data->prop_bindings[j]); + for (j = 0; skeleton_data->method_signals[j]; j++) + g_signal_handler_disconnect (interface, skeleton_data->method_signals[j]); + + g_free (skeleton_data->prop_bindings); + g_free (skeleton_data->method_signals); + g_slice_free (SkeletonData, skeleton_data); + + g_object_unref (interface); +} + static void nm_exported_object_destroy_skeletons (NMExportedObject *self) { @@ -454,22 +495,10 @@ nm_exported_object_destroy_skeletons (NMExportedObject *self) g_return_if_fail (priv->interfaces); while (priv->interfaces) { - gs_unref_object GDBusInterfaceSkeleton *interface = G_DBUS_INTERFACE_SKELETON (priv->interfaces->data); - SkeletonData *skeleton_data; - guint j; + GDBusInterfaceSkeleton *interface = priv->interfaces->data; - priv->interfaces = g_slist_remove (priv->interfaces, interface); - - skeleton_data = g_object_steal_qdata ((GObject *) interface, _skeleton_data_quark ()); - - for (j = 0; skeleton_data->prop_bindings[j]; j++) - g_object_unref (skeleton_data->prop_bindings[j]); - for (j = 0; skeleton_data->method_signals[j]; j++) - g_signal_handler_disconnect (interface, skeleton_data->method_signals[j]); - - g_free (skeleton_data->prop_bindings); - g_free (skeleton_data->method_signals); - g_slice_free (SkeletonData, skeleton_data); + priv->interfaces = g_slist_delete_link (priv->interfaces, priv->interfaces); + nm_exported_object_skeleton_release (interface); } } diff --git a/src/nm-exported-object.h b/src/nm-exported-object.h index cf2cacd1cb..3f69a7bda8 100644 --- a/src/nm-exported-object.h +++ b/src/nm-exported-object.h @@ -25,6 +25,25 @@ G_BEGIN_DECLS +/*****************************************************************************/ + +char *nm_exported_object_skeletonify_method_name (const char *dbus_method_name); + +typedef struct { + GType dbus_skeleton_type; + char *method_name; + GCallback impl; +} NMExportedObjectDBusMethodImpl; + +GDBusInterfaceSkeleton *nm_exported_object_skeleton_create (GType dbus_skeleton_type, + GObjectClass *object_class, + const NMExportedObjectDBusMethodImpl *methods, + guint methods_len, + GObject *target); +void nm_exported_object_skeleton_release (GDBusInterfaceSkeleton *interface); + +/*****************************************************************************/ + #define NM_TYPE_EXPORTED_OBJECT (nm_exported_object_get_type ()) #define NM_EXPORTED_OBJECT(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_EXPORTED_OBJECT, NMExportedObject)) #define NM_EXPORTED_OBJECT_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_EXPORTED_OBJECT, NMExportedObjectClass)) From cf146e9a0dbfc80ec44134d8556fd786b437ad95 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Nov 2015 16:44:11 +0100 Subject: [PATCH 8/8] ifcfg-rh: use distinct D-Bus connection for ifcfg-rh service Prevsiouly, the ifcfg-rh service and the regular NetworkManager were both exported on the same D-Bus connection. That had the effect, that on both services ("com.redhat.ifcfgrh1" and "org.freedesktop.NetworkManager") all objects were visible. This is also problematic later when we use GDBusObjectManager for the org.freedesktop.NetworkManager service. Export the ifcfg service on a separate bus connection. One downside is, that we don't bother exporting the service on the private socket and thus the service is not available without D-Bus daemon. Also, if the bus disconnects, we don't retry or recover. Instead the D-Bus service is dead until restart. --- src/settings/plugins/ifcfg-rh/plugin.c | 278 ++++++++++++++++++++----- src/settings/plugins/ifcfg-rh/plugin.h | 4 +- 2 files changed, 230 insertions(+), 52 deletions(-) diff --git a/src/settings/plugins/ifcfg-rh/plugin.c b/src/settings/plugins/ifcfg-rh/plugin.c index e7ce97a28d..15b3a33446 100644 --- a/src/settings/plugins/ifcfg-rh/plugin.c +++ b/src/settings/plugins/ifcfg-rh/plugin.c @@ -50,6 +50,7 @@ #include "writer.h" #include "utils.h" #include "nm-dbus-compat.h" +#include "nm-exported-object.h" #include "nmdbus-ifcfg-rh.h" @@ -78,14 +79,23 @@ static NMIfcfgConnection *update_connection (SettingsPluginIfcfg *plugin, static void settings_plugin_interface_init (NMSettingsPluginInterface *plugin_iface); -G_DEFINE_TYPE_EXTENDED (SettingsPluginIfcfg, settings_plugin_ifcfg, NM_TYPE_EXPORTED_OBJECT, 0, - G_IMPLEMENT_INTERFACE (NM_TYPE_SETTINGS_PLUGIN, - settings_plugin_interface_init)) +G_DEFINE_TYPE_EXTENDED (SettingsPluginIfcfg, settings_plugin_ifcfg, G_TYPE_OBJECT, 0, + G_IMPLEMENT_INTERFACE (NM_TYPE_SETTINGS_PLUGIN, + settings_plugin_interface_init)) #define SETTINGS_PLUGIN_IFCFG_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), SETTINGS_TYPE_PLUGIN_IFCFG, SettingsPluginIfcfgPrivate)) typedef struct { + NMConfig *config; + + struct { + GDBusConnection *connection; + GDBusInterfaceSkeleton *interface; + GCancellable *cancellable; + guint signal_id; + } dbus; + GHashTable *connections; /* uuid::connection */ gboolean initialized; @@ -725,7 +735,7 @@ impl_ifcfgrh_get_ifcfg_details (SettingsPluginIfcfg *plugin, "unable to get the UUID"); return; } - + path = nm_connection_get_path (NM_CONNECTION (connection)); if (!path) { g_dbus_method_invocation_return_error (context, @@ -739,6 +749,202 @@ impl_ifcfgrh_get_ifcfg_details (SettingsPluginIfcfg *plugin, g_variant_new ("(so)", uuid, path)); } +/*****************************************************************************/ + +static void +_dbus_clear (SettingsPluginIfcfg *self) +{ + SettingsPluginIfcfgPrivate *priv = SETTINGS_PLUGIN_IFCFG_GET_PRIVATE (self); + + nm_clear_g_signal_handler (priv->dbus.connection, &priv->dbus.signal_id); + + nm_clear_g_cancellable (&priv->dbus.cancellable); + + if (priv->dbus.interface) { + g_dbus_interface_skeleton_unexport (priv->dbus.interface); + nm_exported_object_skeleton_release (priv->dbus.interface); + priv->dbus.interface = NULL; + } + + g_clear_object (&priv->dbus.connection); +} + +static void +_dbus_connection_closed (GDBusConnection *connection, + gboolean remote_peer_vanished, + GError *error, + gpointer user_data) +{ + _LOGW ("dbus: %s bus closed", IFCFGRH1_DBUS_SERVICE_NAME); + _dbus_clear (SETTINGS_PLUGIN_IFCFG (user_data)); + + /* Retry or recover? */ +} + +static void +_dbus_request_name_done (GObject *source_object, + GAsyncResult *res, + gpointer user_data) +{ + GDBusConnection *connection = G_DBUS_CONNECTION (source_object); + SettingsPluginIfcfg *self; + SettingsPluginIfcfgPrivate *priv; + gs_free_error GError *error = NULL; + gs_unref_variant GVariant *ret = NULL; + guint32 result; + + ret = g_dbus_connection_call_finish (connection, res, &error); + if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = SETTINGS_PLUGIN_IFCFG (user_data); + priv = SETTINGS_PLUGIN_IFCFG_GET_PRIVATE (self); + + g_clear_object (&priv->dbus.cancellable); + + if (!ret) { + _LOGW ("dbus: couldn't acquire D-Bus service: %s", error->message); + _dbus_clear (self); + return; + } + + g_variant_get (ret, "(u)", &result); + + if (result != DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER) { + _LOGW ("dbus: couldn't acquire ifcfgrh1 D-Bus service (already taken)"); + _dbus_clear (self); + return; + } + + { + GType skeleton_type = NMDBUS_TYPE_IFCFGRH1_SKELETON; + gs_free char *method_name_get_ifcfg_details = NULL; + NMExportedObjectDBusMethodImpl methods[] = { + { + .method_name = (method_name_get_ifcfg_details = nm_exported_object_skeletonify_method_name ("GetIfcfgDetails")), + .impl = G_CALLBACK (impl_ifcfgrh_get_ifcfg_details), + }, + }; + + priv->dbus.interface = nm_exported_object_skeleton_create (skeleton_type, + g_type_class_peek (SETTINGS_TYPE_PLUGIN_IFCFG), + methods, + G_N_ELEMENTS (methods), + (GObject *) self); + + if (!g_dbus_interface_skeleton_export (priv->dbus.interface, + priv->dbus.connection, + IFCFGRH1_DBUS_OBJECT_PATH, + &error)) { + nm_exported_object_skeleton_release (priv->dbus.interface); + priv->dbus.interface = NULL; + _LOGW ("dbus: failed exporting interface: %s", error->message); + _dbus_clear (self); + return; + } + } + + _LOGD ("dbus: aquired D-Bus service %s and exported %s object", + IFCFGRH1_DBUS_SERVICE_NAME, + IFCFGRH1_DBUS_OBJECT_PATH); +} + +static void +_dbus_create_done (GObject *source_object, + GAsyncResult *res, + gpointer user_data) +{ + SettingsPluginIfcfg *self; + SettingsPluginIfcfgPrivate *priv; + gs_free_error GError *error = NULL; + GDBusConnection *connection; + + connection = g_dbus_connection_new_for_address_finish (res, &error); + if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = SETTINGS_PLUGIN_IFCFG (user_data); + priv = SETTINGS_PLUGIN_IFCFG_GET_PRIVATE (self); + + g_clear_object (&priv->dbus.cancellable); + + if (!connection) { + _LOGW ("dbus: couldn't initialize system bus: %s", error->message); + return; + } + + priv->dbus.connection = connection; + priv->dbus.cancellable = g_cancellable_new (); + + priv->dbus.signal_id = g_signal_connect (priv->dbus.connection, + "closed", + G_CALLBACK (_dbus_connection_closed), + self); + + g_dbus_connection_call (priv->dbus.connection, + DBUS_SERVICE_DBUS, + DBUS_PATH_DBUS, + DBUS_INTERFACE_DBUS, + "RequestName", + g_variant_new ("(su)", + IFCFGRH1_DBUS_SERVICE_NAME, + DBUS_NAME_FLAG_DO_NOT_QUEUE), + G_VARIANT_TYPE ("(u)"), + G_DBUS_CALL_FLAGS_NONE, + -1, + priv->dbus.cancellable, + _dbus_request_name_done, + self); +} + +static void +_dbus_setup (SettingsPluginIfcfg *self) +{ + SettingsPluginIfcfgPrivate *priv = SETTINGS_PLUGIN_IFCFG_GET_PRIVATE (self); + gs_free char *address = NULL; + gs_free_error GError *error = NULL; + + g_return_if_fail (!priv->dbus.connection); + + address = g_dbus_address_get_for_bus_sync (G_BUS_TYPE_SYSTEM, NULL, &error); + if (address == NULL) { + _LOGW ("dbus: failed getting address for system bus: %s", error->message); + return; + } + + priv->dbus.cancellable = g_cancellable_new (); + + g_dbus_connection_new_for_address (address, + G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT + | G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION, + NULL, + priv->dbus.cancellable, + _dbus_create_done, + self); +} + +static void +config_changed_cb (NMConfig *config, + NMConfigData *config_data, + NMConfigChangeFlags changes, + NMConfigData *old_data, + SettingsPluginIfcfg *self) +{ + /* If the dbus connection for some reason is borked the D-Bus service + * won't be offered. + * + * On SIGHUP and SIGUSR1 try to re-connect to D-Bus. So in the unlikely + * event that the D-Bus conneciton is broken, that allows for recovery + * without need for restarting NetworkManager. */ + if ( NM_FLAGS_HAS (changes, NM_CONFIG_CHANGE_SIGHUP) + || NM_FLAGS_HAS (changes, NM_CONFIG_CHANGE_SIGUSR1)) { + if (!SETTINGS_PLUGIN_IFCFG_GET_PRIVATE (self)->dbus.connection) + _dbus_setup (self); + } +} + +/*****************************************************************************/ + static void init (NMSettingsPlugin *config) { @@ -756,53 +962,33 @@ static void constructed (GObject *object) { SettingsPluginIfcfg *self = SETTINGS_PLUGIN_IFCFG (object); - GError *error = NULL; - GDBusConnection *bus; - GVariant *ret; - guint32 result; + SettingsPluginIfcfgPrivate *priv = SETTINGS_PLUGIN_IFCFG_GET_PRIVATE (self); G_OBJECT_CLASS (settings_plugin_ifcfg_parent_class)->constructed (object); - bus = g_bus_get_sync (G_BUS_TYPE_SYSTEM, NULL, &error); - if (!bus) { - _LOGW ("Couldn't connect to D-Bus: %s", error->message); - g_clear_error (&error); - return; - } + priv->config = nm_config_get (); + g_object_add_weak_pointer ((GObject *) priv->config, (gpointer *) &priv->config); + g_signal_connect (priv->config, + NM_CONFIG_SIGNAL_CONFIG_CHANGED, + G_CALLBACK (config_changed_cb), + self); - ret = g_dbus_connection_call_sync (bus, - DBUS_SERVICE_DBUS, - DBUS_PATH_DBUS, - DBUS_INTERFACE_DBUS, - "RequestName", - g_variant_new ("(su)", - IFCFGRH1_DBUS_SERVICE_NAME, - DBUS_NAME_FLAG_DO_NOT_QUEUE), - G_VARIANT_TYPE ("(u)"), - G_DBUS_CALL_FLAGS_NONE, - -1, - NULL, &error); - g_object_unref (bus); - if (ret) { - g_variant_get (ret, "(u)", &result); - g_variant_unref (ret); - - if (result == DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER) { - nm_exported_object_export (NM_EXPORTED_OBJECT (self)); - _LOGD ("Acquired D-Bus service %s", IFCFGRH1_DBUS_SERVICE_NAME); - } else - _LOGW ("Couldn't acquire ifcfgrh1 D-Bus service (already taken)"); - } else { - _LOGW ("Couldn't acquire D-Bus service: %s", error->message); - g_clear_error (&error); - } + _dbus_setup (self); } static void dispose (GObject *object) { - SettingsPluginIfcfg *plugin = SETTINGS_PLUGIN_IFCFG (object); - SettingsPluginIfcfgPrivate *priv = SETTINGS_PLUGIN_IFCFG_GET_PRIVATE (plugin); + SettingsPluginIfcfg *self = SETTINGS_PLUGIN_IFCFG (object); + SettingsPluginIfcfgPrivate *priv = SETTINGS_PLUGIN_IFCFG_GET_PRIVATE (self); + + if (priv->config) { + g_object_remove_weak_pointer ((GObject *) priv->config, (gpointer *) &priv->config); + g_signal_handlers_disconnect_by_func (priv->config, config_changed_cb, self); + priv->config = NULL; + } + + _dbus_clear (self); if (priv->connections) { g_hash_table_destroy (priv->connections); @@ -855,12 +1041,9 @@ static void settings_plugin_ifcfg_class_init (SettingsPluginIfcfgClass *req_class) { GObjectClass *object_class = G_OBJECT_CLASS (req_class); - NMExportedObjectClass *exported_object_class = NM_EXPORTED_OBJECT_CLASS (req_class); g_type_class_add_private (req_class, sizeof (SettingsPluginIfcfgPrivate)); - exported_object_class->export_path = IFCFGRH1_DBUS_OBJECT_PATH; - object_class->constructed = constructed; object_class->dispose = dispose; object_class->get_property = get_property; @@ -877,11 +1060,6 @@ settings_plugin_ifcfg_class_init (SettingsPluginIfcfgClass *req_class) g_object_class_override_property (object_class, NM_SETTINGS_PLUGIN_PROP_CAPABILITIES, NM_SETTINGS_PLUGIN_CAPABILITIES); - - nm_exported_object_class_add_interface (NM_EXPORTED_OBJECT_CLASS (req_class), - NMDBUS_TYPE_IFCFGRH1_SKELETON, - "GetIfcfgDetails", impl_ifcfgrh_get_ifcfg_details, - NULL); } static void diff --git a/src/settings/plugins/ifcfg-rh/plugin.h b/src/settings/plugins/ifcfg-rh/plugin.h index e2020ce6c7..eba734cff8 100644 --- a/src/settings/plugins/ifcfg-rh/plugin.h +++ b/src/settings/plugins/ifcfg-rh/plugin.h @@ -37,11 +37,11 @@ typedef struct _SettingsPluginIfcfg SettingsPluginIfcfg; typedef struct _SettingsPluginIfcfgClass SettingsPluginIfcfgClass; struct _SettingsPluginIfcfg { - NMExportedObject parent; + GObject parent; }; struct _SettingsPluginIfcfgClass { - NMExportedObjectClass parent; + GObjectClass parent; }; GType settings_plugin_ifcfg_get_type (void);