Comment 84 for bug 1754671

Revision history for this message
In , Thomas Haller (thaller-1) wrote :

(In reply to Beniamino Galvani from comment #18)
> (In reply to Thomas Haller from comment #17)
> > tl;dr: +1 for a "dns" section.
>
> Ok, I'll add the new 'default' property to a 'dns' setting and I'll also
> move the existing connection.mdns property there, since we haven't done any
> official release that includes it.

I agree.

> I pushed some preliminary patches to branch bg/dns-domains-pt1-bgo746422,
> please review.

+ if (search_only && domain_is_routing_only (str))
+ continue;

it's a bit confusing that the parameter is called "search_only", while it compares it with "routing_only". Could you rename "search_only"?
Also, the inverse naming is confusing to me:

  add_dns_domains (array, ip_config, FALSE, FALSE);

has search_only=FALSE, the double-inverse will result in ~all~. Could we rename "search_only" to "with_routing_only" (and inverse meaning)?

+ if (!domain_is_valid (str, FALSE))
+ continue;

@str has possible a leading tilde. Shouldn't you strip it with "nm_utils_parse_dns_domain (str, NULL)" ?

- /* If this link is never the default (e.g. only used for resources on this
- * network) add a routing domain. */
- route_only = addr_family == AF_INET
- ? !nm_ip4_config_best_default_route_get (NM_IP4_CONFIG (config))
- : !nm_ip6_config_best_default_route_get (NM_IP6_CONFIG (config));
-

this behaviour came originally from https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=c4864ba63f4a13e9938a978787490005f5ba48fb. But despite the commit message, I don't understand why we would set routing-only if nm_ip4_config_get_never_default(). I guess, this new behavior makes much more sense to me.

pushed one fixup.