The previous implementation would print
./tools/check-gitlab-ci.sh: line 22: ci-fairy: command not found
also, it's not nice to execute the program, just to find out whether
we have it.
This will be used on our gitlab-ci tests. We do have a separate,
dedicated test on gitlab-ci for checking that "gitlab-ci.yml" is
correct.
We may intentionally want to violate that condition, but then our
`make check` should not fail.
Add a way to skip that test.
gcc-11.0.0-0.7.fc34 warns here:
CC libnm-core/libnm_core_la-nm-setting-team.lo
libnm-core/nm-setting-team.c: In function ‘nm_team_link_watcher_new_ethtool’:
libnm-core/nm-setting-team.c:127:33: error: array subscript ‘NMTeamLinkWatcher[0]’ is partly outside array bounds of ‘unsigned char[16]’ [-Werror=array-bounds]
127 | watcher->ref_count = 1;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
libnm-core/nm-setting-team.c:125:15: note: referencing an object of size 16 allocated by ‘g_malloc’
125 | watcher = g_malloc(nm_offsetofend(NMTeamLinkWatcher, ethtool));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libnm-core/nm-setting-team.c:128:33: error: array subscript ‘NMTeamLinkWatcher[0]’ is partly outside array bounds of ‘unsigned char[16]’ [-Werror=array-bounds]
128 | watcher->type = LINK_WATCHER_ETHTOOL;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
libnm-core/nm-setting-team.c:125:15: note: referencing an object of size 16 allocated by ‘g_malloc’
125 | watcher = g_malloc(nm_offsetofend(NMTeamLinkWatcher, ethtool));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libnm-core/nm-setting-team.c:129:33: error: array subscript ‘NMTeamLinkWatcher[0]’ is partly outside array bounds of ‘unsigned char[16]’ [-Werror=array-bounds]
129 | watcher->ethtool.delay_up = delay_up;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~
libnm-core/nm-setting-team.c:125:15: note: referencing an object of size 16 allocated by ‘g_malloc’
125 | watcher = g_malloc(nm_offsetofend(NMTeamLinkWatcher, ethtool));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libnm-core/nm-setting-team.c:130:33: error: array subscript ‘NMTeamLinkWatcher[0]’ is partly outside array bounds of ‘unsigned char[16]’ [-Werror=array-bounds]
130 | watcher->ethtool.delay_down = delay_down;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
libnm-core/nm-setting-team.c:125:15: note: referencing an object of size 16 allocated by ‘g_malloc’
125 | watcher = g_malloc(nm_offsetofend(NMTeamLinkWatcher, ethtool));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Maybe we should not use this trick and just malloc() a struct of the
intended size, however:
- the code below does a similar thing, doing it differently for ethtool
watcher is confusing.
- the NMTeamLinkWatcher is a union which cannot alter its type. In no
case is it correct to access the fields of the wrong union type. By
allocating a smaller chunk, valgrind might catch such bugs.
Also, NMTeamLinkWatcher's definition is private to the C source file,
in no case must anybody assume that the rest of the buffer actually
exists.
Hence, workaround the warning by suppressing it.
We allow normalizing the slave-type, but sometimes, we may want to
validate a profile according to the set slave-type.
For example, a "ovs-external-ids" setting should only be allowed for
"connection.slave-type=ovs-interface". But during verify, the slave-type
may be missing and not yet normalized. We need to be able to obtain
the actually used slave-type.
musl added support for reallocarray, but the function prototype is
declared in stdlib.h instead of malloc.h.
Update the check for reallocarray to check both in malloc.h and
stdlib.h.
https://man7.org/linux/man-pages/man3/reallocarray.3.html
wpa_supplicant has a property "scanning" and a "state=scanning".
Previously, NetworkManager considered both parts to indicate whether
supplicant is currently scanning (if either the property or the state
indicated scanning, it took that as indication for scanning).
If NetworkManager thinks that supplicant is scanning, it suppresses
explicit "Scan" requests. That alone is not severe, because the "Scan"
request is only to trigger a scan in supplicant (which supplicant
possibly is already doing in state "scanning").
However, what is severe is that NetworkManager will also block autoconnect
while supplicant is scanning. That is because NetworkManager wants to get
a complete scan result before deciding which network to connect to.
It seems that wpa_supplicant can get into "state=scanning" and stay
there indefinitely. This prevents NetworkManager from autoactivating
a profile.
Fix that, to only honor the "scanning" property.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/597
Fixes: b83f07916a ('supplicant: large rework of wpa_supplicant handling')
In commit 201c153e25 ('libnm: fix GObject Introspection annotations
for functions returning a GPtrArray') these annotations were changed
to fix Vala bindings. However, bindings may treat the transfer
annotation for GPtrArray differently, so depending on the binding
we either get a leak or a double free.
It's unclear how to fix that. For now, just add a warning to the
documentation to avoid it.
The following reproducer leads to a crash:
#!/bin/python
import gi
gi.require_version("NM", "1.0")
from gi.repository import NM
def _pr(msg):
NM.utils_print(0, msg + "\n")
def process(nmc):
for device in nmc.get_devices():
cons = device.filter_connections(nmc.get_connections())
_pr(
"device %s (%s) has %s compatible connections"
% (device.get_iface(), NM.Object.get_path(device), len(cons))
)
process(NM.Client.new())
See-also: https://gitlab.gnome.org/GNOME/gobject-introspection/-/issues/305
We cannot drop the reference count to zero while having
no lock. Otherwise, another thread might race doing
s = nm_ref_string_new("...");
nm_ref_string_unref(s);
and already successfully delete the instance.
Hitting this race should be rather difficult, especially because
we tend to use NMRefString only from one thread. But still, access
to global variables must be race free.
Fixes: 908fadec96 ('shared: add NMRefString')
GObject's dispose() functions may be called multiple times
to break reference cycles.
As dispose() calls _init_release_all(), the object might
already be partially destroyed.
Fixes: ce0e898fb4 ('libnm: refactor caching of D-Bus objects in NMClient')
During shutdown, NM always tries to remove from ovsdb all bridges,
ports, interfaces that it previously added. Currently NM doesn't run
the main loop during shutdown and so it's not possible to perform
asynchronous operations. In particular, the NMOvsdb singleton is
disposed in a destructor where it's not possible to send out all the
queued deletions.
The result is that NM deletes only one OVS interface, keeping the
others. This needs to be fixed, but requires a rework of the shutdown
procedure that involves many parts of NM.
Even when a better shutdown procedure will be implemented, we should
support an unclean shutdown caused by e.g. a kernel panic or a NM
crash. In these cases, the interfaces added by NM would still linger
in the ovsdb.
Delete all those interface at NM startup. If there are connections
profiles for them, NM will create them again.
https://bugzilla.redhat.com/show_bug.cgi?id=1861296https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/700
With Fedora 33+ and RHEL 9+, the default plugins are
"plugins=keyfile,ifcfg-rh", instead of "plugins=ifcfg-rh,keyfile".
Update our "NetworkManager.conf" file to reflect that.
By now, each NMDevice does the reverse lookup and caches the result
via nm_device_get_hostname_from_dns_lookup().
The code is no longer used in NMPolicy.
Fixes: 09c8387114 ('policy: use the hostname setting')
"build_clean.sh" (and "build.sh") scripts can both create a source
tarball (via `make dist`/`make distcheck`), an SRPM (and a spec file),
or build RPMs from the SRPM.
Note that the generated spec file has various options, like
%bcond_without nmtui
%bcond_without debug
%bcond_without test
When building an RPM from the SRPM, you can specify the "--with" or
"--without" option for rpmbuild. This is also what the "-w" / "-W" options
for "build_clean.sh" do.
However, the SRPM still has the intrinsic defaults, and if you later
build an RPM from it, you would have to pass "--with" / "--without"
to rpmbuild.
Often that is not conveniently possible, for example, when you build the
SRPM in koji.
Extend the scripts so that also the defaults for "-w debug" and "-w
test" can be specified when generating the SRPM. You can do that with
the new options "--default-for-{debug,test}" to "build_clean.sh".
Alternatively, it suffices to specify the previously supported
"-w" / "-W" options. That way, we will pass those options to rpmbuild,
but also set them as defaults in the generate spec file. The new
options "--default-for-{debug,test}" are only needed if you want
the default in the spec file to be different then what you use
when creating the SRPM.
By default, "build_clean.sh" script likes to automatically add "-w test"
-- unless the user specified "-w test" or "-W test" on the command line.
That is mostly fine. However, the spec file has an internal default for the
"test" option. So if you want to use the default that gets determined
by the spec file, then we should suppress that automatism.
g_hash_table_iter_next() wants a (gpointer *), not an (int *).
Fixes: f70ee67058 ('dns: sd-resolved: reset interface configuration on deactivation')
(cherry picked from commit 526b484be1)
When a connection is reactivated, there could be two active
connections tracked by the manager: the deactivating one and the new
one. Ensure that we first return the most recent one so that slaves
will pick the right master.
Fixes-test: @iptunnel_gretap_doc_procedure
Fixes: dc6ec6ce7b ('core: reverse the order of active connections in the manager')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/697
We now always use SetLinkDefaultRoute(), but that API was only added in
systemd v240 ([1]).
We could just always call the non-existing method, and ignore the
error. However, that feels ugly. Would systemd-resolved log warnings
about that? Should we suppress all messages about that failure (not
good for debugging).
Instead, make an effort to detect support of the function, and avoid
calling it. That is significantly more complicated than just always
calling the method and not care.
Note that even if systemd-resolved does not support SetLinkDefaultRoute(),
we cannot do anything smart about that. We would simply rely on
systemd-resolved (hopefully) doing the right thing automatically.
That's better and simpler than explicitly adding a "~." domain in
the fallback case.
Also, detecting support is straight forward in the common case, where
there is either success or a clear "org.freedesktop.DBus.Error.UnknownMethod"
error. In cases where there is any other failure, we don't really know.
In that case, we keep trying to use the API under the assumption that
it should work.
[1] https://github.com/systemd/systemd/commit/7 ## 7673795dcf5797491e7f785cbf5077d29a15db4
(cherry picked from commit 44ebb99cfa)
When the DNS settings change, we update the request_queue_lst_head list,
with all the requests we want to send.
Then, send_updates() will try to send it. It might not do it right away,
if resolved is not on the bus or the D-Bus connection is not fully inialized
(meaning, we don't know the name owner yet). In those cases, we would
keep the list of requests, and send them later.
However, when sending them, we would also forget about the configuration.
That means, if you restart systemd-resolved, then the daemon drops off
the bus and reappears. I think that systemd-resolved in fact persists
the configuration during restart. So, usually the settings are still the
same after restart. However, we should do better here: if the service
appears, we should send the settings again.
This means to not forget the requests after we send them once -- at
least, until a new update replaces them.
(cherry picked from commit 4fc44952f7)
We will need these changes next:
- add "self" and "ifindex" fields to RequestItem struct. We will
pass on these structs are user-data for the callbacks, so that
we afterwards know which request completed.
- add DBUS_OP_SET_LINK_DEFAULT_ROUTE global variable. We don't
clone the "operation" string but use string literals. However,
string literals are not guaranteed to be deduplicated, so we
should only compare them with strcmp(). The static variable
avoids this: we can use pointer equality to compare it.
This will be used next.
(cherry picked from commit 8af6647cda)
We now always use SetLinkDefaultRoute(), but that API was only added in
systemd v240 ([1]).
We could just always call the non-existing method, and ignore the
error. However, that feels ugly. Would systemd-resolved log warnings
about that? Should we suppress all messages about that failure (not
good for debugging).
Instead, make an effort to detect support of the function, and avoid
calling it. That is significantly more complicated than just always
calling the method and not care.
Note that even if systemd-resolved does not support SetLinkDefaultRoute(),
we cannot do anything smart about that. We would simply rely on
systemd-resolved (hopefully) doing the right thing automatically.
That's better and simpler than explicitly adding a "~." domain in
the fallback case.
Also, detecting support is straight forward in the common case, where
there is either success or a clear "org.freedesktop.DBus.Error.UnknownMethod"
error. In cases where there is any other failure, we don't really know.
In that case, we keep trying to use the API under the assumption that
it should work.
[1] https://github.com/systemd/systemd/commit/7 ## 7673795dcf5797491e7f785cbf5077d29a15db4
When the DNS settings change, we update the request_queue_lst_head list,
with all the requests we want to send.
Then, send_updates() will try to send it. It might not do it right away,
if resolved is not on the bus or the D-Bus connection is not fully inialized
(meaning, we don't know the name owner yet). In those cases, we would
keep the list of requests, and send them later.
However, when sending them, we would also forget about the configuration.
That means, if you restart systemd-resolved, then the daemon drops off
the bus and reappears. I think that systemd-resolved in fact persists
the configuration during restart. So, usually the settings are still the
same after restart. However, we should do better here: if the service
appears, we should send the settings again.
This means to not forget the requests after we send them once -- at
least, until a new update replaces them.
We will need these changes next:
- add "self" and "ifindex" fields to RequestItem struct. We will
pass on these structs are user-data for the callbacks, so that
we afterwards know which request completed.
- add DBUS_OP_SET_LINK_DEFAULT_ROUTE global variable. We don't
clone the "operation" string but use string literals. However,
string literals are not guaranteed to be deduplicated, so we
should only compare them with strcmp(). The static variable
avoids this: we can use pointer equality to compare it.
This will be used next.
