Commit b288ea13 authored by Thomas Haller's avatar Thomas Haller

dns: move PID handling from NMDnsPlugin to NMDnsDnsmasq implementation

We only have two real DNS plugins: "dnsmasq" and "systemd-resolved" (the "unbound"
plugin is very incomplete and should eventually be dropped).

Of these two, only "dnsmasq" spawns a child process. A lot of the logic
for that is in the parent class NMDnsPlugin, with the purpose for that
logic to be reusable.

However:

 - We are unlikely to add more DNS plugins. Especially because
   systemd-resolved seems the way forward.

 - If we happen to add more plugins, then probably NetworkManager
   should not spawn the process itself. That causes problems with
   restarting the service. Rather, we should let the service manager
   handle the lifetime of such "child" processes. Aside separating
   the lifetime of the DNS plugin process from NetworkManager's,
   this also would allow to sandbox NetworkManager and the DNS plugin
   differently. Currently, NetworkManager itself may might need
   capabilities only to pass them on to the DNS plugin, or (more likely)
   NetworkManager would want to drop additional capabilities for the
   DNS plugin (which we would rather not implement ourself, since that
   seems job of the service management already).

 - The current implementation is far from beautiful. For example,
   it does synchronous (blocking) killing of the running process
   from the PID file, and it uses PID fils. This is not something
   we would want to reuse for other plugins. Also, note that
   dnsmasq already spawns the service asynchronosly (of course).
   Hence, we should also kill it asynchronously, but that is complicated
   by having the logic separated in two different classes while
   providing an abstract API between the two.

