NetworkManager should definitely publish DNS list via D-Bus (with the possibility to follow changes).

This would allow you to build a simple program to monitor it and/or integrate this funcionality elsewhere.

This could also be potentially useful for NetworkManager's integration with
local recursive DNS servers, if they choose to follow NetworkManager's DNS
state.

@dcbw: Where does nm-tool get this information? I can't see it here:

http://projects.gnome.org/NetworkManager/developers/api/09/spec.html

The DNS info is available via IP(4|6)Config objects
http://projects.gnome.org/NetworkManager/developers/api/09/spec.html#org.freedesktop.NetworkManager.IP4Config
http://projects.gnome.org/NetworkManager/developers/api/09/spec.html#org.freedesktop.NetworkManager.IP6Config
org.freedesktop.NetworkManager.Device properties points to them when the device is connected.

Both nm-tool and nmcli get IP configuration via D-Bus (using libnm-glib).

Example:
nm-tool
nmcli -f IP4 dev list iface eth0

Thanks. So it's split between IP4Config and IP6Config. Maybe we could add something like DNSConfig to report exactly what would go into resolv.conf?

Based on some discussions a few weeks ago, I started working on this, although I'm abandoning it half-finished for now; I'm going to try to get back to it later. You can see the current state in the danw/dnspub branch. (The server side of it seems to basically work; you can look at the o.fd.NM.DnsManager properties in d-feet or via dbus-send/gdbus, and they're correct. The client side is unfinished, and I wasn't 100% sure about the best / most-libnm-glib-like way to expose the split-dns data.)

The idea is that using this, it should be possible to make NMDnsDnsmasq be an external program, reading the DnsManager properties to generate its config. And likewise, by allowing NMDnsManager to call arbitrary dns-setting-up scripts, which could access these properties, people could implement behavior like in bug 695384 and bug 673793 and bug 656260.

The SplitDns property here does not exactly match the current dns=dnsmasq behavior; instead of dividing the world into VPNs and everyone else, it divides the world into devices with never-default=TRUE configurations, and everyone else. Because if your VPN isn't set never-default=TRUE, then it doesn't really make a whole lot of sense to avoid talking to its DNS servers, right? (OK, no, actually, that's not right either; there are three kinds of names: VPN-internal names that only your VPN nameservers can resolve, site-local names that only your non-VPN nameservers can resolve, and global names that anyone can resolve. So, the interfaces still need a little work. I guess switching it back to the way that dns=dnsmasq currently works might be the right thing.)

(In reply to comment #2)
> The DNS info is available via IP(4|6)Config objects

FTR, note that VPNs currently have no D-Bus-accessible IP config objects, so in current NM, it's impossible to get the full DNS config via D-Bus.

NM bugzilla reorganization. Sorry for the bug spam.

There's another bug report regarding publishing DNS changes so we can merge the dispatcher and API bug as they are closely related and used for similar purposes.

*** This bug has been marked as a duplicate of bug 703395 ***

Un-duplicating bug, because DNS is still not exposed via D-Bus.


There is some old WIP on branch danw/dnspub-bgo603321 (previously danw/wip/dnspub)

Pushed new version to branch bg/dns-dbus-bgo603321 for review.

>> dns: export DNS manager D-Bus object
    
missing _notify() when mode or rc_manager changes. Possibly, the signal should be emitted at the very end of init_resolv_conf_mode().



     /* virtual methods */

+    /* properties */
+    obj_properties[PROP_MODE] =

     /* signals */

let's drop these comments. They add little value as every object should follow the very same pattern here.



>> dns: export current configuration through D-Bus

I pushed two follow up commits, adding a function _collect_resolv_conf_data(). Do you like that?

_get_configs_variant() largely repeats the collecting of the data. Maybe it should be implemented in terms of _collect_resolv_conf_data(). Or maybe even better, _collect_resolv_conf_data() should *also* return a variant in one run -- because effectively, the variant is always needed.





+    if (NM_IN_STRSET (pspec->name,
+        NM_DNS_MANAGER_MODE,
+        NM_DNS_MANAGER_RC_MANAGER,

indention


  g_snprintf (pname, sizeof (pname), "dns-%s", pspec->name);

I think nm_sprintf_buf() is nicer, because it has an nm_assert() that the buffer is large enough.

(In reply to Thomas Haller from comment #9)
> 
> >> dns: export DNS manager D-Bus object
>     
> missing _notify() when mode or rc_manager changes. Possibly, the signal
> should be emitted at the very end of init_resolv_conf_mode().

Fixed.

>      /* virtual methods */
> 
> +    /* properties */
> +    obj_properties[PROP_MODE] =
> 
>      /* signals */
> 
> let's drop these comments. They add little value as every object should
> follow the very same pattern here.

Ok.

> >> dns: export current configuration through D-Bus
> 
> I pushed two follow up commits, adding a function
> _collect_resolv_conf_data(). Do you like that?

Looks good. Pushed a fixup.

> _get_configs_variant() largely repeats the collecting of the data. Maybe it
> should be implemented in terms of _collect_resolv_conf_data(). Or maybe even
> better, _collect_resolv_conf_data() should *also* return a variant in one
> run -- because effectively, the variant is always needed.

_get_config_variant() is already quite verbose and I have the feeling that folding it into _collect_resolv_conf_data() would make it harder to understand.

> +    if (NM_IN_STRSET (pspec->name,
> +        NM_DNS_MANAGER_MODE,
> +        NM_DNS_MANAGER_RC_MANAGER,
> 
> indention
> 
> 
>   g_snprintf (pname, sizeof (pname), "dns-%s", pspec->name);
> 
> I think nm_sprintf_buf() is nicer, because it has an nm_assert() that the
> buffer is large enough.

Fixed and repushed, thanks.

Looks good to me.

One detail: NMClient creation should not fail when the DNSManager object is not present (e.g. newer nmcli run against older NetworkManager).

The nm_client_*_dns() functions should return NULL instead (as if the property reads failed) and documentation should state that those are allowed to fail with NULL.

(In reply to Lubomir Rintel from comment #11)
> Looks good to me.
> 
> One detail: NMClient creation should not fail when the DNSManager object is
> not present (e.g. newer nmcli run against older NetworkManager).
> 
> The nm_client_*_dns() functions should return NULL instead (as if the
> property reads failed) and documentation should state that those are allowed
> to fail with NULL.

I've added a couple of fixups, thanks.

lgtm

> libnm: implement support for DNS manager properties

Do we ever want to add more fields to the NMDnsEntry structs?  If so, maybe hardcoding the args in nm_dns_entry_new() isn't the way to go.  Though I guess we could just add setter functions later.

Also, some spurious whitespace changes in libnm/nm-manager.c/h

Rest LGTM.

(In reply to Dan Williams from comment #14)
> > libnm: implement support for DNS manager properties
> 
> Do we ever want to add more fields to the NMDnsEntry structs?  If so, maybe
> hardcoding the args in nm_dns_entry_new() isn't the way to go.  Though I
> guess we could just add setter functions later.

Or maybe, nm_dns_entry_new() shouldn't be in a public header in the first place as it's not needed by libnm users and is not exported in libnm.ver. How about the fixup?

> Also, some spurious whitespace changes in libnm/nm-manager.c/h

Fixed, thanks.

ACK after moving the nm_dns_entry functions to a private header.

Merged:

https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=597f327b200139665a919202d632229ca20df33d