device: eliminate direct calls to check_connection_available() in favor of nm_device_check_connection_available()

It was confusing to understand the difference between calling nm_device_connection_is_available()
and check_connection_available(), they behaved similar, but not really
the same. Especially nm_device_connection_is_available() would look
first into @available_connetions, and might call check_connection_available()
itself. Whereas @available_connetions was also populated by testing
check_connection_available(). This interrelation makes it hard to
understand when nm_device_connection_is_available() returned true.

Rename nm_device_connection_is_available() to nm_device_check_connection_available()
and remove all direct calls of check_connection_available() in favor of
the wrapper nm_device_check_connection_available().

Now we only call nm_device_check_connection_available() with different
parameters (@flags and @specific_object). We also have the additional
guarantee that specifying more @flags will widen the result and making
a connection "more" available, while specifying a @specific_object will
restrict it.

This also changes behavior in several cases. For example before
nm_device_connection_is_available() for user-requests would always
declare matching connections available on Wi-Fi devices (only)
regardless of the device state. Now the device state gets consistently
considered.

For default-unmanaged devices it also changes behavior in complicated
ways, because before we would put connections into @available_connetions
for every device-state, but nm_device_connection_is_available() had a
special over-ride only for unmanaged-state.

This also fixes a bug, that user can activate an unavailable Wi-Fi
device:
  nmcli radio wifi off
  nmcli connection up wlan0
This commit is contained in:
Thomas Haller 2015-01-16 16:43:48 +01:00
parent e524be2c34
commit d80f1bf4f0
4 changed files with 37 additions and 67 deletions

View file

@ -1898,7 +1898,7 @@ can_auto_connect (NMDevice *self,
if (!nm_setting_connection_get_autoconnect (s_con))
return FALSE;
return nm_device_connection_is_available (self, connection, FALSE);
return nm_device_check_connection_available (self, connection, NM_DEVICE_CHECK_CON_AVAILABLE_NONE, NULL);
}
/**
@ -6896,64 +6896,42 @@ nm_device_set_dhcp_anycast_address (NMDevice *self, const char *addr)
}
/**
* nm_device_connection_is_available():
* nm_device_check_connection_available():
* @self: the #NMDevice
* @connection: the #NMConnection to check for availability
* @for_user_activation_request: set to %TRUE if we are checking whether
* the connection is available on an explicit user request. This
* also checks for device specific overrides.
* @flags: flags to affect the decision making of whether a connection
* is available. Adding a flag can only make a connection more available,
* not less.
* @specific_object: a device type dependent argument to further
* filter the result. Passing a non %NULL specific object can only reduce
* the availability of a connection.
*
* Check if @connection is available to be activated on @self. Normally this
* only checks if the connection is in @self's AvailableConnections property.
* If @for_user_activation_request is %TRUE then the device is asked to do specific
* checks that may bypass the AvailableConnections property.
* Check if @connection is available to be activated on @self.
*
* Returns: %TRUE if @connection can be activated on @self
*/
gboolean
nm_device_connection_is_available (NMDevice *self,
NMConnection *connection,
gboolean for_user_activation_request)
nm_device_check_connection_available (NMDevice *self,
NMConnection *connection,
NMDeviceCheckConAvailableFlags flags,
const char *specific_object)
{
NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
gboolean is_default_unmanaged;
NMDeviceState state;
if (g_hash_table_contains (priv->available_connections, connection))
return TRUE;
is_default_unmanaged = priv->state == NM_DEVICE_STATE_UNMANAGED && nm_device_get_default_unmanaged (self);
if (!for_user_activation_request && !is_default_unmanaged) {
/* Shortcut: there are only additional checks for either @for_user_activation_request
* or @is_default_unmanaged. Return FALSE right away. */
state = nm_device_get_state (self);
if (state < NM_DEVICE_STATE_UNMANAGED)
return FALSE;
}
if (!nm_device_check_connection_compatible (self, connection)) {
/* An incompatilbe connection is never available. */
if ( state < NM_DEVICE_STATE_UNAVAILABLE
&& nm_device_get_unmanaged_flag (self, NM_UNMANAGED_ALL & ~NM_UNMANAGED_DEFAULT))
return FALSE;
if ( state < NM_DEVICE_STATE_DISCONNECTED
&& !nm_device_is_available (self, NM_DEVICE_CHECK_DEV_AVAILABLE_NONE))
return FALSE;
}
if ( is_default_unmanaged
&& NM_DEVICE_GET_CLASS (self)->check_connection_available (self, connection, NM_DEVICE_CHECK_CON_AVAILABLE_NONE, NULL)) {
/* default-unmanaged devices in UNMANAGED state have no available connections
* so we must manually check whether the connection is available here. */
return TRUE;
}
if (!nm_device_check_connection_compatible (self, connection))
return FALSE;
if ( for_user_activation_request
&& NM_DEVICE_GET_CLASS (self)->check_connection_available_has_user_override
&& NM_DEVICE_GET_CLASS (self)->check_connection_available (self, connection, NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST, NULL)) {
/* Connections for an explicit user activation request might only be available after
* additional checking.
*
* For example in case of hidden Wi-Fi, the connection might not have the 'hidden' property
* set. Support this by allowing device specific overrides.
*/
return TRUE;
}
return FALSE;
return NM_DEVICE_GET_CLASS (self)->check_connection_available (self, connection, flags, specific_object);
}
static void
@ -6973,16 +6951,10 @@ _clear_available_connections (NMDevice *self, gboolean do_signal)
static gboolean
_try_add_available_connection (NMDevice *self, NMConnection *connection)
{
if ( nm_device_get_state (self) < NM_DEVICE_STATE_DISCONNECTED
&& !nm_device_get_default_unmanaged (self))
return FALSE;
if (nm_device_check_connection_compatible (self, connection)) {
if (NM_DEVICE_GET_CLASS (self)->check_connection_available (self, connection, NM_DEVICE_CHECK_CON_AVAILABLE_NONE, NULL)) {
g_hash_table_add (NM_DEVICE_GET_PRIVATE (self)->available_connections,
g_object_ref (connection));
return TRUE;
}
if (nm_device_check_connection_available (self, connection, NM_DEVICE_CHECK_CON_AVAILABLE_NONE, NULL)) {
g_hash_table_add (NM_DEVICE_GET_PRIVATE (self)->available_connections,
g_object_ref (connection));
return TRUE;
}
return FALSE;
}
@ -7057,8 +7029,8 @@ nm_device_get_available_connections (NMDevice *self, const char *specific_object
/* If a specific object is given, only include connections that are
* compatible with it.
*/
if ( !specific_object
|| NM_DEVICE_GET_CLASS (self)->check_connection_available (self, connection, NM_DEVICE_CHECK_CON_AVAILABLE_NONE, specific_object))
if ( !specific_object /* << Optimization: we know that the connection is available without @specific_object. */
|| nm_device_check_connection_available (self, connection, NM_DEVICE_CHECK_CON_AVAILABLE_NONE, specific_object))
g_ptr_array_add (array, connection);
}
}

