This takes current HEAD branch, and finds all the commits what
are not on master or one of the nm-1-* branches, and runs
checkpatch.pl on each.
The use is to run checkpatch.pl on all patches of a feature
branch.
(cherry picked from commit 369446eae6)
This wouldn't even dereference the dangling pointer, but
merely comparing it for pointer equality. Still, it's actually
undefined behavior. Avoid it.
(cherry picked from commit cfc0565604)
When a software device is removed by nmcli in parallel with a
disconnection, e.g.:
nmcli connection add type team ifname t1 con-name t1
sleep 1
nmcli connection down t1 & nmcli device delete t1
nmcli sometimes crashes in the following way:
...
Connection 't1' (e4701688-d1a9-4942-85f0-a2081e120023) successfully added.
Connection 't1' successfully deactivated (D-Bus active path: /org/freedesktop/NetworkManager/ActiveConnection/36)
Device 't1' successfully removed.
AddressSanitizer:DEADLYSIGNAL
=================================================================
==15217==ERROR: AddressSanitizer: SEGV on unknown address 0x00000000000b (pc 0x7fa6d92d1c9d bp 0x0000004ba260 sp 0x7ffffe6a6f40 T0)
==15217==The signal is caused by a READ memory access.
==15217==Hint: address points to the zero page.
0 0x7fa6d92d1c9c in g_string_truncate (/lib64/libglib-2.0.so.0+0x6ec9c)
1 0x7fa6d92d2d7b in g_string_printf (/lib64/libglib-2.0.so.0+0x6fd7b)
2 0x45a6d7 in delete_device_cb clients/cli/devices.c:2465
3 0x7fa6d9849289 in g_simple_async_result_complete /usr/src/debug/glib2-2.56.1-1.fc28.x86_64/gio/gsimpleasyncresult.c:802
4 0x7fa6dbaa9836 in device_delete_cb libnm/nm-device.c:2458
5 0x7fa6d985bcf3 in g_task_return_now /usr/src/debug/glib2-2.56.1-1.fc28.x86_64/gio/gtask.c:1148
6 0x7fa6d985c7a5 in g_task_return /usr/src/debug/glib2-2.56.1-1.fc28.x86_64/gio/gtask.c:1206
7 0x7fa6d989ca6c in reply_cb /usr/src/debug/glib2-2.56.1-1.fc28.x86_64/gio/gdbusproxy.c:2586
8 0x7fa6d985bcf3 in g_task_return_now /usr/src/debug/glib2-2.56.1-1.fc28.x86_64/gio/gtask.c:1148
9 0x7fa6d985c7a5 in g_task_return /usr/src/debug/glib2-2.56.1-1.fc28.x86_64/gio/gtask.c:1206
10 0x7fa6d98913c0 in g_dbus_connection_call_done /usr/src/debug/glib2-2.56.1-1.fc28.x86_64/gio/gdbusconnection.c:5722
11 0x7fa6d985bcf3 in g_task_return_now /usr/src/debug/glib2-2.56.1-1.fc28.x86_64/gio/gtask.c:1148
12 0x7fa6d985bd2c in complete_in_idle_cb /usr/src/debug/glib2-2.56.1-1.fc28.x86_64/gio/gtask.c:1162
13 0x7fa6d92ac0ea in g_idle_dispatch gmain.c:5535
14 0x7fa6d92af7cc in g_main_dispatch gmain.c:3177
15 0x7fa6d92afb97 in g_main_context_iterate gmain.c:3903
16 0x7fa6d92afec1 in g_main_loop_run (/lib64/libglib-2.0.so.0+0x4cec1)
17 0x472892 in main clients/cli/nmcli.c:1067
18 0x7fa6d8cc31ba in __libc_start_main (/lib64/libc.so.6+0x231ba)
19 0x4162b9 in _start (/usr/bin/nmcli+0x4162b9)
The reason is that after calling nm_device_delete_async() we also
listen for the manager device-removed signal. When the signal is
received, device_removed_cb() destroy the @info structure and calls
g_main_loop_quit (loop). However, if the delete_device_cb() callback
has already been dispatched it is executed anyway and it tries to
access a stale @info.
It makes little sense to listen for the device-removed signal since
the return value of nm_device_delete_async() already tells us whether
the device was removed successfully or not.
The only advantage would be that when the device goes away for other
reasons we can still return success, but that is racy and should not
be relied upon.
https://bugzilla.redhat.com/show_bug.cgi?id=1639208
(cherry picked from commit 6130a4561e)
This is a change in behavior regarding the filename that we choose when
writing files to "/etc/NetworkManager/system-connections/".
(cherry picked from commit d37ad15f12)
initrd does not use keyfile API from "src/settings/plugins/keyfile",
hence it does not use nms_keyfile_utils_escape_filename() to add
the ".nmconnection" file extension.
I think that is problematic, because it also misses escapings which
are necessary so that NetworkManager will accept the file.
Anyway, the proper solution here would be to move the keyfile utility
functions to libnm-core, alongside base keyfile API. That way, it
could be used by initrd generator.
For now, just dirty fix the generated filename.
Fixes: 648c256b90
(cherry picked from commit 4ca7fa7f4a)
For profiles in "/etc/NetworkManager/system-connections", we did not enforce
that the keyfiles have a special suffix, nor did we generate the
filenames in such a manner. In hindsight, I think that was a mistake.
Recently we added "/run/NetworkManager/system-connections" as additional
keyfile directory. Enforce a suffix and write keyfiles with such a name.
In principle, we could also start writing keyfiles in /etc with the
same suffix. But let's not do that, because we anyway cannot enforce
it.
An ugly part is, that during `nmcli connection load` we need to
determine whether the to-be-loaded connection is under /etc or /run.
Preferably, we would allow any kind of symlinking as what matters
is the file object (inode) and not the path. Anyway, we don't do
that but compare plain paths. That means, paths which are not
in an expected form, will be rejected. In particular, the paths
starting with "/run/..." and "/var/run/..." will be treated differently,
and one of them will be rejected.
Note that ifcfg-rh plugin strictly enforces that the path
starts with IFCFG_DIR as well. So, while this is a breaking
change for keyfile, I think it's reasonable.
(cherry picked from commit 648c256b90)
keyfile already supports omitting the "connection.id" and
"connection.uuid". In that case, the ID would be taken from the
keyfile's name, and the UUID was generated by md5 hashing the
full filename.
No longer do this during nm_keyfile_read(), instead let all
callers call nm_keyfile_read_ensure_*() to their liking. This is done
for two reasons:
- a minor reason is, that one day we want to expose keyfile API
as public API. That means, we also want to read keyfiles from
stdin, where there is no filename available. The implementation
which parses stdio needs to define their own way of auto-generating
ID and UUID. Note how nm_keyfile_read()'s API no longer takes a
filename as argument, which would be awkward for the stdin case.
- Currently, we only support one keyfile directory, which (configurably)
is "/etc/NetworkManager/system-connections".
In the future, we want to support multiple keyfile dirctories, like
"/var/run/NetworkManager/profiles" or "/usr/lib/NetworkManager/profiles".
Here we want that a file "foo" (which does not specify a UUID) gets the
same UUID regardless of the directory it is in. That seems better, because
then the UUID won't change as you move the file between directories.
Yes, that means, that the same UUID will be provided by multiple
files, but NetworkManager must already cope with that situation anyway.
Unfortunately, the UUID generation scheme hashes the full path. That
means, we must hash the path name of the file "foo" inside the
original "system-connections" directory.
Refactor the code so that it accounds for a difference between the
filename of the keyfile, and the profile_dir used for generating
the UUID.
(cherry picked from commit 837d44ffa4)
Split out the functionality for auto-detecting the ID and UUID of
a connection. First of all, nm_keyfile_read() is already overcomplicated.
The next commit will require the caller to explicitly call these
functions.
(cherry picked from commit 02c8844178)
check_prefix() was only ever called with "." as prefix.
Simplify the implementation to explicitly check for a leading
dot.
(cherry picked from commit 2e5985f2e9)
Already since 1.0.0 release and commit "3784678177 cli: create a connection
if none exist in 'nmcli dev connect' (rh #1113941)", device-connect can
also create a profile.
That is useful, in particular as opposed to
$ nmcli connection up ifname "$DEVICE"
which wouldn't create a profile (ever).
Document it.
(cherry picked from commit 1b732e28f7)
nm-initrd-generator scans the command line for options relevant to network
configuration and creates configuration files for an early instance of
NetworkManager run from the initial ramdisk during early boot.
(cherry picked from commit 9f9609555d)
This is loosely based on nms-ibft-reader, but with some significant
changes. Notably, it parses /sys/firmware/ibft directly instead of
iscsiadm output.
iscsiadm is not available on early boot (perhaps it's too large) and
turns out that parsing sysfs directly is easier and more
straightforwared anyways. A win-win situation.
It is not useful alone, it's in a separate commit just for the sake of
easier review.
(cherry picked from commit b544f7243d)
We need a mode that:
* doesn't leave processes behind
* doesn't force an internal dhclient
* doesn't auto-generate default connections
* doesn't write out files into libdir, only /run
The original configure-and-quit mode doesn't really fit the initrd use. But
it's proobably not a good idea to just change its behavior.
(cherry picked from commit c263f5355c)
This is useful for in-memory connections to persist NetworkManager
restarts (as opposed to machine restarts).
Perhaps most improtantly, this allows generating in-memory connections outside
NetworkManager, e.g. passing configuration from early boot firmware in initrd.
Note that this does *not* aspire to do more than it says on the tin:
Notably, it doesn't touch the problem of provisioning connections in multiple
persistent connection directories and thus doesn't have to deal with the
problem of deleting or overlaying the connections tha (rh #772414) deals
with.
(cherry picked from commit ce4dbd7daf)
Especially with configure-and-quit, it's easy to encounter a condition,
where the device reached a failed state, policy decides to quit, but the
active connection is not yet torn down from the device.
Upon the next start NetworkManager would think the connection succeeded
activating.
(cherry picked from commit e98ebc7e3b)
Make them just ask for connections from GDBus, as other D-Bus clients
do. GDBus anyway reuses the connection if it has one, but allows us to
deal with errors in a more civilized manner.
(cherry picked from commit e1fc005239)
Using these unormalized was wrong all along, but by chance didn't hit
paths that needed normalized connections. This may change if we
actually write in memory connections to /run with the keyfile plugin,
because that one wants them normalized.
This also saves some work, because normalization does boring things for
us, such as adding default ipv4/ipv6/proxy settings everywhere.
(cherry picked from commit 89d1c9fb30)
nm_setting_to_string() operates on the setting alone, without a
connection. Tolerate that.
This fixed nm_connection_dump(vlan_connection).
(cherry picked from commit c39b134da1)
On networked boot we need to somehow communicate this to the early boot
machinery. Sadly, no DBus there and we're running in configure-and-quit
mode.
Abusing the state file for this sounds almost reasonable and is
reasonably straightforward thing to do.
(cherry picked from commit 55d24ba94e)
