merge: branch 'bg/dns-update-on-sigusr1'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/832
(cherry picked from commit a3306d538c)

clients/cli/connections.c

@@ -19,6 +19,7 @@
 #include "nm-vpn-helpers.h"
 #include "nm-meta-setting-access.h"
 #include "nm-secret-agent-simple.h"
+#include "nm-glib-aux/nm-dbus-aux.h"
 
 #include "utils.h"
 #include "common.h"
@@ -9184,28 +9185,42 @@ do_connection_monitor(const NMCCommand *cmd, NmCli *nmc, int argc, const char *c
 }
 
 static void
-do_connection_reload(const NMCCommand *cmd, NmCli *nmc, int argc, const char *const *argv)
+connection_reload_cb(GObject *source, GAsyncResult *result, gpointer user_data)
 {
-    gs_unref_variant GVariant *result = NULL;
+    NmCli *       nmc              = user_data;
-    gs_free_error GError *error       = NULL;
+    gs_free_error GError *error    = NULL;
+    gs_unref_variant GVariant *ret = NULL;
 
-    next_arg(nmc, &argc, &argv, NULL);
+    ret = nm_dbus_call_finish(result, &error);
-    if (nmc->complete)
-        return;
-
-    result = nmc_dbus_call_sync(nmc,
-                                "/org/freedesktop/NetworkManager/Settings",
-                                "org.freedesktop.NetworkManager.Settings",
-                                "ReloadConnections",
-                                g_variant_new("()"),
-                                G_VARIANT_TYPE("(b)"),
-                                &error);
     if (error) {
         g_string_printf(nmc->return_text,
                         _("Error: failed to reload connections: %s."),
                         nmc_error_get_simple_message(error));
         nmc->return_value = NMC_RESULT_ERROR_UNKNOWN;
     }
+
+    quit();
+}
+
+static void
+do_connection_reload(const NMCCommand *cmd, NmCli *nmc, int argc, const char *const *argv)
+{
+    next_arg(nmc, &argc, &argv, NULL);
+    if (nmc->complete)
+        return;
+
+    nmc->should_wait++;
+    nm_dbus_call(G_BUS_TYPE_SYSTEM,
+                 NM_DBUS_SERVICE,
+                 NM_DBUS_PATH_SETTINGS,
+                 NM_DBUS_INTERFACE_SETTINGS,
+                 "ReloadConnections",
+                 g_variant_new("()"),
+                 G_VARIANT_TYPE("(b)"),
+                 NULL,
+                 (nmc->timeout == -1 ? 90 : nmc->timeout) * 1000,
+                 connection_reload_cb,
+                 nmc);
 }
 
 static void

clients/cli/general.c

@@ -10,6 +10,7 @@
 #include "nm-libnm-core-intern/nm-common-macros.h"
 
 #include "nm-client-utils.h"
+#include "nm-glib-aux/nm-dbus-aux.h"
 
 #include "polkit-agent.h"
 #include "utils.h"
@@ -344,7 +345,8 @@ usage_general_reload(void)
                  "                can be reloaded through 'nmcli connection reload' instead.\n"
                  "\n"
                  "  'dns-rc'      Update DNS configuration, which usually involves writing\n"
-                 "                /etc/resolv.conf anew.\n"
+                 "                /etc/resolv.conf anew. This is equivalent to sending the\n"
+                 "                SIGUSR1 signal to the NetworkManager process.\n"
                  "\n"
                  "  'dns-full'    Restart the DNS plugin. This is for example useful when\n"
                  "                using dnsmasq plugin, which uses additional configuration\n"
@@ -598,15 +600,31 @@ show_nm_permissions(NmCli *nmc)
     return TRUE;
 }
 
