[systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP
Michael Marineau
michael.marineau at coreos.com
Tue Jul 29 16:54:24 PDT 2014
On Tue, Jul 29, 2014 at 3:37 PM, Zbigniew Jędrzejewski-Szmek
<zbyszek at in.waw.pl> wrote:
> 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.
I was just matching the above lines which may write DNS= or NTP= with
blank values. I don't think it matters either way. Omitting
DOMAINNAME= if it is blank certainly looks a little cleaner since the
writes get squashed into a single fprintf. Will update.
>
>>
>> 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);
This was copied verbatim from similar functions in this file, should I
update the style of the others to match your suggestion? Why the
preference of manually calculating a buffer length than using
asprintf?
>
>> + 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?
sd_network_get_domainname() returns NULL for empty strings following
the behavior of parse_env_file() so simply checking for NULL elsewhere
is sufficient.
More information about the systemd-devel
mailing list