View file

@ -173,8 +173,6 @@ typedef struct {
NMDeviceCheckConAvailableFlags flags,
const char *specific_object);
gboolean check_connection_available_has_user_override;
gboolean (* complete_connection) (NMDevice *self,
NMConnection *connection,
const char *specific_object,
@ -397,9 +395,10 @@ gboolean nm_device_has_pending_action (NMDevice *device);
GPtrArray *nm_device_get_available_connections (NMDevice *device,
const char *specific_object);
gboolean nm_device_connection_is_available (NMDevice *device,
NMConnection *connection,
gboolean for_user_activation_request);
gboolean nm_device_check_connection_available (NMDevice *device,
NMConnection *connection,
NMDeviceCheckConAvailableFlags flags,
const char *specific_object);
gboolean nm_device_notify_component_added (NMDevice *device, GObject *component);

View file

@ -3327,7 +3327,6 @@ nm_device_wifi_class_init (NMDeviceWifiClass *klass)
parent_class->is_available = is_available;
parent_class->check_connection_compatible = check_connection_compatible;
parent_class->check_connection_available = check_connection_available;
parent_class->check_connection_available_has_user_override = TRUE;
parent_class->complete_connection = complete_connection;
parent_class->set_enabled = set_enabled;

View file

@ -2538,7 +2538,7 @@ ensure_master_active_connection (NMManager *self,
if (!is_compatible_with_slave (candidate, connection))
continue;
if (nm_device_connection_is_available (master_device, candidate, TRUE)) {
if (nm_device_check_connection_available (master_device, candidate, NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST, NULL)) {
master_ac = nm_manager_activate_connection (self,
candidate,
NULL,
@ -2579,7 +2579,7 @@ ensure_master_active_connection (NMManager *self,
continue;
}
if (!nm_device_connection_is_available (candidate, master_connection, TRUE))
if (!nm_device_check_connection_available (candidate, master_connection, NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST, NULL))
continue;
found_device = TRUE;
@ -2729,7 +2729,7 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError *
}
/* Final connection must be available on device */
if (!nm_device_connection_is_available (device, connection, TRUE)) {
if (!nm_device_check_connection_available (device, connection, NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST, NULL)) {
g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_UNKNOWN_CONNECTION,
"Connection '%s' is not available on the device %s at this time.",
nm_connection_get_id (connection), nm_device_get_iface (device));