fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-05-03 21:48:21 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

device: mark uses of device's state-reason with nm_device_state_reason_check()

Browse source 
The state-change of a device has a reason argument, which is mostly for information
only.

There are many places in code that are the source of a state-reason.
Mostly these are calls to:
  - nm_device_state_changed()
  - nm_device_queue_state()
  - nm_device_queue_recheck_available()
  - nm_device_set_unmanaged_by_*()
  - nm_device_master_release_one_slave()
  - nm_device_ip_method_failed()
  - nm_modem_emit_prepare_result()
  - nm_modem_emit_ppp_failed()
  - nm_manager_deactivate_connection()
  - NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_*);

However, there are a few places in code that look at the reason
to decide how to proceed. I think this is a bad pattern, because
cause and effect are decoupled and it gets hard to understand where
a certain reason is set and what consequences that has.

Add a nop-function nm_device_state_reason_check() to mark all uses
of the device state reason that derive decisions from it. That is,
highlight the "effect" part.
This commit is contained in:
Thomas Haller 2017-02-23 15:19:03 +01:00
parent 71a22df337
commit 405ee7cad0
10 changed files with 46 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/devices/bluetooth/nm-device-bt.c
View file

@ -486,7 +486,7 @@ modem_prepare_result (NMModem *modem,
break;
}
} else {
if (reason == NM_DEVICE_STATE_REASON_SIM_PIN_INCORRECT) {
if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_SIM_PIN_INCORRECT) {
/* If the connect failed because the SIM PIN was wrong don't allow
* the device to be auto-activated anymore, which would risk locking
* the SIM if the incorrect PIN continues to be used.

2
src/devices/nm-device-macsec.c
View file

@ -110,7 +110,7 @@ parent_state_changed (NMDevice *parent,
NMDeviceMacsec *self = NM_DEVICE_MACSEC (user_data);
/* We'll react to our own carrier state notifications. Ignore the parent's. */
if (reason == NM_DEVICE_STATE_REASON_CARRIER)
if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_CARRIER)
return;
nm_device_set_unmanaged_by_flags (NM_DEVICE (self), NM_UNMANAGED_PARENT, !nm_device_get_managed (parent, FALSE), reason);

2
src/devices/nm-device-macvlan.c
View file

@ -131,7 +131,7 @@ parent_state_changed (NMDevice *parent,
NMDeviceMacvlan *self = NM_DEVICE_MACVLAN (user_data);
/* We'll react to our own carrier state notifications. Ignore the parent's. */
if (reason == NM_DEVICE_STATE_REASON_CARRIER)
if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_CARRIER)
return;
nm_device_set_unmanaged_by_flags (NM_DEVICE (self), NM_UNMANAGED_PARENT, !nm_device_get_managed (parent, FALSE), reason);

2
src/devices/nm-device-vlan.c
View file

@ -79,7 +79,7 @@ parent_state_changed (NMDevice *parent,
NMDeviceVlan *self = NM_DEVICE_VLAN (user_data);
/* We'll react to our own carrier state notifications. Ignore the parent's. */
if (reason == NM_DEVICE_STATE_REASON_CARRIER)
if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_CARRIER)
return;
nm_device_set_unmanaged_by_flags (NM_DEVICE (self), NM_UNMANAGED_PARENT, !nm_device_get_managed (parent, FALSE), reason);

25
src/devices/nm-device.c
View file

@ -2967,7 +2967,7 @@ slave_state_changed (NMDevice *slave,
}
/* Don't touch the device if its state changed externally. */
if (reason == NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED)
if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED)
configure = FALSE;
if (release) {
@ -3247,10 +3247,10 @@ nm_device_slave_notify_release (NMDevice *self, NMDeviceStateReason reason)
if ( priv->state > NM_DEVICE_STATE_DISCONNECTED
&& priv->state <= NM_DEVICE_STATE_ACTIVATED) {
if (reason == NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED) {
if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED) {
new_state = NM_DEVICE_STATE_FAILED;
master_status = "failed";
} else if (reason == NM_DEVICE_STATE_REASON_USER_REQUESTED) {
} else if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_USER_REQUESTED) {
new_state = NM_DEVICE_STATE_DEACTIVATING;
master_status = "deactivated by user request";
} else {
@ -4232,7 +4232,6 @@ activate_stage1_device_prepare (NMDevice *self)
{
NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
NMActStageReturn ret = NM_ACT_STAGE_RETURN_SUCCESS;
NMDeviceStateReason failure_reason = NM_DEVICE_STATE_REASON_NONE;
NMActiveConnection *active = NM_ACTIVE_CONNECTION (priv->act_request);
_set_ip_state (self, AF_INET, IP_NONE);
@ -4246,6 +4245,8 @@ activate_stage1_device_prepare (NMDevice *self)
/* Assumed connections were already set up outside NetworkManager */
if (!nm_active_connection_get_assumed (active)) {
NMDeviceStateReason failure_reason = NM_DEVICE_STATE_REASON_NONE;
ret = NM_DEVICE_GET_CLASS (self)->act_stage1_prepare (self, &failure_reason);
if (ret == NM_ACT_STAGE_RETURN_POSTPONE) {
return;
@ -11914,11 +11915,14 @@ _set_state_full (NMDevice *self,
case NM_DEVICE_STATE_UNMANAGED:
nm_device_set_firmware_missing (self, FALSE);
if (old_state > NM_DEVICE_STATE_UNMANAGED) {
if (reason == NM_DEVICE_STATE_REASON_REMOVED) {
switch (nm_device_state_reason_check (reason)) {
case NM_DEVICE_STATE_REASON_REMOVED:
nm_device_cleanup (self, reason, CLEANUP_TYPE_REMOVED);
} else if (reason == NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED) {
break;
case NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED:
nm_device_cleanup (self, reason, CLEANUP_TYPE_KEEP);
} else {
break;
default:
/* Clean up if the device is now unmanaged but was activated */
if (nm_device_get_act_request (self))
nm_device_cleanup (self, reason, CLEANUP_TYPE_DECONFIGURE);
@ -11926,17 +11930,18 @@ _set_state_full (NMDevice *self,
nm_device_hw_addr_reset (self, "unmanage");
set_nm_ipv6ll (self, FALSE);
restore_ip6_properties (self);
break;
}
}
break;
case NM_DEVICE_STATE_UNAVAILABLE:
if (old_state == NM_DEVICE_STATE_UNMANAGED) {
save_ip6_properties (self);
if (reason != NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED)
if (nm_device_state_reason_check (reason) != NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED)
ip6_managed_setup (self);
}
if (reason != NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED) {
if (nm_device_state_reason_check (reason) != NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED) {
if (old_state == NM_DEVICE_STATE_UNMANAGED || priv->firmware_missing) {
if (!nm_device_bring_up (self, TRUE, &no_firmware) && no_firmware)
_LOGW (LOGD_PLATFORM, "firmware may be missing.");
@ -11963,7 +11968,7 @@ _set_state_full (NMDevice *self,
nm_device_cleanup (self, reason, CLEANUP_TYPE_DECONFIGURE);
} else if (old_state < NM_DEVICE_STATE_DISCONNECTED) {
if (reason != NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED) {
if (nm_device_state_reason_check (reason) != NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED) {
/* Ensure IPv6 is set up as it may not have been done when
* entering the UNAVAILABLE state depending on the reason.
*/

15
src/devices/nm-device.h
View file

@ -30,6 +30,21 @@
#include "nm-rfkill-manager.h"
#include "NetworkManagerUtils.h"
static inline NMDeviceStateReason
nm_device_state_reason_check (NMDeviceStateReason reason)
{
/* the device-state-reason serves mostly informational purpse during a state
* change. In some cases however, decisions are made based on the reason.
* I tend to think that interpreting the state reason to derive some behaviors
* is confusing, because the cause and effect are so far apart.
*
* This function is here to mark source that inspects the reason to make
* a decision -- contrary to places that set the reason. Thus, by grepping
* for nm_device_state_reason_check() you can find the "effect" to a certain
* reason.
*/
return reason;
}
#define NM_PENDING_ACTION_AUTOACTIVATE "autoactivate"
#define NM_PENDING_ACTION_DHCP4 "dhcp4"

4
src/devices/wwan/nm-device-modem.c
View file

@ -127,7 +127,7 @@ modem_prepare_result (NMModem *modem,
if (success)
nm_device_activate_schedule_stage2_device_config (device);
else {
if (reason == NM_DEVICE_STATE_REASON_SIM_PIN_INCORRECT) {
if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_SIM_PIN_INCORRECT) {
/* If the connect failed because the SIM PIN was wrong don't allow
* the device to be auto-activated anymore, which would risk locking
* the SIM if the incorrect PIN continues to be used.
@ -377,7 +377,7 @@ device_state_changed (NMDevice *device,
nm_modem_device_state_changed (priv->modem, new_state, old_state);
switch (reason) {
switch (nm_device_state_reason_check (reason)) {
case NM_DEVICE_STATE_REASON_GSM_REGISTRATION_DENIED:
case NM_DEVICE_STATE_REASON_GSM_REGISTRATION_NOT_SEARCHING:
case NM_DEVICE_STATE_REASON_GSM_SIM_NOT_INSERTED:

8
src/devices/wwan/nm-modem-ofono.c
View file

@ -745,7 +745,6 @@ context_property_changed (GDBusProxy *proxy,
{
NMModemOfono *self = NM_MODEM_OFONO (user_data);
NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self);
NMDeviceStateReason reason = NM_DEVICE_STATE_REASON_NONE;
NMPlatformIP4Address addr;
gboolean ret = FALSE;
GVariant *v_dict;
@ -910,9 +909,10 @@ context_property_changed (GDBusProxy *proxy,
out:
if (nm_modem_get_state (NM_MODEM (self)) != NM_MODEM_STATE_CONNECTED) {
_LOGI ("emitting PREPARE_RESULT: %s", ret ? "TRUE" : "FALSE");
if (!ret)
reason = NM_DEVICE_STATE_REASON_IP_CONFIG_UNAVAILABLE;
nm_modem_emit_prepare_result (NM_MODEM (self), ret, reason);
nm_modem_emit_prepare_result (NM_MODEM (self), ret,
ret
? NM_DEVICE_STATE_REASON_NONE
: NM_DEVICE_STATE_REASON_IP_CONFIG_UNAVAILABLE);
} else {
_LOGW ("MODEM_PPP_FAILED");
nm_modem_emit_ppp_failed (NM_MODEM (self), NM_DEVICE_STATE_REASON_PPP_FAILED);

3
src/nm-manager.c
View file

@ -3924,8 +3924,9 @@ nm_manager_deactivate_connection (NMManager *manager,
if (NM_IS_VPN_CONNECTION (active)) {
NMVpnConnectionStateReason vpn_reason = NM_VPN_CONNECTION_STATE_REASON_USER_DISCONNECTED;
if (reason == NM_DEVICE_STATE_REASON_CONNECTION_REMOVED)
if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_CONNECTION_REMOVED)
vpn_reason = NM_VPN_CONNECTION_STATE_REASON_CONNECTION_REMOVED;
if (nm_vpn_connection_deactivate (NM_VPN_CONNECTION (active), vpn_reason, FALSE))
success = TRUE;
else

7
src/nm-policy.c
View file

@ -1465,7 +1465,7 @@ device_state_changed (NMDevice *device,
&& old_state <= NM_DEVICE_STATE_ACTIVATED) {
int tries = nm_settings_connection_get_autoconnect_retries (connection);
if (reason == NM_DEVICE_STATE_REASON_NO_SECRETS) {
if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_NO_SECRETS) {
_LOGD (LOGD_DEVICE, "connection '%s' now blocked from autoconnect due to no secrets",
nm_settings_connection_get_id (connection));
@ -1524,7 +1524,7 @@ device_state_changed (NMDevice *device,
update_routing_and_dns (self, FALSE);
break;
case NM_DEVICE_STATE_DEACTIVATING:
if (reason == NM_DEVICE_STATE_REASON_USER_REQUESTED) {
if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_USER_REQUESTED) {
if (!nm_device_get_autoconnect (device)) {
/* The device was disconnected; block all connections on it */
block_autoconnect_for_device (self, device);
@ -1544,7 +1544,8 @@ device_state_changed (NMDevice *device,
/* Reset retry counts for a device's connections when carrier on; if cable
* was unplugged and plugged in again, we should try to reconnect.
*/
if (reason == NM_DEVICE_STATE_REASON_CARRIER && old_state == NM_DEVICE_STATE_UNAVAILABLE)
if ( nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_CARRIER
&& old_state == NM_DEVICE_STATE_UNAVAILABLE)
reset_autoconnect_all (self, device);
if (old_state > NM_DEVICE_STATE_DISCONNECTED)