+static void
+reload_cb(GObject *source, GAsyncResult *result, gpointer user_data)
+{
+    NmCli *       nmc              = user_data;
+    gs_free_error GError *error    = NULL;
+    gs_unref_variant GVariant *ret = NULL;
+
+    ret = nm_dbus_call_finish(result, &error);
+    if (error) {
+        g_string_printf(nmc->return_text,
+                        _("Error: failed to reload: %s"),
+                        nmc_error_get_simple_message(error));
+        nmc->return_value = NMC_RESULT_ERROR_UNKNOWN;
+    }
+
+    quit();
+}
+
 static void
 do_general_reload(const NMCCommand *cmd, NmCli *nmc, int argc, const char *const *argv)
 {
-    gs_unref_variant GVariant *result = NULL;
+    gs_free const char **values    = NULL;
-    gs_free_error GError *error       = NULL;
+    gs_free char *       err_token = NULL;
-    gs_free const char ** values      = NULL;
+    gs_free char *       joined    = NULL;
-    gs_free char *        err_token   = NULL;
+    int                  flags     = 0;
-    gs_free char *        joined      = NULL;
-    int                   flags       = 0;
 
     next_arg(nmc, &argc, &argv, NULL);
 
@@ -649,20 +667,18 @@ do_general_reload(const NMCCommand *cmd, NmCli *nmc, int argc, const char *const
         return;
     }
 
-    result = nmc_dbus_call_sync(nmc,
+    nmc->should_wait++;
-                                "/org/freedesktop/NetworkManager",
+    nm_dbus_call(G_BUS_TYPE_SYSTEM,
-                                "org.freedesktop.NetworkManager",
+                 NM_DBUS_SERVICE,
-                                "Reload",
+                 NM_DBUS_PATH,
-                                g_variant_new("(u)", flags),
+                 NM_DBUS_INTERFACE,
-                                G_VARIANT_TYPE("()"),
+                 "Reload",
-                                &error);
+                 g_variant_new("(u)", flags),
-
+                 G_VARIANT_TYPE("()"),
-    if (error) {
+                 NULL,
-        g_string_printf(nmc->return_text,
+                 (nmc->timeout == -1 ? 90 : nmc->timeout) * 1000,
-                        _("Error: failed to reload: %s"),
+                 reload_cb,
-                        nmc_error_get_simple_message(error));
+                 nmc);
-        nmc->return_value = NMC_RESULT_ERROR_UNKNOWN;
-    }
 }
 
 static void

man/NetworkManager.xml

@@ -475,9 +475,17 @@
         <varlistentry>
           <term><varname>SIGUSR1</varname></term>
           <listitem><para>
-            The signal forces a rewrite of DNS configuration. Contrary to
+            The signal forces a rewrite of DNS configuration. Contrary
-            SIGHUP, this does not restart the DNS plugin and will not interrupt
+            to SIGHUP, this does not restart the DNS plugin and will not
-            name resolution.
+            interrupt name resolution.
+
+            When NetworkManager is not managing DNS, the signal forces
+            a restart of operations that depend on the DNS
+            configuration (like the resolution of the system hostname
+            via reverse DNS, or the resolution of WireGuard peers);
+            therefore, it can be used to tell NetworkManager that the
+            content of resolv.conf was changed externally.
+
             In the future, further actions may be added. A SIGUSR1
             means to write out data like resolv.conf, or refresh a cache.
             It is a subset of what is done for SIGHUP without reloading

man/nmcli.xml

@@ -356,6 +356,7 @@
         <arg choice='plain'><command>hostname</command></arg>
         <arg choice='plain'><command>permissions</command></arg>
         <arg choice='plain'><command>logging</command></arg>
+        <arg choice='plain'><command>reload</command></arg>
       </group>
       <arg rep='repeat'><replaceable>ARGUMENTS</replaceable></arg>
     </cmdsynopsis>
@@ -419,6 +420,66 @@
           for available level and domain values.</para>
         </listitem>
       </varlistentry>
+
+      <varlistentry>
+        <term>
+          <command>reload</command>
+          <arg rep='repeat'><replaceable>flags</replaceable></arg>
+        </term>
+
+        <listitem>
+          <para>
+            Reload NetworkManager's configuration and perform certain
+            updates, like flushing caches or rewriting external state to
+            disk. This is similar to sending SIGHUP to NetworkManager
+            but it allows for more fine-grained control over what to
+            reload through the flags argument. It also allows non-root
+            access via PolicyKit and contrary to signals it is
+            synchronous. Available flags are:
+          </para>
+          <variablelist>
+            <varlistentry>
+              <term><option>conf</option></term>
+              <listitem><para>
+                Reload the NetworkManager.conf configuration from
+                disk. Note that this does not include connections, which
+                can be reloaded through <command>nmcli connection
+                reload</command> instead.
+              </para></listitem>
+            </varlistentry>
+
+            <varlistentry>
+              <term><option>dns-rc</option></term>
+              <listitem><para>
+                Update DNS configuration, which usually involves writing
+                /etc/resolv.conf anew. This is equivalent to sending the
+                SIGUSR1 signal to the NetworkManager process.
+              </para></listitem>
+            </varlistentry>
+
+            <varlistentry>
+              <term><option>dns-full</option></term>
+              <listitem><para>
+                Restart the DNS plugin. This is for example useful when
+                using dnsmasq plugin, which uses additional
+                configuration in
+                <filename>/etc/NetworkManager/dnsmasq.d</filename>. If
+                you edit those files, you can restart the DNS
+                plugin. This action shortly interrupts name resolution.
+              </para></listitem>
+            </varlistentry>
+          </variablelist>
+          <para>
+            With no flags, everything that is supported is reloaded,
+            which is identical to sending a SIGHUP. See
+            <citerefentry>
+              <refentrytitle>NetworkManager</refentrytitle>
+              <manvolnum>8</manvolnum>
+            </citerefentry>
+            for more details about signals.
+          </para>
+        </listitem>
+      </varlistentry>
     </variablelist>
   </refsect1>
 