Move the code to NMDnsDnsmasq. This is the only place that cares about
this. Also, that makes it actually clearer what is happening, by seeing
the lifetime handling of the child proceess all in one place.
parent 5f513d06
......@@ -39,6 +39,10 @@ typedef struct {
gboolean running;
GVariant *set_server_ex_args;
GPid pid;
guint watch_id;
char *progname;
} NMDnsDnsmasqPrivate;
struct _NMDnsDnsmasq {
......@@ -61,6 +65,53 @@ G_DEFINE_TYPE (NMDnsDnsmasq, nm_dns_dnsmasq, NM_TYPE_DNS_PLUGIN)
/*****************************************************************************/
static void
kill_existing (const char *progname, const char *pidfile, const char *kill_match)
{
long pid;
gs_free char *contents = NULL;
gs_free char *cmdline_contents = NULL;
guint64 start_time;
char proc_path[256];
gs_free_error GError *error = NULL;
if (!pidfile)
return;
if (!kill_match)
g_return_if_reached ();
if (!g_file_get_contents (pidfile, &contents, NULL, &error)) {
if (g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NOENT))
return;
goto out;
}
pid = _nm_utils_ascii_str_to_int64 (contents, 10, 2, INT_MAX, -1);
if (pid == -1)
goto out;
start_time = nm_utils_get_start_time_for_pid (pid, NULL, NULL);
if (start_time == 0)
goto out;
nm_sprintf_buf (proc_path, "/proc/%ld/cmdline", pid);
if (!g_file_get_contents (proc_path, &cmdline_contents, NULL, NULL))
goto out;
if (!strstr (cmdline_contents, kill_match))
goto out;
nm_utils_kill_process_sync (pid, start_time, SIGKILL, _NMLOG_DOMAIN,
progname ?: "<dns-process>",
0, 0, 1000);
out:
unlink (pidfile);
}
/*****************************************************************************/
static void
add_dnsmasq_nameserver (NMDnsDnsmasq *self,
GVariantBuilder *servers,
......@@ -266,7 +317,7 @@ name_owner_changed (GObject *object,
if (priv->running) {
_LOGI ("dnsmasq disappeared");
priv->running = FALSE;
g_signal_emit_by_name (self, NM_DNS_PLUGIN_FAILED);
_nm_dns_plugin_emit_failed (self, FALSE);
} else {
/* The only reason for which (!priv->running) here
* is that the dnsmasq process quit. We don't care
......@@ -294,7 +345,7 @@ dnsmasq_proxy_cb (GObject *source, GAsyncResult *res, gpointer user_data)
if (!proxy) {
_LOGW ("failed to connect to dnsmasq via DBus: %s", error->message);
g_signal_emit_by_name (self, NM_DNS_PLUGIN_FAILED);
_nm_dns_plugin_emit_failed (self, FALSE);
return;
}
......@@ -314,21 +365,55 @@ dnsmasq_proxy_cb (GObject *source, GAsyncResult *res, gpointer user_data)
send_dnsmasq_update (self);
}
static void
watch_cb (GPid pid, int status, gpointer user_data)
{
NMDnsDnsmasq *self = user_data;
NMDnsDnsmasqPrivate *priv = NM_DNS_DNSMASQ_GET_PRIVATE (self);
int err;
priv->pid = 0;
priv->watch_id = 0;
g_clear_pointer (&priv->progname, g_free);
(void) unlink (PIDFILE);
if (WIFEXITED (status)) {
err = WEXITSTATUS (status);
if (err) {
_LOGW ("dnsmasq exited with error: %s",
nm_utils_dnsmasq_status_to_string (err, NULL, 0));
} else
_LOGD ("dnsmasq exited normally");
} else if (WIFSTOPPED (status))
_LOGW ("dnsmasq stopped unexpectedly with signal %d", WSTOPSIG (status));
else if (WIFSIGNALED (status))
_LOGW ("dnsmasq died with signal %d", WTERMSIG (status));
else
_LOGW ("dnsmasq died from an unknown cause");
priv->running = FALSE;
_nm_dns_plugin_emit_failed (self, TRUE);
}
static void
start_dnsmasq (NMDnsDnsmasq *self)
{
NMDnsDnsmasqPrivate *priv = NM_DNS_DNSMASQ_GET_PRIVATE (self);
gs_free_error GError *local = NULL;
const char *dm_binary;
const char *argv[15];
GPid pid = 0;
guint idx = 0;
gs_free char *cmdline = NULL;
gs_free char *progname = NULL;
if (priv->running) {
/* the dnsmasq process is running. Nothing to do. */
return;
}
if (nm_dns_plugin_child_pid ((NMDnsPlugin *) self) > 0) {
if (priv->pid > 0) {
/* if we already have a child process spawned, don't do
* it again. */
return;
......@@ -360,10 +445,33 @@ start_dnsmasq (NMDnsDnsmasq *self)
argv[idx++] = NULL;
nm_assert (idx <= G_N_ELEMENTS (argv));
/* And finally spawn dnsmasq */
pid = nm_dns_plugin_child_spawn (NM_DNS_PLUGIN (self), argv, PIDFILE, "bin/dnsmasq");
if (!pid)
nm_assert (priv->pid == 0);
nm_assert (!priv->progname);
nm_assert (!priv->watch_id);
progname = g_path_get_basename (argv[0]);
kill_existing (progname, PIDFILE, "bin/dnsmasq");
_LOGI ("starting %s...", progname);
_LOGD ("command line: %s",
(cmdline = g_strjoinv (" ", (char **) argv)));
if (!g_spawn_async (NULL,
(char **) argv,
NULL,
G_SPAWN_DO_NOT_REAP_CHILD,
nm_utils_setpgid,
NULL,
&pid,
&local)) {
_LOGW ("failed to spawn dnsmasq: %s", local->message);
return;
}
_LOGD ("%s started with pid %d", progname, pid);
priv->watch_id = g_child_watch_add (pid, (GChildWatchFunc) watch_cb, self);
priv->pid = pid;
priv->progname = g_steal_pointer (&progname);
if ( priv->dnsmasq
|| priv->dnsmasq_cancellable) {
......@@ -409,33 +517,26 @@ update (NMDnsPlugin *plugin,
/*****************************************************************************/
static void
child_quit (NMDnsPlugin *plugin, int status)
kill_child (NMDnsDnsmasq *self)
{
NMDnsDnsmasq *self = NM_DNS_DNSMASQ (plugin);
NMDnsDnsmasqPrivate *priv = NM_DNS_DNSMASQ_GET_PRIVATE (self);
gboolean failed = TRUE;
int err;
if (WIFEXITED (status)) {
err = WEXITSTATUS (status);
if (err) {
_LOGW ("dnsmasq exited with error: %s",
nm_utils_dnsmasq_status_to_string (err, NULL, 0));
} else {
_LOGD ("dnsmasq exited normally");
failed = FALSE;
}
} else if (WIFSTOPPED (status))
_LOGW ("dnsmasq stopped unexpectedly with signal %d", WSTOPSIG (status));
else if (WIFSIGNALED (status))
_LOGW ("dnsmasq died with signal %d", WTERMSIG (status));
else
_LOGW ("dnsmasq died from an unknown cause");
nm_clear_g_source (&priv->watch_id);
if (priv->pid) {
nm_utils_kill_child_sync (priv->pid, SIGTERM, _NMLOG_DOMAIN,
priv->progname ?: "<dns-process>", NULL, 1000, 0);
priv->pid = 0;
g_clear_pointer (&priv->progname, g_free);
}
(void) unlink (PIDFILE);
}
priv->running = FALSE;
static void
stop (NMDnsPlugin *plugin)
{
NMDnsDnsmasq *self = NM_DNS_DNSMASQ (plugin);
if (failed)
g_signal_emit_by_name (self, NM_DNS_PLUGIN_FAILED);
kill_child (self);
}
/*****************************************************************************/
......@@ -454,7 +555,10 @@ nm_dns_dnsmasq_new (void)
static void
dispose (GObject *object)
{
NMDnsDnsmasqPrivate *priv = NM_DNS_DNSMASQ_GET_PRIVATE ((NMDnsDnsmasq *) object);
NMDnsDnsmasq *self = NM_DNS_DNSMASQ (object);
NMDnsDnsmasqPrivate *priv = NM_DNS_DNSMASQ_GET_PRIVATE (self);
kill_child (self);
nm_clear_g_cancellable (&priv->dnsmasq_cancellable);
nm_clear_g_cancellable (&priv->update_cancellable);
......@@ -476,6 +580,6 @@ nm_dns_dnsmasq_class_init (NMDnsDnsmasqClass *dns_class)
plugin_class->plugin_name = "dnsmasq";
plugin_class->is_caching = TRUE;
plugin_class->child_quit = child_quit;
plugin_class->stop = stop;
plugin_class->update = update;
}
......@@ -1558,9 +1558,8 @@ update_dns (NMDnsManager *self,
}
static void
plugin_failed (NMDnsPlugin *plugin, gpointer user_data)
plugin_failed (NMDnsManager *self, NMDnsPlugin *plugin)
{
NMDnsManager *self = NM_DNS_MANAGER (user_data);
GError *error = NULL;
/* Errors with non-caching plugins aren't fatal */
......@@ -1590,34 +1589,41 @@ plugin_child_quit_update_dns (gpointer user_data)
}
static void
plugin_child_quit (NMDnsPlugin *plugin, int exit_status, gpointer user_data)
plugin_failed_cb (NMDnsPlugin *plugin, gboolean is_fatal, NMDnsManager *self)
{
NMDnsManager *self = NM_DNS_MANAGER (user_data);
NMDnsManagerPrivate *priv = NM_DNS_MANAGER_GET_PRIVATE (self);
gint64 ts = nm_utils_get_monotonic_timestamp_ms ();
_LOGW ("plugin %s child quit unexpectedly", nm_dns_plugin_get_name (plugin));
if (is_fatal) {
gint64 ts = nm_utils_get_monotonic_timestamp_ms ();
if ( !priv->plugin_ratelimit.ts
|| (ts - priv->plugin_ratelimit.ts) / 1000 > PLUGIN_RATELIMIT_INTERVAL) {
priv->plugin_ratelimit.ts = ts;
priv->plugin_ratelimit.num_restarts = 0;
} else {
priv->plugin_ratelimit.num_restarts++;
if (priv->plugin_ratelimit.num_restarts > PLUGIN_RATELIMIT_BURST) {
plugin_failed (plugin, self);
_LOGW ("plugin %s child respawning too fast, delaying update for %u seconds",
nm_dns_plugin_get_name (plugin), PLUGIN_RATELIMIT_DELAY);
priv->plugin_ratelimit.timer = g_timeout_add_seconds (PLUGIN_RATELIMIT_DELAY,
plugin_child_quit_update_dns,
self);
return;
_LOGW ("plugin %s died unexpectedly", nm_dns_plugin_get_name (plugin));
if ( !priv->plugin_ratelimit.ts
|| (ts - priv->plugin_ratelimit.ts) / 1000 > PLUGIN_RATELIMIT_INTERVAL) {
priv->plugin_ratelimit.ts = ts;
priv->plugin_ratelimit.num_restarts = 0;
} else {
priv->plugin_ratelimit.num_restarts++;
if (priv->plugin_ratelimit.num_restarts > PLUGIN_RATELIMIT_BURST) {
plugin_failed (self, plugin);
_LOGW ("plugin %s child respawning too fast, delaying update for %u seconds",
nm_dns_plugin_get_name (plugin), PLUGIN_RATELIMIT_DELAY);
priv->plugin_ratelimit.timer = g_timeout_add_seconds (PLUGIN_RATELIMIT_DELAY,
plugin_child_quit_update_dns,
self);
return;
}
}
plugin_child_quit_update_dns (self);
return;
}
plugin_child_quit_update_dns (self);
plugin_failed (self, plugin);
}
/*****************************************************************************/
static void
_ip_config_dns_priority_changed (gpointer config,
GParamSpec *pspec,
......@@ -1846,15 +1852,15 @@ _clear_plugin (NMDnsManager *self)
{
NMDnsManagerPrivate *priv = NM_DNS_MANAGER_GET_PRIVATE (self);
priv->plugin_ratelimit.ts = 0;
nm_clear_g_source (&priv->plugin_ratelimit.timer);
if (priv->plugin) {
g_signal_handlers_disconnect_by_func (priv->plugin, plugin_failed, self);
g_signal_handlers_disconnect_by_func (priv->plugin, plugin_child_quit, self);
g_signal_handlers_disconnect_by_func (priv->plugin, plugin_failed_cb, self);
nm_dns_plugin_stop (priv->plugin);
g_clear_object (&priv->plugin);
return TRUE;
}
priv->plugin_ratelimit.ts = 0;
nm_clear_g_source (&priv->plugin_ratelimit.timer);
return FALSE;
}
......@@ -2074,10 +2080,8 @@ again:
systemd_resolved_changed = TRUE;
if ( plugin_changed
&& priv->plugin) {
g_signal_connect (priv->plugin, NM_DNS_PLUGIN_FAILED, G_CALLBACK (plugin_failed), self);
g_signal_connect (priv->plugin, NM_DNS_PLUGIN_CHILD_QUIT, G_CALLBACK (plugin_child_quit), self);
}
&& priv->plugin)
g_signal_connect (priv->plugin, NM_DNS_PLUGIN_FAILED, G_CALLBACK (plugin_failed_cb), self);
g_object_freeze_notify (G_OBJECT (self));
......
......@@ -19,7 +19,6 @@
enum {
FAILED,
CHILD_QUIT,
LAST_SIGNAL,
};
......@@ -94,159 +93,25 @@ nm_dns_plugin_get_name (NMDnsPlugin *self)
return klass->plugin_name;
}
/*****************************************************************************/
static void
_clear_pidfile (NMDnsPlugin *self)
{
NMDnsPluginPrivate *priv = NM_DNS_PLUGIN_GET_PRIVATE (self);
if (priv->pidfile) {
unlink (priv->pidfile);
g_clear_pointer (&priv->pidfile, g_free);
}
}
static void
kill_existing (const char *progname, const char *pidfile, const char *kill_match)
{
long pid;
gs_free char *contents = NULL;
gs_free char *cmdline_contents = NULL;
guint64 start_time;
char proc_path[256];
gs_free_error GError *error = NULL;
if (!pidfile)
return;
if (!kill_match)
g_return_if_reached ();
if (!g_file_get_contents (pidfile, &contents, NULL, &error)) {
if (g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NOENT))
return;
goto out;
}
pid = _nm_utils_ascii_str_to_int64 (contents, 10, 2, INT_MAX, -1);
if (pid == -1)
goto out;
start_time = nm_utils_get_start_time_for_pid (pid, NULL, NULL);
if (start_time == 0)
goto out;
nm_sprintf_buf (proc_path, "/proc/%ld/cmdline", pid);
if (!g_file_get_contents (proc_path, &cmdline_contents, NULL, NULL))
goto out;
if (!strstr (cmdline_contents, kill_match))
goto out;
nm_utils_kill_process_sync (pid, start_time, SIGKILL, _NMLOG_DOMAIN,
progname ?: "<dns-process>",
0, 0, 1000);
out:
unlink (pidfile);
}
static void
watch_cb (GPid pid, int status, gpointer user_data)
{
NMDnsPlugin *self = NM_DNS_PLUGIN (user_data);
NMDnsPluginPrivate *priv = NM_DNS_PLUGIN_GET_PRIVATE (self);
priv->pid = 0;
priv->watch_id = 0;
g_clear_pointer (&priv->progname, g_free);
_clear_pidfile (self);
g_signal_emit (self, signals[CHILD_QUIT], 0, status);
}
GPid
nm_dns_plugin_child_pid (NMDnsPlugin *self)
{
NMDnsPluginPrivate *priv;
g_return_val_if_fail (NM_IS_DNS_PLUGIN (self), 0);
priv = NM_DNS_PLUGIN_GET_PRIVATE (self);
return priv->pid;
}
GPid
nm_dns_plugin_child_spawn (NMDnsPlugin *self,
const char **argv,
const char *pidfile,
const char *kill_match)
{
NMDnsPluginPrivate *priv;
GError *error = NULL;
GPid pid;
gs_free char *cmdline = NULL;
gs_free char *progname = NULL;
g_return_val_if_fail (argv && argv[0], 0);
g_return_val_if_fail (NM_IS_DNS_PLUGIN (self), 0);
priv = NM_DNS_PLUGIN_GET_PRIVATE (self);
g_return_val_if_fail (!priv->pid, 0);
nm_assert (!priv->progname);
nm_assert (!priv->watch_id);
nm_assert (!priv->pidfile);
progname = g_path_get_basename (argv[0]);
kill_existing (progname, pidfile, kill_match);
_LOGI ("starting %s...", progname);
_LOGD ("command line: %s",
(cmdline = g_strjoinv (" ", (char **) argv)));
if (!g_spawn_async (NULL, (char **) argv, NULL,
G_SPAWN_DO_NOT_REAP_CHILD,
nm_utils_setpgid, NULL,
&pid,
&error)) {
_LOGW ("failed to spawn %s: %s",
progname, error->message);
g_clear_error (&error);
return 0;
}
_LOGD ("%s started with pid %d", progname, pid);
priv->watch_id = g_child_watch_add (pid, (GChildWatchFunc) watch_cb, self);
priv->pid = pid;
priv->progname = g_steal_pointer (&progname);
priv->pidfile = g_strdup (pidfile);
return pid;
}
gboolean
nm_dns_plugin_child_kill (NMDnsPlugin *self)
void
nm_dns_plugin_stop (NMDnsPlugin *self)
{
NMDnsPluginPrivate *priv = NM_DNS_PLUGIN_GET_PRIVATE (self);
NMDnsPluginClass *klass;
nm_clear_g_source (&priv->watch_id);
if (priv->pid) {
nm_utils_kill_child_sync (priv->pid, SIGTERM, _NMLOG_DOMAIN,
priv->progname ?: "<dns-process>", NULL, 1000, 0);
priv->pid = 0;
g_clear_pointer (&priv->progname, g_free);
}
_clear_pidfile (self);
g_return_if_fail (NM_IS_DNS_PLUGIN (self));
return TRUE;
klass = NM_DNS_PLUGIN_GET_CLASS (self);
if (klass->stop)
klass->stop (self);
}
void
nm_dns_plugin_stop (NMDnsPlugin *self)
_nm_dns_plugin_emit_failed (gpointer /* NMDnsPlugin * */ self,
gboolean is_fatal)
{
nm_dns_plugin_child_kill (self);
nm_assert (NM_IS_DNS_PLUGIN (self));
g_signal_emit (self, signals[FAILED], 0, (gboolean) (!!is_fatal));
}
/*****************************************************************************/
......@@ -254,17 +119,6 @@ nm_dns_plugin_stop (NMDnsPlugin *self)
static void
nm_dns_plugin_init (NMDnsPlugin *self)
{
self->_priv = G_TYPE_INSTANCE_GET_PRIVATE (self, NM_TYPE_DNS_PLUGIN, NMDnsPluginPrivate);
}
static void
dispose (GObject *object)
{
NMDnsPlugin *self = NM_DNS_PLUGIN (object);
nm_dns_plugin_stop (self);
G_OBJECT_CLASS (nm_dns_plugin_parent_class)->dispose (object);
}
static void
......@@ -272,10 +126,6 @@ nm_dns_plugin_class_init (NMDnsPluginClass *plugin_class)
{
GObjectClass *object_class = G_OBJECT_CLASS (plugin_class);
g_type_class_add_private (plugin_class, sizeof (NMDnsPluginPrivate));
object_class->dispose = dispose;
/* Emitted by the plugin and consumed by NMDnsManager when
* some error happens with the nameserver subprocess. Causes NM to fall
* back to writing out a non-local-caching resolv.conf until the next
......@@ -286,15 +136,6 @@ nm_dns_plugin_class_init (NMDnsPluginClass *plugin_class)
G_OBJECT_CLASS_TYPE (object_class),
G_SIGNAL_RUN_FIRST,
0, NULL, NULL,
g_cclosure_marshal_VOID__VOID,
G_TYPE_NONE, 0);
signals[CHILD_QUIT] =
g_signal_new (NM_DNS_PLUGIN_CHILD_QUIT,
G_OBJECT_CLASS_TYPE (object_class),
G_SIGNAL_RUN_FIRST,
G_STRUCT_OFFSET (NMDnsPluginClass, child_quit),
NULL, NULL,
g_cclosure_marshal_VOID__INT,
G_TYPE_NONE, 1, G_TYPE_INT);
g_cclosure_marshal_VOID__BOOLEAN,
G_TYPE_NONE, 1, G_TYPE_BOOLEAN);
}
......@@ -16,13 +16,9 @@
#define NM_DNS_PLUGIN_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_DNS_PLUGIN, NMDnsPluginClass))
#define NM_DNS_PLUGIN_FAILED "failed"
#define NM_DNS_PLUGIN_CHILD_QUIT "child-quit"
struct _NMDnsPluginPrivate;
typedef struct {
GObject parent;
struct _NMDnsPluginPrivate *_priv;
} NMDnsPlugin;
typedef struct {
......@@ -39,14 +35,9 @@ typedef struct {
const char *hostname,
GError **error);
const char *plugin_name;
void (*stop) (NMDnsPlugin *self);
/* Emitted by the plugin base class when the nameserver subprocess
* quits. This signal is consumed by the plugin subclasses and not
* by NMDnsManager. If the subclass decides the exit status (as returned
* by waitpid(2)) is fatal it should then emit the 'failed' signal.
*/
void (*child_quit) (NMDnsPlugin *self, int status);
const char *plugin_name;
/* Types should set to TRUE if they start a local caching nameserver
* that listens on localhost and would block any other local caching
......@@ -70,21 +61,9 @@ gboolean nm_dns_plugin_update (NMDnsPlugin *self,
void nm_dns_plugin_stop (NMDnsPlugin *self);
/* For subclasses/plugins */
/* Spawn a child process and watch for it to quit. 'argv' is the NULL-terminated
* argument vector to spawn the child with, where argv[0] is the full path to
* the child's executable. If 'pidfile' is given the process owning the PID
* contained in 'pidfile' will be killed if its command line matches 'kill_match'
* and the pidfile will be deleted.
*/
GPid nm_dns_plugin_child_spawn (NMDnsPlugin *self,
const char **argv,
const char *pidfile,
const char *kill_match);
GPid nm_dns_plugin_child_pid (NMDnsPlugin *self);
/*****************************************************************************/
gboolean nm_dns_plugin_child_kill (NMDnsPlugin *self);
void _nm_dns_plugin_emit_failed (gpointer /* NMDnsPlugin * */ self,
gboolean is_fatal);
#endif /* __NM_DNS_PLUGIN_H__ */
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment