diff options
author | Thomas Haller <thaller@redhat.com> | 2018-10-17 15:26:02 +0200 |
---|---|---|
committer | Thomas Haller <thaller@redhat.com> | 2018-10-17 15:26:02 +0200 |
commit | 9cf64e2ee23fc1bebe13196460a78a4ee8651ade (patch) | |
tree | 1df4e27fc4098648301f97f2d73204f3450f9ccd | |
parent | fd7115eeed527c92925b3b176add81d9a3605455 (diff) | |
parent | 0cb8bed23c22b01cce38e03d14ce29e5f73c28d2 (diff) | |
download | NetworkManager-9cf64e2ee23fc1bebe13196460a78a4ee8651ade.tar.gz |
core: merge branch 'th/device-availability'
https://bugzilla.redhat.com/show_bug.cgi?id=1639254
-rw-r--r-- | src/devices/nm-device.c | 35 | ||||
-rw-r--r-- | src/devices/nm-device.h | 29 | ||||
-rw-r--r-- | src/nm-manager.c | 54 |
3 files changed, 99 insertions, 19 deletions
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index ce21803bfa..1a7b90cb57 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -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)) { + 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"); + "device is strictly unmanaged"); return FALSE; } - } else { - if (!nm_device_get_managed (self, 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 unmanaged for internal request"); + "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; } } diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index 199ef51b0b..bf867f4852 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -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, diff --git a/src/nm-manager.c b/src/nm-manager.c index 9ea83ef8db..d2fad95519 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -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; } |