networkd: manager/link - only serialize once per event-loop iteration

Every time the state is written out we may trigger third-party apps, so
let's be a bit more careful about writing this out unnecessarily.

1 files changed, 229 insertions, 185 deletions

diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c
index 2b83ee81ed..76d5aa5585 100644
--- a/src/network/networkd-manager.c
+++ b/src/network/networkd-manager.c
@@ -573,6 +573,213 @@ static int manager_connect_rtnl(Manager *m) {
         return 0;
 }
 
+static int set_put_in_addr(Set *s, const struct in_addr *address) {
+        char *p;
+        int r;
+
+        assert(s);
+
+        r = in_addr_to_string(AF_INET, (const union in_addr_union*) address, &p);
+        if (r < 0)
+                return r;
+
+        r = set_consume(s, p);
+        if (r == -EEXIST)
+                return 0;
+
+        return r;
+}
+
+static int set_put_in_addrv(Set *s, const struct in_addr *addresses, int n) {
+        int r, i, c = 0;
+
+        assert(s);
+        assert(n <= 0 || addresses);
+
+        for (i = 0; i < n; i++) {
+                r = set_put_in_addr(s, addresses+i);
+                if (r < 0)
+                        return r;
+
+                c += r;
+        }
+
+        return c;
+}
+
+static void print_string_set(FILE *f, const char *field, Set *s) {
+        bool space = false;
+        Iterator i;
+        char *p;
+
+        if (set_isempty(s))
+                return;
+
+        fputs(field, f);
+
+        SET_FOREACH(p, s, i) {
+                if (space)
+                        fputc(' ', f);
+                fputs(p, f);
+                space = true;
+        }
+        fputc('\n', f);
+}
+
+static int manager_save(Manager *m) {
+        _cleanup_set_free_free_ Set *dns = NULL, *ntp = NULL, *domains = NULL;
+        Link *link;
+        Iterator i;
+        _cleanup_free_ char *temp_path = NULL;
+        _cleanup_fclose_ FILE *f = NULL;
+        LinkOperationalState operstate = LINK_OPERSTATE_OFF;
+        const char *operstate_str;
+        int r;
+
+        assert(m);
+        assert(m->state_file);
+
+        /* We add all NTP and DNS server to a set, to filter out duplicates */
+        dns = set_new(&string_hash_ops);
+        if (!dns)
+                return -ENOMEM;
+
+        ntp = set_new(&string_hash_ops);
+        if (!ntp)
+                return -ENOMEM;
+
+        domains = set_new(&string_hash_ops);
+        if (!domains)
+                return -ENOMEM;
+
+        HASHMAP_FOREACH(link, m->links, i) {
+                if (link->flags & IFF_LOOPBACK)
+                        continue;
+
+                if (link->operstate > operstate)
+                        operstate = link->operstate;
+
+                if (!link->network)
+                        continue;
+
+                /* First add the static configured entries */
+                r = set_put_strdupv(dns, link->network->dns);
+                if (r < 0)
+                        return r;
+
+                r = set_put_strdupv(ntp, link->network->ntp);
+                if (r < 0)
+                        return r;
+
+                r = set_put_strdupv(domains, link->network->domains);
+                if (r < 0)
+                        return r;
+
+                if (!link->dhcp_lease)
+                        continue;
+
+                /* Secondly, add the entries acquired via DHCP */
+                if (link->network->dhcp_dns) {
+                        const struct in_addr *addresses;
+
+                        r = sd_dhcp_lease_get_dns(link->dhcp_lease, &addresses);
+                        if (r > 0) {
+                                r = set_put_in_addrv(dns, addresses, r);
+                                if (r < 0)
+                                        return r;
+                        } else if (r < 0 && r != -ENODATA)
+                                return r;
+                }
+
+                if (link->network->dhcp_ntp) {
+                        const struct in_addr *addresses;
+
+                        r = sd_dhcp_lease_get_ntp(link->dhcp_lease, &addresses);
+                        if (r > 0) {
+                                r = set_put_in_addrv(ntp, addresses, r);
+                                if (r < 0)
+                                        return r;
+                        } else if (r < 0 && r != -ENODATA)
+                                return r;
+                }
+
+                if (link->network->dhcp_domains) {
+                        const char *domainname;
+
+                        r = sd_dhcp_lease_get_domainname(link->dhcp_lease, &domainname);
+                        if (r >= 0) {
+                                r = set_put_strdup(domains, domainname);
+                                if (r < 0)
+                                        return r;
+                        } else if (r != -ENODATA)
+                                return r;
+                }
+        }
+
+        operstate_str = link_operstate_to_string(operstate);
+        assert(operstate_str);
+
+        r = fopen_temporary(m->state_file, &f, &temp_path);
+        if (r < 0)
+                return r;
+
+        fchmod(fileno(f), 0644);
+
+        fprintf(f,
+                "# This is private data. Do not parse.\n"
+                "OPER_STATE=%s\n", operstate_str);
+
+        print_string_set(f, "DNS=", dns);
+        print_string_set(f, "NTP=", ntp);
+        print_string_set(f, "DOMAINS=", domains);
+
+        r = fflush_and_check(f);
+        if (r < 0)
+                goto fail;
+
+        if (rename(temp_path, m->state_file) < 0) {
+                r = -errno;
+                goto fail;
+        }
+
+        if (m->operational_state != operstate) {
+                m->operational_state = operstate;
+                r = manager_send_changed(m, "OperationalState", NULL);
+                if (r < 0)
+                        log_error_errno(r, "Could not emit changed OperationalState: %m");
+        }
+
+        m->dirty = false;
+
+        return 0;
+
+fail:
+        (void) unlink(m->state_file);
+        (void) unlink(temp_path);
+
+        return log_error_errno(r, "Failed to save network state to %s: %m", m->state_file);
+}
+
+static int manager_dirty_handler(sd_event_source *s, void *userdata) {
+        Manager *m = userdata;
+        Link *link;
+        Iterator i;
+        int r;
+
+        assert(m);
+
+        if (m->dirty)
+                manager_save(m);
+
+        SET_FOREACH(link, m->dirty_links, i) {
+                r = link_save(link);
+                if (r >= 0)
+                        link_clean(link);
+        }
+
+        return 1;
+}
+
 int manager_new(Manager **ret) {
         _cleanup_manager_free_ Manager *m = NULL;
         int r;
@@ -594,6 +801,10 @@ int manager_new(Manager **ret) {
         sd_event_add_signal(m->event, NULL, SIGTERM, NULL, NULL);
         sd_event_add_signal(m->event, NULL, SIGINT, NULL, NULL);
 
+        r = sd_event_add_post(m->event, NULL, manager_dirty_handler, m);
+        if (r < 0)
+                return r;
+
         r = manager_connect_rtnl(m);
         if (r < 0)
                 return r;
@@ -688,8 +899,19 @@ static bool manager_check_idle(void *userdata) {
 }
 
 int manager_run(Manager *m) {
+        Link *link;
+        Iterator i;
+
         assert(m);
 
+        /* The dirty handler will deal with future serialization, but the first one
+           must be done explicitly. */
+
+        manager_save(m);
+
+        HASHMAP_FOREACH(link, m->links, i)
+                link_save(link);
+
         if (m->bus)
                 return bus_event_loop_with_idle(
                                 m->event,
@@ -795,191 +1017,6 @@ int manager_rtnl_enumerate_addresses(Manager *m) {
         return r;
 }
 
-static int set_put_in_addr(Set *s, const struct in_addr *address) {
-        char *p;
-        int r;
-
-        assert(s);
-
-        r = in_addr_to_string(AF_INET, (const union in_addr_union*) address, &p);
-        if (r < 0)
-                return r;
-
-        r = set_consume(s, p);
-        if (r == -EEXIST)
-                return 0;
-
-        return r;
-}
-
-static int set_put_in_addrv(Set *s, const struct in_addr *addresses, int n) {
-        int r, i, c = 0;
-
-        assert(s);
-        assert(n <= 0 || addresses);
-
-        for (i = 0; i < n; i++) {
-                r = set_put_in_addr(s, addresses+i);
-                if (r < 0)
-                        return r;
-
-                c += r;
-        }
-
-        return c;
-}
-
-static void print_string_set(FILE *f, const char *field, Set *s) {
-        bool space = false;
-        Iterator i;
-        char *p;
-
-        if (set_isempty(s))
-                return;
-
-        fputs(field, f);
-
-        SET_FOREACH(p, s, i) {
-                if (space)
-                        fputc(' ', f);
-                fputs(p, f);
-                space = true;
-        }
-        fputc('\n', f);
-}
-
-int manager_save(Manager *m) {
-        _cleanup_set_free_free_ Set *dns = NULL, *ntp = NULL, *domains = NULL;
-        Link *link;
-        Iterator i;
-        _cleanup_free_ char *temp_path = NULL;
-        _cleanup_fclose_ FILE *f = NULL;
-        LinkOperationalState operstate = LINK_OPERSTATE_OFF;
-        const char *operstate_str;
-        int r;
-
-        assert(m);
-        assert(m->state_file);
-
-        /* We add all NTP and DNS server to a set, to filter out duplicates */
-        dns = set_new(&string_hash_ops);
-        if (!dns)
-                return -ENOMEM;
-
-        ntp = set_new(&string_hash_ops);
-        if (!ntp)
-                return -ENOMEM;
-
-        domains = set_new(&string_hash_ops);
-        if (!domains)
-                return -ENOMEM;
-
-        HASHMAP_FOREACH(link, m->links, i) {
-                if (link->flags & IFF_LOOPBACK)
-                        continue;
-
-                if (link->operstate > operstate)
-                        operstate = link->operstate;
-
-                if (!link->network)
-                        continue;
-
-                /* First add the static configured entries */
-                r = set_put_strdupv(dns, link->network->dns);
-                if (r < 0)
-                        return r;
-
-                r = set_put_strdupv(ntp, link->network->ntp);
-                if (r < 0)
-                        return r;
-
-                r = set_put_strdupv(domains, link->network->domains);
-                if (r < 0)
-                        return r;
-
-                if (!link->dhcp_lease)
-                        continue;
-
-                /* Secondly, add the entries acquired via DHCP */
-                if (link->network->dhcp_dns) {
-                        const struct in_addr *addresses;
-
-                        r = sd_dhcp_lease_get_dns(link->dhcp_lease, &addresses);
-                        if (r > 0) {
-                                r = set_put_in_addrv(dns, addresses, r);
-                                if (r < 0)
-                                        return r;
-                        } else if (r < 0 && r != -ENODATA)
-                                return r;
-                }
-
-                if (link->network->dhcp_ntp) {
-                        const struct in_addr *addresses;
-
-                        r = sd_dhcp_lease_get_ntp(link->dhcp_lease, &addresses);
-                        if (r > 0) {
-                                r = set_put_in_addrv(ntp, addresses, r);
-                                if (r < 0)
-                                        return r;
-                        } else if (r < 0 && r != -ENODATA)
-                                return r;
-                }
-
-                if (link->network->dhcp_domains) {
-                        const char *domainname;
-
-                        r = sd_dhcp_lease_get_domainname(link->dhcp_lease, &domainname);
-                        if (r >= 0) {
-                                r = set_put_strdup(domains, domainname);
-                                if (r < 0)
-                                        return r;
-                        } else if (r != -ENODATA)
-                                return r;
-                }
-        }
-
-        operstate_str = link_operstate_to_string(operstate);
-        assert(operstate_str);
-
-        r = fopen_temporary(m->state_file, &f, &temp_path);
-        if (r < 0)
-                return r;
-
-        fchmod(fileno(f), 0644);
-
-        fprintf(f,
-                "# This is private data. Do not parse.\n"
-                "OPER_STATE=%s\n", operstate_str);
-
-        print_string_set(f, "DNS=", dns);
-        print_string_set(f, "NTP=", ntp);
-        print_string_set(f, "DOMAINS=", domains);
-
-        r = fflush_and_check(f);
-        if (r < 0)
-                goto fail;
-
-        if (rename(temp_path, m->state_file) < 0) {
-                r = -errno;
-                goto fail;
-        }
-
-        if (m->operational_state != operstate) {
-                m->operational_state = operstate;
-                r = manager_send_changed(m, "OperationalState", NULL);
-                if (r < 0)
-                        log_error_errno(r, "Could not emit changed OperationalState: %m");
-        }
-
-        return 0;
-
-fail:
-        (void) unlink(m->state_file);
-        (void) unlink(temp_path);
-
-        return log_error_errno(r, "Failed to save network state to %s: %m", m->state_file);
-}
-
 int manager_address_pool_acquire(Manager *m, int family, unsigned prefixlen, union in_addr_union *found) {
         AddressPool *p;
         int r;
@@ -1036,3 +1073,10 @@ Link* manager_find_uplink(Manager *m, Link *exclude) {
 
         return NULL;
 }
+
+void manager_dirty(Manager *manager) {
+        assert(manager);
+
+        /* the serialized state in /run is no longer up-to-date */
+        manager->dirty = true;
+}