ppp_exit_code() does too much or too little. Either it should log
about all reasons why pppd exited, including signals, or it should
just do the status to string conversion. Split it.
It's useless (and in some cases also harmful) to commit the
configuration to update the default route metric when the device has
no default route. Also, don't commit configuration for externally
activated devices.
https://bugzilla.redhat.com/show_bug.cgi?id=1459604
Don't log in a function that basically just inspects state, without
mutating it. Instead, pass the reason why a connection could not be
generated to the caller so that we have one sensible log message.
The device's RECHECK_ASSUME signal has only NMManager as subscriber
and it immediately calls recheck_assume_connection().
With the previous commit, recheck_assume_connection() always logs
a debug message, so we don't need this duplicate message anymore.
Originally 850c977 "device: track system interface state in NMDevice",
intended that a connection can only be assumed initially when seeing
a device for the first time. Assuming a connection later was to be
prevented by setting device's sys-iface-state to MANAGED.
That changed too much in behavior, because we used to assume external
connections also when they are activated later on. So this was attempted
to get fixed by
- acf1067 nm-manager: try assuming connections on managed devices
- b6b7d90 manager: avoid generating in memory connections during startup for managed devices
It's probably just wrong to prevent assuming connections based on the
sys-iface-state. So drop the check for sys-iface-state from
recheck_assume_connection(). Now, we can assume anytime on managed,
disconnected interfaces, like previously.
Btw, note that priv->startup is totally wrong to check there, because
priv->startup has the sole purpose of tracking startup-complete property.
Startup, as far as NMManager is concerned, is platform_query_devices().
However, the problem is that we only assume connections (contrary to
doing external activation) when we have a connection-uuid from the state
file or with guess-assume during startup.
When assuming a master device, it can fail with
(nm-bond): ignoring generated connection (IPv6LL-only and not in master-slave relationship)
thus, for internal reason the device cannot be assumed yet.
Fix that by attatching the assume-state to the device, so that on multiple
recheck_assume_connection() calls we still try to assume. Whenever we try
to assume the connection and it fails due to external reasons (like, the connection
no longer matching), we clear the assume state, so that we only try as
long as there are internal reasons why assuming fails.
https://bugzilla.redhat.com/show_bug.cgi?id=1452062
The state file should only be read initially when NM starts, that is:
during NMManager's platform_query_devices().
At all later points, for example when a software device gets destroyed
and re-realized, the state file is clearly no longer relevant.
Hence, pass the set-nm-owned flag from NMManager to realize_start_setup().
This is very much the same as with the NM_UNMANAGED_FLAG_USER_EXPLICT flag,
which we also read from the state-file.
curl must bind to the interface that has IP configuration, not the
underlying device. Without this commit, connectivity check fails on
certain connection types (PPPoE, WWAN).
Fixes: 9d43869e47
Don't crash if the bond mode can't be read from sysfs - for example
when the interface disappears. The generated connection will be bogus,
but at that point it doesn't matter because the in-memory connection
will be destroyed.
Fixes: 056a973a4fhttps://bugzilla.redhat.com/show_bug.cgi?id=1459580
Let's explicitly unexports on dispose(). Probably that already
happened, because NMExportedObject asserts that it is unexported
during !quitting.
During quitting, we probably don't tear down the manager.
Anyway, we should always unexport.
After a daemon restart, any software device is considered !nm-owned,
even if it was created by NM. Therefore, a device stays around even if
the connection which created it gets deactivated or deleted.
Fix this by remembering the previous nm-owned state in the device
state file.
https://bugzilla.redhat.com/show_bug.cgi?id=1376199
The list field should have a unique name and not the same as the
list head. This avoids
c_list_link_before (&priv->network_servers, &network_server->network_servers);
c_list_for_each_entry (network_server, &priv->network_servers, network_servers) {
Instead of having a complex _find_network_server() function with several
arguments, split it.
Having multiple optional arguments to a find() function is fine,
if all arguments consistently narrow down the search.
For example nmp_cache_lookup_link_full(), where each argument is
optional, and it restricts the search.
For _find_network_server() that was not the case, because setting
"addr" and "device" together would be non-sensical.
The 3 bluetooth NAP hooks are called each only once. Inline them.
It is still very easy to understand where the bluetooth related
functions are invoked: grep for nm_bt_vtable_network_server.
In deactivate(), don't bother checking whether the current active
connection is a bluetooth type. Just always call unregister_bridge().
It's fast, and does nothing in case the bridge isn't registered.
I change it because I disagree with the previous naming.
For example bt_network_server_available() would not only call
is_available(). Instead, it checks whether the connection can
activate regarding availability of the bluetooth connection
(meaning, it returns TRUE if it's not a bluetooth connection or
if the bluez manager gives green light). In the bridge case,
it doesn't check any network-server availability.
There is already a function with a meaningful name for this behavior:
check_connection_available().
Same with bt_network_server_register(). It would indicate success,
if the applied connection is not a bluetooth connection. In cases,
where it didn't actually register anything. A function called
bt_network_server_register() should only return success if it actually
registered anything.
Branch f9b1bc16e9 added bluetooth NAP
support. A NAP connection is of connection.type "bluetooth", but it
also has a "bridge" setting. Also, it is primarily handled by NMDeviceBridge
and NMBridgeDeviceFactory (with help from NMBluezManager).
However, don't let nm_connection_get_connection_type() and
nm_connnection_is_type() lie about what the connection.type is.
The type is "bluetooth" for most purposes -- at least, as far as
the client is concerned (and the public API of libnm). This restores
previous API behavior, where nm_connection_get_connection_type()
and nm_connection_is_type() would be simple accessors to the
"connection.type" property.
Only a few places care about the bridge aspect, and those places need special
treatment. For example NMDeviceBridge needs to be fully aware that it can
handle bluetooth NAP connection. That is nothing new: if you handle a
connection of any type, you must know which fields matter and what they
mean. It's not enough that nm_connection_get_connection_type() for bluetooth
NAP connectins is claiming to be a bridge.
Counter examples, where the original behavior is right:
src/nm-manager.c- g_set_error (error,
src/nm-manager.c- NM_MANAGER_ERROR,
src/nm-manager.c- NM_MANAGER_ERROR_FAILED,
src/nm-manager.c- "NetworkManager plugin for '%s' unavailable",
src/nm-manager.c: nm_connection_get_connection_type (connection));
the correct message is: "no bluetooth plugin available", not "bridge".
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: if ( ( nm_connection_is_type (connection, NM_SETTING_WIRED_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: && !nm_connection_get_setting_pppoe (connection))
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_VLAN_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_WIRELESS_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_INFINIBAND_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_BOND_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_TEAM_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_BRIDGE_SETTING_NAME))
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c- return TRUE;
the correct behavior is for ifcfg-rh plugin to reject bluetooth NAP
connections, not proceed and store it.
The duplicate check is based only on the link-types and setting-types.
However, that doesn't really cut it because in case of bluetooth
NAP connections, NMBridgeDeviceFactory is responsible for the connection,
although connection.type is "bluetooth".
This is only here to catch loading invalid plugins. But at the point
where we load an invalid plugin, the process is hosed. This is at best
an assertion, but rather pointless really.
The simple link-types/setting-types mechanism doesn't really cut it
because for the bluetooth NAP connection.type is "bluetooth", but
it shall be primarily handled by the bridge factory.
It's internal API that allows for a basic matching of the factory.
It is however not sophisticated enough for the full complexity.
Make it as internal API only.
Using plain numbers make it cumbersome to grep for
setting types by priority.
The only downside is, that with the enum values it
is no longer obvious which value has higher or lower
priority.
Also, introduce NM_SETTING_PRIORITY_INVALID. This is what
_nm_setting_type_get_base_type_priority() returns. For the moment
it still has the same numerical value 0 as before. Later, that
shall be distinct from NM_SETTING_PRIORITY_CONNECTION.
Since commit 74dac5f (nm-manager: try assuming connections on managed devices),
and commit f4226e7 (manager: avoid generating in memory connections
during startup for managed devices), recheck_assume_connection() also
assumes connections on devices that are currently not in sys-iface-state
"external".
That is correct, as also for fully managed devices (which are currently
in disconnected state), we want to assume external connections. However,
when doing that, we must reset the sys-iface-state to external.
https://bugzilla.redhat.com/show_bug.cgi?id=1457242
In AP mode we should not look up an access point. It is wrong to
do, and it ends up marking the connection as hidden.
It seems wrong to me that if the client explicitly set
hidden=FALSE before AddAndActivate(), that complete_connection()
would still set it to TRUE if it cannot find the access
point. That is, because complete_connection() does not know
whether hidden was omitted or set intentionally by the user.
A better name is link_speed_update(), because it re-reads and
sets the speed value.
Also, move _notfiy() after logging. It doesn't matter in this
case, but we should first log, and then do actions that have potentially
complex side-effects.
Now, that NMDeviceClass:carrier_changed_notify() is no longer called as
deferred action, we can check for DCB state there, instead or registering
to the NM_DEVICE_CARRIER notifications.
Note that:
- carrier_changed_notify() has only one implementation: NMDeviceEthernet
to call get_link_speed() when carrier comes back.
- currently, calling carrier_changed_notify() with carrier=FALSE
has no effect, because NMDeviceEthernet only acts on carrier=TRUE.
- when carrier appears, nm_device_set_carrier() will call
carrier_changed() right away. We only call carrier_changed()
with carrier=TRUE only at one place. The change merley moves
carrier_changed_notify() out of the function. Apart from
that it has no effect.
- when carrier disappears, previoulsy we would delay action for
4 seconds. Hence, we would delay carrier_changed_notify() as well
-- although it has no effect.
The last point is at least ugly. Fix it by moving
carrier_changed_notify() to nm_device_set_carrier().
Coverity complains about not checking the return value:
src/settings/nm-settings-connection.c:2329: check_return: Calling "g_key_file_load_from_file" without checking return value (as is done elsewhere 6 out of 7 times).
While at it, refactor the code and check whether the timestamp
is valid.
NetworkManager-1.8.0/src/supplicant/nm-supplicant-interface.c:232: check_return: Calling "g_async_initable_init_finish" without checking return value (as is done elsewhere 7 out of 8 times).
