core: merge branch 'th/device-availability'

https://bugzilla.redhat.com/show_bug.cgi?id=1639254
This commit is contained in:
Thomas Haller 2018-10-17 15:26:02 +02:00
commit 9cf64e2ee2
3 changed files with 101 additions and 21 deletions

View file

@ -13615,6 +13615,19 @@ nm_device_update_metered (NMDevice *self)
}
}
static NMDeviceCheckDevAvailableFlags
_device_check_dev_available_flags_from_con (NMDeviceCheckConAvailableFlags con_flags)
{
NMDeviceCheckDevAvailableFlags dev_flags;
dev_flags = NM_DEVICE_CHECK_DEV_AVAILABLE_NONE;
if (NM_FLAGS_HAS (con_flags, _NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST_WAITING_CARRIER))
dev_flags |= _NM_DEVICE_CHECK_DEV_AVAILABLE_IGNORE_CARRIER;
return dev_flags;
}
static gboolean
_nm_device_check_connection_available (NMDevice *self,
NMConnection *connection,
@ -13656,34 +13669,30 @@ _nm_device_check_connection_available (NMDevice *self,
return FALSE;
}
if (state < NM_DEVICE_STATE_UNAVAILABLE) {
if (NM_FLAGS_ANY (flags, NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST)) {
if (!nm_device_get_managed (self, TRUE)) {
nm_utils_error_set_literal (error, NM_UTILS_ERROR_CONNECTION_AVAILABLE_UNMANAGED_DEVICE,
"device is unmanaged");
return FALSE;
}
} else {
if (!nm_device_get_managed (self, TRUE)) {
if (!nm_device_get_managed (self, FALSE)) {
nm_utils_error_set_literal (error, NM_UTILS_ERROR_CONNECTION_AVAILABLE_UNMANAGED_DEVICE,
"device is unmanaged for internal request");
"device is strictly unmanaged");
return FALSE;
}
if (!NM_FLAGS_HAS (flags, _NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST_OVERRULE_UNMANAGED)) {
nm_utils_error_set_literal (error, NM_UTILS_ERROR_CONNECTION_AVAILABLE_UNMANAGED_DEVICE,
"device is currently unmanaged");
return FALSE;
}
}
}
if ( state < NM_DEVICE_STATE_DISCONNECTED
&& !nm_device_is_software (self)) {
if (NM_FLAGS_ANY (flags, NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST)) {
if (!nm_device_is_available (self, NM_DEVICE_CHECK_DEV_AVAILABLE_FOR_USER_REQUEST)) {
if (!nm_device_is_available (self, _device_check_dev_available_flags_from_con (flags))) {
if (NM_FLAGS_HAS (flags, _NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST)) {
nm_utils_error_set_literal (error, NM_UTILS_ERROR_CONNECTION_AVAILABLE_TEMPORARY,
"device is not available");
return FALSE;
}
} else {
if (!nm_device_is_available (self, NM_DEVICE_CHECK_DEV_AVAILABLE_NONE)) {
} else {
nm_utils_error_set_literal (error, NM_UTILS_ERROR_CONNECTION_AVAILABLE_TEMPORARY,
"device is not available for internal request");
return FALSE;
}
return FALSE;
}
}

View file

@ -168,14 +168,34 @@ typedef enum NMActStageReturn NMActStageReturn;
typedef enum { /*< skip >*/
NM_DEVICE_CHECK_CON_AVAILABLE_NONE = 0,
/* since NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST is a collection of flags with more fine grained
* parts, this flag in general indicates that this is a user-request. */
_NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST = (1L << 0),
/* we also consider devices which have no carrier but are still waiting for the driver
* to detect carrier. Usually, such devices are not yet available, however for a user-request
* they are. They might fail later if carrier doesn't come. */
_NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST_WAITING_CARRIER = (1L << 1),
/* usually, a profile is only available if the Wi-Fi AP is in range. For an
* explicit user request, we also consider profiles for APs that are not (yet)
* visible. */
_NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST_IGNORE_AP = (1L << 2),
/* a device can be marked as unmanaged for various reasons. Some of these reasons
* are authorative, others not. Non-authoritative reasons can be overruled by
* `nmcli device set $DEVICE managed yes`. Also, for an explicit user activation
* request we may want to consider the device as managed. This flag makes devices
* that are unmanaged appear available. */
_NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST_OVERRULE_UNMANAGED = (1L << 3),
/* a collection of flags, that are commonly set for an explict user-request. */
NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST = _NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST
| _NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST_WAITING_CARRIER
| _NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST_IGNORE_AP,
| _NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST_IGNORE_AP
| _NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST_OVERRULE_UNMANAGED,
NM_DEVICE_CHECK_CON_AVAILABLE_ALL = (1L << 3) - 1,
NM_DEVICE_CHECK_CON_AVAILABLE_ALL = (1L << 4) - 1,
} NMDeviceCheckConAvailableFlags;
struct _NMDevicePrivate;
@ -191,7 +211,12 @@ struct _NMDevice {
typedef enum { /*< skip >*/
NM_DEVICE_CHECK_DEV_AVAILABLE_NONE = 0,
/* the device is considered available, even if it has no carrier.
*
* For various device types (software devices) we ignore carrier based
* on the type. So, for them, this flag has no effect anyway. */
_NM_DEVICE_CHECK_DEV_AVAILABLE_IGNORE_CARRIER = (1L << 0),
NM_DEVICE_CHECK_DEV_AVAILABLE_FOR_USER_REQUEST = _NM_DEVICE_CHECK_DEV_AVAILABLE_IGNORE_CARRIER,
NM_DEVICE_CHECK_DEV_AVAILABLE_ALL = (1L << 1) - 1,

View file

@ -3394,6 +3394,7 @@ nm_manager_get_best_device_for_connection (NMManager *self,
NMDeviceCheckConAvailableFlags flags;
gs_unref_ptrarray GPtrArray *all_ac_arr = NULL;
gs_free_error GError *local_best = NULL;
NMConnectionMultiConnect multi_connect;
nm_assert (!sett_conn || NM_IS_SETTINGS_CONNECTION (sett_conn));
nm_assert (!connection || NM_IS_CONNECTION (connection));
@ -3403,9 +3404,44 @@ nm_manager_get_best_device_for_connection (NMManager *self,
if (!connection)
connection = nm_settings_connection_get_connection (sett_conn);
flags = for_user_request ? NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST : NM_DEVICE_CHECK_CON_AVAILABLE_NONE;
multi_connect = _nm_connection_get_multi_connect (connection);
if ( _nm_connection_get_multi_connect (nm_settings_connection_get_connection (sett_conn)) == NM_CONNECTION_MULTI_CONNECT_SINGLE
if (!for_user_request)
flags = NM_DEVICE_CHECK_CON_AVAILABLE_NONE;
else {
/* if the profile is multi-connect=single, we also consider devices which
* are marked as unmanaged. And explicit user-request shows sufficent user
* intent to make the device managed.
* That is also, because we expect that such profile is suitably tied
* to the intended device. So when an unmanaged device matches, the user's
* intent is clear.
*
* For multi-connect != single devices that is different. The profile
* is not restricted to a particular device.
* For that reason, plain `nmcli connection up "$MULIT_PROFILE"` seems
* less suitable for multi-connect profiles, because the target device is
* left unspecified. Anyway, if a user issues
*
* $ nmcli device set "$DEVICE" managed no
* $ nmcli connection up "$MULIT_PROFILE"
*
* then it is reasonable for multi-connect profiles to not consider
* the device a suitable candidate.
*
* This may be seen inconsistent, but I think that it makes a lot of
* sense. Also note that "connection.multi-connect" work quite differently
* in aspects like activation. E.g. `nmcli connection up` of multi-connect
* "single" profile, will deactivate the profile if it is active already.
* That is different from multi-connect profiles, where it will aim to
* activate the profile one more time on an hitherto disconnected device.
*/
if (multi_connect == NM_CONNECTION_MULTI_CONNECT_SINGLE)
flags = NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST;
else
flags = NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST & ~_NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST_OVERRULE_UNMANAGED;
}
if ( multi_connect == NM_CONNECTION_MULTI_CONNECT_SINGLE
&& (ac = active_connection_find_by_connection (self, sett_conn, connection, NM_ACTIVE_CONNECTION_STATE_DEACTIVATING, &all_ac_arr))) {
/* if we have a profile which may activate on only one device (multi-connect single), then
* we prefer the device on which the profile is already active. It means to reactivate
@ -3492,13 +3528,23 @@ found_better:
/* determine the priority of this device. Currently this priority is independent
* of the profile (connection) and the device's details (aside the state).
*
* Maybe nm_device_check_connection_available() should instead return a priority,
* as it has more information available. For now, that is not needed nor implemented. */
* as it has more information available.
*
* For example, if you have multiple Wi-Fi devices, currently a user-request would
* also select the device if the AP is not visible. Optimally, if one of the two
* devices sees the AP and the other one doesn't, the former would be preferred.
* For that, the priority would need to be determined by nm_device_check_connection_available(). */
prio = _device_get_activation_prio (device);
if ( prio <= best.prio
&& best.device) {
/* we already have a matching device with a better priority. This candidate
* cannot be better. Skip the check. */
* cannot be better. Skip the check.
*
* Also note, that below we collect the best error message @local_best.
* Since we already have best.device, the error message does not matter
* either, and we can skip nm_device_check_connection_available() altogether. */
continue;
}