From 07281f21a71fa333288821ad863accfeedfff567 Mon Sep 17 00:00:00 2001
From: Dan Williams <dcbw@redhat.com>
Date: Mon, 9 Dec 2013 12:55:04 -0600
Subject: [PATCH] core: delay startup complete until carrier is found or
timeout (rh #1034921) (rh #1030583)
---
src/devices/nm-device.c | 177 ++++++++++++++++++++++++++++++++++-----------
src/nm-active-connection.c | 8 ++
2 files changed, 142 insertions(+), 43 deletions(-)
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 578ccb0..96a5d44 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -184,15 +184,15 @@ typedef struct {
gboolean initialized;
gboolean in_state_changed;
NMDeviceState state;
NMDeviceStateReason state_reason;
QueuedState queued_state;
guint queued_ip_config_id;
- guint pending_actions;
+ GArray *pending_actions;
char * udi;
char * path;
char * iface; /* may change, could be renamed by user */
int ifindex;
gboolean is_software;
char * ip_iface;
@@ -224,14 +224,15 @@ typedef struct {
gpointer act_source6_func;
/* Link stuff */
guint link_connected_id;
guint link_disconnected_id;
guint carrier_defer_id;
gboolean carrier;
+ guint carrier_wait_id;
gboolean ignore_carrier;
/* Generic DHCP stuff */
NMDHCPManager * dhcp_manager;
guint32 dhcp_timeout;
GByteArray * dhcp_anycast_address;
@@ -390,14 +391,15 @@ nm_device_init (NMDevice *self)
priv->capabilities = NM_DEVICE_CAP_NM_SUPPORTED;
priv->state = NM_DEVICE_STATE_UNMANAGED;
priv->state_reason = NM_DEVICE_STATE_REASON_NONE;
priv->dhcp_timeout = 0;
priv->rfkill_type = RFKILL_TYPE_UNKNOWN;
priv->autoconnect = DEFAULT_AUTOCONNECT;
priv->available_connections = g_hash_table_new_full (g_direct_hash, g_direct_equal, g_object_unref, NULL);
+ priv->pending_actions = g_array_sized_new (FALSE, TRUE, sizeof (GQuark), 5);
}
static void
update_accept_ra_save (NMDevice *self)
{
NMDevicePrivate *priv;
const char *ip_iface;
@@ -1154,14 +1156,20 @@ nm_device_set_carrier (NMDevice *device, gboolean carrier)
if (priv->carrier) {
nm_log_info (LOGD_DEVICE, "(%s): link connected", iface);
if (priv->carrier_defer_id) {
g_source_remove (priv->carrier_defer_id);
priv->carrier_defer_id = 0;
}
klass->carrier_changed (device, TRUE);
+
+ if (priv->carrier_wait_id) {
+ g_source_remove (priv->carrier_wait_id);
+ priv->carrier_wait_id = 0;
+ nm_device_remove_pending_action (device, "carrier wait");
+ }
} else if (state <= NM_DEVICE_STATE_DISCONNECTED) {
nm_log_info (LOGD_DEVICE, "(%s): link disconnected", iface);
klass->carrier_changed (device, FALSE);
} else {
nm_log_info (LOGD_DEVICE, "(%s): link disconnected (deferring action for %d seconds)",
iface, LINK_DISCONNECT_DELAY);
priv->carrier_defer_id = g_timeout_add_seconds (LINK_DISCONNECT_DELAY,
@@ -5110,17 +5118,28 @@ nm_device_start_ip_check (NMDevice *self)
/* If no ping was started, just advance to SECONDARIES */
if (!priv->gw_ping.pid)
nm_device_queue_state (self, NM_DEVICE_STATE_SECONDARIES, NM_DEVICE_STATE_REASON_NONE);
}
/****************************************************************/
+static gboolean
+carrier_wait_timeout (gpointer user_data)
+{
+ NMDevice *self = NM_DEVICE (user_data);
+
+ NM_DEVICE_GET_PRIVATE (self)->carrier_wait_id = 0;
+ nm_device_remove_pending_action (self, "carrier wait");
+ return G_SOURCE_REMOVE;
+}
+
gboolean
nm_device_bring_up (NMDevice *self, gboolean block, gboolean *no_firmware)
{
+ NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
gboolean success;
guint32 tries = 0;
g_return_val_if_fail (NM_IS_DEVICE (self), FALSE);
if (nm_device_is_up (self))
goto out;
@@ -5138,14 +5157,28 @@ nm_device_bring_up (NMDevice *self, gboolean block, gboolean *no_firmware)
g_usleep (200);
if (!nm_device_is_up (self)) {
nm_log_warn (LOGD_HW, "(%s): device not up after timeout!", nm_device_get_iface (self));
return FALSE;
}
+ /* Devices that support carrier detect must be IFF_UP to report carrier
+ * changes; so after setting the device IFF_UP we must suppress startup
+ * complete (via a pending action) until either the carrier turns on, or
+ * a timeout is reached.
+ */
+ if (device_has_capability (self, NM_DEVICE_CAP_CARRIER_DETECT)) {
+ if (priv->carrier_wait_id) {
+ g_source_remove (priv->carrier_wait_id);
+ nm_device_remove_pending_action (self, "carrier wait");
+ }
+ priv->carrier_wait_id = g_timeout_add_seconds (5, carrier_wait_timeout, self);
+ nm_device_add_pending_action (self, "carrier wait");
+ }
+
out:
/* Can only get HW address of some devices when they are up */
nm_device_update_hw_address (self);
_update_ip4_address (self);
return TRUE;
}
@@ -5301,14 +5334,19 @@ dispose (GObject *object)
}
if (priv->cp_updated_id) {
g_signal_handler_disconnect (priv->con_provider, priv->cp_updated_id);
priv->cp_updated_id = 0;
}
+ if (priv->carrier_wait_id) {
+ g_source_remove (priv->carrier_wait_id);
+ priv->carrier_wait_id = 0;
+ }
+
g_hash_table_unref (priv->available_connections);
priv->available_connections = NULL;
activation_source_clear (self, TRUE, AF_INET);
activation_source_clear (self, TRUE, AF_INET6);
clear_act_request (self);
@@ -5331,14 +5369,17 @@ finalize (GObject *object)
if (priv->dhcp_manager)
g_object_unref (priv->dhcp_manager);
if (priv->fw_manager)
g_object_unref (priv->fw_manager);
+ g_array_free (priv->pending_actions, TRUE);
+ priv->pending_actions = NULL;
+
g_free (priv->udi);
g_free (priv->path);
g_free (priv->iface);
g_free (priv->ip_iface);
g_free (priv->driver);
g_free (priv->driver_version);
g_free (priv->firmware_version);
@@ -5931,46 +5972,54 @@ nm_device_set_firmware_missing (NMDevice *self, gboolean new_missing)
gboolean
nm_device_get_firmware_missing (NMDevice *self)
{
return NM_DEVICE_GET_PRIVATE (self)->firmware_missing;
}
+#define QUEUED_PREFIX "queued state change to "
+
static const char *
-state_to_string (NMDeviceState state)
+queued_state_to_string (NMDeviceState state)
{
switch (state) {
case NM_DEVICE_STATE_UNMANAGED:
- return "unmanaged";
+ return QUEUED_PREFIX "unmanaged";
case NM_DEVICE_STATE_UNAVAILABLE:
- return "unavailable";
+ return QUEUED_PREFIX "unavailable";
case NM_DEVICE_STATE_DISCONNECTED:
- return "disconnected";
+ return QUEUED_PREFIX "disconnected";
case NM_DEVICE_STATE_PREPARE:
- return "prepare";
+ return QUEUED_PREFIX "prepare";
case NM_DEVICE_STATE_CONFIG:
- return "config";
+ return QUEUED_PREFIX "config";
case NM_DEVICE_STATE_NEED_AUTH:
- return "need-auth";
+ return QUEUED_PREFIX "need-auth";
case NM_DEVICE_STATE_IP_CONFIG:
- return "ip-config";
+ return QUEUED_PREFIX "ip-config";
case NM_DEVICE_STATE_IP_CHECK:
- return "ip-check";
+ return QUEUED_PREFIX "ip-check";
case NM_DEVICE_STATE_SECONDARIES:
- return "secondaries";
+ return QUEUED_PREFIX "secondaries";
case NM_DEVICE_STATE_ACTIVATED:
- return "activated";
+ return QUEUED_PREFIX "activated";
case NM_DEVICE_STATE_DEACTIVATING:
- return "deactivating";
+ return QUEUED_PREFIX "deactivating";
case NM_DEVICE_STATE_FAILED:
- return "failed";
+ return QUEUED_PREFIX "failed";
default:
break;
}
- return "unknown";
+ return QUEUED_PREFIX "unknown";
+}
+
+static const char *
+state_to_string (NMDeviceState state)
+{
+ return queued_state_to_string (state) + strlen (QUEUED_PREFIX);
}
static const char *
reason_to_string (NMDeviceStateReason reason)
{
switch (reason) {
case NM_DEVICE_STATE_REASON_NONE:
@@ -6083,21 +6132,14 @@ reason_to_string (NMDeviceStateReason reason)
return "secondary-connection-failed";
default:
break;
}
return "unknown";
}
-static inline gboolean
-state_implies_pending_action (NMDeviceState state)
-{
- return ( state >= NM_DEVICE_STATE_PREPARE
- && state < NM_DEVICE_STATE_ACTIVATED);
-}
-
void
nm_device_state_changed (NMDevice *device,
NMDeviceState state,
NMDeviceStateReason reason)
{
NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device);
NMDeviceState old_state;
@@ -6131,18 +6173,14 @@ nm_device_state_changed (NMDevice *device,
return;
}
old_state = priv->state;
priv->state = state;
priv->state_reason = reason;
- if ( state_implies_pending_action (state)
- && !state_implies_pending_action (old_state))
- nm_device_add_pending_action (device, "activation");
-
nm_log_info (LOGD_DEVICE, "(%s): device state change: %s -> %s (reason '%s') [%d %d %d]",
nm_device_get_iface (device),
state_to_string (old_state),
state_to_string (state),
reason_to_string (reason),
old_state,
state,
@@ -6300,18 +6338,14 @@ nm_device_state_changed (NMDevice *device,
if (old_state == NM_DEVICE_STATE_ACTIVATED)
nm_dispatcher_call (DISPATCHER_ACTION_DOWN, nm_act_request_get_connection (req), device, NULL, NULL);
/* Dispose of the cached activation request */
if (req)
g_object_unref (req);
- if ( state_implies_pending_action (old_state)
- && !state_implies_pending_action (state))
- nm_device_remove_pending_action (device, "activation");
-
priv->in_state_changed = FALSE;
}
static gboolean
queued_set_state (gpointer user_data)
{
NMDevice *self = NM_DEVICE (user_data);
@@ -6330,15 +6364,15 @@ queued_set_state (gpointer user_data)
*/
priv->queued_state.id = 0;
new_state = priv->queued_state.state;
new_reason = priv->queued_state.reason;
nm_device_queued_state_clear (self);
nm_device_state_changed (self, new_state, new_reason);
- nm_device_remove_pending_action (self, "queued state change");
+ nm_device_remove_pending_action (self, queued_state_to_string (new_state));
} else {
g_warn_if_fail (priv->queued_state.state == NM_DEVICE_STATE_UNKNOWN);
g_warn_if_fail (priv->queued_state.reason == NM_DEVICE_STATE_REASON_NONE);
}
return FALSE;
}
@@ -6349,29 +6383,37 @@ nm_device_queue_state (NMDevice *self,
{
NMDevicePrivate *priv;
g_return_if_fail (NM_IS_DEVICE (self));
priv = NM_DEVICE_GET_PRIVATE (self);
+ /* "lock" the pending actions so that if there was a previously
+ * queued action that's about to be cleared, that doesn't drop
+ * the pending actions to 0 before we add the new pending action.
+ */
+ nm_device_add_pending_action (self, "queued state lock");
+
/* We should only ever have one delayed state transition at a time */
if (priv->queued_state.id) {
if (priv->queued_state.state == state)
return;
nm_log_warn (LOGD_DEVICE, "(%s): overwriting previously queued state change to %s (%s)",
nm_device_get_iface (self),
state_to_string (priv->queued_state.state),
reason_to_string (priv->queued_state.reason));
nm_device_queued_state_clear (self);
}
priv->queued_state.state = state;
priv->queued_state.reason = reason;
priv->queued_state.id = g_idle_add (queued_set_state, self);
- nm_device_add_pending_action (self, "queued state change");
+ nm_device_add_pending_action (self, queued_state_to_string (state));
+
+ nm_device_remove_pending_action (self, "queued state lock");
nm_log_dbg (LOGD_DEVICE, "(%s): queued state change to %s due to %s (id %d)",
nm_device_get_iface (self), state_to_string (state), reason_to_string (reason),
priv->queued_state.id);
}
NMDeviceState
@@ -6391,15 +6433,15 @@ nm_device_queued_state_clear (NMDevice *self)
{
NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
if (priv->queued_state.id) {
nm_log_dbg (LOGD_DEVICE, "(%s): clearing queued state transition (id %d)",
nm_device_get_iface (self), priv->queued_state.id);
g_source_remove (priv->queued_state.id);
- nm_device_remove_pending_action (self, "queued state change");
+ nm_device_remove_pending_action (self, queued_state_to_string (priv->queued_state.state));
}
memset (&priv->queued_state, 0, sizeof (priv->queued_state));
}
NMDeviceState
nm_device_get_state (NMDevice *device)
{
@@ -7102,40 +7144,89 @@ nm_device_set_hw_addr (NMDevice *device, const guint8 *addr,
}
nm_device_bring_up (device, TRUE, NULL);
g_free (mac_str);
return success;
}
+/**
+ * nm_device_add_pending_action():
+ * @device: the #NMDevice to add the pending action to
+ * @action: a static string that identifies the action
+ *
+ * Adds a pending action to the device.
+ */
void
nm_device_add_pending_action (NMDevice *device, const char *action)
{
NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device);
+ GQuark qaction;
+ guint i;
- priv->pending_actions++;
- nm_log_dbg (LOGD_DEVICE, "(%s): add_pending_action (%d): %s",
- nm_device_get_iface (device), priv->pending_actions, action);
+ qaction = g_quark_from_static_string (action);
- if (priv->pending_actions == 1)
+ /* Shouldn't ever add the same pending action twice */
+ for (i = 0; i < priv->pending_actions->len; i++) {
+ if (qaction == g_array_index (priv->pending_actions, GQuark, i)) {
+ nm_log_warn (LOGD_DEVICE, "(%s): add_pending_action (%d): '%s' already added",
+ nm_device_get_iface (device),
+ priv->pending_actions->len,
+ action);
+ g_warn_if_reached ();
+ return;
+ }
+ }
+
+ g_array_prepend_val (priv->pending_actions, qaction);
+ nm_log_dbg (LOGD_DEVICE, "(%s): add_pending_action (%d): '%s'",
+ nm_device_get_iface (device),
+ priv->pending_actions->len,
+ action);
+
+ if (priv->pending_actions->len == 1)
g_object_notify (G_OBJECT (device), NM_DEVICE_HAS_PENDING_ACTION);
}
+/**
+ * nm_device_remove_pending_action():
+ * @device: the #NMDevice to remove the pending action from
+ * @action: a static string that identifies the action
+ *
+ * Removes a pending action previously added by nm_device_add_pending_action().
+ */
void
nm_device_remove_pending_action (NMDevice *device, const char *action)
{
NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device);
+ GQuark qaction;
+ guint i;
- priv->pending_actions--;
- nm_log_dbg (LOGD_DEVICE, "(%s): remove_pending_action (%d): %s",
- nm_device_get_iface (device), priv->pending_actions, action);
+ qaction = g_quark_from_static_string (action);
- if (priv->pending_actions == 0)
- g_object_notify (G_OBJECT (device), NM_DEVICE_HAS_PENDING_ACTION);
+ /* Shouldn't ever add the same pending action twice */
+ for (i = 0; i < priv->pending_actions->len; i++) {
+ if (qaction == g_array_index (priv->pending_actions, GQuark, i)) {
+ g_array_remove_index_fast (priv->pending_actions, i);
+ nm_log_dbg (LOGD_DEVICE, "(%s): remove_pending_action (%d): '%s'",
+ nm_device_get_iface (device),
+ priv->pending_actions->len,
+ action);
+
+ if (priv->pending_actions->len == 0)
+ g_object_notify (G_OBJECT (device), NM_DEVICE_HAS_PENDING_ACTION);
+ return;
+ }
+ }
+
+ nm_log_warn (LOGD_DEVICE, "(%s): remove_pending_action (%d): '%s' never added",
+ nm_device_get_iface (device),
+ priv->pending_actions->len,
+ action);
}
gboolean
nm_device_has_pending_action (NMDevice *device)
{
NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device);
- return priv->pending_actions > 0;
+ return priv->pending_actions->len > 0;
}
diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c
index 6a825bc..34f438c 100644
--- a/src/nm-active-connection.c
+++ b/src/nm-active-connection.c
@@ -135,14 +135,20 @@ nm_active_connection_set_state (NMActiveConnection *self,
if ( new_state == NM_ACTIVE_CONNECTION_STATE_ACTIVATED
|| old_state == NM_ACTIVE_CONNECTION_STATE_ACTIVATED) {
nm_settings_connection_update_timestamp (NM_SETTINGS_CONNECTION (priv->connection),
(guint64) time (NULL), TRUE);
}
+ if (priv->device) {
+ if ( old_state < NM_ACTIVE_CONNECTION_STATE_ACTIVATED
+ && new_state >= NM_ACTIVE_CONNECTION_STATE_ACTIVATED)
+ nm_device_remove_pending_action (priv->device, "activation");
+ }
+
if (priv->state == NM_ACTIVE_CONNECTION_STATE_DEACTIVATED) {
/* Device is no longer relevant when deactivated */
g_clear_object (&priv->device);
g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_DEVICES);
}
}
@@ -262,8 +268,10 @@ set_property (GObject *object, guint pro
case PROP_INT_DEVICE:
g_warn_if_fail (priv->device == NULL);
priv->device = g_value_dup_object (value);
- if (priv->device)
+ if (priv->device) {
g_warn_if_fail (priv->device != priv->master);
+ nm_device_add_pending_action (priv->device, "activation");
+ }
break;
case PROP_INT_USER_REQUESTED:
priv->user_requested = g_value_get_boolean (value);
--
1.8.3.1
From 813ea5995bb5a35c115c46f30eefe18db2886afb Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 16 Dec 2013 16:52:36 +0100
Subject: [PATCH 1/2] core: allow dynamic strings for pending action names
Use a GSList of the string values, instead of an array of GQuarks.
Using GQuarks does not allow to add arbitrary strings, because they
would leak the internalized strings. The next patch will begin
using unique, non-const action strings.
Given the rather small number of expected pending states, a singly
linked list seems appropriate.
Signed-off-by: Thomas Haller <thaller@redhat.com>
(some fixes and simplifications by dcbw based on patch reviews)
---
src/devices/nm-device.c | 49 +++++++++++++++++++++++++------------------------
1 file changed, 25 insertions(+), 24 deletions(-)
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index c1c7c69..e4644bf 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -184,15 +184,15 @@ typedef struct {
gboolean initialized;
gboolean in_state_changed;
NMDeviceState state;
NMDeviceStateReason state_reason;
QueuedState queued_state;
guint queued_ip_config_id;
- GArray *pending_actions;
+ GSList *pending_actions;
char * udi;
char * path;
char * iface; /* may change, could be renamed by user */
int ifindex;
gboolean is_software;
char * ip_iface;
@@ -391,15 +391,14 @@ nm_device_init (NMDevice *self)
priv->capabilities = NM_DEVICE_CAP_NM_SUPPORTED;
priv->state = NM_DEVICE_STATE_UNMANAGED;
priv->state_reason = NM_DEVICE_STATE_REASON_NONE;
priv->dhcp_timeout = 0;
priv->rfkill_type = RFKILL_TYPE_UNKNOWN;
priv->autoconnect = DEFAULT_AUTOCONNECT;
priv->available_connections = g_hash_table_new_full (g_direct_hash, g_direct_equal, g_object_unref, NULL);
- priv->pending_actions = g_array_sized_new (FALSE, TRUE, sizeof (GQuark), 5);
}
static void
update_accept_ra_save (NMDevice *self)
{
NMDevicePrivate *priv;
const char *ip_iface;
@@ -5368,16 +5367,15 @@ finalize (GObject *object)
if (priv->dhcp_manager)
g_object_unref (priv->dhcp_manager);
if (priv->fw_manager)
g_object_unref (priv->fw_manager);
- g_array_free (priv->pending_actions, TRUE);
- priv->pending_actions = NULL;
+ g_slist_free_full (priv->pending_actions, g_free);
g_free (priv->udi);
g_free (priv->path);
g_free (priv->iface);
g_free (priv->ip_iface);
g_free (priv->driver);
g_free (priv->driver_version);
@@ -7153,78 +7152,80 @@ nm_device_set_hw_addr (NMDevice *device, const guint8 *addr,
*
* Adds a pending action to the device.
*/
void
nm_device_add_pending_action (NMDevice *device, const char *action)
{
NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device);
- GQuark qaction;
- guint i;
+ GSList *iter;
+ guint count;
- qaction = g_quark_from_static_string (action);
+ g_return_if_fail (action);
/* Shouldn't ever add the same pending action twice */
- for (i = 0; i < priv->pending_actions->len; i++) {
- if (qaction == g_array_index (priv->pending_actions, GQuark, i)) {
+ for (iter = priv->pending_actions; iter; iter = iter->next) {
+ if (!strcmp (action, iter->data)) {
nm_log_warn (LOGD_DEVICE, "(%s): add_pending_action (%d): '%s' already added",
nm_device_get_iface (device),
- priv->pending_actions->len,
+ g_slist_length (priv->pending_actions),
action);
- g_warn_if_reached ();
- return;
+ g_return_val_if_reached (FALSE);
}
}
- g_array_prepend_val (priv->pending_actions, qaction);
+ priv->pending_actions = g_slist_append (priv->pending_actions, g_strdup (action));
+ count = g_slist_length (priv->pending_actions);
+
nm_log_dbg (LOGD_DEVICE, "(%s): add_pending_action (%d): '%s'",
nm_device_get_iface (device),
- priv->pending_actions->len,
+ count,
action);
- if (priv->pending_actions->len == 1)
+ if (count == 1)
g_object_notify (G_OBJECT (device), NM_DEVICE_HAS_PENDING_ACTION);
}
/**
* nm_device_remove_pending_action():
* @device: the #NMDevice to remove the pending action from
* @action: a static string that identifies the action
*
* Removes a pending action previously added by nm_device_add_pending_action().
*/
void
nm_device_remove_pending_action (NMDevice *device, const char *action)
{
NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device);
- GQuark qaction;
- guint i;
+ GSList *iter;
- qaction = g_quark_from_static_string (action);
+ g_return_if_fail (action);
/* Shouldn't ever add the same pending action twice */
- for (i = 0; i < priv->pending_actions->len; i++) {
- if (qaction == g_array_index (priv->pending_actions, GQuark, i)) {
- g_array_remove_index_fast (priv->pending_actions, i);
+ for (iter = priv->pending_actions; iter; iter = iter->next) {
+ if (!strcmp (action, iter->data)) {
+ g_free (iter->data);
+ priv->pending_actions = g_slist_delete_link (priv->pending_actions, iter);
nm_log_dbg (LOGD_DEVICE, "(%s): remove_pending_action (%d): '%s'",
nm_device_get_iface (device),
- priv->pending_actions->len,
+ g_slist_length (priv->pending_actions),
action);
- if (priv->pending_actions->len == 0)
+ if (priv->pending_actions == NULL)
g_object_notify (G_OBJECT (device), NM_DEVICE_HAS_PENDING_ACTION);
return;
}
}
nm_log_warn (LOGD_DEVICE, "(%s): remove_pending_action (%d): '%s' never added",
nm_device_get_iface (device),
- priv->pending_actions->len,
+ g_slist_length (priv->pending_actions),
action);
+ g_return_if_reached ();
}
gboolean
nm_device_has_pending_action (NMDevice *device)
{
NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device);
- return priv->pending_actions->len > 0;
+ return !!priv->pending_actions;
}
--
1.8.3.1
From b6ef165cfefba44a496cc336e1f91bb93b28f3ff Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 16 Dec 2013 15:02:38 +0100
Subject: [PATCH 2/2] core: fix NMActiveConnection to properly add/remove
pending action "activation"
When a new activation request is received, NetworkManager creates a new
NMActiveConnection to track that request. The device may already be activated,
in which case NetworkManager stops the old activation and starts the new one,
but both exist in parallel for a short period of time. If the old
NMActiveConnection is activating and already has a pending 'activation'
action, when the new NMActiveConnection adds its own 'activating' action,
they will clash. This is fixed by making each NMActiveConnection's activation
pending action uniquely named.
This fixes a g_warning with the following back trace:
#0 0x000000328224edfd in g_logv () from /lib64/libglib-2.0.so.0
#1 0x000000328224efe2 in g_log () from /lib64/libglib-2.0.so.0
#2 0x000000328224f2e6 in g_warn_message () from /lib64/libglib-2.0.so.0
#3 0x0000000000440aee in nm_device_add_pending_action (device=0x14002e0, action=0x50704a "activation") at devices/nm-device.c:7172
#4 0x000000000047525c in nm_active_connection_set_device (self=0x141b3c0, device=0x14002e0) at nm-active-connection.c:364
#5 0x0000000000475ec1 in set_property (object=0x141b3c0, prop_id=11, value=0x7fff7ff36c20, pspec=0x1405f70) at nm-active-connection.c:647
#6 0x0000003282615d3e in g_object_newv () from /lib64/libgobject-2.0.so.0
#7 0x00000032826162e6 in g_object_new_valist () from /lib64/libgobject-2.0.so.0
#8 0x0000003282616654 in g_object_new () from /lib64/libgobject-2.0.so.0
#9 0x0000000000474193 in nm_act_request_new (connection=0x13bb0e0, specific_object=0x0, subject=0x1447740, device=0x14002e0) at nm-activation-request.c:376
#10 0x000000000048e477 in _new_active_connection (self=0x13e8060, connection=0x13bb0e0, specific_object=0x0, device=0x14002e0, subject=0x1447740, error=0x7fff7ff36f90) at nm-manager.c:2947
#11 0x000000000048ed77 in impl_manager_activate_connection (self=0x13e8060, connection_path=0x134d590 "/org/freedesktop/NetworkManager/Settings/9", device_path=0x134d550 "/org/freedesktop/NetworkManager/Devices/1",
specific_object_path=0x0, context=0x143a9b0) at nm-manager.c:3206
#12 0x00000000004876c8 in dbus_glib_marshal_nm_manager_VOID__BOXED_BOXED_BOXED_POINTER (closure=0x7fff7ff37220, return_value=0x0, n_param_values=5, param_values=0x1448830, invocation_hint=0x0,
marshal_data=0x48e9dd <impl_manager_activate_connection>) at nm-manager-glue.h:189
#13 0x0000003284a0d6a9 in object_registration_message () from /lib64/libdbus-glib-1.so.2
#14 0x000000348ea1ce66 in _dbus_object_tree_dispatch_and_unlock () from /lib64/libdbus-1.so.3
#15 0x000000348ea0fa31 in dbus_connection_dispatch () from /lib64/libdbus-1.so.3
#16 0x0000003284a0acc5 in message_queue_dispatch () from /lib64/libdbus-glib-1.so.2
#17 0x0000003282247df6 in g_main_context_dispatch () from /lib64/libglib-2.0.so.0
#18 0x0000003282248148 in g_main_context_iterate.isra.22 () from /lib64/libglib-2.0.so.0
#19 0x000000328224854a in g_main_loop_run () from /lib64/libglib-2.0.so.0
#20 0x000000000042c6c0 in main (argc=1, argv=0x7fff7ff379b8) at main.c:629
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
src/nm-active-connection.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c
index cb42417..a8a422c 100644
--- a/src/nm-active-connection.c
+++ b/src/nm-active-connection.c
@@ -42,14 +42,16 @@ G_DEFINE_ABSTRACT_TYPE (NMActiveConnection, nm_active_connection, G_TYPE_OBJECT)
NMDevice *device;
gboolean is_default;
gboolean is_default6;
NMActiveConnectionState state;
gboolean vpn;
+ char *pending_activation_id;
+
gboolean user_requested;
gulong user_uid;
NMDevice *master;
} NMActiveConnectionPrivate;
enum {
PROP_0,
@@ -138,16 +140,20 @@ nm_active_connection_set_state (NMActiveConnection *self,
|| old_state == NM_ACTIVE_CONNECTION_STATE_ACTIVATED) {
nm_settings_connection_update_timestamp (NM_SETTINGS_CONNECTION (priv->connection),
(guint64) time (NULL), TRUE);
}
if (priv->device) {
if ( old_state < NM_ACTIVE_CONNECTION_STATE_ACTIVATED
- && new_state >= NM_ACTIVE_CONNECTION_STATE_ACTIVATED)
- nm_device_remove_pending_action (priv->device, "activation");
+ && new_state >= NM_ACTIVE_CONNECTION_STATE_ACTIVATED &&
+ priv->pending_activation_id)
+ {
+ nm_device_remove_pending_action (priv->device, priv->pending_activation_id);
+ g_clear_pointer (&priv->pending_activation_id, g_free);
+ }
}
if (priv->state == NM_ACTIVE_CONNECTION_STATE_DEACTIVATED) {
/* Device is no longer relevant when deactivated */
g_clear_object (&priv->device);
g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_DEVICES);
}
@@ -357,15 +363,16 @@ set_property (GObject *object, guint prop_id,
priv->connection = g_value_dup_object (value);
break;
case PROP_INT_DEVICE:
g_warn_if_fail (priv->device == NULL);
priv->device = g_value_dup_object (value);
if (priv->device) {
g_warn_if_fail (priv->device != priv->master);
- nm_device_add_pending_action (priv->device, "activation");
+ priv->pending_activation_id = g_strdup_printf ("activation::%p", (void *)object);
+ nm_device_add_pending_action (priv->device, priv->pending_activation_id);
}
break;
case PROP_INT_USER_REQUESTED:
priv->user_requested = g_value_get_boolean (value);
break;
case PROP_INT_USER_UID:
priv->user_uid = g_value_get_ulong (value);
@@ -732,14 +739,18 @@ static void
g_free (priv->path);
priv->path = NULL;
g_free (priv->specific_object);
priv->specific_object = NULL;
g_clear_object (&priv->connection);
+ if (priv->pending_activation_id) {
+ nm_device_remove_pending_action (priv->device, priv->pending_activation_id);
+ g_clear_pointer (&priv->pending_activation_id, g_free);
+ }
g_clear_object (&priv->device);
g_clear_object (&priv->master);
G_OBJECT_CLASS (nm_active_connection_parent_class)->dispose (object);
}
static void
--
1.8.3.1