Currently the cancellable is not yet used. Drop it again.
However, I think the code might be useful, so I hope to revert
this commit afterwards to use it.
Also support reapply. During reapply we try to preserve
keys that are added externally.
However, the current implementation does not properly use transactions
to ensure there is no race here.
We don't need every log line repeat all the parameters
of the call. Each call should have a unique identifier
(which is NM_HASH_OBFUSCATE_PTR(call)) and only the first
message from a call contains all the details.
API should fail with a "cancelled" error if (and only if)
the user cancelled the operation. An error to communicate
with OVSDB or being disconnected should fail with a different
reason.
GArray is nice, because it is more memory efficient and encourages
local access. However, growing the array also means that all entries
get relocated, that means, we cannot pass on a pointer to a call
instance.
Next, we will add a way to cancel operations via a GCancellable.
That means, we subscribe to the cancelled signal and need to pass
on a user-data that indicates which call got cancelled. Since with
an GArray the calls can be relocated, we would anyway need an
additional allocation (just for the user data). So, we wouldn't
actually safe anything but make the code more complicated.
As often, CList works great in this use case. Use it.
It's in general nicer to have all definitions of structs and enums
a the top of the source file. As enums and structs are commonly used
by functions, it's often nice to have the enums and structs defined
first, before all functions.
External connections are devices that are configured outside of
NetworkManager. Such devices should be mostly ignored and not
be interfered with.
Note that we tend to create external connection profiles for
such devices. That happens for example if you use wg-quick to
manage a WireGuard interface outside of NetworkManager. But it
really happens for any interface.
This generated profile has no DNS configuration. Unless we use
the systemd-resolved backend, they thus don't contribute to the DNS
settings (which is fine).
However, with systemd-resolved, NetworkManager would also reset
the DNS configuration of those external interfaces. That is clearly
wrong. NetworkManager should only care about the interfaces that it
actively manages and leave others alone.
How to reproduce: use systemd-resolved and configure an interface outside
of NetworkManager. Note that `nmcli device` shows the state as
"connected (externally)". Note that `resolvectl` shows the DNS configuration
on that external interface. Do something in NetworkManager to trigger
a DNS update (e.g. SIGHUB or reactivate a profile). Note in `resolvectl`
that the external interface's DNS configuration was wiped.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/563#note_673283
Rework update_system_hostname() to use the new properties from the
hostname setting.
In the default configuration where all the 3 boolean properties
hostname.{from-dhcp,from-dns,only-from-default} are true, the behavior
is the same as before.
When a new active connection is created, it gets added at the
beginning of manager's list. This means that the list contains most
recently activated connections first. Since the list is doubly-linked,
it is possible to efficiently iterate in both directions, so the order
of the list is mostly a matter of convention.
I think it is preferable to have oldest active connections at the
beginning of the list; let's reverse the order.
In most places where the list is iterated, the order doesn't
matter. Where it does, use the *_prev() variant to maintain the old
iteration order.
The following merge request in ModemManager introduces a more or less
common timeout value for the connection attempts in all plugin and
protocol implementations:
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/merge_requests/391
The value chosen by default for the steps that may take long to
complete in a connection attempt is 180s, and 120s for the steps in
the disconnection path.
Until now, every different plugin or protocol had a different timeout
value, all of them <= 180s, and with that change in ModemManager, the
values are now aligned for all.
Note, though, that this does not mean that a connection attempt will
take always less than 180s, as there may be multiple other steps in
addition to the one that took the maximum timeout. The value chosen
for NetworkManager is a compromise between the new defaults from MM
and what the user would expect under e.g. very low quality conditions.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/678
In connection_removed we use the id.name that was being g_freed a few
lines further down.
Fixes: bea6c40367 ('wifi/iwd: handle forgetting connection profiles')
C casts unconditionally force the type, and as such they don't
necessarily improve type safety, but rather overcome restrictions
from the compiler when necessary.
Casting a void pointer is unnecessary (in C), it does not make the
code more readable nor more safe. In particular for g_object_new(),
which is known to return a void pointer of the right type.
Drop such casts.
sed 's/([A-Za-z_0-9]\+ *\* *) *g_object_new/g_object_new/g' $(git grep -l g_object_new) -i
./contrib/scripts/nm-code-format-container.sh
If an NMSettingsConnection with the VOLATILE or EXTENRAL flags is created
and passed to nm_manager_activate_connection, it's immediately scheduled
for deletion in an idle callback and will likely be deleted before the
authorization step in nm_manager_activate_connection finishes and the
connection will be aborted. This is because there's no
NMActiveConnection in priv->active_connection_lst_head referencing it
until _internal_activate_device(). Change
active_connection_find_by_connection to also look for connections in
priv->async_op_lst_head.
New _delete_volatile_connection_do() calls are added. Previously it
would be called when an active connection may have been removed from
priv->active_connection_lst_head, now also call it when an active
connection may have been removed from priv->async_op_lst_head without
being added to priv->active_connection_lst_head.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/671
Before, ovsdb_call_method() has a long list of arguments
to account for all possible commands. That does not scale.
Instead, introduce a separate OvsdbMethodPayload type and
only add a macro to allow passing the right parameters.
The text should match the OvsdbCommand enum. If the enum
value is named OVSDB_ADD_INTERFACE, then we should print
"add-interface". Or alternatively, if you think spelling
out interface is too long, then the enum should be renamed.
I don't care, but name should correspond.
ovsdb sends monitor updates, with "new" and "old" values that indicate
whether this is an addition, and update, or a removal.
Since we also cache the entries, we might not agree with what ovsdb
says. E.g. if ovsdb says this is an update, but we didn't have the
interface in our cache, we should rather pretend that the interface
was added. Even if this possibly indicates some inconsistency between
what OVS says and what we have cached, we should make the best of it.
Rework the code. On update, we compare the result with our cache
and care less about the "new" / "old" values.
We will call the function directly as well. Lets aim to
get the types right.
Also the compiler would warn if the cast to (GDestroyNotify)
would be to a fundamtally different function signature.
GHashTable is optimized for data that has no separate value
pointer. We can use the OpenvswitchBridge structs as key themselves,
by having the id as first field of the structure and only use
g_hash_table_add().
Of course, in practice "mtu" is much smaller than 2^31, and
also is sizeof(int) >= sizeof(uint32_t) (on our systems). Hence,
this was correct. Still, it feels ugly to pass a unsigned integer
where not the entire range is covered.
- rename "id" to something more distinct: "call_id".
- consistently use guint64 type. We don't want nor need
to handle negative values. For CALL_ID_UNSPEC we can use
G_MAXUINT64.
- don't use "i" format string for the call id. That expects
an "int", so it's not clear how this was working correctly
previously. Also, "int" has a smaller range than our 64bits.
Use instead "json_int_t" and cast properly in the variadic
arguments of json_pack().
Also, never update the value to %NULL. If the current
message does not contain a UUID, keep the previous one.
Fixes: 830a5a14cb ('device: add support for OpenVSwitch devices')
"nm-device-logging.h" defines logging macros for a NMDevice instance.
It also expects a "self" variable in the call environment, and that
variable had to be in the type of NMDevice or the NMDevice subclass.
Extend the macro foo, so that @self can be either a NMDevice* pointer
or a NMDevice$SUBTYPE.
Of course, that would have always been possible, if we would simply cast
to "(NMDevice *)" where we need it. The trick is that the macro only
works if @self is one of the two expected types, and not some arbitrary
unrelated type.
