diff options
author | Dan Williams <dcbw@redhat.com> | 2015-03-27 16:19:33 -0500 |
---|---|---|
committer | Dan Williams <dcbw@redhat.com> | 2015-03-27 16:19:33 -0500 |
commit | a4c3973a842b6b6d7f9f2c2b608ffb61e2700f2e (patch) | |
tree | 042d18377a3c3aea5cb0479bdf3f53862573bde9 | |
parent | d997cde995ecf8b75ac46f3c63abd13956262782 (diff) | |
parent | 09a05f6c3e0b4502252d70cb121654e7312520c5 (diff) | |
download | NetworkManager-a4c3973a842b6b6d7f9f2c2b608ffb61e2700f2e.tar.gz |
merge: respawn DNS plugin children if they quit unexpectedly (bgo #728342)
-rw-r--r-- | src/NetworkManagerUtils.c | 19 | ||||
-rw-r--r-- | src/NetworkManagerUtils.h | 2 | ||||
-rw-r--r-- | src/dns-manager/nm-dns-manager.c | 196 | ||||
-rw-r--r-- | src/dns-manager/nm-dns-plugin.c | 5 | ||||
-rw-r--r-- | src/dns-manager/nm-dns-unbound.c | 2 |
5 files changed, 131 insertions, 93 deletions
diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c index f28b783e58..66108afe2b 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -152,27 +152,26 @@ nm_utils_ip6_address_clear_host_address (struct in6_addr *dst, const struct in6_ int -nm_spawn_process (const char *args) +nm_spawn_process (const char *args, GError **error) { + GError *local = NULL; gint num_args; char **argv = NULL; int status = -1; - GError *error = NULL; g_return_val_if_fail (args != NULL, -1); + g_return_val_if_fail (!error || !*error, -1); - if (!g_shell_parse_argv (args, &num_args, &argv, &error)) { - nm_log_warn (LOGD_CORE, "could not parse arguments for '%s': %s", args, error->message); - g_error_free (error); - return -1; + if (g_shell_parse_argv (args, &num_args, &argv, &local)) { + g_spawn_sync ("/", argv, NULL, 0, NULL, NULL, NULL, NULL, &status, &local); + g_strfreev (argv); } - if (!g_spawn_sync ("/", argv, NULL, 0, NULL, NULL, NULL, NULL, &status, &error)) { - nm_log_warn (LOGD_CORE, "could not spawn process '%s': %s", args, error->message); - g_error_free (error); + if (local) { + nm_log_warn (LOGD_CORE, "could not spawn process '%s': %s", args, local->message); + g_propagate_error (error, local); } - g_strfreev (argv); return status; } diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h index 11dc496b4f..b74fd1cbf3 100644 --- a/src/NetworkManagerUtils.h +++ b/src/NetworkManagerUtils.h @@ -49,7 +49,7 @@ nm_utils_ip6_route_metric_normalize (guint32 metric) return metric ? metric : 1024 /*NM_PLATFORM_ROUTE_METRIC_DEFAULT_IP6*/; } -int nm_spawn_process (const char *args); +int nm_spawn_process (const char *args, GError **error); int nm_utils_modprobe (GError **error, const char *arg1, ...) G_GNUC_NULL_TERMINATED; diff --git a/src/dns-manager/nm-dns-manager.c b/src/dns-manager/nm-dns-manager.c index f9a4fd44ae..2b8fb7e6eb 100644 --- a/src/dns-manager/nm-dns-manager.c +++ b/src/dns-manager/nm-dns-manager.c @@ -310,12 +310,19 @@ dispatch_netconfig (char **searches, again: - ret = waitpid (pid, NULL, 0); - if (ret < 0 && errno == EINTR) - goto again; - else if (ret < 0 && errno == ECHILD) { - /* When the netconfig exist, the errno is ECHILD, it should return TRUE */ - return TRUE; + if (waitpid (pid, NULL, 0) < 0) { + if (errno == EINTR) + goto again; + else if (errno == ECHILD) { + /* child already exited */ + ret = pid; + } else { + g_set_error_literal (error, + NM_MANAGER_ERROR, + NM_MANAGER_ERROR_FAILED, + "Error waiting for netconfig to exit: %s", + strerror (errno)); + } } return ret > 0; @@ -331,22 +338,12 @@ write_resolv_conf (FILE *f, { char *searches_str = NULL; char *nameservers_str = NULL; - int i; gboolean retval = FALSE; + char *tmp_str; GString *str; - - if (fprintf (f, "%s","# Generated by NetworkManager\n") < 0) { - g_set_error (error, - NM_MANAGER_ERROR, - NM_MANAGER_ERROR_FAILED, - "Could not write " _PATH_RESCONF ": %s\n", - g_strerror (errno)); - return FALSE; - } + int i; if (searches) { - char *tmp_str; - tmp_str = g_strjoinv (" ", searches); searches_str = g_strconcat ("search ", tmp_str, "\n", NULL); g_free (tmp_str); @@ -374,10 +371,17 @@ write_resolv_conf (FILE *f, nameservers_str = g_string_free (str, FALSE); - if (fprintf (f, "%s%s", + if (fprintf (f, "# Generated by NetworkManager\n%s%s", searches_str ? searches_str : "", - strlen (nameservers_str) ? nameservers_str : "") != -1) + nameservers_str) > 0) retval = TRUE; + else { + g_set_error (error, + NM_MANAGER_ERROR, + NM_MANAGER_ERROR_FAILED, + "Could not write " _PATH_RESCONF ": %s\n", + g_strerror (errno)); + } g_free (searches_str); g_free (nameservers_str); @@ -394,9 +398,15 @@ dispatch_resolvconf (char **searches, char *cmd; FILE *f; gboolean retval = FALSE; + int errnosv, err; - if (! g_file_test (RESOLVCONF_PATH, G_FILE_TEST_IS_EXECUTABLE)) + if (!g_file_test (RESOLVCONF_PATH, G_FILE_TEST_IS_EXECUTABLE)) { + g_set_error_literal (error, + NM_MANAGER_ERROR, + NM_MANAGER_ERROR_FAILED, + RESOLVCONF_PATH " is not executable"); return FALSE; + } if (searches || nameservers) { cmd = g_strconcat (RESOLVCONF_PATH, " -a ", "NetworkManager", NULL); @@ -410,12 +420,21 @@ dispatch_resolvconf (char **searches, g_strerror (errno)); else { retval = write_resolv_conf (f, searches, nameservers, error); - retval &= (pclose (f) == 0); + err = pclose (f); + if (err < 0) { + errnosv = errno; + g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errnosv), + "Failed to close pipe to resolvconf: %d", errnosv); + retval = FALSE; + } else if (err > 0) { + nm_log_warn (LOGD_DNS, "resolvconf failed with status %d", err); + retval = FALSE; + } } } else { cmd = g_strconcat (RESOLVCONF_PATH, " -d ", "NetworkManager", NULL); nm_log_info (LOGD_DNS, "Removing DNS information from %s", RESOLVCONF_PATH); - if (nm_spawn_process (cmd) == 0) + if (nm_spawn_process (cmd, error) == 0) retval = TRUE; } @@ -585,6 +604,42 @@ compute_hash (NMDnsManager *self, guint8 buffer[HASH_LEN]) g_checksum_free (sum); } +static void +build_plugin_config_lists (NMDnsManager *self, + GSList **out_vpn_configs, + GSList **out_dev_configs, + GSList **out_other_configs) +{ + NMDnsManagerPrivate *priv = NM_DNS_MANAGER_GET_PRIVATE (self); + GSList *iter; + + g_return_if_fail (out_vpn_configs && !*out_vpn_configs); + g_return_if_fail (out_dev_configs && !*out_dev_configs); + g_return_if_fail (out_other_configs && !*out_other_configs); + + /* Build up config lists for plugins; we use the raw configs here, not the + * merged information that we write to resolv.conf so that the plugins can + * still use the domain information in each config to provide split DNS if + * they want to. + */ + if (priv->ip4_vpn_config) + *out_vpn_configs = g_slist_append (*out_vpn_configs, priv->ip4_vpn_config); + if (priv->ip6_vpn_config) + *out_vpn_configs = g_slist_append (*out_vpn_configs, priv->ip6_vpn_config); + if (priv->ip4_device_config) + *out_dev_configs = g_slist_append (*out_dev_configs, priv->ip4_device_config); + if (priv->ip6_device_config) + *out_dev_configs = g_slist_append (*out_dev_configs, priv->ip6_device_config); + + for (iter = priv->configs; iter; iter = g_slist_next (iter)) { + if ( (iter->data != priv->ip4_vpn_config) + && (iter->data != priv->ip4_device_config) + && (iter->data != priv->ip6_vpn_config) + && (iter->data != priv->ip6_device_config)) + *out_other_configs = g_slist_append (*out_other_configs, iter->data); + } +} + static gboolean update_dns (NMDnsManager *self, gboolean no_caching, @@ -592,7 +647,7 @@ update_dns (NMDnsManager *self, { NMDnsManagerPrivate *priv; NMResolvConfData rc; - GSList *iter, *vpn_configs = NULL, *dev_configs = NULL, *other_configs = NULL; + GSList *iter; const char *nis_domain = NULL; char **searches = NULL; char **nameservers = NULL; @@ -600,8 +655,7 @@ update_dns (NMDnsManager *self, int num, i, len; gboolean success = FALSE, caching = FALSE; - g_return_val_if_fail (error != NULL, FALSE); - g_return_val_if_fail (*error == NULL, FALSE); + g_return_val_if_fail (!error || !*error, FALSE); priv = NM_DNS_MANAGER_GET_PRIVATE (self); @@ -698,32 +752,11 @@ update_dns (NMDnsManager *self, nis_domain = rc.nis_domain; - /* Build up config lists for plugins; we use the raw configs here, not the - * merged information that we write to resolv.conf so that the plugins can - * still use the domain information in each config to provide split DNS if - * they want to. - */ - if (priv->ip4_vpn_config) - vpn_configs = g_slist_append (vpn_configs, priv->ip4_vpn_config); - if (priv->ip6_vpn_config) - vpn_configs = g_slist_append (vpn_configs, priv->ip6_vpn_config); - if (priv->ip4_device_config) - dev_configs = g_slist_append (dev_configs, priv->ip4_device_config); - if (priv->ip6_device_config) - dev_configs = g_slist_append (dev_configs, priv->ip6_device_config); - - for (iter = priv->configs; iter; iter = g_slist_next (iter)) { - if ( (iter->data != priv->ip4_vpn_config) - && (iter->data != priv->ip4_device_config) - && (iter->data != priv->ip6_vpn_config) - && (iter->data != priv->ip6_device_config)) - other_configs = g_slist_append (other_configs, iter->data); - } - /* Let any plugins do their thing first */ if (priv->plugin) { NMDnsPlugin *plugin = priv->plugin; const char *plugin_name = nm_dns_plugin_get_name (plugin); + GSList *vpn_configs = NULL, *dev_configs = NULL, *other_configs = NULL; if (nm_dns_plugin_is_caching (plugin)) { if (no_caching) { @@ -734,6 +767,8 @@ update_dns (NMDnsManager *self, caching = TRUE; } + build_plugin_config_lists (self, &vpn_configs, &dev_configs, &other_configs); + nm_log_dbg (LOGD_DNS, "DNS: updating plugin %s", plugin_name); if (!nm_dns_plugin_update (plugin, vpn_configs, @@ -747,15 +782,14 @@ update_dns (NMDnsManager *self, */ caching = FALSE; } + g_slist_free (vpn_configs); + g_slist_free (dev_configs); + g_slist_free (other_configs); skip: ; } - g_slist_free (vpn_configs); - g_slist_free (dev_configs); - g_slist_free (other_configs); - /* If caching was successful, we only send 127.0.0.1 to /etc/resolv.conf * to ensure that the glibc resolver doesn't try to round-robin nameservers, * but only uses the local caching nameserver. @@ -807,9 +841,23 @@ plugin_failed (NMDnsPlugin *plugin, gpointer user_data) /* Disable caching until the next DNS update */ if (!update_dns (self, TRUE, &error)) { - nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s", - error ? error->code : -1, - error && error->message ? error->message : "(unknown)"); + nm_log_warn (LOGD_DNS, "could not commit DNS changes: %s", error->message); + g_clear_error (&error); + } +} + +static void +plugin_child_quit (NMDnsPlugin *plugin, int exit_status, gpointer user_data) +{ + NMDnsManager *self = NM_DNS_MANAGER (user_data); + GError *error = NULL; + + nm_log_warn (LOGD_DNS, "DNS: plugin %s child quit unexpectedly; refreshing DNS", + nm_dns_plugin_get_name (plugin)); + + /* Let the plugin try to spawn the child again */ + if (!update_dns (self, FALSE, &error)) { + nm_log_warn (LOGD_DNS, "could not commit DNS changes: %s", error->message); g_clear_error (&error); } } @@ -846,9 +894,7 @@ nm_dns_manager_add_ip4_config (NMDnsManager *mgr, priv->configs = g_slist_append (priv->configs, g_object_ref (config)); if (!priv->updates_queue && !update_dns (mgr, FALSE, &error)) { - nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s", - error ? error->code : -1, - error && error->message ? error->message : "(unknown)"); + nm_log_warn (LOGD_DNS, "could not commit DNS changes: %s", error->message); g_clear_error (&error); } @@ -880,9 +926,7 @@ nm_dns_manager_remove_ip4_config (NMDnsManager *mgr, NMIP4Config *config) g_object_unref (config); if (!priv->updates_queue && !update_dns (mgr, FALSE, &error)) { - nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s", - error ? error->code : -1, - error && error->message ? error->message : "(unknown)"); + nm_log_warn (LOGD_DNS, "could not commit DNS changes: %s", error->message); g_clear_error (&error); } @@ -923,9 +967,7 @@ nm_dns_manager_add_ip6_config (NMDnsManager *mgr, priv->configs = g_slist_append (priv->configs, g_object_ref (config)); if (!priv->updates_queue && !update_dns (mgr, FALSE, &error)) { - nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s", - error ? error->code : -1, - error && error->message ? error->message : "(unknown)"); + nm_log_warn (LOGD_DNS, "could not commit DNS changes: %s", error->message); g_clear_error (&error); } @@ -957,9 +999,7 @@ nm_dns_manager_remove_ip6_config (NMDnsManager *mgr, NMIP6Config *config) g_object_unref (config); if (!priv->updates_queue && !update_dns (mgr, FALSE, &error)) { - nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s", - error ? error->code : -1, - error && error->message ? error->message : "(unknown)"); + nm_log_warn (LOGD_DNS, "could not commit DNS changes: %s", error->message); g_clear_error (&error); } @@ -1001,9 +1041,7 @@ nm_dns_manager_set_hostname (NMDnsManager *mgr, priv->hostname = g_strdup (filtered); if (!priv->updates_queue && !update_dns (mgr, FALSE, &error)) { - nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s", - error ? error->code : -1, - error && error->message ? error->message : "(unknown)"); + nm_log_warn (LOGD_DNS, "could not commit DNS changes: %s", error->message); g_clear_error (&error); } } @@ -1057,9 +1095,7 @@ nm_dns_manager_end_updates (NMDnsManager *mgr, const char *func) /* Commit all the outstanding changes */ nm_log_dbg (LOGD_DNS, "(%s): committing DNS changes (%d)", func, priv->updates_queue); if (!update_dns (mgr, FALSE, &error)) { - nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s", - error ? error->code : -1, - error && error->message ? error->message : "(unknown)"); + nm_log_warn (LOGD_DNS, "could not commit DNS changes: %s", error->message); g_clear_error (&error); } @@ -1111,6 +1147,7 @@ init_resolv_conf_mode (NMDnsManager *self) if (priv->plugin) { nm_log_info (LOGD_DNS, "DNS: loaded plugin %s", nm_dns_plugin_get_name (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); } } @@ -1128,8 +1165,7 @@ config_changed_cb (NMConfig *config, init_resolv_conf_mode (self); if (!update_dns (self, TRUE, &error)) { - nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s", - error->code, error->message); + nm_log_warn (LOGD_DNS, "could not commit DNS changes: %s", error->message); g_clear_error (&error); } } @@ -1157,7 +1193,11 @@ dispose (GObject *object) NMDnsManagerPrivate *priv = NM_DNS_MANAGER_GET_PRIVATE (self); GError *error = NULL; - g_clear_object (&priv->plugin); + 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_clear_object (&priv->plugin); + } /* If we're quitting, leave a valid resolv.conf in place, not one * pointing to 127.0.0.1 if any plugins were active. Thus update @@ -1165,9 +1205,7 @@ dispose (GObject *object) * DNS updates yet, there's no reason to touch resolv.conf on shutdown. */ if (priv->dns_touched && !update_dns (self, TRUE, &error)) { - nm_log_warn (LOGD_DNS, "could not commit DNS changes on shutdown: (%d) %s", - error ? error->code : -1, - error && error->message ? error->message : "(unknown)"); + nm_log_warn (LOGD_DNS, "could not commit DNS changes on shutdown: %s", error->message); g_clear_error (&error); priv->dns_touched = FALSE; } diff --git a/src/dns-manager/nm-dns-plugin.c b/src/dns-manager/nm-dns-plugin.c index 3d593cce8d..5b95d1c50a 100644 --- a/src/dns-manager/nm-dns-plugin.c +++ b/src/dns-manager/nm-dns-plugin.c @@ -183,7 +183,8 @@ nm_dns_plugin_child_spawn (NMDnsPlugin *self, return priv->pid; } -gboolean nm_dns_plugin_child_kill (NMDnsPlugin *self) +gboolean +nm_dns_plugin_child_kill (NMDnsPlugin *self) { NMDnsPluginPrivate *priv = NM_DNS_PLUGIN_GET_PRIVATE (self); @@ -193,7 +194,7 @@ gboolean nm_dns_plugin_child_kill (NMDnsPlugin *self) } if (priv->pid) { - nm_utils_kill_child_async (priv->pid, SIGTERM, LOGD_DNS, priv->progname, 2000, NULL, NULL); + nm_utils_kill_child_sync (priv->pid, SIGTERM, LOGD_DNS, priv->progname, NULL, 1000, 0); priv->pid = 0; g_free (priv->progname); priv->progname = NULL; diff --git a/src/dns-manager/nm-dns-unbound.c b/src/dns-manager/nm-dns-unbound.c index 533290c11d..5723bf8229 100644 --- a/src/dns-manager/nm-dns-unbound.c +++ b/src/dns-manager/nm-dns-unbound.c @@ -42,7 +42,7 @@ update (NMDnsPlugin *plugin, * without calling custom scripts. The dnssec-trigger functionality * may be eventually merged into NetworkManager. */ - return nm_spawn_process ("/usr/libexec/dnssec-trigger-script --async --update") == 0; + return nm_spawn_process ("/usr/libexec/dnssec-trigger-script --async --update", NULL) == 0; } static gboolean |