- use _NM_GET_PRIVATE() and _NM_GET_PRIVATE_PTR() everywhere.
- reorder statements, to have GObject related functions (init, dispose,
constructed) at the bottom of each file and in a consistent order w.r.t.
each other.
- unify whitespaces in signal and properties declarations.
- use NM_GOBJECT_PROPERTIES_DEFINE() and _notify()
- drop unused signal slots in class structures
- drop unused header files for device factories
_clear_plugin() should explicitly stop the DNS plugin, instead of just
unreferencing it. Unreferencing does not necessarily mean, that the
plugin will be destroyed right away.
Otherwise, when killing dnsmasq it does not get respawned:
dnsmasq[0x560dd7e43cf0]: dnsmasq exited normally
dns-mgr: plugin dnsmasq child quit unexpectedly
dns-mgr: update-dns: updating resolv.conf
dns-mgr: config: 100 best v4 enp0s25
dns-mgr: config: 100 best v6 enp0s25
dns-mgr: config: 100 default v6 lo
dns-mgr: config: 100 default v4 lo
dns-mgr: update-dns: updating plugin dnsmasq
dnsmasq[0x560dd7e43cf0]: adding nameserver '192.168.0.2@enp0s25'
dnsmasq[0x560dd7e43cf0]: trying to update dnsmasq nameservers
dns-mgr: update-resolv-conf: write internal file /var/run/NetworkManager/resolv.conf succeeded but don't update /etc/resolv.conf as it points to resolv.conf.nm
dnsmasq[0x560dd7e43cf0]: dnsmasq disappeared
Previously, we would create priv->dnsmasq proxy only once,
and not respawn the process at all.
https://bugzilla.gnome.org/show_bug.cgi?id=766996
The 4 private fields pid, watch_id, progname and pidfile strictly
belong together. When spawning a child, we set all 4 of them and
when killing the child all get cleared. Cleanup to code to always
set those 4 fields together.
When a pidfile exists, it is always stale after this point
and kill_existing() should always unlink it.
Also, refactor kill_existing() to use nm_utils_kill_process_sync()
which waits for the process to be gone.
GError codes are only unique per domain, so logging the code without
also indicating the domain is not helpful. And anyway, if the error
messages are not distinctive enough to tell the whole story then we
should fix the error messages.
Based-on-patch-by: Dan Winship <danw@gnome.org>
Functions that take a GError** MUST fill it in on error. There is no
need to check whether error is NULL if the function it was passed to
had a failing return value.
Likewise, a proper GError must have a non-NULL message, so there's no
need to double-check that either.
Based-on-patch-by: Dan Winship <danw@gnome.org>
- All internal source files (except "examples", which are not internal)
should include "config.h" first. As also all internal source
files should include "nm-default.h", let "config.h" be included
by "nm-default.h" and include "nm-default.h" as first in every
source file.
We already wanted to include "nm-default.h" before other headers
because it might contains some fixes (like "nm-glib.h" compatibility)
that is required first.
- After including "nm-default.h", we optinally allow for including the
corresponding header file for the source file at hand. The idea
is to ensure that each header file is self contained.
- Don't include "config.h" or "nm-default.h" in any header file
(except "nm-sd-adapt.h"). Public headers anyway must not include
these headers, and internal headers are never included after
"nm-default.h", as of the first previous point.
- Include all internal headers with quotes instead of angle brackets.
In practice it doesn't matter, because in our public headers we must
include other headers with angle brackets. As we use our public
headers also to compile our interal source files, effectively the
result must be the same. Still do it for consistency.
- Except for <config.h> itself. Include it with angle brackets as suggested by
https://www.gnu.org/software/autoconf/manual/autoconf.html#Configuration-Headers
If dnsmasq (or another DNS plugin) exits immediately (for example due
to an already used port), the DNS manager keeps restarting it forever,
wasting system resources and filling logs.
Add a simple rate-limiting mechanism.
https://bugzilla.gnome.org/show_bug.cgi?id=760691
Modify the DNS manager to use the static global DNS configuration when
available. In addition, change DNS plugins interface to accept a new
argument for global configuration and add support for this new
parameter to the dnsmasq plugin.
Rather than randomly including one or more of <glib.h>,
<glib-object.h>, and <gio/gio.h> everywhere (and forgetting to include
"nm-glib-compat.h" most of the time), rename nm-glib-compat.h to
nm-glib.h, include <gio/gio.h> from there, and then change all .c
files in NM to include "nm-glib.h" rather than including the glib
headers directly.
(Public headers files still have to include the real glib headers,
since nm-glib.h isn't installed...)
Also, remove glib includes from header files that are already
including a base object header file (which must itself already include
the glib headers).
NM was killing the dnsmasq local caching nameserver process and immediately
starting a new one, and new process couldn't bind to 127.0.0.1 because the
old one hadn't quit yet. Thus the new process quit, and the user was
left with no split DNS at all.
While this does introduce more synchronous waiting into the connection
process, it's not that much time and NM will kill dnsmasq if it hasn't
quit after 1 second. The longer-term fix is to use dnsmasq's D-Bus
interface to update DNS without respawning it.
https://bugzilla.gnome.org/show_bug.cgi?id=728342https://bugzilla.redhat.com/show_bug.cgi?id=1161232
Add nm_utils_setpgid() as a g_spawn*() child setup function for
calling setpgid(), and use it where appropriate rather than
reimplementing it every time.
Replace the pthread_sigwait()-based signal handling with
g_unix_signal_add()-based handling, and get rid of all the
now-unnecessary calls to nm_unblock_posix_signals() when spawning
subprocesses.
As a bonus, this also fixes the "^C in gdb kills NM too" bug.
config.h should be included from every .c file, and it should be
included before any other include. Fix that.
(As a side effect of how I did this, this also changes us to
consistently use "config.h" rather than <config.h>. To the extent that
it matters [which is not much], quotes are more correct anyway, since
we're talking about a file in our own build tree, not a system
include.)
Various bits of code want the network interface which an IP config
came from, for example when distinguishing which interface to
send DNS requests to when the DNS servers are link-local. DNS
plugins may also want this data for various reasons.
So it makes sense to attach the interface name to the IP config
object when the DNS manager gets it, so that later DNS updates
that don't have any interface information (hostname changes, etc)
can still generate correct DNS information.
This also eliminates the "last_iface" hack, which was often
inaccurate.
It also now sends "NetworkManager" to SUSE netconfig as the
interface name, because the DNS information being sent is already
merged/prioritized and not specific to a network interface, so
it's time to stop lying about where it came from.
Commit 217c5bf6ac fixed processing of unix
signals: signals are blocked in all threads and a dedicated thread handles the
signals using sigwait().
However, the commit forgot that child processes inherit signal mask as well.
That is why we have to unblock signals for child processes we spawn from NM, so
that they can receive signals.
inet_ntop() either returns 'address%interface' or just 'address'. In the first
case, we replace '%' with '@' since dnsmasq supports '%' only since version
2.58. In the second case, we append '@interface' to make it work.
(small fixes by dcbw)
config.h defines _GNU_SOURCE, which in turn defines the bits necessary
for kill, isblank, and isascii. So wherever we use those, we need
to make sure config.h is included.