[systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

[Zbigniew Jędrzejewski-Szmek]

On Tue, Jul 29, 2014 at 02:48:18PM -0700, Michael Marineau wrote:
> When the code for generating resolv.conf was moved from networkd to
> resolved the DHCP domain name code was dropped.
> ---
> 
> This is a resend, rebased since some recent changes changed how this
> patch needed to be implemented.
> 
>  src/network/networkd-link.c    | 13 +++++++++++++
>  src/network/sd-network.c       | 24 ++++++++++++++++++++++++
>  src/resolve/resolved-link.c    | 20 ++++++++++++++++++++
>  src/resolve/resolved-link.h    |  2 ++
>  src/resolve/resolved-manager.c | 10 +++++++++-
>  src/systemd/sd-network.h       |  3 +++
>  6 files changed, 71 insertions(+), 1 deletion(-)
> 
> diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
> index 3b8b7ed..827c428 100644
> --- a/src/network/networkd-link.c
> +++ b/src/network/networkd-link.c
> @@ -2451,6 +2451,19 @@ int link_save(Link *link) {
>                                  (address + 1 ? " " : ""));
>  
>                  fputs("\n", f);
> +
> +                fprintf(f, "DOMAINNAME=");
> +
> +                if (link->network->dhcp_domainname &&
> +                    link->dhcp_lease) {
> +                        const char *domainname;
> +
> +                        r = sd_dhcp_lease_get_domainname(link->dhcp_lease, &domainname);
> +                        if (r >= 0)
> +                                fputs(domainname, f);
> +                }
> +
> +                fputs("\n", f);
Is it really necessary to write anything if the name is not available?
Other parts of this function don't write anyting in similar cases.

>  
>          if (link->dhcp_lease) {
> diff --git a/src/network/sd-network.c b/src/network/sd-network.c
> index bfb8321..a427a27 100644
> --- a/src/network/sd-network.c
> +++ b/src/network/sd-network.c
> @@ -175,6 +175,30 @@ _public_ int sd_network_get_ntp(int ifindex, char ***ret) {
>          return network_get_strv("NTP", ifindex, ret);
>  }
>  
> +_public_ int sd_network_get_domainname(int ifindex, char **domainname) {
> +        _cleanup_free_ char *s = NULL, *p = NULL;
> +        int r;
> +
> +        assert_return(ifindex > 0, -EINVAL);
> +        assert_return(domainname, -EINVAL);
> +
> +        if (asprintf(&p, "/run/systemd/netif/links/%d", ifindex) < 0)
> +                return -ENOMEM;
Not terribly important, but please spell that as:

           char p[sizeof("/run/systemd/netif/links/") + DECIMAL_STRING_MAX(int)];
	   snprintf(p, sizeof(p), "/run/systemd/netif/links/%d", ifindex);

> +        r = parse_env_file(p, NEWLINE, "DOMAINNAME", &s, NULL);
> +        if (r == -ENOENT)
> +                return -ENODATA;
> +        else if (r < 0)
> +                return r;
> +        else if (!s)
> +                return -EIO;
> +
> +        *domainname = s;
> +        s = NULL;
> +
> +        return 0;
> +}
> +
>  static inline int MONITOR_TO_FD(sd_network_monitor *m) {
>          return (int) (unsigned long) m - 1;
>  }
> diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c
> index 6ac7c5b..f6b7f6a 100644
> --- a/src/resolve/resolved-link.c
> +++ b/src/resolve/resolved-link.c
> @@ -77,6 +77,7 @@ Link *link_free(Link *l) {
>          while (l->dns_servers)
>                  dns_server_free(l->dns_servers);
>  
> +        free(l->domainname);
>          free(l);
>          return NULL;
>  }
> @@ -191,10 +192,29 @@ clear:
>          return r;
>  }
>  
> +static int link_update_domainname(Link *l) {
> +        char *domainname = NULL;
> +        int r;
> +
> +        assert(l);
> +
> +        free(l->domainname);
> +        l->domainname = NULL;
> +
> +        r = sd_network_get_domainname(l->ifindex, &domainname);
> +        if (r < 0)
> +                return r;
> +
> +        l->domainname = domainname;
> +
> +        return 0;
> +}
> +
>  int link_update_monitor(Link *l) {
>          assert(l);
>  
>          link_update_dns_servers(l);
> +        link_update_domainname(l);
>          link_allocate_scopes(l);
>          link_add_rrs(l);
>  
> diff --git a/src/resolve/resolved-link.h b/src/resolve/resolved-link.h
> index f58bd54..9730aec 100644
> --- a/src/resolve/resolved-link.h
> +++ b/src/resolve/resolved-link.h
> @@ -68,6 +68,8 @@ struct Link {
>  
>          RateLimit mdns_ratelimit;
>          RateLimit llmnr_ratelimit;
> +
> +        char *domainname;
>  };
>  
>  int link_new(Manager *m, Link **ret, int ifindex);
> diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
> index a8715bd..253a97e 100644
> --- a/src/resolve/resolved-manager.c
> +++ b/src/resolve/resolved-manager.c
> @@ -522,6 +522,7 @@ int manager_write_resolv_conf(Manager *m) {
>          const char *path = "/run/systemd/resolve/resolv.conf";
>          _cleanup_free_ char *temp_path = NULL;
>          _cleanup_fclose_ FILE *f = NULL;
> +        const char *domainname = NULL;
>          unsigned count = 0;
>          DnsServer *s;
>          Iterator i;
> @@ -542,13 +543,20 @@ int manager_write_resolv_conf(Manager *m) {
>                "# resolv.conf(5) in a different way, replace the symlink by a\n"
>                "# static file or a different symlink.\n\n", f);
>  
> -        HASHMAP_FOREACH(l, m->links, i)
> +        HASHMAP_FOREACH(l, m->links, i) {
>                  LIST_FOREACH(servers, s, l->dns_servers)
>                          write_resolve_conf_server(s, f, &count);
>  
> +                if (!domainname && l->domainname)
> +                        domainname = l->domainname;
> +        }
> +
>          LIST_FOREACH(servers, s, m->dns_servers)
>                  write_resolve_conf_server(s, f, &count);
>  
> +        if (domainname)
> +                fprintf(f, "domain %s\n", domainname);
> +

How does this deal with empty domain names? Let's say the server is
misconfigure and returns "". Will this still be valid syntax? Also,
are the contents checked for special characters, etc, earlier?

Zbyszek