Every branch (for example "nm-1-40") has exactly one next branch, from
which patches should be backported (in that example that branch is
"nm-1-42").
While "find-backports" searches all newer branches for patches, it does
not make it clear form where the patch should come from.
That means, if you run the script `contrib/scripts/find-backports origin/nm-1-40`
it will check nm-1-42 and main branch, and might suggest to backport
patches that are only on main, but not "nm-1-42". That would be wrong,
because patches need to first go into nm-1-42, and then backported (from
there) further to nm-1-40.
Print a warning to highlight that.
If the client was waiting for IPv6 DAD to complete and the lease was
updated or lost, `wait_ipv6_dad` needs to be cleared; otherwise, at
the next platform change the client will try to evaluate the DAD state
with a different or no lease. In particular if there is no lease the
client will try to decline it because there are no valid addresses,
leading to an assertion failure:
../src/core/dhcp/nm-dhcp-client.c:997:_dhcp_client_decline: assertion failed: (l3cd)
Backtrace:
__GI_raise ()
__GI_abort ()
g_assertion_message ()
g_assertion_message_expr ()
_dhcp_client_decline (self=0x1af13b0, l3cd=0x0, error_message=0x8e25e1 "DAD failed", error=0x7ffec2c45cb0) at ../src/core/dhcp/nm-dhcp-client.c:997
l3_cfg_notify_cb (l3cfg=0x1bc47f0, notify_data=0x7ffec2c46c60, self=0x1af13b0) at ../src/core/dhcp/nm-dhcp-client.c:1190
g_closure_invoke ()
g_signal_emit_valist ()
g_signal_emit ()
_nm_l3cfg_emit_signal_notify () at ../src/core/nm-l3cfg.c:629
_nm_l3cfg_notify_platform_change_on_idle () at ../src/core/nm-l3cfg.c:1390
_platform_signal_on_idle_cb () at ../src/core/nm-netns.c:411
g_idle_dispatch ()
Fixes: 393bc628ff ('dhcp: wait DAD completion for DHCPv6 addresses')
https://bugzilla.redhat.com/show_bug.cgi?id=2179890https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1594
<error> is mostly about "really should not happen" scenarios. It's
closer to an assertion failure, and something that NetworkManager should
not happen.
Of course, things can go wrong, but <warn> is a sufficient. When ovsdb
gives unexpected communication, it's just a warning. At least, that's
also what all the similar cases in "nm-ovsdb.c" already do
GSocketConnection/GOutputStream/GInputStream seems rather unnecessary.
Maybe they make sense when you want to write portable code (for
Windows). Otherwise, watching a file descriptor and reading/writing it
directly is simpler (and also more efficient).
For example, we passed no GCancellable to g_input_stream_read_async().
What does that mean w.r.t. destroying the NMOvsdb instance? I suspect
it's wrong, but it's hard to say, because there are so many layers of
code.
Note that we anyway keep state in NMOvsdb, namely the data we want to
send (output_buf) and the data we partially received (input_buf). All we
need, are poll notifications when the file descriptor is ready. To
those, we hook up the read/write callbacks. Also before was the code
async, and there were callbacks when when read/write was done. That does
not simplify the code in any way.
- we no longer use separate NMOvsdbPrivate.buf and NMOvsdbPrivate.input
buffers. There is just a NMOvsdbPrivate.input_buf that can we can fill
directly.
The "priv->bufp" offset is only used while parsing a message at a time.
It's unnecessary to track it in NMOvsdbPrivate and keep it between
parsing messages. Tracking the state in NMOvsdbPrivate makes it more
complicated to understand, because one needs to reason at which times
the state is used (when it really is not used).
Also, move the parsing to a separate function.
We did not initialize "child_stderr". If that were necessary, we would need
to add it too. However, it is clearly not necessary to initialize those fields.
It's not used. It's better to use SOCK_NONBLOCK flag for socket(), as we do.
Also, the implementation that blindly calls F_SETFL without merging the
existing flags from F_GETFL is just wrong. Drop it altogether.
F_SETFL will reset the flags. That is wrong, as we only want to add
O_NONBLOCK flag and leaving the other flags alone. Usually, we would
need to call F_GETFL first.
Note that on Linux, F_SETFL can only set certain flags, so the
O_RDWR|O_CLOEXEC flags were unaffected by this. That means, most likely
there are no other flags that our use of F_SETFL would wrongly clear.
Still, it's ugly, because it's not obvious whether there might be other
flags.
Avoid that altogether, by setting the flag already during open().
Fixes: 67e092abcb ('core: better handling of rfkill for WiMAX and WiFi (bgo #629589) (rh #599002)')
The device shouldn't change state from DEACTIVATING to DISCONNECTED
until its detached from its controller; otherwise, the port detach
that is in progress can conflict with the following activation.
This changes the signature of detach_port() to be asynchronous,
similarly to attach_port(). The implementation can return TRUE/FALSE
on immediate completion.
Current implementations return immediately and so there is no change
in behavior for now.
It is wrong trying to send the signal still. Just error out.
Note that ECHILD indicates that the process is either not a child
or was already reaped. In both cases, that is a bug of the caller
who must keep accurate track of the child's process ID.
It's easy enough to know how many bytes are needed. Just allocate the
right size (+1, because NMStrBuf really likes to reserve that extra byte
for the trailing NUL, even if it's not needed in this case).
src/libnm-core-impl/tests/test-keyfile.c: In function '_invalid_option_write_handler':
src/libnm-core-impl/tests/test-keyfile.c:917:9: error: 'message' may be used uninitialized [-Werror=maybe-uninitialized]
917 | g_assert(message && strstr(message, "ethtool.bogus"));
| ^
src/libnm-core-impl/tests/test-keyfile.c:905:29: note: 'message' was declared here
905 | const char *message;
| ^
lto1: all warnings being treated as errors
"preexec_fn" is not great, because it is not generally safe in multi
threaded code (and we don't know whether the test didn't start other
threads already, like a GDBus worker thread). Well, it probably is safe,
if you only call async signal safe code, but it's not clear what os.dup2()
does under the hood.
Just avoid that. We can pass on the FD directly.
test_ec2 (__main__.TestNmCloudSetup.test_ec2) ... /usr/lib64/python3.11/unittest/case.py:579: ResourceWarning: unclosed file <_io.BufferedWriter name=5>
if method() is not None:
ResourceWarning: Enable tracemalloc to get the object allocation traceback
ok
Fixes: d89d42bf23 ('tests/client: test nm-cloud-setup')
This seems unnecessary, because we spawn the child process via subprocess.Popen and
set "pass_fds". That already ensures to pass on the FD.
Worse, socket.set_inheritable() is only added in Python 3.4, that means the
test is gonna break for Python 3.2 and 3.3. Just avoid that by not using the
unnecessary function. For the same reason, drop "inheritable=True" from
os.dup2(). "inheritable=True" is already the default, but only exists
since Python 3.4.
Fixes: d89d42bf23 ('tests/client: test nm-cloud-setup')
The test uses subprocess.Popen()'s "pass_fd" argument. That is only
available since Python 3.2. Possibly it could be solved differently, but
that is not implemented. Instead, skip the test.
Also, socket.socket.set_inheritable() is Python 3.4. But presumably
we don't need it.
Fixes: d89d42bf23 ('tests/client: test nm-cloud-setup')
