> dns: move rate limiting to NMDnsPlugin

+       /* Lets the subclass know * when the nameserver subprocess

Extra '*'.


> dns: replace the FAILED and CHILD_QUIT signals with a STATE property

+       g_object_class_install_property (object_class, PROP_STATE,
+           g_param_spec_boolean (NM_DNS_PLUGIN_STATE, "", "", FALSE,
+                                 G_PARAM_READABLE | G_PARAM_STATIC_STRINGS));

g_param_spec_uint ?


		_LOGD ("update-dns: updating plugin %s", plugin_name);
+               g_signal_handlers_block_by_func (plugin, plugin_state_changed, self);
		if (!nm_dns_plugin_update (plugin,
                                           priv->configs,
                                           global_config,
                                           priv->hostname)) {

Why do we need to temporarily block the signal handler? It seems to me
that ->update() doesn't directly trigger a state change, and it
shouldn't matter whether an asynchronous signal is handled in
nm_dns_plugin_update() or just after.

(In reply to Beniamino Galvani from comment #1)
> > dns: move rate limiting to NMDnsPlugin
> 
> +       /* Lets the subclass know * when the nameserver subprocess
> 
> Extra '*'.

Fixed.

> > dns: replace the FAILED and CHILD_QUIT signals with a STATE property
> 
> +       g_object_class_install_property (object_class, PROP_STATE,
> +           g_param_spec_boolean (NM_DNS_PLUGIN_STATE, "", "", FALSE,
> +                                 G_PARAM_READABLE |
> G_PARAM_STATIC_STRINGS));
> 
> g_param_spec_uint ?

Fixed.

> 		_LOGD ("update-dns: updating plugin %s", plugin_name);
> +               g_signal_handlers_block_by_func (plugin,
> plugin_state_changed, self);
> 		if (!nm_dns_plugin_update (plugin,
>                                            priv->configs,
>                                            global_config,
>                                            priv->hostname)) {
> 
> Why do we need to temporarily block the signal handler? It seems to me
> that ->update() doesn't directly trigger a state change,

NMDnsUnbound's update() does trigger a state change.

> and it
> shouldn't matter whether an asynchronous signal is handled in
> nm_dns_plugin_update() or just after.

The signal emitted during update() is not handled after unblocking it; it's simply dropped. Which is good, we don't want to do anything about it, we just inspect the state below the update() call.

Pushed an updated branch.

> dns: move rate limiting to NMDnsPlugin
    
when watch_cb() ratelimites the restart, it pretends the plugin is still running. That seems ugly. Now that you add states, can there be a special state RATE_LIMITED.



pushed some fixups.



Previously, NMDnsManager had a detection of resolved (_resolvconf_resolved_managed), but it would only enable resolved, when "dns" configuration was unspecified and it was detected.

Now, it enables resolved, when the "dns" configuration is unspecified. Don't you yet errors in the logfiles when running without resolved now?

If somebody configures "dns" explicitly, and updating the DNS fails, there should be errors in the logfile. If somebody does not configure resolved, then there should be no errors.

I think, the default should be the same as before: "default" or (if detected) "systemd-resolved", and resurrect _resolvconf_resolved_managed().

bugzilla.gnome.org is being shut down in favor of a GitLab instance. 
We are closing all old bug reports and feature requests in GNOME Bugzilla which have not seen updates for a long time.

If you still use NetworkManager and if you still see this bug / want this feature in a recent and supported version of NetworkManager, then please feel free to report it at https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/

Thank you for creating this report and we are sorry it could not be implemented (workforce and time is unfortunately limited).