shared/nm-glib-aux/nm-dbus-aux.c

@@ -268,6 +268,119 @@ nm_dbus_connection_call_finish_variant_strip_dbus_error_cb(GObject *     source,
 
 /*****************************************************************************/
 
+typedef struct {
+    char *              bus_name;
+    char *              object_path;
+    char *              interface_name;
+    char *              method_name;
+    GVariant *          parameters;
+    GDBusConnection *   connection;
+    const GVariantType *reply_type;
+    int                 timeout_msec;
+} CallAsyncInfo;
+
+static void
+call_async_info_destroy(CallAsyncInfo *info)
+{
+    g_free(info->bus_name);
+    g_free(info->object_path);
+    g_free(info->interface_name);
+    g_free(info->method_name);
+    g_variant_unref(info->parameters);
+    nm_g_object_unref(info->connection);
+    g_free(info);
+}
+
+static void
+call_cb(GObject *source, GAsyncResult *result, gpointer user_data)
+{
+    gs_unref_object GTask *task  = user_data;
+    GError *               error = NULL;
+    GVariant *             ret;
+
+    ret = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), result, &error);
+    if (!ret) {
+        g_task_return_error(task, error);
+        return;
+    }
+
+    g_task_return_pointer(task, ret, (GDestroyNotify) g_variant_unref);
+}
+
+static void
+call_bus_get_cb(GObject *source, GAsyncResult *result, gpointer user_data)
+{
+    gs_unref_object GTask *task = user_data;
+    GCancellable *         cancellable;
+    CallAsyncInfo *        info;
+    GError *               error = NULL;
+
+    info             = g_task_get_task_data(task);
+    info->connection = g_bus_get_finish(result, &error);
+    cancellable      = g_task_get_cancellable(task);
+
+    if (!info->connection) {
+        g_task_return_error(task, g_steal_pointer(&error));
+        return;
+    }
+
+    g_dbus_connection_call(info->connection,
+                           info->bus_name,
+                           info->object_path,
+                           info->interface_name,
+                           info->method_name,
+                           info->parameters,
+                           info->reply_type,
+                           G_DBUS_CALL_FLAGS_NONE,
+                           info->timeout_msec,
+                           cancellable,
+                           call_cb,
+                           g_steal_pointer(&task));
+}
+
+void
+nm_dbus_call(GBusType            bus_type,
+             const char *        bus_name,
+             const char *        object_path,
+             const char *        interface_name,
+             const char *        method_name,
+             GVariant *          parameters,
+             const GVariantType *reply_type,
+             GCancellable *      cancellable,
+             int                 timeout_msec,
+             GAsyncReadyCallback callback,
+             gpointer            user_data)
+{
+    GTask *        task;
+    CallAsyncInfo *info;
+
+    info  = g_new(CallAsyncInfo, 1);
+    *info = (CallAsyncInfo){
+        .bus_name       = g_strdup(bus_name),
+        .object_path    = g_strdup(object_path),
+        .interface_name = g_strdup(interface_name),
+        .method_name    = g_strdup(method_name),
+        .parameters     = g_variant_ref_sink(parameters),
+        .reply_type     = reply_type,
+        .timeout_msec   = timeout_msec,
+    };
+
+    task = nm_g_task_new(NULL, cancellable, nm_dbus_call, callback, user_data);
+    g_task_set_task_data(task, info, (GDestroyNotify) call_async_info_destroy);
+
+    g_bus_get(bus_type, cancellable, call_bus_get_cb, task);
+}
+
+GVariant *
+nm_dbus_call_finish(GAsyncResult *result, GError **error)
+{
+    nm_assert(nm_g_task_is_valid(result, NULL, nm_dbus_call));
+
+    return g_task_propagate_pointer(G_TASK(result), error);
+}
+
+/*****************************************************************************/
+
 gboolean
 _nm_dbus_error_is(GError *error, ...)
 {

shared/nm-glib-aux/nm-dbus-aux.h

@@ -186,6 +186,22 @@ void nm_dbus_connection_call_finish_variant_strip_dbus_error_cb(GObject *     so
 
 /*****************************************************************************/
 
+void nm_dbus_call(GBusType            bus_type,
+                  const char *        bus_name,
+                  const char *        object_path,
+                  const char *        interface_name,
+                  const char *        method_name,
+                  GVariant *          parameters,
+                  const GVariantType *reply_type,
+                  GCancellable *      cancellable,
+                  int                 timeout_msec,
+                  GAsyncReadyCallback callback,
+                  gpointer            user_data);
+
+GVariant *nm_dbus_call_finish(GAsyncResult *result, GError **error);
+
+/*****************************************************************************/
+
 gboolean _nm_dbus_error_is(GError *error, ...) G_GNUC_NULL_TERMINATED;
 
 #define nm_dbus_error_is(error, ...)                           \

src/core/dns/nm-dns-manager.c

@@ -1624,7 +1624,7 @@ _mgr_configs_data_clear(NMDnsManager *self)
 /*****************************************************************************/
 
 static gboolean
-update_dns(NMDnsManager *self, gboolean no_caching, GError **error)
+update_dns(NMDnsManager *self, gboolean no_caching, gboolean force_emit, GError **error)
 {
     NMDnsManagerPrivate *priv                = NM_DNS_MANAGER_GET_PRIVATE(self);
     const char *         nis_domain          = NULL;
@@ -1816,7 +1816,7 @@ plugin_skip:;
     }
 
     /* signal that DNS resolution configs were changed */
-    if ((do_update || caching) && result == SR_SUCCESS)
+    if ((do_update || caching || force_emit) && result == SR_SUCCESS)
         g_signal_emit(self, signals[CONFIG_CHANGED], 0);
 
     nm_clear_pointer(&priv->config_variant, g_variant_unref);
@@ -1924,7 +1924,7 @@ changed:
     if (!priv->updates_queue) {
         gs_free_error GError *error = NULL;
 
-        if (!update_dns(self, FALSE, &error))
+        if (!update_dns(self, FALSE, FALSE, &error))
             _LOGW("could not commit DNS changes: %s", error->message);
     }
 
@@ -1965,7 +1965,7 @@ nm_dns_manager_set_hostname(NMDnsManager *self, const char *hostname, gboolean s
     if (!priv->updates_queue) {
         gs_free_error GError *error = NULL;
 
-        if (!update_dns(self, FALSE, &error))
+        if (!update_dns(self, FALSE, FALSE, &error))
             _LOGW("could not commit DNS changes: %s", error->message);
     }
 }
@@ -2012,7 +2012,7 @@ nm_dns_manager_end_updates(NMDnsManager *self, const char *func)
 
     /* Commit all the outstanding changes */
     _LOGD("(%s): committing DNS changes (%d)", func, priv->updates_queue);
-    if (!update_dns(self, FALSE, &error))
+    if (!update_dns(self, FALSE, FALSE, &error))
         _LOGW("could not commit DNS changes: %s", error->message);
 
     memset(priv->prev_hash, 0, sizeof(priv->prev_hash));
@@ -2038,7 +2038,7 @@ nm_dns_manager_stop(NMDnsManager *self)
     if (priv->dns_touched && priv->plugin && NM_IS_DNS_DNSMASQ(priv->plugin)) {
         gs_free_error GError *error = NULL;
 
-        if (!update_dns(self, TRUE, &error))
+        if (!update_dns(self, TRUE, FALSE, &error))
             _LOGW("could not commit DNS changes on shutdown: %s", error->message);
 
         priv->dns_touched = FALSE;
@@ -2363,7 +2363,7 @@ config_changed_cb(NMConfig *          config,
                          | NM_CONFIG_CHANGE_GLOBAL_DNS_CONFIG)) {
         gs_free_error GError *error = NULL;
 
-        if (!update_dns(self, FALSE, &error))
+        if (!update_dns(self, FALSE, TRUE, &error))
             _LOGW("could not commit DNS changes: %s", error->message);
     }
 }