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.
The python example is (also) used to test the feature. That is because
currently nmcli does not yet have support for ovs.external-ids and this
API is only accessible via D-Bus (or a tool like this example).
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
Ubuntu/Debian and CentOS/Fedora are sufficiently similar that it's
better that we have only one variant of ".gitlab-ci/*-install.sh"
and "contrib/*/REQUIRED_PACKAGES".
This was already the case, however, we used to symlink
".gitlab-ci/centos-install.sh" to "fedora-install.sh". That
worked, but it didn't scale very well. For example, if we would follow
that pattern, we would also need a symlink "contrib/centos/REQUIRED_PACKAGES"
Or should "contrib/centos" symlink to "contrib/fedora"? That seems even
more wrong.
We already had the "distro.base_type" variable for that. Make use of
that also for the install script.
It's not clear that ci-fairy has a stable API. Instead, we pin the
version by specifying the git commit sha.
In the build script, install the pinned version.
ci-templates builds and caches the test containers. When the build
scripts, the ci-template or "config.yml" changes, we need to bump
the tag so that the containers get rebuild.
Partly automate this. The tag now gets generated by the template and
contains a checksum of certain build files. Thus, if you change
any build files, then `ci-fairy generate-template` would generate a
different tag. You can not miss that, because we have tests that ensure
that our ".gitlab-ci.yml" is up to date. Also, you no longer need to
manually bump the tag when a build script changes, just regenerate
".gitlab-ci.yml" with `ci-fairy generate-template`.
See also: https://gitlab.freedesktop.org/freedesktop/ci-templates/-/merge_requests/54
> If there is no network [1] to connect to (if network interface is down [2])
> NetworkManager will make "nm-online -s" stop [3] without waiting.
[1] we usually talk about connection profiles or devices/interfaces
[2] it would be better to say, if no suitable profile is ready to
autoconnect.
[3] `nm-online -s` waits until NetworkManager declares "startup
complete" reached.
I find the wording of the comment not very clear or useful.
Possibly I don't understand it properly, but that is a problem
for a comment that is supposed to help.
Remove it.
We encountered the following situation:
* nm-online (NetworkManager-wait-online.service) is started
* NetworkManager tries to configure network
* the first try of using dhclient fails with timeout for some reasons
* the second try of running dhclient completes successfully (NMs tries himself)
We get a working network, but it takes more that 30 seconds.
NetworkManager-wait-online.service fails, it may be a false negative
to checkers of network being online. It will also be a false positive signal
to start mounting network (CIFS, NFS etc.) shares, for example, because
they depend from network-online.target and network-online.target will become
reached right after NetworkManager-wait-online.service fails.
So, it makes sense to increase default timeout from 30 to 60 seconds.
@thaller wrote in https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/237
that there may be cases where, for example, incorrectly set up bridges
or just network may lead to delays when booting the system.
Yes, but:
* if there is no possibility to start trying to connect the network, e.g. if all network interfaces,
managed by NetworkManager, are DOWN, NM will not try to connect network and "nm-online -s" will
exit with 0 without waiting (so, this will not prolong start up of workstations or servers
when ethernet cable is just not plugged in)
* an additional potential 30 seconds delay seems to be less harmful then a false positive to start
mounting network shares or doing abything else too eraly, when an additional wait for a few seconds
would have prevented those things from failing.
All things considered, it seems reasonable to increase this timeout from 30 seconds to 1 minute (60 seconds).
Signed-off-by: Mikhail Novosyolov <m.novosyolov@rosalinux.ru>
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/675
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
The goal is to run most distros only manually. However, it would be nice
to avoid (manually) clicking twice to start the tests for one distro:
once for the container preparation, and once for the actual test.
Previously, the container prep part was set to manual and the actual
test automatic. It worked almost as desired, except that this leads
to the entire gitlab-ci pipeline be be in running state indefinitely.
To fix that, always run the container prep steps. If the container is
cached, this is supposed to be fast and cheap. Now only the actual tests
are marked as "manual".
It seems "pages" test does not get properly triggered, if only
t_fedora:33 completes. It should, because the other distros are
optional. Try to set "needs" to fix that.
We now install black by default via REQUIRED_PACKAGES script.
Thus, also when we build on Fedora 30, `make check` would run
python black. However, the formatting depends on the version
of python black, and the one in Fedora 30 is not right.
Skip all black tests during `make check`. We have a deicated
gitlab-ci test that runs black already (with the desired version
of black).
The checkpatch test tests the patches on the merg-request, as they
branch off from master (or one of the stable branches).
It thus need the full git history, but the git repository might be a
shallow clone. Fix it.
Certain parts of the code are entirely generated or must follow
a certain format that can be enforced by a tool. These invariants
must never fail:
- ci-fairy generate-template (check-ci-script)
- black python formatting
- clang-format C formatting
- msgfmt -vs
On the other hand, we also have a checkpatch script that checks
the current patch for common errors. These are heuristics and
only depend on the current patch (contrary to the previous type
that depend on the entire source tree).
Refactor the gitlab-ci tests:
- split "checkpatch" into "check-patch" and "check-tree".
- merge the "check-ci-script" test into "check-tree".
Now that the individual steps are no longer in .gitlab.yml but we
run a full shell script, clean it up to be better readable.
Also, we need to fail the script when any command fails.
"debian/REQUIRED_PACKAGES" is used by gitlab-ci to prepare the image. We require
"udev" package, if only to install "/usr/share/pkgconfig/udev.pc" to get the
udev directory.
Otherwise build fails with:
Run-time dependency udev found: NO (tried pkgconfig)
meson.build:371:2: ERROR: Dependency "udev" not found, tried pkgconfig
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.
