From fa994f917d51997c6bc125161a473d1ed8d40229 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 31 Jan 2016 23:53:31 +0100 Subject: [PATCH 01/22] activate: fix memory allocation for execv() parameters Make sure we construct the full environment block on the heap, so that we can clean things up properly if execv() fails. --- src/activate/activate.c | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/src/activate/activate.c b/src/activate/activate.c index 95083441ab..6709d9bb80 100644 --- a/src/activate/activate.c +++ b/src/activate/activate.c @@ -151,24 +151,44 @@ static int launch(char* name, char **argv, char **env, int fds) { return log_oom(); STRV_FOREACH(s, arg_setenv) { - if (strchr(*s, '=')) - envp[n_env++] = *s; - else { + if (strchr(*s, '=')) { + char *k; + + k = strdup(*s); + if (!k) + return log_oom(); + + envp[n_env++] = k; + } else { _cleanup_free_ char *p; + const char *n; p = strappend(*s, "="); if (!p) return log_oom(); - envp[n_env] = strv_find_prefix(env, p); - if (envp[n_env]) - n_env ++; + + n = strv_find_prefix(env, p); + if (!n) + continue; + + envp[n_env] = strdup(n); + if (!envp[n_env]) + return log_oom(); } } for (i = 0; i < ELEMENTSOF(tocopy); i++) { - envp[n_env] = strv_find_prefix(env, tocopy[i]); - if (envp[n_env]) - n_env ++; + const char *n; + + n = strv_find_prefix(env, tocopy[i]); + if (!n) + continue; + + envp[n_env] = strdup(n); + if (!envp[n_env]) + return log_oom(); + + n_env ++; } if ((asprintf((char**)(envp + n_env++), "LISTEN_FDS=%d", fds) < 0) || From 7e0cdf8f1ff366a1d1770ea92bcce686db8c2dfc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 31 Jan 2016 23:54:14 +0100 Subject: [PATCH 02/22] activate: reorder --help text Make sure the --help and --version options are mentioned first, like in all our other tools. --- src/activate/activate.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/activate/activate.c b/src/activate/activate.c index 6709d9bb80..558d16824a 100644 --- a/src/activate/activate.c +++ b/src/activate/activate.c @@ -329,12 +329,12 @@ static void help(void) { printf("%s [OPTIONS...]\n\n" "Listen on sockets and launch child on connection.\n\n" "Options:\n" - " -d --datagram Datagram sockets\n" - " -l --listen=ADDR Listen for raw connections at ADDR\n" - " -a --accept Spawn separate child for each connection\n" " -h --help Show this help and exit\n" + " --version Print version string and exit\n" + " -l --listen=ADDR Listen for raw connections at ADDR\n" + " -d --datagram Listen on datagram instead of stream socket\n" + " -a --accept Spawn separate child for each connection\n" " -E --setenv=NAME[=VALUE] Pass an environment variable to children\n" - " --version Print version string and exit\n" "\n" "Note: file descriptors from sd_listen_fds() will be passed through.\n" , program_invocation_short_name); From 2afcd6902b5c04e8b05c9d1ffd8bc6175fac6efb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 31 Jan 2016 23:55:06 +0100 Subject: [PATCH 03/22] journal-cat: don't allocate memory for the syslog identifier Fixes: #2490 --- src/journal/cat.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/journal/cat.c b/src/journal/cat.c index 7fd4198df8..07c3df522c 100644 --- a/src/journal/cat.c +++ b/src/journal/cat.c @@ -34,7 +34,7 @@ #include "syslog-util.h" #include "util.h" -static char *arg_identifier = NULL; +static const char *arg_identifier = NULL; static int arg_priority = LOG_INFO; static bool arg_level_prefix = true; @@ -82,14 +82,10 @@ static int parse_argv(int argc, char *argv[]) { return version(); case 't': - free(arg_identifier); if (isempty(optarg)) arg_identifier = NULL; - else { - arg_identifier = strdup(optarg); - if (!arg_identifier) - return log_oom(); - } + else + arg_identifier = optarg; break; case 'p': From 011696f76233486bc56c266b18a328924f70269c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 00:00:01 +0100 Subject: [PATCH 04/22] resolved: rework what ResolveHostname() with family == AF_UNSPEC means Previously, if a hostanem is resolved with AF_UNSPEC specified, this would be used as indication to resolve both an AF_INET and an AF_INET6 address. With this change this logic is altered: an AF_INET address is only resolved if there's actually a routable IPv4 address on the specific interface, and similar an AF_INET6 address is only resolved if there's a routable IPv6 address. With this in place, it's ensured that the returned data is actually connectable by applications. This logic mimics glibc's resolver behaviour. Note that if the client asks explicitly for AF_INET or AF_INET6 it will get what it asked for. This also simplifies the logic how it is determined whether a specific lookup shall take place on a scope. Specifically, the checks with dns_scope_good_key() are now moved out of the transaction code and into the query code, so that we don't even create a transaction object on a specific scope if we cannot execute the resolution on it anyway. --- src/resolve/resolved-bus.c | 1 + src/resolve/resolved-dns-query.c | 47 ++++++++++++++++++++++++-- src/resolve/resolved-dns-query.h | 4 +++ src/resolve/resolved-dns-scope.c | 27 ++++++--------- src/resolve/resolved-dns-scope.h | 2 +- src/resolve/resolved-dns-transaction.c | 13 ------- src/resolve/resolved-link.c | 21 +++++++----- src/resolve/resolved-link.h | 4 +-- src/resolve/resolved-manager.c | 15 ++++++++ src/resolve/resolved-manager.h | 2 ++ 10 files changed, 93 insertions(+), 43 deletions(-) diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c index 834ae837de..251e7a50a4 100644 --- a/src/resolve/resolved-bus.c +++ b/src/resolve/resolved-bus.c @@ -281,6 +281,7 @@ static int bus_method_resolve_hostname(sd_bus_message *message, void *userdata, q->request = sd_bus_message_ref(message); q->request_family = family; q->complete = bus_method_resolve_hostname_complete; + q->suppress_unroutable_family = family == AF_UNSPEC; r = dns_query_bus_track(q, message); if (r < 0) diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index a00851658e..06d30d7863 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -21,6 +21,7 @@ #include "alloc-util.h" #include "dns-domain.h" +#include "dns-type.h" #include "hostname-util.h" #include "local-addresses.h" #include "resolved-dns-query.h" @@ -161,6 +162,7 @@ static int dns_query_candidate_go(DnsQueryCandidate *c) { DnsTransaction *t; Iterator i; int r; + unsigned n = 0; assert(c); @@ -172,8 +174,14 @@ static int dns_query_candidate_go(DnsQueryCandidate *c) { r = dns_transaction_go(t); if (r < 0) return r; + + n++; } + /* If there was nothing to start, then let's proceed immediately */ + if (n == 0) + dns_query_candidate_notify(c); + return 0; } @@ -222,6 +230,31 @@ static DnsTransactionState dns_query_candidate_state(DnsQueryCandidate *c) { return state; } +static bool dns_query_candidate_is_routable(DnsQueryCandidate *c, uint16_t type) { + int family; + + assert(c); + + /* Checks whether the specified RR type matches an address family that is routable on the link(s) the scope of + * this candidate belongs to. Specifically, whether there's a routable IPv4 address on it if we query an A RR, + * or a routable IPv6 address if we query an AAAA RR. */ + + if (!c->query->suppress_unroutable_family) + return true; + + if (c->scope->protocol != DNS_PROTOCOL_DNS) + return true; + + family = dns_type_to_af(type); + if (family < 0) + return true; + + if (c->scope->link) + return link_relevant(c->scope->link, family, false); + else + return manager_routable(c->scope->manager, family); +} + static int dns_query_candidate_setup_transactions(DnsQueryCandidate *c) { DnsQuestion *question; DnsResourceKey *key; @@ -236,14 +269,24 @@ static int dns_query_candidate_setup_transactions(DnsQueryCandidate *c) { /* Create one transaction per question key */ DNS_QUESTION_FOREACH(key, question) { _cleanup_(dns_resource_key_unrefp) DnsResourceKey *new_key = NULL; + DnsResourceKey *qkey; + + if (!dns_query_candidate_is_routable(c, key->type)) + continue; if (c->search_domain) { r = dns_resource_key_new_append_suffix(&new_key, key, c->search_domain->name); if (r < 0) goto fail; - } - r = dns_query_candidate_add_transaction(c, new_key ?: key); + qkey = new_key; + } else + qkey = key; + + if (!dns_scope_good_key(c->scope, qkey)) + continue; + + r = dns_query_candidate_add_transaction(c, qkey); if (r < 0) goto fail; diff --git a/src/resolve/resolved-dns-query.h b/src/resolve/resolved-dns-query.h index c7e4ce9a00..75c2c14c1f 100644 --- a/src/resolve/resolved-dns-query.h +++ b/src/resolve/resolved-dns-query.h @@ -69,6 +69,10 @@ struct DnsQuery { uint64_t flags; int ifindex; + /* If true, A or AAAA RR lookups will be suppressed on links with no routable address of the matching address + * family */ + bool suppress_unroutable_family; + DnsTransactionState state; unsigned n_cname_redirects; diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index ac4887abea..03239794ee 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -490,7 +490,9 @@ DnsScopeMatch dns_scope_good_domain(DnsScope *s, int ifindex, uint64_t flags, co } } -int dns_scope_good_key(DnsScope *s, DnsResourceKey *key) { +bool dns_scope_good_key(DnsScope *s, const DnsResourceKey *key) { + int key_family; + assert(s); assert(key); @@ -498,6 +500,9 @@ int dns_scope_good_key(DnsScope *s, DnsResourceKey *key) { * this scope. Note that this call assumes as fully qualified * name, i.e. the search suffixes already appended. */ + if (key->class != DNS_CLASS_IN) + return false; + if (s->protocol == DNS_PROTOCOL_DNS) { /* On classic DNS, looking up non-address RRs is always @@ -519,13 +524,11 @@ int dns_scope_good_key(DnsScope *s, DnsResourceKey *key) { /* On mDNS and LLMNR, send A and AAAA queries only on the * respective scopes */ - if (s->family == AF_INET && key->class == DNS_CLASS_IN && key->type == DNS_TYPE_AAAA) - return false; + key_family = dns_type_to_af(key->type); + if (key_family < 0) + return true; - if (s->family == AF_INET6 && key->class == DNS_CLASS_IN && key->type == DNS_TYPE_A) - return false; - - return true; + return key_family == s->family; } static int dns_scope_multicast_membership(DnsScope *s, bool b, struct in_addr in, struct in6_addr in6) { @@ -1017,9 +1020,6 @@ bool dns_scope_name_needs_search_domain(DnsScope *s, const char *name) { } bool dns_scope_network_good(DnsScope *s) { - Iterator i; - Link *l; - /* Checks whether the network is in good state for lookups on this scope. For mDNS/LLMNR/Classic DNS scopes * bound to links this is easy, as they don't even exist if the link isn't in a suitable state. For the global * DNS scope we check whether there are any links that are up and have an address. */ @@ -1027,10 +1027,5 @@ bool dns_scope_network_good(DnsScope *s) { if (s->link) return true; - HASHMAP_FOREACH(l, s->manager->links, i) { - if (link_relevant(l, AF_UNSPEC, false)) - return true; - } - - return false; + return manager_routable(s->manager, AF_UNSPEC); } diff --git a/src/resolve/resolved-dns-scope.h b/src/resolve/resolved-dns-scope.h index f9b63d56d9..05b8d66de0 100644 --- a/src/resolve/resolved-dns-scope.h +++ b/src/resolve/resolved-dns-scope.h @@ -87,7 +87,7 @@ int dns_scope_socket_tcp(DnsScope *s, int family, const union in_addr_union *add int dns_scope_socket_udp(DnsScope *s, DnsServer *server, uint16_t port); DnsScopeMatch dns_scope_good_domain(DnsScope *s, int ifindex, uint64_t flags, const char *domain); -int dns_scope_good_key(DnsScope *s, DnsResourceKey *key); +bool dns_scope_good_key(DnsScope *s, const DnsResourceKey *key); DnsServer *dns_scope_get_dns_server(DnsScope *s); void dns_scope_next_dns_server(DnsScope *s); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 637b99aaa5..501f13063e 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1411,12 +1411,6 @@ static int dns_transaction_make_packet(DnsTransaction *t) { if (r < 0) return r; - r = dns_scope_good_key(t->scope, t->key); - if (r < 0) - return r; - if (r == 0) - return -EDOM; - r = dns_packet_append_key(p, t->key, NULL); if (r < 0) return r; @@ -1498,13 +1492,6 @@ int dns_transaction_go(DnsTransaction *t) { /* Otherwise, we need to ask the network */ r = dns_transaction_make_packet(t); - if (r == -EDOM) { - /* Not the right request to make on this network? - * (i.e. an A request made on IPv6 or an AAAA request - * made on IPv4, on LLMNR or mDNS.) */ - dns_transaction_complete(t, DNS_TRANSACTION_NO_SERVERS); - return 0; - } if (r < 0) return r; diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c index 37dd4a6e78..7412e64622 100644 --- a/src/resolve/resolved-link.c +++ b/src/resolve/resolved-link.c @@ -515,14 +515,17 @@ int link_update_monitor(Link *l) { return 0; } -bool link_relevant(Link *l, int family, bool multicast) { +bool link_relevant(Link *l, int family, bool local_multicast) { _cleanup_free_ char *state = NULL; LinkAddress *a; assert(l); - /* A link is relevant for multicast traffic if it isn't a loopback or pointopoint device, has a link beat, can - * do multicast and has at least one relevant IP address */ + /* A link is relevant for local multicast traffic if it isn't a loopback or pointopoint device, has a link + * beat, can do multicast and has at least one link-local (or better) IP address. + * + * A link is relevant for non-multicast traffic if it isn't a loopback device, has a link beat, and has at + * least one routable address.*/ if (l->flags & (IFF_LOOPBACK|IFF_DORMANT)) return false; @@ -530,7 +533,7 @@ bool link_relevant(Link *l, int family, bool multicast) { if ((l->flags & (IFF_UP|IFF_LOWER_UP)) != (IFF_UP|IFF_LOWER_UP)) return false; - if (multicast) { + if (local_multicast) { if (l->flags & IFF_POINTOPOINT) return false; @@ -548,7 +551,7 @@ bool link_relevant(Link *l, int family, bool multicast) { return false; LIST_FOREACH(addresses, a, l->addresses) - if ((family == AF_UNSPEC || a->family == family) && link_address_relevant(a)) + if ((family == AF_UNSPEC || a->family == family) && link_address_relevant(a, local_multicast)) return true; return false; @@ -692,7 +695,7 @@ void link_address_add_rrs(LinkAddress *a, bool force_remove) { if (a->family == AF_INET) { if (!force_remove && - link_address_relevant(a) && + link_address_relevant(a, true) && a->link->llmnr_ipv4_scope && a->link->llmnr_support == RESOLVE_SUPPORT_YES && a->link->manager->llmnr_support == RESOLVE_SUPPORT_YES) { @@ -749,7 +752,7 @@ void link_address_add_rrs(LinkAddress *a, bool force_remove) { if (a->family == AF_INET6) { if (!force_remove && - link_address_relevant(a) && + link_address_relevant(a, true) && a->link->llmnr_ipv6_scope && a->link->llmnr_support == RESOLVE_SUPPORT_YES && a->link->manager->llmnr_support == RESOLVE_SUPPORT_YES) { @@ -826,13 +829,13 @@ int link_address_update_rtnl(LinkAddress *a, sd_netlink_message *m) { return 0; } -bool link_address_relevant(LinkAddress *a) { +bool link_address_relevant(LinkAddress *a, bool local_multicast) { assert(a); if (a->flags & (IFA_F_DEPRECATED|IFA_F_TENTATIVE)) return false; - if (IN_SET(a->scope, RT_SCOPE_HOST, RT_SCOPE_NOWHERE)) + if (a->scope >= (local_multicast ? RT_SCOPE_HOST : RT_SCOPE_LINK)) return false; return true; diff --git a/src/resolve/resolved-link.h b/src/resolve/resolved-link.h index 3b6aafb8f0..29e7b72247 100644 --- a/src/resolve/resolved-link.h +++ b/src/resolve/resolved-link.h @@ -89,7 +89,7 @@ int link_new(Manager *m, Link **ret, int ifindex); Link *link_free(Link *l); int link_update_rtnl(Link *l, sd_netlink_message *m); int link_update_monitor(Link *l); -bool link_relevant(Link *l, int family, bool multicast); +bool link_relevant(Link *l, int family, bool local_multicast); LinkAddress* link_find_address(Link *l, int family, const union in_addr_union *in_addr); void link_add_rrs(Link *l, bool force_remove); @@ -107,7 +107,7 @@ bool link_dnssec_supported(Link *l); int link_address_new(Link *l, LinkAddress **ret, int family, const union in_addr_union *in_addr); LinkAddress *link_address_free(LinkAddress *a); int link_address_update_rtnl(LinkAddress *a, sd_netlink_message *m); -bool link_address_relevant(LinkAddress *l); +bool link_address_relevant(LinkAddress *l, bool local_multicast); void link_address_add_rrs(LinkAddress *a, bool force_remove); DEFINE_TRIVIAL_CLEANUP_FUNC(Link*, link_free); diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index fbd188c2ac..f1dbda1a6a 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -1226,3 +1226,18 @@ void manager_dnssec_verdict(Manager *m, DnssecVerdict verdict, const DnsResource m->n_dnssec_verdict[verdict]++; } + +bool manager_routable(Manager *m, int family) { + Iterator i; + Link *l; + + assert(m); + + /* Returns true if the host has at least one interface with a routable address of the specified type */ + + HASHMAP_FOREACH(l, m->links, i) + if (link_relevant(l, family, false)) + return true; + + return false; +} diff --git a/src/resolve/resolved-manager.h b/src/resolve/resolved-manager.h index 1af49c8fb9..e2c539d3d2 100644 --- a/src/resolve/resolved-manager.h +++ b/src/resolve/resolved-manager.h @@ -169,3 +169,5 @@ DnssecMode manager_get_dnssec_mode(Manager *m); bool manager_dnssec_supported(Manager *m); void manager_dnssec_verdict(Manager *m, DnssecVerdict verdict, const DnsResourceKey *key); + +bool manager_routable(Manager *m, int family); From 53f555b6f6b0a9fb58a781e40e3db67924a2a9c8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 16:13:51 +0100 Subject: [PATCH 05/22] util: add usec_add() which adds two usec_t values with overflow handling --- src/basic/time-util.h | 13 +++++++++++++ src/test/test-time.c | 15 +++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/basic/time-util.h b/src/basic/time-util.h index 7321e3c670..b37d5ad5dc 100644 --- a/src/basic/time-util.h +++ b/src/basic/time-util.h @@ -127,3 +127,16 @@ time_t mktime_or_timegm(struct tm *tm, bool utc); struct tm *localtime_or_gmtime_r(const time_t *t, struct tm *tm, bool utc); unsigned long usec_to_jiffies(usec_t usec); + +static inline usec_t usec_add(usec_t a, usec_t b) { + usec_t c; + + /* Adds two time values, and makes sure USEC_INFINITY as input results as USEC_INFINITY in output, and doesn't + * overflow. */ + + c = a + b; + if (c < a || c < b) /* overflow check */ + return USEC_INFINITY; + + return c; +} diff --git a/src/test/test-time.c b/src/test/test-time.c index 8896b2c92b..ca44f81f9c 100644 --- a/src/test/test-time.c +++ b/src/test/test-time.c @@ -176,9 +176,19 @@ static void test_get_timezones(void) { r = get_timezones(&zones); assert_se(r == 0); - STRV_FOREACH(zone, zones) { + STRV_FOREACH(zone, zones) assert_se(timezone_is_valid(*zone)); - } +} + +static void test_usec_add(void) { + assert_se(usec_add(0, 0) == 0); + assert_se(usec_add(1, 4) == 5); + assert_se(usec_add(USEC_INFINITY, 5) == USEC_INFINITY); + assert_se(usec_add(5, USEC_INFINITY) == USEC_INFINITY); + assert_se(usec_add(USEC_INFINITY-5, 2) == USEC_INFINITY-3); + assert_se(usec_add(USEC_INFINITY-2, 2) == USEC_INFINITY); + assert_se(usec_add(USEC_INFINITY-1, 2) == USEC_INFINITY); + assert_se(usec_add(USEC_INFINITY, 2) == USEC_INFINITY); } int main(int argc, char *argv[]) { @@ -190,6 +200,7 @@ int main(int argc, char *argv[]) { test_format_timespan(USEC_PER_SEC); test_timezone_is_valid(); test_get_timezones(); + test_usec_add(); return 0; } From 1bce0ffa66f329bd50d8bfaa943a755caa65b269 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 00:19:14 +0100 Subject: [PATCH 06/22] sd-event: when determining the last allowed time a time event may elapse, deal with overflows --- src/libsystemd/sd-event/sd-event.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c index 11c7330b9b..8552d5a54e 100644 --- a/src/libsystemd/sd-event/sd-event.c +++ b/src/libsystemd/sd-event/sd-event.c @@ -327,6 +327,10 @@ static int earliest_time_prioq_compare(const void *a, const void *b) { return 0; } +static usec_t time_event_source_latest(const sd_event_source *s) { + return usec_add(s->time.next, s->time.accuracy); +} + static int latest_time_prioq_compare(const void *a, const void *b) { const sd_event_source *x = a, *y = b; @@ -346,9 +350,9 @@ static int latest_time_prioq_compare(const void *a, const void *b) { return 1; /* Order by time */ - if (x->time.next + x->time.accuracy < y->time.next + y->time.accuracy) + if (time_event_source_latest(x) < time_event_source_latest(y)) return -1; - if (x->time.next + x->time.accuracy > y->time.next + y->time.accuracy) + if (time_event_source_latest(x) > time_event_source_latest(y)) return 1; return 0; @@ -1995,7 +1999,7 @@ static int event_arm_timer( b = prioq_peek(d->latest); assert_se(b && b->enabled != SD_EVENT_OFF); - t = sleep_between(e, a->time.next, b->time.next + b->time.accuracy); + t = sleep_between(e, a->time.next, time_event_source_latest(b)); if (d->next == t) return 0; From 393003e1debf7c7f75beaacbd532b92c3e3dc729 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 00:20:18 +0100 Subject: [PATCH 07/22] sd-event: permit a USEC_INFINITY timeout as an alternative to a disabling an event source This should simplify handling of time events in clients and is in-line with the USEC_INFINITY macro we already have. This way setting a timeout to 0 indicates "elapse immediately", and a timeout of USEC_INFINITY "elapse never". --- man/sd_event_add_time.xml | 57 ++++++++++++------------------ src/libsystemd/sd-event/sd-event.c | 6 ++-- 2 files changed, 25 insertions(+), 38 deletions(-) diff --git a/man/sd_event_add_time.xml b/man/sd_event_add_time.xml index 142fa80f8f..a2c0d54b56 100644 --- a/man/sd_event_add_time.xml +++ b/man/sd_event_add_time.xml @@ -114,41 +114,28 @@ Description - sd_event_add_time() adds a new timer - event source to an event loop. The event loop object is specified - in the event parameter, the event source - object is returned in the source - parameter. The clock parameter takes a - clock identifier, one of CLOCK_REALTIME, - CLOCK_MONOTONIC, - CLOCK_BOOTTIME, - CLOCK_REALTIME_ALARM, or - CLOCK_BOOTTIME_ALARM. See - timerfd_create2 - for details regarding the various types of clocks. The - usec parameter specifies the earliest time, - in microseconds (µs), relative to the clock's epoch, when - the timer shall be triggered. If a time already in the past is - specified (including 0), this timer source - "fires" immediately and is ready to be dispatched. The - accuracy parameter specifies an additional - accuracy value in µs specifying how much the timer event may be - delayed. Use 0 to select the default accuracy - (250ms). Use 1µs for maximum accuracy. Consider specifying - 60000000µs (1min) or larger for long-running events that may be - delayed substantially. Picking higher accuracy values allows the - system to coalesce timer events more aggressively, improving - power efficiency. The handler parameter - shall reference a function to call when the timer elapses. The - handler function will be passed the - userdata pointer, which may be chosen - freely by the caller. The handler is also passed the configured - trigger time, even if it is actually called - slightly later, subject to the specified accuracy value, - the kernel timer slack (see - prctl2), - and additional scheduling latencies. To query the actual time the - handler was called use + sd_event_add_time() adds a new timer event source to an event loop. The event loop + object is specified in the event parameter, the event source object is returned in the + source parameter. The clock parameter takes a clock identifier, one + of CLOCK_REALTIME, CLOCK_MONOTONIC, CLOCK_BOOTTIME, + CLOCK_REALTIME_ALARM, or CLOCK_BOOTTIME_ALARM. See + timerfd_create2 for details + regarding the various types of clocks. The usec parameter specifies the earliest time, in + microseconds (µs), relative to the clock's epoch, when the timer shall be triggered. If a time already in the past + is specified (including 0), this timer source "fires" immediately and is ready to be + dispatched. If the paramater is specified as UINT64_MAX the timer event will never elapse, + which may be used as an alternative to explicitly disabling a timer event source with + sd_event_source_set_enabled3. The + accuracy parameter specifies an additional accuracy value in µs specifying how much the + timer event may be delayed. Use 0 to select the default accuracy (250ms). Use 1µs for maximum + accuracy. Consider specifying 60000000µs (1min) or larger for long-running events that may be delayed + substantially. Picking higher accuracy values allows the system to coalesce timer events more aggressively, + improving power efficiency. The handler parameter shall reference a function to call when + the timer elapses. The handler function will be passed the userdata pointer, which may be + chosen freely by the caller. The handler is also passed the configured trigger time, even if it is actually called + slightly later, subject to the specified accuracy value, the kernel timer slack (see + prctl2), and additional + scheduling latencies. To query the actual time the handler was called use sd_event_now3. By default, the timer will elapse once diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c index 8552d5a54e..eeb3453919 100644 --- a/src/libsystemd/sd-event/sd-event.c +++ b/src/libsystemd/sd-event/sd-event.c @@ -1070,7 +1070,6 @@ _public_ int sd_event_add_time( int r; assert_return(e, -EINVAL); - assert_return(usec != (uint64_t) -1, -EINVAL); assert_return(accuracy != (uint64_t) -1, -EINVAL); assert_return(e->state != SD_EVENT_FINISHED, -ESTALE); assert_return(!event_pid_changed(e), -ECHILD); @@ -1760,7 +1759,6 @@ _public_ int sd_event_source_set_time(sd_event_source *s, uint64_t usec) { struct clock_data *d; assert_return(s, -EINVAL); - assert_return(usec != (uint64_t) -1, -EINVAL); assert_return(EVENT_SOURCE_IS_TIME(s->type), -EDOM); assert_return(s->event->state != SD_EVENT_FINISHED, -ESTALE); assert_return(!event_pid_changed(s->event), -ECHILD); @@ -1890,6 +1888,8 @@ static usec_t sleep_between(sd_event *e, usec_t a, usec_t b) { if (a <= 0) return 0; + if (a >= USEC_INFINITY) + return USEC_INFINITY; if (b <= a + 1) return a; @@ -1979,7 +1979,7 @@ static int event_arm_timer( d->needs_rearm = false; a = prioq_peek(d->earliest); - if (!a || a->enabled == SD_EVENT_OFF) { + if (!a || a->enabled == SD_EVENT_OFF || a->time.next == USEC_INFINITY) { if (d->fd < 0) return 0; From a483fb59a8dd908a3e4b20d62410d022d511eaa8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 16:01:25 +0100 Subject: [PATCH 08/22] core: store for each unit when the last low-level unit state change took place This adds a new timestamp field to the Unit struct, storing when the last low-level state change took place, and make sure this is restored after a daemon reload. This new field is useful to allow restarting of per-state timers exactly where they originally started. --- src/core/dbus-unit.c | 1 + src/core/unit.c | 32 ++++++++++++++++++++++++-------- src/core/unit.h | 4 ++++ 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index dca9f77528..d7929e5566 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -674,6 +674,7 @@ const sd_bus_vtable bus_unit_vtable[] = { SD_BUS_PROPERTY("DropInPaths", "as", NULL, offsetof(Unit, dropin_paths), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("UnitFileState", "s", property_get_unit_file_state, 0, 0), SD_BUS_PROPERTY("UnitFilePreset", "s", property_get_unit_file_preset, 0, 0), + BUS_PROPERTY_DUAL_TIMESTAMP("StateChangeTimestamp", offsetof(Unit, state_change_timestamp), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), BUS_PROPERTY_DUAL_TIMESTAMP("InactiveExitTimestamp", offsetof(Unit, inactive_exit_timestamp), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), BUS_PROPERTY_DUAL_TIMESTAMP("ActiveEnterTimestamp", offsetof(Unit, active_enter_timestamp), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), BUS_PROPERTY_DUAL_TIMESTAMP("ActiveExitTimestamp", offsetof(Unit, active_exit_timestamp), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), diff --git a/src/core/unit.c b/src/core/unit.c index b6fbf4e785..05466a84f9 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -869,6 +869,7 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) { Iterator i; const char *prefix2; char + timestamp0[FORMAT_TIMESTAMP_MAX], timestamp1[FORMAT_TIMESTAMP_MAX], timestamp2[FORMAT_TIMESTAMP_MAX], timestamp3[FORMAT_TIMESTAMP_MAX], @@ -890,6 +891,7 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) { "%s\tInstance: %s\n" "%s\tUnit Load State: %s\n" "%s\tUnit Active State: %s\n" + "%s\nState Change Timestamp: %s\n" "%s\tInactive Exit Timestamp: %s\n" "%s\tActive Enter Timestamp: %s\n" "%s\tActive Exit Timestamp: %s\n" @@ -907,6 +909,7 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) { prefix, strna(u->instance), prefix, unit_load_state_to_string(u->load_state), prefix, unit_active_state_to_string(unit_active_state(u)), + prefix, strna(format_timestamp(timestamp0, sizeof(timestamp0), u->state_change_timestamp.realtime)), prefix, strna(format_timestamp(timestamp1, sizeof(timestamp1), u->inactive_exit_timestamp.realtime)), prefix, strna(format_timestamp(timestamp2, sizeof(timestamp2), u->active_enter_timestamp.realtime)), prefix, strna(format_timestamp(timestamp3, sizeof(timestamp3), u->active_exit_timestamp.realtime)), @@ -1821,19 +1824,17 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su /* Update timestamps for state changes */ if (m->n_reloading <= 0) { - dual_timestamp ts; - - dual_timestamp_get(&ts); + dual_timestamp_get(&u->state_change_timestamp); if (UNIT_IS_INACTIVE_OR_FAILED(os) && !UNIT_IS_INACTIVE_OR_FAILED(ns)) - u->inactive_exit_timestamp = ts; + u->inactive_exit_timestamp = u->state_change_timestamp; else if (!UNIT_IS_INACTIVE_OR_FAILED(os) && UNIT_IS_INACTIVE_OR_FAILED(ns)) - u->inactive_enter_timestamp = ts; + u->inactive_enter_timestamp = u->state_change_timestamp; if (!UNIT_IS_ACTIVE_OR_RELOADING(os) && UNIT_IS_ACTIVE_OR_RELOADING(ns)) - u->active_enter_timestamp = ts; + u->active_enter_timestamp = u->state_change_timestamp; else if (UNIT_IS_ACTIVE_OR_RELOADING(os) && !UNIT_IS_ACTIVE_OR_RELOADING(ns)) - u->active_exit_timestamp = ts; + u->active_exit_timestamp = u->state_change_timestamp; } /* Keep track of failed units */ @@ -2553,10 +2554,13 @@ int unit_serialize(Unit *u, FILE *f, FDSet *fds, bool serialize_jobs) { } } + dual_timestamp_serialize(f, "state-change-timestamp", &u->state_change_timestamp); + dual_timestamp_serialize(f, "inactive-exit-timestamp", &u->inactive_exit_timestamp); dual_timestamp_serialize(f, "active-enter-timestamp", &u->active_enter_timestamp); dual_timestamp_serialize(f, "active-exit-timestamp", &u->active_exit_timestamp); dual_timestamp_serialize(f, "inactive-enter-timestamp", &u->inactive_enter_timestamp); + dual_timestamp_serialize(f, "condition-timestamp", &u->condition_timestamp); dual_timestamp_serialize(f, "assert-timestamp", &u->assert_timestamp); @@ -2695,7 +2699,7 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { /* End marker */ if (isempty(l)) - return 0; + break; k = strcspn(l, "="); @@ -2735,6 +2739,9 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { } else /* legacy for pre-44 */ log_unit_warning(u, "Update from too old systemd versions are unsupported, cannot deserialize job: %s", v); continue; + } else if (streq(l, "state-change-timestamp")) { + dual_timestamp_deserialize(v, &u->state_change_timestamp); + continue; } else if (streq(l, "inactive-exit-timestamp")) { dual_timestamp_deserialize(v, &u->inactive_exit_timestamp); continue; @@ -2841,6 +2848,15 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { log_unit_warning(u, "Failed to deserialize unit parameter '%s', ignoring.", l); } } + + /* Versions before 228 did not carry a state change timestamp. In this case, take the current time. This is + * useful, so that timeouts based on this timestamp don't trigger too early, and is in-line with the logic from + * before 228 where the base for timeouts was not peristet across reboots. */ + + if (!dual_timestamp_is_set(&u->state_change_timestamp)) + dual_timestamp_get(&u->state_change_timestamp); + + return 0; } int unit_add_node_link(Unit *u, const char *what, bool wants, UnitDependency dep) { diff --git a/src/core/unit.h b/src/core/unit.h index 3eb3484fb7..f86a0f687b 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -121,6 +121,10 @@ struct Unit { dual_timestamp condition_timestamp; dual_timestamp assert_timestamp; + /* Updated whenever the low-level state changes */ + dual_timestamp state_change_timestamp; + + /* Updated whenever the (high-level) active state enters or leaves the active or inactive states */ dual_timestamp inactive_exit_timestamp; dual_timestamp active_enter_timestamp; dual_timestamp active_exit_timestamp; From d0a7c5f69207b6719bab94893035fc8f5f6f87cb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 21:07:09 +0100 Subject: [PATCH 09/22] core: move parsing of rlimits into rlimit-util.[ch] This way we can reuse it for parsing rlimit settings in "systemctl set-property" and related commands. --- src/basic/rlimit-util.c | 176 +++++++++++++++++++++ src/basic/rlimit-util.h | 3 + src/core/load-fragment-gperf.gperf.m4 | 20 +-- src/core/load-fragment.c | 212 +++----------------------- src/core/load-fragment.h | 3 - src/core/main.c | 32 ++-- src/test/test-unit-file.c | 29 ++-- 7 files changed, 243 insertions(+), 232 deletions(-) diff --git a/src/basic/rlimit-util.c b/src/basic/rlimit-util.c index 44f885db16..11476ac8d1 100644 --- a/src/basic/rlimit-util.c +++ b/src/basic/rlimit-util.c @@ -22,10 +22,14 @@ #include #include +#include "alloc-util.h" +#include "extract-word.h" +#include "formats-util.h" #include "macro.h" #include "missing.h" #include "rlimit-util.h" #include "string-table.h" +#include "time-util.h" int setrlimit_closest(int resource, const struct rlimit *rlim) { struct rlimit highest, fixed; @@ -51,6 +55,178 @@ int setrlimit_closest(int resource, const struct rlimit *rlim) { return 0; } +static int rlimit_parse_u64(const char *val, rlim_t *ret) { + uint64_t u; + int r; + + assert(val); + assert(ret); + + if (streq(val, "infinity")) { + *ret = RLIM_INFINITY; + return 0; + } + + /* setrlimit(2) suggests rlim_t is always 64bit on Linux. */ + assert_cc(sizeof(rlim_t) == sizeof(uint64_t)); + + r = safe_atou64(val, &u); + if (r < 0) + return r; + if (u >= (uint64_t) RLIM_INFINITY) + return -ERANGE; + + *ret = (rlim_t) u; + return 0; +} + +static int rlimit_parse_size(const char *val, rlim_t *ret) { + uint64_t u; + int r; + + assert(val); + assert(ret); + + if (streq(val, "infinity")) { + *ret = RLIM_INFINITY; + return 0; + } + + r = parse_size(val, 1024, &u); + if (r < 0) + return r; + if (u >= (uint64_t) RLIM_INFINITY) + return -ERANGE; + + *ret = (rlim_t) u; + return 0; +} + +static int rlimit_parse_sec(const char *val, rlim_t *ret) { + uint64_t u; + usec_t t; + int r; + + assert(val); + assert(ret); + + if (streq(val, "infinity")) { + *ret = RLIM_INFINITY; + return 0; + } + + r = parse_sec(val, &t); + if (r < 0) + return r; + if (t == USEC_INFINITY) { + *ret = RLIM_INFINITY; + return 0; + } + + u = (uint64_t) DIV_ROUND_UP(t, USEC_PER_SEC); + if (u >= (uint64_t) RLIM_INFINITY) + return -ERANGE; + + *ret = (rlim_t) u; + return 0; +} + +static int rlimit_parse_usec(const char *val, rlim_t *ret) { + usec_t t; + int r; + + assert(val); + assert(ret); + + if (streq(val, "infinity")) { + *ret = RLIM_INFINITY; + return 0; + } + + r = parse_time(val, &t, 1); + if (r < 0) + return r; + if (t == USEC_INFINITY) { + *ret = RLIM_INFINITY; + return 0; + } + + *ret = (rlim_t) t; + return 0; +} + +static int (*const rlimit_parse_table[_RLIMIT_MAX])(const char *val, rlim_t *ret) = { + [RLIMIT_CPU] = rlimit_parse_sec, + [RLIMIT_FSIZE] = rlimit_parse_size, + [RLIMIT_DATA] = rlimit_parse_size, + [RLIMIT_STACK] = rlimit_parse_size, + [RLIMIT_CORE] = rlimit_parse_size, + [RLIMIT_RSS] = rlimit_parse_size, + [RLIMIT_NOFILE] = rlimit_parse_u64, + [RLIMIT_AS] = rlimit_parse_size, + [RLIMIT_NPROC] = rlimit_parse_u64, + [RLIMIT_MEMLOCK] = rlimit_parse_size, + [RLIMIT_LOCKS] = rlimit_parse_u64, + [RLIMIT_SIGPENDING] = rlimit_parse_u64, + [RLIMIT_MSGQUEUE] = rlimit_parse_size, + [RLIMIT_NICE] = rlimit_parse_u64, + [RLIMIT_RTPRIO] = rlimit_parse_u64, + [RLIMIT_RTTIME] = rlimit_parse_usec, +}; + +int rlimit_parse_one(int resource, const char *val, rlim_t *ret) { + assert(val); + assert(ret); + + if (resource < 0) + return -EINVAL; + if (resource >= _RLIMIT_MAX) + return -EINVAL; + + return rlimit_parse_table[resource](val, ret); +} + +int rlimit_parse(int resource, const char *val, struct rlimit *ret) { + _cleanup_free_ char *hard = NULL, *soft = NULL; + rlim_t hl, sl; + int r; + + assert(val); + assert(ret); + + r = extract_first_word(&val, &soft, ":", EXTRACT_DONT_COALESCE_SEPARATORS); + if (r < 0) + return r; + if (r == 0) + return -EINVAL; + + r = rlimit_parse_one(resource, soft, &sl); + if (r < 0) + return r; + + r = extract_first_word(&val, &hard, ":", EXTRACT_DONT_COALESCE_SEPARATORS); + if (r < 0) + return r; + if (!isempty(val)) + return -EINVAL; + if (r == 0) + hl = sl; + else { + r = rlimit_parse_one(resource, hard, &hl); + if (r < 0) + return r; + if (sl > hl) + return -EILSEQ; + } + + *ret = (struct rlimit) { + .rlim_cur = sl, + .rlim_max = hl, + }; + + return 0; +} + static const char* const rlimit_table[_RLIMIT_MAX] = { [RLIMIT_CPU] = "LimitCPU", [RLIMIT_FSIZE] = "LimitFSIZE", diff --git a/src/basic/rlimit-util.h b/src/basic/rlimit-util.h index 262f86dd04..fd2f7ff7cf 100644 --- a/src/basic/rlimit-util.h +++ b/src/basic/rlimit-util.h @@ -30,4 +30,7 @@ int rlimit_from_string(const char *s) _pure_; int setrlimit_closest(int resource, const struct rlimit *rlim); +int rlimit_parse_one(int resource, const char *val, rlim_t *ret); +int rlimit_parse(int resource, const char *val, struct rlimit *ret); + #define RLIMIT_MAKE_CONST(lim) ((struct rlimit) { lim, lim }) diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index 29ab1b6b9e..7211738685 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -60,22 +60,22 @@ $1.RestrictAddressFamilies, config_parse_address_families, 0, $1.SystemCallArchitectures, config_parse_warn_compat, DISABLED_CONFIGURATION, 0 $1.SystemCallErrorNumber, config_parse_warn_compat, DISABLED_CONFIGURATION, 0 $1.RestrictAddressFamilies, config_parse_warn_compat, DISABLED_CONFIGURATION, 0') -$1.LimitCPU, config_parse_sec_limit, RLIMIT_CPU, offsetof($1, exec_context.rlimit) -$1.LimitFSIZE, config_parse_bytes_limit, RLIMIT_FSIZE, offsetof($1, exec_context.rlimit) -$1.LimitDATA, config_parse_bytes_limit, RLIMIT_DATA, offsetof($1, exec_context.rlimit) -$1.LimitSTACK, config_parse_bytes_limit, RLIMIT_STACK, offsetof($1, exec_context.rlimit) -$1.LimitCORE, config_parse_bytes_limit, RLIMIT_CORE, offsetof($1, exec_context.rlimit) -$1.LimitRSS, config_parse_bytes_limit, RLIMIT_RSS, offsetof($1, exec_context.rlimit) +$1.LimitCPU, config_parse_limit, RLIMIT_CPU, offsetof($1, exec_context.rlimit) +$1.LimitFSIZE, config_parse_limit, RLIMIT_FSIZE, offsetof($1, exec_context.rlimit) +$1.LimitDATA, config_parse_limit, RLIMIT_DATA, offsetof($1, exec_context.rlimit) +$1.LimitSTACK, config_parse_limit, RLIMIT_STACK, offsetof($1, exec_context.rlimit) +$1.LimitCORE, config_parse_limit, RLIMIT_CORE, offsetof($1, exec_context.rlimit) +$1.LimitRSS, config_parse_limit, RLIMIT_RSS, offsetof($1, exec_context.rlimit) $1.LimitNOFILE, config_parse_limit, RLIMIT_NOFILE, offsetof($1, exec_context.rlimit) -$1.LimitAS, config_parse_bytes_limit, RLIMIT_AS, offsetof($1, exec_context.rlimit) +$1.LimitAS, config_parse_limit, RLIMIT_AS, offsetof($1, exec_context.rlimit) $1.LimitNPROC, config_parse_limit, RLIMIT_NPROC, offsetof($1, exec_context.rlimit) -$1.LimitMEMLOCK, config_parse_bytes_limit, RLIMIT_MEMLOCK, offsetof($1, exec_context.rlimit) +$1.LimitMEMLOCK, config_parse_limit, RLIMIT_MEMLOCK, offsetof($1, exec_context.rlimit) $1.LimitLOCKS, config_parse_limit, RLIMIT_LOCKS, offsetof($1, exec_context.rlimit) $1.LimitSIGPENDING, config_parse_limit, RLIMIT_SIGPENDING, offsetof($1, exec_context.rlimit) -$1.LimitMSGQUEUE, config_parse_bytes_limit, RLIMIT_MSGQUEUE, offsetof($1, exec_context.rlimit) +$1.LimitMSGQUEUE, config_parse_limit, RLIMIT_MSGQUEUE, offsetof($1, exec_context.rlimit) $1.LimitNICE, config_parse_limit, RLIMIT_NICE, offsetof($1, exec_context.rlimit) $1.LimitRTPRIO, config_parse_limit, RLIMIT_RTPRIO, offsetof($1, exec_context.rlimit) -$1.LimitRTTIME, config_parse_usec_limit, RLIMIT_RTTIME, offsetof($1, exec_context.rlimit) +$1.LimitRTTIME, config_parse_limit, RLIMIT_RTTIME, offsetof($1, exec_context.rlimit) $1.ReadWriteDirectories, config_parse_namespace_path_strv, 0, offsetof($1, exec_context.read_write_dirs) $1.ReadOnlyDirectories, config_parse_namespace_path_strv, 0, offsetof($1, exec_context.read_only_dirs) $1.InaccessibleDirectories, config_parse_namespace_path_strv, 0, offsetof($1, exec_context.inaccessible_dirs) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 903e6f0cf6..3b1fbd8b90 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -54,6 +54,7 @@ #include "parse-util.h" #include "path-util.h" #include "process-util.h" +#include "rlimit-util.h" #ifdef HAVE_SECCOMP #include "seccomp-util.h" #endif @@ -1101,122 +1102,6 @@ int config_parse_capability_set( return 0; } -static int rlim_parse_u64(const char *val, rlim_t *res) { - int r = 0; - - if (streq(val, "infinity")) - *res = RLIM_INFINITY; - else { - uint64_t u; - - /* setrlimit(2) suggests rlim_t is always 64bit on Linux. */ - assert_cc(sizeof(rlim_t) == sizeof(uint64_t)); - - r = safe_atou64(val, &u); - if (r >= 0 && u >= (uint64_t) RLIM_INFINITY) - r = -ERANGE; - if (r == 0) - *res = (rlim_t) u; - } - return r; -} - -static int rlim_parse_size(const char *val, rlim_t *res) { - int r = 0; - - if (streq(val, "infinity")) - *res = RLIM_INFINITY; - else { - uint64_t u; - - r = parse_size(val, 1024, &u); - if (r >= 0 && u >= (uint64_t) RLIM_INFINITY) - r = -ERANGE; - if (r == 0) - *res = (rlim_t) u; - } - return r; -} - -static int rlim_parse_sec(const char *val, rlim_t *res) { - int r = 0; - - if (streq(val, "infinity")) - *res = RLIM_INFINITY; - else { - usec_t t; - - r = parse_sec(val, &t); - if (r < 0) - return r; - if (t == USEC_INFINITY) - *res = RLIM_INFINITY; - else - *res = (rlim_t) (DIV_ROUND_UP(t, USEC_PER_SEC)); - - } - return r; -} - -static int rlim_parse_usec(const char *val, rlim_t *res) { - int r = 0; - - if (streq(val, "infinity")) - *res = RLIM_INFINITY; - else { - usec_t t; - - r = parse_time(val, &t, 1); - if (r < 0) - return r; - if (t == USEC_INFINITY) - *res = RLIM_INFINITY; - else - *res = (rlim_t) t; - } - return r; -} - -static int parse_rlimit_range( - const char *unit, - const char *filename, - unsigned line, - const char *value, - struct rlimit **rl, - int (*rlim_parser)(const char *, rlim_t *)) { - - const char *whole_value = value; - rlim_t soft, hard; - _cleanup_free_ char *sword = NULL, *hword = NULL; - int nwords, r; - - assert(value); - - /* or */ - nwords = extract_many_words(&value, ":", EXTRACT_DONT_COALESCE_SEPARATORS, &sword, &hword, NULL); - r = nwords < 0 ? nwords : nwords == 0 ? -EINVAL : 0; - - if (r == 0) - r = rlim_parser(sword, &soft); - if (r == 0 && nwords == 2) - r = rlim_parser(hword, &hard); - if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse resource value, ignoring: %s", whole_value); - return 0; - } - if (nwords == 2 && soft > hard) - return log_syntax(unit, LOG_WARNING, filename, line, 0, "Invalid resource value ("RLIM_FMT" > "RLIM_FMT"), ignoring: %s", soft, hard, whole_value); - - if (!*rl) { - *rl = new(struct rlimit, 1); - if (!*rl) - return log_oom(); - } - (*rl)->rlim_cur = soft; - (*rl)->rlim_max = nwords == 2 ? hard : soft; - return 0; -} - int config_parse_limit( const char *unit, const char *filename, @@ -1229,88 +1114,35 @@ int config_parse_limit( void *data, void *userdata) { - struct rlimit **rl = data; + struct rlimit **rl = data, d = {}; + int r; assert(filename); assert(lvalue); assert(rvalue); assert(data); - rl += ltype; - return parse_rlimit_range(unit, filename, line, rvalue, rl, rlim_parse_u64); + r = rlimit_parse(ltype, rvalue, &d); + if (r == -EILSEQ) { + log_syntax(unit, LOG_WARNING, filename, line, r, "Soft resource limit chosen higher than hard limit, ignoring: %s", rvalue); + return 0; + } + if (r < 0) { + log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse resource value, ignoring: %s", rvalue); + return 0; + } + + if (rl[ltype]) + *rl[ltype] = d; + else { + rl[ltype] = newdup(struct rlimit, &d, 1); + if (!rl[ltype]) + return log_oom(); + } + + return 0; } -int config_parse_bytes_limit( - const char *unit, - const char *filename, - unsigned line, - const char *section, - unsigned section_line, - const char *lvalue, - int ltype, - const char *rvalue, - void *data, - void *userdata) { - - struct rlimit **rl = data; - - assert(filename); - assert(lvalue); - assert(rvalue); - assert(data); - - rl += ltype; - return parse_rlimit_range(unit, filename, line, rvalue, rl, rlim_parse_size); -} - -int config_parse_sec_limit( - const char *unit, - const char *filename, - unsigned line, - const char *section, - unsigned section_line, - const char *lvalue, - int ltype, - const char *rvalue, - void *data, - void *userdata) { - - struct rlimit **rl = data; - - assert(filename); - assert(lvalue); - assert(rvalue); - assert(data); - - rl += ltype; - return parse_rlimit_range(unit, filename, line, rvalue, rl, rlim_parse_sec); -} - -int config_parse_usec_limit( - const char *unit, - const char *filename, - unsigned line, - const char *section, - unsigned section_line, - const char *lvalue, - int ltype, - const char *rvalue, - void *data, - void *userdata) { - - struct rlimit **rl = data; - - assert(filename); - assert(lvalue); - assert(rvalue); - assert(data); - - rl += ltype; - return parse_rlimit_range(unit, filename, line, rvalue, rl, rlim_parse_usec); -} - - - #ifdef HAVE_SYSV_COMPAT int config_parse_sysv_priority(const char *unit, const char *filename, diff --git a/src/core/load-fragment.h b/src/core/load-fragment.h index f0027a6b43..20dd84ba95 100644 --- a/src/core/load-fragment.h +++ b/src/core/load-fragment.h @@ -58,9 +58,6 @@ int config_parse_exec_capabilities(const char *unit, const char *filename, unsig int config_parse_exec_secure_bits(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_capability_set(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_limit(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); -int config_parse_bytes_limit(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); -int config_parse_sec_limit(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); -int config_parse_usec_limit(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_sysv_priority(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_kill_signal(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_exec_mount_flags(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); diff --git a/src/core/main.c b/src/core/main.c index 27ba6af031..2e1d07811c 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -676,22 +676,22 @@ static int parse_config_file(void) { { "Manager", "DefaultStartLimitInterval", config_parse_sec, 0, &arg_default_start_limit_interval }, { "Manager", "DefaultStartLimitBurst", config_parse_unsigned, 0, &arg_default_start_limit_burst }, { "Manager", "DefaultEnvironment", config_parse_environ, 0, &arg_default_environment }, - { "Manager", "DefaultLimitCPU", config_parse_sec_limit, 0, &arg_default_rlimit[RLIMIT_CPU] }, - { "Manager", "DefaultLimitFSIZE", config_parse_bytes_limit, 0, &arg_default_rlimit[RLIMIT_FSIZE] }, - { "Manager", "DefaultLimitDATA", config_parse_bytes_limit, 0, &arg_default_rlimit[RLIMIT_DATA] }, - { "Manager", "DefaultLimitSTACK", config_parse_bytes_limit, 0, &arg_default_rlimit[RLIMIT_STACK] }, - { "Manager", "DefaultLimitCORE", config_parse_bytes_limit, 0, &arg_default_rlimit[RLIMIT_CORE] }, - { "Manager", "DefaultLimitRSS", config_parse_bytes_limit, 0, &arg_default_rlimit[RLIMIT_RSS] }, - { "Manager", "DefaultLimitNOFILE", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_NOFILE] }, - { "Manager", "DefaultLimitAS", config_parse_bytes_limit, 0, &arg_default_rlimit[RLIMIT_AS] }, - { "Manager", "DefaultLimitNPROC", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_NPROC] }, - { "Manager", "DefaultLimitMEMLOCK", config_parse_bytes_limit, 0, &arg_default_rlimit[RLIMIT_MEMLOCK] }, - { "Manager", "DefaultLimitLOCKS", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_LOCKS] }, - { "Manager", "DefaultLimitSIGPENDING", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_SIGPENDING] }, - { "Manager", "DefaultLimitMSGQUEUE", config_parse_bytes_limit, 0, &arg_default_rlimit[RLIMIT_MSGQUEUE] }, - { "Manager", "DefaultLimitNICE", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_NICE] }, - { "Manager", "DefaultLimitRTPRIO", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_RTPRIO] }, - { "Manager", "DefaultLimitRTTIME", config_parse_usec_limit, 0, &arg_default_rlimit[RLIMIT_RTTIME] }, + { "Manager", "DefaultLimitCPU", config_parse_limit, RLIMIT_CPU, arg_default_rlimit }, + { "Manager", "DefaultLimitFSIZE", config_parse_limit, RLIMIT_FSIZE, arg_default_rlimit }, + { "Manager", "DefaultLimitDATA", config_parse_limit, RLIMIT_DATA, arg_default_rlimit }, + { "Manager", "DefaultLimitSTACK", config_parse_limit, RLIMIT_STACK, arg_default_rlimit }, + { "Manager", "DefaultLimitCORE", config_parse_limit, RLIMIT_CORE, arg_default_rlimit }, + { "Manager", "DefaultLimitRSS", config_parse_limit, RLIMIT_RSS, arg_default_rlimit }, + { "Manager", "DefaultLimitNOFILE", config_parse_limit, RLIMIT_NOFILE, arg_default_rlimit }, + { "Manager", "DefaultLimitAS", config_parse_limit, RLIMIT_AS, arg_default_rlimit }, + { "Manager", "DefaultLimitNPROC", config_parse_limit, RLIMIT_NPROC, arg_default_rlimit }, + { "Manager", "DefaultLimitMEMLOCK", config_parse_limit, RLIMIT_MEMLOCK, arg_default_rlimit }, + { "Manager", "DefaultLimitLOCKS", config_parse_limit, RLIMIT_LOCKS, arg_default_rlimit }, + { "Manager", "DefaultLimitSIGPENDING", config_parse_limit, RLIMIT_SIGPENDING, arg_default_rlimit }, + { "Manager", "DefaultLimitMSGQUEUE", config_parse_limit, RLIMIT_MSGQUEUE, arg_default_rlimit }, + { "Manager", "DefaultLimitNICE", config_parse_limit, RLIMIT_NICE, arg_default_rlimit }, + { "Manager", "DefaultLimitRTPRIO", config_parse_limit, RLIMIT_RTPRIO, arg_default_rlimit }, + { "Manager", "DefaultLimitRTTIME", config_parse_limit, RLIMIT_RTTIME, arg_default_rlimit }, { "Manager", "DefaultCPUAccounting", config_parse_bool, 0, &arg_default_cpu_accounting }, { "Manager", "DefaultBlockIOAccounting", config_parse_bool, 0, &arg_default_blockio_accounting }, { "Manager", "DefaultMemoryAccounting", config_parse_bool, 0, &arg_default_memory_accounting }, diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c index 199623e025..a458870846 100644 --- a/src/test/test-unit-file.c +++ b/src/test/test-unit-file.c @@ -712,12 +712,15 @@ static void test_config_parse_rlimit(void) { assert_se(rl[RLIMIT_NOFILE]->rlim_cur == RLIM_INFINITY); assert_se(rl[RLIMIT_NOFILE]->rlim_cur == rl[RLIMIT_NOFILE]->rlim_max); + rl[RLIMIT_NOFILE]->rlim_cur = 10; + rl[RLIMIT_NOFILE]->rlim_max = 20; + + /* Invalid values don't change rl */ assert_se(config_parse_limit(NULL, "fake", 1, "section", 1, "LimitNOFILE", RLIMIT_NOFILE, "10:20:30", rl, NULL) >= 0); assert_se(rl[RLIMIT_NOFILE]); assert_se(rl[RLIMIT_NOFILE]->rlim_cur == 10); assert_se(rl[RLIMIT_NOFILE]->rlim_max == 20); - /* Invalid values don't change rl */ assert_se(config_parse_limit(NULL, "fake", 1, "section", 1, "LimitNOFILE", RLIMIT_NOFILE, "wat:wat", rl, NULL) >= 0); assert_se(rl[RLIMIT_NOFILE]); assert_se(rl[RLIMIT_NOFILE]->rlim_cur == 10); @@ -735,64 +738,64 @@ static void test_config_parse_rlimit(void) { rl[RLIMIT_NOFILE] = mfree(rl[RLIMIT_NOFILE]); - assert_se(config_parse_sec_limit(NULL, "fake", 1, "section", 1, "LimitCPU", RLIMIT_CPU, "56", rl, NULL) >= 0); + assert_se(config_parse_limit(NULL, "fake", 1, "section", 1, "LimitCPU", RLIMIT_CPU, "56", rl, NULL) >= 0); assert_se(rl[RLIMIT_CPU]); assert_se(rl[RLIMIT_CPU]->rlim_cur == 56); assert_se(rl[RLIMIT_CPU]->rlim_cur == rl[RLIMIT_CPU]->rlim_max); - assert_se(config_parse_sec_limit(NULL, "fake", 1, "section", 1, "LimitCPU", RLIMIT_CPU, "57s", rl, NULL) >= 0); + assert_se(config_parse_limit(NULL, "fake", 1, "section", 1, "LimitCPU", RLIMIT_CPU, "57s", rl, NULL) >= 0); assert_se(rl[RLIMIT_CPU]); assert_se(rl[RLIMIT_CPU]->rlim_cur == 57); assert_se(rl[RLIMIT_CPU]->rlim_cur == rl[RLIMIT_CPU]->rlim_max); - assert_se(config_parse_sec_limit(NULL, "fake", 1, "section", 1, "LimitCPU", RLIMIT_CPU, "40s:1m", rl, NULL) >= 0); + assert_se(config_parse_limit(NULL, "fake", 1, "section", 1, "LimitCPU", RLIMIT_CPU, "40s:1m", rl, NULL) >= 0); assert_se(rl[RLIMIT_CPU]); assert_se(rl[RLIMIT_CPU]->rlim_cur == 40); assert_se(rl[RLIMIT_CPU]->rlim_max == 60); - assert_se(config_parse_sec_limit(NULL, "fake", 1, "section", 1, "LimitCPU", RLIMIT_CPU, "infinity", rl, NULL) >= 0); + assert_se(config_parse_limit(NULL, "fake", 1, "section", 1, "LimitCPU", RLIMIT_CPU, "infinity", rl, NULL) >= 0); assert_se(rl[RLIMIT_CPU]); assert_se(rl[RLIMIT_CPU]->rlim_cur == RLIM_INFINITY); assert_se(rl[RLIMIT_CPU]->rlim_cur == rl[RLIMIT_CPU]->rlim_max); - assert_se(config_parse_sec_limit(NULL, "fake", 1, "section", 1, "LimitCPU", RLIMIT_CPU, "1234ms", rl, NULL) >= 0); + assert_se(config_parse_limit(NULL, "fake", 1, "section", 1, "LimitCPU", RLIMIT_CPU, "1234ms", rl, NULL) >= 0); assert_se(rl[RLIMIT_CPU]); assert_se(rl[RLIMIT_CPU]->rlim_cur == 2); assert_se(rl[RLIMIT_CPU]->rlim_cur == rl[RLIMIT_CPU]->rlim_max); rl[RLIMIT_CPU] = mfree(rl[RLIMIT_CPU]); - assert_se(config_parse_usec_limit(NULL, "fake", 1, "section", 1, "LimitRTTIME", RLIMIT_RTTIME, "58", rl, NULL) >= 0); + assert_se(config_parse_limit(NULL, "fake", 1, "section", 1, "LimitRTTIME", RLIMIT_RTTIME, "58", rl, NULL) >= 0); assert_se(rl[RLIMIT_RTTIME]); assert_se(rl[RLIMIT_RTTIME]->rlim_cur == 58); assert_se(rl[RLIMIT_RTTIME]->rlim_cur == rl[RLIMIT_RTTIME]->rlim_max); - assert_se(config_parse_usec_limit(NULL, "fake", 1, "section", 1, "LimitRTTIME", RLIMIT_RTTIME, "58:60", rl, NULL) >= 0); + assert_se(config_parse_limit(NULL, "fake", 1, "section", 1, "LimitRTTIME", RLIMIT_RTTIME, "58:60", rl, NULL) >= 0); assert_se(rl[RLIMIT_RTTIME]); assert_se(rl[RLIMIT_RTTIME]->rlim_cur == 58); assert_se(rl[RLIMIT_RTTIME]->rlim_max == 60); - assert_se(config_parse_usec_limit(NULL, "fake", 1, "section", 1, "LimitRTTIME", RLIMIT_RTTIME, "59s", rl, NULL) >= 0); + assert_se(config_parse_limit(NULL, "fake", 1, "section", 1, "LimitRTTIME", RLIMIT_RTTIME, "59s", rl, NULL) >= 0); assert_se(rl[RLIMIT_RTTIME]); assert_se(rl[RLIMIT_RTTIME]->rlim_cur == 59 * USEC_PER_SEC); assert_se(rl[RLIMIT_RTTIME]->rlim_cur == rl[RLIMIT_RTTIME]->rlim_max); - assert_se(config_parse_usec_limit(NULL, "fake", 1, "section", 1, "LimitRTTIME", RLIMIT_RTTIME, "59s:123s", rl, NULL) >= 0); + assert_se(config_parse_limit(NULL, "fake", 1, "section", 1, "LimitRTTIME", RLIMIT_RTTIME, "59s:123s", rl, NULL) >= 0); assert_se(rl[RLIMIT_RTTIME]); assert_se(rl[RLIMIT_RTTIME]->rlim_cur == 59 * USEC_PER_SEC); assert_se(rl[RLIMIT_RTTIME]->rlim_max == 123 * USEC_PER_SEC); - assert_se(config_parse_usec_limit(NULL, "fake", 1, "section", 1, "LimitRTTIME", RLIMIT_RTTIME, "infinity", rl, NULL) >= 0); + assert_se(config_parse_limit(NULL, "fake", 1, "section", 1, "LimitRTTIME", RLIMIT_RTTIME, "infinity", rl, NULL) >= 0); assert_se(rl[RLIMIT_RTTIME]); assert_se(rl[RLIMIT_RTTIME]->rlim_cur == RLIM_INFINITY); assert_se(rl[RLIMIT_RTTIME]->rlim_cur == rl[RLIMIT_RTTIME]->rlim_max); - assert_se(config_parse_usec_limit(NULL, "fake", 1, "section", 1, "LimitRTTIME", RLIMIT_RTTIME, "infinity:infinity", rl, NULL) >= 0); + assert_se(config_parse_limit(NULL, "fake", 1, "section", 1, "LimitRTTIME", RLIMIT_RTTIME, "infinity:infinity", rl, NULL) >= 0); assert_se(rl[RLIMIT_RTTIME]); assert_se(rl[RLIMIT_RTTIME]->rlim_cur == RLIM_INFINITY); assert_se(rl[RLIMIT_RTTIME]->rlim_cur == rl[RLIMIT_RTTIME]->rlim_max); - assert_se(config_parse_usec_limit(NULL, "fake", 1, "section", 1, "LimitRTTIME", RLIMIT_RTTIME, "2345ms", rl, NULL) >= 0); + assert_se(config_parse_limit(NULL, "fake", 1, "section", 1, "LimitRTTIME", RLIMIT_RTTIME, "2345ms", rl, NULL) >= 0); assert_se(rl[RLIMIT_RTTIME]); assert_se(rl[RLIMIT_RTTIME]->rlim_cur == 2345 * USEC_PER_MSEC); assert_se(rl[RLIMIT_RTTIME]->rlim_cur == rl[RLIMIT_RTTIME]->rlim_max); From cb9712492f94153b7ce6fc03d6dd3fd95c87baa5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 21:08:50 +0100 Subject: [PATCH 10/22] gpt-auto: don't log an EIO error if blkid finds something which is not a partition table Fixes: #2483 --- src/gpt-auto-generator/gpt-auto-generator.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c index ce8cecc5cb..a1bad9fcbe 100644 --- a/src/gpt-auto-generator/gpt-auto-generator.c +++ b/src/gpt-auto-generator/gpt-auto-generator.c @@ -635,16 +635,19 @@ static int enumerate_partitions(dev_t devnum) { if (r == 1) return 0; /* no results */ else if (r == -2) { - log_warning("%s: probe gave ambiguous results, ignoring", node); + log_warning("%s: probe gave ambiguous results, ignoring.", node); return 0; } else if (r != 0) return log_error_errno(errno ?: EIO, "%s: failed to probe: %m", node); errno = 0; r = blkid_probe_lookup_value(b, "PTTYPE", &pttype, NULL); - if (r != 0) - return log_error_errno(errno ?: EIO, - "%s: failed to determine partition table type: %m", node); + if (r != 0) { + if (errno == 0) + return 0; /* No partition table found. */ + + return log_error_errno(errno, "%s: failed to determine partition table type: %m", node); + } /* We only do this all for GPT... */ if (!streq_ptr(pttype, "gpt")) { From 99d4f5e5c0d2532159542519e683f976f881f0f5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 21:15:07 +0100 Subject: [PATCH 11/22] basic: add new rlimit_format() call This formats a struct rlimit the way rlimit_parse() expects it. --- src/basic/rlimit-util.c | 24 ++++++++++++++++++++++++ src/basic/rlimit-util.h | 2 ++ src/test/test-rlimit-util.c | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+) diff --git a/src/basic/rlimit-util.c b/src/basic/rlimit-util.c index 11476ac8d1..8a921a27cb 100644 --- a/src/basic/rlimit-util.c +++ b/src/basic/rlimit-util.c @@ -227,6 +227,30 @@ int rlimit_parse(int resource, const char *val, struct rlimit *ret) { return 0; } +int rlimit_format(const struct rlimit *rl, char **ret) { + char *s = NULL; + + assert(rl); + assert(ret); + + if (rl->rlim_cur >= RLIM_INFINITY && rl->rlim_max >= RLIM_INFINITY) + s = strdup("infinity"); + else if (rl->rlim_cur >= RLIM_INFINITY) + (void) asprintf(&s, "infinity:" RLIM_FMT, rl->rlim_max); + else if (rl->rlim_max >= RLIM_INFINITY) + (void) asprintf(&s, RLIM_FMT ":infinity", rl->rlim_cur); + else if (rl->rlim_cur == rl->rlim_max) + (void) asprintf(&s, RLIM_FMT, rl->rlim_cur); + else + (void) asprintf(&s, RLIM_FMT ":" RLIM_FMT, rl->rlim_cur, rl->rlim_max); + + if (!s) + return -ENOMEM; + + *ret = s; + return 0; +} + static const char* const rlimit_table[_RLIMIT_MAX] = { [RLIMIT_CPU] = "LimitCPU", [RLIMIT_FSIZE] = "LimitFSIZE", diff --git a/src/basic/rlimit-util.h b/src/basic/rlimit-util.h index fd2f7ff7cf..abf3c57934 100644 --- a/src/basic/rlimit-util.h +++ b/src/basic/rlimit-util.h @@ -33,4 +33,6 @@ int setrlimit_closest(int resource, const struct rlimit *rlim); int rlimit_parse_one(int resource, const char *val, rlim_t *ret); int rlimit_parse(int resource, const char *val, struct rlimit *ret); +int rlimit_format(const struct rlimit *rl, char **ret); + #define RLIMIT_MAKE_CONST(lim) ((struct rlimit) { lim, lim }) diff --git a/src/test/test-rlimit-util.c b/src/test/test-rlimit-util.c index 24bfe7a60e..d9ac9368cd 100644 --- a/src/test/test-rlimit-util.c +++ b/src/test/test-rlimit-util.c @@ -17,12 +17,37 @@ #include +#include "alloc-util.h" #include "capability-util.h" #include "macro.h" #include "rlimit-util.h" #include "string-util.h" #include "util.h" +static void test_rlimit_parse_format(int resource, const char *string, rlim_t soft, rlim_t hard, int ret, const char *formatted) { + _cleanup_free_ char *f = NULL; + struct rlimit rl = { + .rlim_cur = 4711, + .rlim_max = 4712, + }, rl2 = { + .rlim_cur = 4713, + .rlim_max = 4714 + }; + + assert_se(rlimit_parse(resource, string, &rl) == ret); + if (ret < 0) + return; + + assert_se(rl.rlim_cur == soft); + assert_se(rl.rlim_max == hard); + + assert_se(rlimit_format(&rl, &f) >= 0); + assert_se(streq(formatted, f)); + + assert_se(rlimit_parse(resource, formatted, &rl2) >= 0); + assert_se(memcmp(&rl, &rl2, sizeof(struct rlimit)) == 0); +} + int main(int argc, char *argv[]) { struct rlimit old, new, high; struct rlimit err = { @@ -65,5 +90,15 @@ int main(int argc, char *argv[]) { assert_se(old.rlim_cur == new.rlim_cur); assert_se(old.rlim_max == new.rlim_max); + test_rlimit_parse_format(RLIMIT_NOFILE, "4:5", 4, 5, 0, "4:5"); + test_rlimit_parse_format(RLIMIT_NOFILE, "6", 6, 6, 0, "6"); + test_rlimit_parse_format(RLIMIT_NOFILE, "infinity", RLIM_INFINITY, RLIM_INFINITY, 0, "infinity"); + test_rlimit_parse_format(RLIMIT_NOFILE, "infinity:infinity", RLIM_INFINITY, RLIM_INFINITY, 0, "infinity"); + test_rlimit_parse_format(RLIMIT_NOFILE, "8:infinity", 8, RLIM_INFINITY, 0, "8:infinity"); + test_rlimit_parse_format(RLIMIT_CPU, "25min:13h", (25*USEC_PER_MINUTE) / USEC_PER_SEC, (13*USEC_PER_HOUR) / USEC_PER_SEC, 0, "1500:46800"); + test_rlimit_parse_format(RLIMIT_NOFILE, "", 0, 0, -EINVAL, NULL); + test_rlimit_parse_format(RLIMIT_NOFILE, "5:4", 0, 0, -EILSEQ, NULL); + test_rlimit_parse_format(RLIMIT_NOFILE, "5:4:3", 0, 0, -EINVAL, NULL); + return 0; } From b93ea5d3681c15155f50a296802e4ab6f04e3cd1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 21:27:33 +0100 Subject: [PATCH 12/22] core: fix handling of AccuracyUSec and RandomDelayUSec bus properties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clear up some confusion regarding the USec and Sec suffixes we use. In configuration files we usually use the Sec suffix, to indicate the implied time unit if none is specified. The respective bus properties however use the USec property, since they expose 64bit unsigned integers containing time in µs. Before this patch timer units exposed a bus property AccuracyUSec (which hence is the correct name) but when parsing transient property data would look for AccuracySec instead (which is incorrect). This patch ensures we look for AccuracySec correctly, but keeps the code for AccuracyUSec in place for compatibility, but adds a warning to ensure that apps are updated to use the right property. --- src/core/dbus-timer.c | 6 ++++-- src/shared/bus-util.c | 28 +++++++++++----------------- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/core/dbus-timer.c b/src/core/dbus-timer.c index ec301df6d7..212f9e1d8d 100644 --- a/src/core/dbus-timer.c +++ b/src/core/dbus-timer.c @@ -267,10 +267,12 @@ static int bus_timer_set_transient_property( return 1; - } else if (streq(name, "AccuracySec")) { - + } else if (STR_IN_SET(name, "AccuracyUSec", "AccuracySec")) { usec_t u = 0; + if (streq(name, "AccuracySec")) + log_notice("Client is using obsolete AccuracySec= transient property, please use AccuracyUSec= instead."); + r = sd_bus_message_read(message, "t", &u); if (r < 0) return r; diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 63fd9b9514..3afe157905 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -1459,18 +1459,22 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen return 0; - } else if (streq(field, "RandomizedDelaySec")) { + } else if (STR_IN_SET(field, "AccuracySec", "RandomizedDelaySec")) { + char *n; usec_t t; - + size_t l; r = parse_sec(eq, &t); if (r < 0) - return log_error_errno(r, "Failed to parse RandomizedDelaySec= parameter: %s", eq); + return log_error_errno(r, "Failed to parse %s= parameter: %s", field, eq); - r = sd_bus_message_append_basic(m, SD_BUS_TYPE_STRING, "RandomizedDelayUSec"); - if (r < 0) - return bus_log_create_error(r); + l = strlen(field); + n = newa(char, l + 2); + if (!n) + return log_oom(); - r = sd_bus_message_append(m, "v", "t", t); + /* Change suffix Sec → USec */ + strcpy(mempcpy(n, field, l - 3), "USec"); + r = sd_bus_message_append(m, "sv", n, "t", t); if (r < 0) return bus_log_create_error(r); @@ -1746,16 +1750,6 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen r = sd_bus_message_append(m, "v", "i", sig); - } else if (streq(field, "AccuracySec")) { - usec_t u; - - r = parse_sec(eq, &u); - if (r < 0) { - log_error("Failed to parse %s value %s", field, eq); - return -EINVAL; - } - - r = sd_bus_message_append(m, "v", "t", u); } else if (streq(field, "TimerSlackNSec")) { nsec_t n; From 5c1f0be1efd2bed25a8759ae0f9927e82afc086b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 21:28:24 +0100 Subject: [PATCH 13/22] =?UTF-8?q?core:=20when=20saving=20timer=20unit=20ti?= =?UTF-8?q?me=20values,=20store=20them=20in=20raw=20=C2=B5s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let's simplify things a bit, and make sure we don't lose accuracy. --- src/core/dbus-timer.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/core/dbus-timer.c b/src/core/dbus-timer.c index 212f9e1d8d..321ed5da37 100644 --- a/src/core/dbus-timer.c +++ b/src/core/dbus-timer.c @@ -278,10 +278,8 @@ static int bus_timer_set_transient_property( return r; if (mode != UNIT_CHECK) { - char time[FORMAT_TIMESPAN_MAX]; - t->accuracy_usec = u; - unit_write_drop_in_private_format(UNIT(t), mode, name, "%s=%s\n", name, format_timespan(time, sizeof(time), u, USEC_PER_MSEC)); + unit_write_drop_in_private_format(UNIT(t), mode, name, "AccuracySec=" USEC_FMT "us\n", u); } return 1; @@ -294,10 +292,8 @@ static int bus_timer_set_transient_property( return r; if (mode != UNIT_CHECK) { - char time[FORMAT_TIMESPAN_MAX]; - t->random_usec = u; - unit_write_drop_in_private_format(UNIT(t), mode, name, "RandomizedDelaySec=%s\n", format_timespan(time, sizeof(time), u, USEC_PER_MSEC)); + unit_write_drop_in_private_format(UNIT(t), mode, name, "RandomizedDelaySec=" USEC_FMT "us\n", u); } return 1; From 1e5b6b4f2861ad2b31571065b60b05fca85047b1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 21:31:44 +0100 Subject: [PATCH 14/22] shared: simplify parsing of bus properties a bit Let's write the property name and value in one call, when that's possible, shorthing our code a bit. --- src/shared/bus-util.c | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 3afe157905..76356a6e31 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -1414,15 +1414,9 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen if (streq(field, "CPUQuota")) { - if (isempty(eq)) { - - r = sd_bus_message_append_basic(m, SD_BUS_TYPE_STRING, "CPUQuotaPerSecUSec"); - if (r < 0) - return bus_log_create_error(r); - - r = sd_bus_message_append(m, "v", "t", USEC_INFINITY); - - } else if (endswith(eq, "%")) { + if (isempty(eq)) + r = sd_bus_message_append(m, "sv", "CPUQuotaPerSecUSec", "t", USEC_INFINITY); + else if (endswith(eq, "%")) { double percent; if (sscanf(eq, "%lf%%", &percent) != 1 || percent <= 0) { @@ -1430,11 +1424,7 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen return -EINVAL; } - r = sd_bus_message_append_basic(m, SD_BUS_TYPE_STRING, "CPUQuotaPerSecUSec"); - if (r < 0) - return bus_log_create_error(r); - - r = sd_bus_message_append(m, "v", "t", (usec_t) percent * USEC_PER_SEC / 100); + r = sd_bus_message_append(m, "sv", "CPUQuotaPerSecUSec", "t", (usec_t) percent * USEC_PER_SEC / 100); } else { log_error("CPU quota needs to be in percent."); return -EINVAL; @@ -1447,11 +1437,7 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen } else if (streq(field, "EnvironmentFile")) { - r = sd_bus_message_append_basic(m, SD_BUS_TYPE_STRING, "EnvironmentFiles"); - if (r < 0) - return bus_log_create_error(r); - - r = sd_bus_message_append(m, "v", "a(sb)", 1, + r = sd_bus_message_append(m, "sv", "EnvironmentFiles", "a(sb)", 1, eq[0] == '-' ? eq + 1 : eq, eq[0] == '-'); if (r < 0) From fb8a9fc9b5ec53b13f9c2acad48b9a7f6579acfa Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 21:32:54 +0100 Subject: [PATCH 15/22] resolve: work around clang limitation clang is apparently not smart enough to detect when a switch statement contains case statements for all possible values of the used type. Work around that. (And while we are at it, normalize indentation a bit) Fixes: #2504 --- src/resolve/dns-type.c | 74 ++++++++++++++++++++++++++++++++---------- 1 file changed, 56 insertions(+), 18 deletions(-) diff --git a/src/resolve/dns-type.c b/src/resolve/dns-type.c index fc2f1826fd..49210b2ca9 100644 --- a/src/resolve/dns-type.c +++ b/src/resolve/dns-type.c @@ -240,31 +240,69 @@ int dns_class_from_string(const char *s) { } const char* tlsa_cert_usage_to_string(uint8_t cert_usage) { - switch(cert_usage) { - case 0: return "CA constraint"; - case 1: return "Service certificate constraint"; - case 2: return "Trust anchor assertion"; - case 3: return "Domain-issued certificate"; - case 4 ... 254: return "Unassigned"; - case 255: return "Private use"; + + switch (cert_usage) { + + case 0: + return "CA constraint"; + + case 1: + return "Service certificate constraint"; + + case 2: + return "Trust anchor assertion"; + + case 3: + return "Domain-issued certificate"; + + case 4 ... 254: + return "Unassigned"; + + case 255: + return "Private use"; } + + return NULL; /* clang cannot count that we covered everything */ } const char* tlsa_selector_to_string(uint8_t selector) { - switch(selector) { - case 0: return "Full Certificate"; - case 1: return "SubjectPublicKeyInfo"; - case 2 ... 254: return "Unassigned"; - case 255: return "Private use"; + switch (selector) { + + case 0: + return "Full Certificate"; + + case 1: + return "SubjectPublicKeyInfo"; + + case 2 ... 254: + return "Unassigned"; + + case 255: + return "Private use"; } + + return NULL; } const char* tlsa_matching_type_to_string(uint8_t selector) { - switch(selector) { - case 0: return "No hash used"; - case 1: return "SHA-256"; - case 2: return "SHA-512"; - case 3 ... 254: return "Unassigned"; - case 255: return "Private use"; + + switch (selector) { + + case 0: + return "No hash used"; + + case 1: + return "SHA-256"; + + case 2: + return "SHA-512"; + + case 3 ... 254: + return "Unassigned"; + + case 255: + return "Private use"; } + + return NULL; } From cab2aca3e7dc9966eb9f45416f34729719f01dd4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 21:37:28 +0100 Subject: [PATCH 16/22] core: fix support for transient resource limit properties Make sure we can properly process resource limit properties. Specifically, allow transient configuration of both the soft and hard limit, the same way from the unit files. Previously, only the the hard rlimits could be configured but they'd implicitly spill into the soft hard rlimits. This also updates the client-side code to be able to parse hard/soft resource limit specifications. Since we need to serialize two properties in bus_append_unit_property_assignment() now, the marshalling of the container around it is now moved into the function itself. This has the benefit of shortening the calling code. As a side effect this now beefs up the rlimit parser of "systemctl set-property" to understand time and disk sizes where that's appropriate. --- src/core/dbus-execute.c | 57 +++++++++++++++++++++++------- src/nspawn/nspawn-register.c | 8 ----- src/run/run.c | 8 ----- src/shared/bus-util.c | 67 ++++++++++++++++++++---------------- src/systemctl/systemctl.c | 8 ----- 5 files changed, 82 insertions(+), 66 deletions(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index eae0808f9e..2de28f43e1 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -835,7 +835,8 @@ int bus_exec_context_set_transient_property( UnitSetPropertiesMode mode, sd_bus_error *error) { - int r; + const char *soft = NULL; + int r, ri; assert(u); assert(c); @@ -1492,7 +1493,23 @@ int bus_exec_context_set_transient_property( return 1; - } else if (rlimit_from_string(name) >= 0) { + } + + ri = rlimit_from_string(name); + if (ri < 0) { + soft = endswith(name, "Soft"); + if (soft) { + const char *n; + + n = strndupa(name, soft - name); + ri = rlimit_from_string(n); + if (ri >= 0) + name = n; + + } + } + + if (ri >= 0) { uint64_t rl; rlim_t x; @@ -1510,22 +1527,36 @@ int bus_exec_context_set_transient_property( } if (mode != UNIT_CHECK) { - int z; + _cleanup_free_ char *f = NULL; + struct rlimit nl; - z = rlimit_from_string(name); + if (c->rlimit[ri]) { + nl = *c->rlimit[ri]; - if (!c->rlimit[z]) { - c->rlimit[z] = new(struct rlimit, 1); - if (!c->rlimit[z]) + if (soft) + nl.rlim_cur = x; + else + nl.rlim_max = x; + } else + /* When the resource limit is not initialized yet, then assign the value to both fields */ + nl = (struct rlimit) { + .rlim_cur = x, + .rlim_max = x, + }; + + r = rlimit_format(&nl, &f); + if (r < 0) + return r; + + if (c->rlimit[ri]) + *c->rlimit[ri] = nl; + else { + c->rlimit[ri] = newdup(struct rlimit, &nl, 1); + if (!c->rlimit[ri]) return -ENOMEM; } - c->rlimit[z]->rlim_cur = c->rlimit[z]->rlim_max = x; - - if (x == RLIM_INFINITY) - unit_write_drop_in_private_format(u, mode, name, "%s=infinity\n", name); - else - unit_write_drop_in_private_format(u, mode, name, "%s=%" PRIu64 "\n", name, rl); + unit_write_drop_in_private_format(u, mode, name, "%s=%s\n", name, f); } return 1; diff --git a/src/nspawn/nspawn-register.c b/src/nspawn/nspawn-register.c index 65ca9c762b..a89de4b324 100644 --- a/src/nspawn/nspawn-register.c +++ b/src/nspawn/nspawn-register.c @@ -166,17 +166,9 @@ int register_machine( } STRV_FOREACH(i, properties) { - r = sd_bus_message_open_container(m, 'r', "sv"); - if (r < 0) - return bus_log_create_error(r); - r = bus_append_unit_property_assignment(m, *i); if (r < 0) return r; - - r = sd_bus_message_close_container(m); - if (r < 0) - return bus_log_create_error(r); } r = sd_bus_message_close_container(m); diff --git a/src/run/run.c b/src/run/run.c index 92a1d5373c..080c15466c 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -422,17 +422,9 @@ static int transient_unit_set_properties(sd_bus_message *m, char **properties) { return r; STRV_FOREACH(i, properties) { - r = sd_bus_message_open_container(m, 'r', "sv"); - if (r < 0) - return r; - r = bus_append_unit_property_assignment(m, *i); if (r < 0) return r; - - r = sd_bus_message_close_container(m); - if (r < 0) - return r; } return 0; diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 76356a6e31..9bd7db9aaa 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -1398,7 +1398,7 @@ int bus_parse_unit_info(sd_bus_message *message, UnitInfo *u) { int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignment) { const char *eq, *field; - int r; + int r, rl; assert(m); assert(assignment); @@ -1409,6 +1409,10 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen return -EINVAL; } + r = sd_bus_message_open_container(m, SD_BUS_TYPE_STRUCT, "sv"); + if (r < 0) + return bus_log_create_error(r); + field = strndupa(assignment, eq - assignment); eq ++; @@ -1430,20 +1434,14 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen return -EINVAL; } - if (r < 0) - return bus_log_create_error(r); - - return 0; + goto finish; } else if (streq(field, "EnvironmentFile")) { r = sd_bus_message_append(m, "sv", "EnvironmentFiles", "a(sb)", 1, eq[0] == '-' ? eq + 1 : eq, eq[0] == '-'); - if (r < 0) - return bus_log_create_error(r); - - return 0; + goto finish; } else if (STR_IN_SET(field, "AccuracySec", "RandomizedDelaySec")) { char *n; @@ -1461,17 +1459,38 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen /* Change suffix Sec → USec */ strcpy(mempcpy(n, field, l - 3), "USec"); r = sd_bus_message_append(m, "sv", n, "t", t); - if (r < 0) - return bus_log_create_error(r); - - return 0; + goto finish; } r = sd_bus_message_append_basic(m, SD_BUS_TYPE_STRING, field); if (r < 0) return bus_log_create_error(r); - if (STR_IN_SET(field, + rl = rlimit_from_string(field); + if (rl >= 0) { + const char *sn; + struct rlimit l; + + r = rlimit_parse(rl, eq, &l); + if (r < 0) + return log_error_errno(r, "Failed to parse resource limit: %s", eq); + + r = sd_bus_message_append(m, "v", "t", l.rlim_max); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_message_close_container(m); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_message_open_container(m, SD_BUS_TYPE_STRUCT, "sv"); + if (r < 0) + return bus_log_create_error(r); + + sn = strjoina(field, "Soft"); + r = sd_bus_message_append(m, "sv", sn, "t", l.rlim_cur); + + } else if (STR_IN_SET(field, "CPUAccounting", "MemoryAccounting", "BlockIOAccounting", "TasksAccounting", "SendSIGHUP", "SendSIGKILL", "WakeSystem", "DefaultDependencies", "IgnoreSIGPIPE", "TTYVHangup", "TTYReset", "RemainAfterExit", @@ -1652,21 +1671,6 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen r = sd_bus_message_append(m, "v", "a(st)", path, u); } - } else if (rlimit_from_string(field) >= 0) { - uint64_t rl; - - if (streq(eq, "infinity")) - rl = (uint64_t) -1; - else { - r = safe_atou64(eq, &rl); - if (r < 0) { - log_error("Invalid resource limit: %s", eq); - return -EINVAL; - } - } - - r = sd_bus_message_append(m, "v", "t", rl); - } else if (streq(field, "Nice")) { int32_t i; @@ -1849,6 +1853,11 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen return -EINVAL; } +finish: + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_message_close_container(m); if (r < 0) return bus_log_create_error(r); diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 078c1b5ea4..536beb28ce 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -4873,17 +4873,9 @@ static int set_property(int argc, char *argv[], void *userdata) { return bus_log_create_error(r); STRV_FOREACH(i, strv_skip(argv, 2)) { - r = sd_bus_message_open_container(m, SD_BUS_TYPE_STRUCT, "sv"); - if (r < 0) - return bus_log_create_error(r); - r = bus_append_unit_property_assignment(m, *i); if (r < 0) return r; - - r = sd_bus_message_close_container(m); - if (r < 0) - return bus_log_create_error(r); } r = sd_bus_message_close_container(m); From c5962fcff05a1e46138f9f8116513947398cb725 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 21:46:11 +0100 Subject: [PATCH 17/22] update TODO --- TODO | 2 ++ 1 file changed, 2 insertions(+) diff --git a/TODO b/TODO index b5e6decdb5..9ca06af63a 100644 --- a/TODO +++ b/TODO @@ -33,6 +33,8 @@ Janitorial Clean-ups: Features: +* rework coredump tool to move actual processing into a socket activated service + * cache sd_event_now() result from before the first iteration... * remove Capabilities=, after all AmbientCapabilities= and CapabilityBoundingSet= should be enough. From 36c16a7cdd6c33d7980efc2cd6a2211941f302b4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 21:48:10 +0100 Subject: [PATCH 18/22] core: rework unit timeout handling, and add new setting RuntimeMaxSec= This clean-ups timeout handling in PID 1. Specifically, instead of storing 0 in internal timeout variables as indication for a disabled timeout, use USEC_INFINITY which is in-line with how we do this in the rest of our code (following the logic that 0 means "no", and USEC_INFINITY means "never"). This also replace all usec_t additions with invocations to usec_add(), so that USEC_INFINITY is properly propagated, and sd-event considers it has indication for turning off the event source. This also alters the deserialization of the units to restart timeouts from the time they were originally started from. Before this patch timeouts would be restarted beginning with the time of the deserialization, which could lead to artificially prolonged timeouts if a daemon reload took place. Finally, a new RuntimeMaxSec= setting is introduced for service units, that specifies a maximum runtime after which a specific service is forcibly terminated. This is useful to put time limits on time-intensive processing jobs. This also simplifies the various xyz_spawn() calls of the various types in that explicit distruction of the timers is removed, as that is done anyway by the state change handlers, and a state change is always done when the xyz_spawn() calls fail. Fixes: #2249 --- src/core/busname.c | 20 ++- src/core/dbus-service.c | 14 ++ src/core/job.c | 11 +- src/core/load-fragment-gperf.gperf.m4 | 1 + src/core/load-fragment.c | 9 ++ src/core/main.c | 10 +- src/core/mount.c | 33 ++-- src/core/scope.c | 37 +++-- src/core/service.c | 212 +++++++++++++------------- src/core/service.h | 1 + src/core/socket.c | 37 ++--- src/core/swap.c | 20 ++- src/core/unit.c | 3 +- src/shared/bus-util.c | 2 +- 14 files changed, 213 insertions(+), 197 deletions(-) diff --git a/src/core/busname.c b/src/core/busname.c index a949cd6d3f..ed083a8412 100644 --- a/src/core/busname.c +++ b/src/core/busname.c @@ -112,29 +112,27 @@ static void busname_done(Unit *u) { n->timer_event_source = sd_event_source_unref(n->timer_event_source); } -static int busname_arm_timer(BusName *n) { +static int busname_arm_timer(BusName *n, usec_t usec) { int r; assert(n); - if (n->timeout_usec <= 0) { - n->timer_event_source = sd_event_source_unref(n->timer_event_source); - return 0; - } - if (n->timer_event_source) { - r = sd_event_source_set_time(n->timer_event_source, now(CLOCK_MONOTONIC) + n->timeout_usec); + r = sd_event_source_set_time(n->timer_event_source, usec); if (r < 0) return r; return sd_event_source_set_enabled(n->timer_event_source, SD_EVENT_ONESHOT); } + if (usec == USEC_INFINITY) + return 0; + r = sd_event_add_time( UNIT(n)->manager->event, &n->timer_event_source, CLOCK_MONOTONIC, - now(CLOCK_MONOTONIC) + n->timeout_usec, 0, + usec, 0, busname_dispatch_timer, n); if (r < 0) return r; @@ -372,7 +370,7 @@ static int busname_coldplug(Unit *u) { if (r < 0) return r; - r = busname_arm_timer(n); + r = busname_arm_timer(n, usec_add(u->state_change_timestamp.monotonic, n->timeout_usec)); if (r < 0) return r; } @@ -397,7 +395,7 @@ static int busname_make_starter(BusName *n, pid_t *_pid) { pid_t pid; int r; - r = busname_arm_timer(n); + r = busname_arm_timer(n, usec_add(now(CLOCK_MONOTONIC), n->timeout_usec)); if (r < 0) goto fail; @@ -475,7 +473,7 @@ static void busname_enter_signal(BusName *n, BusNameState state, BusNameResult f } if (r > 0) { - r = busname_arm_timer(n); + r = busname_arm_timer(n, usec_add(now(CLOCK_MONOTONIC), n->timeout_usec)); if (r < 0) { log_unit_warning_errno(UNIT(n), r, "Failed to arm timer: %m"); goto fail; diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c index 2529689f5a..16f50238a1 100644 --- a/src/core/dbus-service.c +++ b/src/core/dbus-service.c @@ -49,6 +49,7 @@ const sd_bus_vtable bus_service_vtable[] = { SD_BUS_PROPERTY("RestartUSec", "t", bus_property_get_usec, offsetof(Service, restart_usec), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("TimeoutStartUSec", "t", bus_property_get_usec, offsetof(Service, timeout_start_usec), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("TimeoutStopUSec", "t", bus_property_get_usec, offsetof(Service, timeout_stop_usec), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("RuntimeMaxUSec", "t", bus_property_get_usec, offsetof(Service, runtime_max_usec), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("WatchdogUSec", "t", bus_property_get_usec, offsetof(Service, watchdog_usec), SD_BUS_VTABLE_PROPERTY_CONST), BUS_PROPERTY_DUAL_TIMESTAMP("WatchdogTimestamp", offsetof(Service, watchdog_timestamp), 0), SD_BUS_PROPERTY("StartLimitInterval", "t", bus_property_get_usec, offsetof(Service, start_limit.interval), SD_BUS_VTABLE_PROPERTY_CONST), @@ -124,6 +125,19 @@ static int bus_service_set_transient_property( unit_write_drop_in_private_format(UNIT(s), mode, name, "Type=%s\n", service_type_to_string(s->type)); } + return 1; + } else if (streq(name, "RuntimeMaxUSec")) { + usec_t u; + + r = sd_bus_message_read(message, "t", &u); + if (r < 0) + return r; + + if (mode != UNIT_CHECK) { + s->runtime_max_usec = u; + unit_write_drop_in_private_format(UNIT(s), mode, name, "RuntimeMaxSec=" USEC_FMT "us\n", u); + } + return 1; } else if (STR_IN_SET(name, diff --git a/src/core/job.c b/src/core/job.c index d8fdf1b53f..1dcb872019 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -932,14 +932,14 @@ int job_start_timer(Job *j) { j->begin_usec = now(CLOCK_MONOTONIC); - if (j->unit->job_timeout <= 0) + if (j->unit->job_timeout == USEC_INFINITY) return 0; r = sd_event_add_time( j->manager->event, &j->timer_event_source, CLOCK_MONOTONIC, - j->begin_usec + j->unit->job_timeout, 0, + usec_add(j->begin_usec, j->unit->job_timeout), 0, job_dispatch_timer, j); if (r < 0) return r; @@ -1117,17 +1117,16 @@ int job_coldplug(Job *j) { if (j->state == JOB_WAITING) job_add_to_run_queue(j); - if (j->begin_usec == 0 || j->unit->job_timeout == 0) + if (j->begin_usec == 0 || j->unit->job_timeout == USEC_INFINITY) return 0; - if (j->timer_event_source) - j->timer_event_source = sd_event_source_unref(j->timer_event_source); + j->timer_event_source = sd_event_source_unref(j->timer_event_source); r = sd_event_add_time( j->manager->event, &j->timer_event_source, CLOCK_MONOTONIC, - j->begin_usec + j->unit->job_timeout, 0, + usec_add(j->begin_usec, j->unit->job_timeout), 0, job_dispatch_timer, j); if (r < 0) log_debug_errno(r, "Failed to restart timeout for job: %m"); diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index 7211738685..2507db1932 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -214,6 +214,7 @@ Service.RestartSec, config_parse_sec, 0, Service.TimeoutSec, config_parse_service_timeout, 0, offsetof(Service, timeout_start_usec) Service.TimeoutStartSec, config_parse_service_timeout, 0, offsetof(Service, timeout_start_usec) Service.TimeoutStopSec, config_parse_service_timeout, 0, offsetof(Service, timeout_stop_usec) +Service.RuntimeMaxSec, config_parse_sec, 0, offsetof(Service, runtime_max_usec) Service.WatchdogSec, config_parse_sec, 0, offsetof(Service, watchdog_usec) Service.StartLimitInterval, config_parse_sec, 0, offsetof(Service, start_limit.interval) Service.StartLimitBurst, config_parse_unsigned, 0, offsetof(Service, start_limit.burst) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 3b1fbd8b90..3b37cc4cda 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -1743,6 +1743,15 @@ int config_parse_service_timeout(const char *unit, } else if (streq(lvalue, "TimeoutStartSec")) s->start_timeout_defined = true; + /* Traditionally, these options accepted 0 to disable the timeouts. However, a timeout of 0 suggests it happens + * immediately, hence fix this to become USEC_INFINITY instead. This is in-line with how we internally handle + * all other timeouts. */ + + if (s->timeout_start_usec <= 0) + s->timeout_start_usec = USEC_INFINITY; + if (s->timeout_stop_usec <= 0) + s->timeout_stop_usec = USEC_INFINITY; + return 0; } diff --git a/src/core/main.c b/src/core/main.c index 2e1d07811c..84e292afd2 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -710,8 +710,14 @@ static int parse_config_file(void) { CONF_PATHS_NULSTR("systemd/system.conf.d") : CONF_PATHS_NULSTR("systemd/user.conf.d"); - config_parse_many(fn, conf_dirs_nulstr, "Manager\0", - config_item_table_lookup, items, false, NULL); + config_parse_many(fn, conf_dirs_nulstr, "Manager\0", config_item_table_lookup, items, false, NULL); + + /* Traditionally "0" was used to turn off the default unit timeouts. Fix this up so that we used USEC_INFINITY + * like everywhere else. */ + if (arg_default_timeout_start_usec <= 0) + arg_default_timeout_start_usec = USEC_INFINITY; + if (arg_default_timeout_stop_usec <= 0) + arg_default_timeout_stop_usec = USEC_INFINITY; return 0; } diff --git a/src/core/mount.c b/src/core/mount.c index 2ad4ad4f42..7e3a6d578f 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -152,29 +152,27 @@ static void mount_init(Unit *u) { u->ignore_on_isolate = true; } -static int mount_arm_timer(Mount *m) { +static int mount_arm_timer(Mount *m, usec_t usec) { int r; assert(m); - if (m->timeout_usec <= 0) { - m->timer_event_source = sd_event_source_unref(m->timer_event_source); - return 0; - } - if (m->timer_event_source) { - r = sd_event_source_set_time(m->timer_event_source, now(CLOCK_MONOTONIC) + m->timeout_usec); + r = sd_event_source_set_time(m->timer_event_source, usec); if (r < 0) return r; return sd_event_source_set_enabled(m->timer_event_source, SD_EVENT_ONESHOT); } + if (usec == USEC_INFINITY) + return 0; + r = sd_event_add_time( UNIT(m)->manager->event, &m->timer_event_source, CLOCK_MONOTONIC, - now(CLOCK_MONOTONIC) + m->timeout_usec, 0, + usec, 0, mount_dispatch_timer, m); if (r < 0) return r; @@ -653,7 +651,7 @@ static int mount_coldplug(Unit *u) { if (r < 0) return r; - r = mount_arm_timer(m); + r = mount_arm_timer(m, usec_add(u->state_change_timestamp.monotonic, m->timeout_usec)); if (r < 0) return r; } @@ -725,11 +723,11 @@ static int mount_spawn(Mount *m, ExecCommand *c, pid_t *_pid) { r = unit_setup_exec_runtime(UNIT(m)); if (r < 0) - goto fail; + return r; - r = mount_arm_timer(m); + r = mount_arm_timer(m, usec_add(now(CLOCK_MONOTONIC), m->timeout_usec)); if (r < 0) - goto fail; + return r; exec_params.environment = UNIT(m)->manager->environment; exec_params.confirm_spawn = UNIT(m)->manager->confirm_spawn; @@ -745,21 +743,16 @@ static int mount_spawn(Mount *m, ExecCommand *c, pid_t *_pid) { m->exec_runtime, &pid); if (r < 0) - goto fail; + return r; r = unit_watch_pid(UNIT(m), pid); if (r < 0) /* FIXME: we need to do something here */ - goto fail; + return r; *_pid = pid; return 0; - -fail: - m->timer_event_source = sd_event_source_unref(m->timer_event_source); - - return r; } static void mount_enter_dead(Mount *m, MountResult f) { @@ -805,7 +798,7 @@ static void mount_enter_signal(Mount *m, MountState state, MountResult f) { goto fail; if (r > 0) { - r = mount_arm_timer(m); + r = mount_arm_timer(m, usec_add(now(CLOCK_MONOTONIC), m->timeout_usec)); if (r < 0) goto fail; diff --git a/src/core/scope.c b/src/core/scope.c index 1953af1f88..7cddee23b8 100644 --- a/src/core/scope.c +++ b/src/core/scope.c @@ -66,29 +66,27 @@ static void scope_done(Unit *u) { s->timer_event_source = sd_event_source_unref(s->timer_event_source); } -static int scope_arm_timer(Scope *s) { +static int scope_arm_timer(Scope *s, usec_t usec) { int r; assert(s); - if (s->timeout_stop_usec <= 0) { - s->timer_event_source = sd_event_source_unref(s->timer_event_source); - return 0; - } - if (s->timer_event_source) { - r = sd_event_source_set_time(s->timer_event_source, now(CLOCK_MONOTONIC) + s->timeout_stop_usec); + r = sd_event_source_set_time(s->timer_event_source, usec); if (r < 0) return r; return sd_event_source_set_enabled(s->timer_event_source, SD_EVENT_ONESHOT); } + if (usec == USEC_INFINITY) + return 0; + r = sd_event_add_time( UNIT(s)->manager->event, &s->timer_event_source, CLOCK_MONOTONIC, - now(CLOCK_MONOTONIC) + s->timeout_stop_usec, 0, + usec, 0, scope_dispatch_timer, s); if (r < 0) return r; @@ -190,20 +188,19 @@ static int scope_coldplug(Unit *u) { assert(s); assert(s->state == SCOPE_DEAD); - if (s->deserialized_state != s->state) { + if (s->deserialized_state == s->state) + return 0; - if (IN_SET(s->deserialized_state, SCOPE_STOP_SIGKILL, SCOPE_STOP_SIGTERM)) { - r = scope_arm_timer(s); - if (r < 0) - return r; - } - - if (!IN_SET(s->deserialized_state, SCOPE_DEAD, SCOPE_FAILED)) - unit_watch_all_pids(UNIT(s)); - - scope_set_state(s, s->deserialized_state); + if (IN_SET(s->deserialized_state, SCOPE_STOP_SIGKILL, SCOPE_STOP_SIGTERM)) { + r = scope_arm_timer(s, usec_add(u->state_change_timestamp.monotonic, s->timeout_stop_usec)); + if (r < 0) + return r; } + if (!IN_SET(s->deserialized_state, SCOPE_DEAD, SCOPE_FAILED)) + unit_watch_all_pids(UNIT(s)); + + scope_set_state(s, s->deserialized_state); return 0; } @@ -261,7 +258,7 @@ static void scope_enter_signal(Scope *s, ScopeState state, ScopeResult f) { r = 1; if (r > 0) { - r = scope_arm_timer(s); + r = scope_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_stop_usec)); if (r < 0) goto fail; diff --git a/src/core/service.c b/src/core/service.c index 355de3e15d..edda02b8f3 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -112,6 +112,7 @@ static void service_init(Unit *u) { s->timeout_start_usec = u->manager->default_timeout_start_usec; s->timeout_stop_usec = u->manager->default_timeout_stop_usec; s->restart_usec = u->manager->default_restart_usec; + s->runtime_max_usec = USEC_INFINITY; s->type = _SERVICE_TYPE_INVALID; s->socket_fd = -1; s->bus_endpoint_fd = -1; @@ -216,7 +217,7 @@ static void service_start_watchdog(Service *s) { return; if (s->watchdog_event_source) { - r = sd_event_source_set_time(s->watchdog_event_source, s->watchdog_timestamp.monotonic + s->watchdog_usec); + r = sd_event_source_set_time(s->watchdog_event_source, usec_add(s->watchdog_timestamp.monotonic, s->watchdog_usec)); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to reset watchdog timer: %m"); return; @@ -228,7 +229,7 @@ static void service_start_watchdog(Service *s) { UNIT(s)->manager->event, &s->watchdog_event_source, CLOCK_MONOTONIC, - s->watchdog_timestamp.monotonic + s->watchdog_usec, 0, + usec_add(s->watchdog_timestamp.monotonic, s->watchdog_usec), 0, service_dispatch_watchdog, s); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to add watchdog timer: %m"); @@ -433,18 +434,21 @@ static int service_arm_timer(Service *s, usec_t usec) { assert(s); if (s->timer_event_source) { - r = sd_event_source_set_time(s->timer_event_source, now(CLOCK_MONOTONIC) + usec); + r = sd_event_source_set_time(s->timer_event_source, usec); if (r < 0) return r; return sd_event_source_set_enabled(s->timer_event_source, SD_EVENT_ONESHOT); } + if (usec == USEC_INFINITY) + return 0; + r = sd_event_add_time( UNIT(s)->manager->event, &s->timer_event_source, CLOCK_MONOTONIC, - now(CLOCK_MONOTONIC) + usec, 0, + usec, 0, service_dispatch_timer, s); if (r < 0) return r; @@ -509,6 +513,9 @@ static int service_verify(Service *s) { if (!s->usb_function_descriptors && s->usb_function_strings) log_unit_warning(UNIT(s), "Service has USBFunctionStrings= setting, but no USBFunctionDescriptors=. Ignoring."); + if (s->runtime_max_usec != USEC_INFINITY && s->type == SERVICE_ONESHOT) + log_unit_warning(UNIT(s), "MaxRuntimeSec= has no effect in combination with Type=oneshot. Ignoring."); + return 0; } @@ -624,7 +631,7 @@ static int service_add_extras(Service *s) { /* Oneshot services have disabled start timeout by default */ if (s->type == SERVICE_ONESHOT && !s->start_timeout_defined) - s->timeout_start_usec = 0; + s->timeout_start_usec = USEC_INFINITY; service_fix_output(s); @@ -882,6 +889,7 @@ static void service_set_state(Service *s, ServiceState state) { if (!IN_SET(state, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, + SERVICE_RUNNING, SERVICE_RELOAD, SERVICE_STOP, SERVICE_STOP_SIGABRT, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST, SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL, @@ -954,6 +962,37 @@ static void service_set_state(Service *s, ServiceState state) { unit_notify(UNIT(s), table[old_state], table[state], s->reload_result == SERVICE_SUCCESS); } +static usec_t service_coldplug_timeout(Service *s) { + assert(s); + + switch (s->deserialized_state) { + + case SERVICE_START_PRE: + case SERVICE_START: + case SERVICE_START_POST: + case SERVICE_RELOAD: + return usec_add(UNIT(s)->state_change_timestamp.monotonic, s->timeout_start_usec); + + case SERVICE_RUNNING: + return usec_add(UNIT(s)->active_enter_timestamp.monotonic, s->runtime_max_usec); + + case SERVICE_STOP: + case SERVICE_STOP_SIGABRT: + case SERVICE_STOP_SIGTERM: + case SERVICE_STOP_SIGKILL: + case SERVICE_STOP_POST: + case SERVICE_FINAL_SIGTERM: + case SERVICE_FINAL_SIGKILL: + return usec_add(UNIT(s)->state_change_timestamp.monotonic, s->timeout_stop_usec); + + case SERVICE_AUTO_RESTART: + return usec_add(UNIT(s)->inactive_enter_timestamp.monotonic, s->restart_usec); + + default: + return USEC_INFINITY; + } +} + static int service_coldplug(Unit *u) { Service *s = SERVICE(u); int r; @@ -964,31 +1003,9 @@ static int service_coldplug(Unit *u) { if (s->deserialized_state == s->state) return 0; - if (IN_SET(s->deserialized_state, - SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, - SERVICE_RELOAD, - SERVICE_STOP, SERVICE_STOP_SIGABRT, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST, - SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL)) { - - usec_t k; - - k = IN_SET(s->deserialized_state, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, SERVICE_RELOAD) ? s->timeout_start_usec : s->timeout_stop_usec; - - /* For the start/stop timeouts 0 means off */ - if (k > 0) { - r = service_arm_timer(s, k); - if (r < 0) - return r; - } - } - - if (s->deserialized_state == SERVICE_AUTO_RESTART) { - - /* The restart timeouts 0 means immediately */ - r = service_arm_timer(s, s->restart_usec); - if (r < 0) - return r; - } + r = service_arm_timer(s, service_coldplug_timeout(s)); + if (r < 0) + return r; if (s->main_pid > 0 && pid_is_unwaited(s->main_pid) && @@ -1175,7 +1192,7 @@ static int service_spawn( r = unit_setup_exec_runtime(UNIT(s)); if (r < 0) - goto fail; + return r; if (pass_fds || s->exec_context.std_input == EXEC_INPUT_SOCKET || @@ -1184,55 +1201,42 @@ static int service_spawn( r = service_collect_fds(s, &fds, &fd_names); if (r < 0) - goto fail; + return r; n_fds = r; } - if (timeout > 0) { - r = service_arm_timer(s, timeout); - if (r < 0) - goto fail; - } else - s->timer_event_source = sd_event_source_unref(s->timer_event_source); + r = service_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), timeout)); + if (r < 0) + return r; r = unit_full_printf_strv(UNIT(s), c->argv, &argv); if (r < 0) - goto fail; + return r; our_env = new0(char*, 6); - if (!our_env) { - r = -ENOMEM; - goto fail; - } + if (!our_env) + return -ENOMEM; if (is_control ? s->notify_access == NOTIFY_ALL : s->notify_access != NOTIFY_NONE) - if (asprintf(our_env + n_env++, "NOTIFY_SOCKET=%s", UNIT(s)->manager->notify_socket) < 0) { - r = -ENOMEM; - goto fail; - } + if (asprintf(our_env + n_env++, "NOTIFY_SOCKET=%s", UNIT(s)->manager->notify_socket) < 0) + return -ENOMEM; if (s->main_pid > 0) - if (asprintf(our_env + n_env++, "MAINPID="PID_FMT, s->main_pid) < 0) { - r = -ENOMEM; - goto fail; - } + if (asprintf(our_env + n_env++, "MAINPID="PID_FMT, s->main_pid) < 0) + return -ENOMEM; if (UNIT(s)->manager->running_as != MANAGER_SYSTEM) - if (asprintf(our_env + n_env++, "MANAGERPID="PID_FMT, getpid()) < 0) { - r = -ENOMEM; - goto fail; - } + if (asprintf(our_env + n_env++, "MANAGERPID="PID_FMT, getpid()) < 0) + return -ENOMEM; if (s->socket_fd >= 0) { union sockaddr_union sa; socklen_t salen = sizeof(sa); r = getpeername(s->socket_fd, &sa.sa, &salen); - if (r < 0) { - r = -errno; - goto fail; - } + if (r < 0) + return -errno; if (IN_SET(sa.sa.sa_family, AF_INET, AF_INET6)) { _cleanup_free_ char *addr = NULL; @@ -1241,34 +1245,26 @@ static int service_spawn( r = sockaddr_pretty(&sa.sa, salen, true, false, &addr); if (r < 0) - goto fail; + return r; t = strappend("REMOTE_ADDR=", addr); - if (!t) { - r = -ENOMEM; - goto fail; - } + if (!t) + return -ENOMEM; our_env[n_env++] = t; port = sockaddr_port(&sa.sa); - if (port < 0) { - r = port; - goto fail; - } + if (port < 0) + return port; - if (asprintf(&t, "REMOTE_PORT=%u", port) < 0) { - r = -ENOMEM; - goto fail; - } + if (asprintf(&t, "REMOTE_PORT=%u", port) < 0) + return -ENOMEM; our_env[n_env++] = t; } } final_env = strv_env_merge(2, UNIT(s)->manager->environment, our_env, NULL); - if (!final_env) { - r = -ENOMEM; - goto fail; - } + if (!final_env) + return -ENOMEM; if (is_control && UNIT(s)->cgroup_path) { path = strjoina(UNIT(s)->cgroup_path, "/control"); @@ -1280,7 +1276,7 @@ static int service_spawn( r = bus_kernel_create_endpoint(UNIT(s)->manager->running_as == MANAGER_SYSTEM ? "system" : "user", UNIT(s)->id, &bus_endpoint_path); if (r < 0) - goto fail; + return r; /* Pass the fd to the exec_params so that the child process can upload the policy. * Keep a reference to the fd in the service, so the endpoint is kept alive as long @@ -1314,22 +1310,16 @@ static int service_spawn( s->exec_runtime, &pid); if (r < 0) - goto fail; + return r; r = unit_watch_pid(UNIT(s), pid); if (r < 0) /* FIXME: we need to do something here */ - goto fail; + return r; *_pid = pid; return 0; - -fail: - if (timeout) - s->timer_event_source = sd_event_source_unref(s->timer_event_source); - - return r; } static int main_pid_good(Service *s) { @@ -1437,7 +1427,7 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart) if (allow_restart && service_shall_restart(s)) { - r = service_arm_timer(s, s->restart_usec); + r = service_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->restart_usec)); if (r < 0) goto fail; @@ -1545,11 +1535,9 @@ static void service_enter_signal(Service *s, ServiceState state, ServiceResult f goto fail; if (r > 0) { - if (s->timeout_stop_usec > 0) { - r = service_arm_timer(s, s->timeout_stop_usec); - if (r < 0) - goto fail; - } + r = service_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_stop_usec)); + if (r < 0) + goto fail; service_set_state(s, state); } else if (IN_SET(state, SERVICE_STOP_SIGABRT, SERVICE_STOP_SIGTERM) && s->kill_context.send_sigkill) @@ -1577,8 +1565,7 @@ static void service_enter_stop_by_notify(Service *s) { unit_watch_all_pids(UNIT(s)); - if (s->timeout_stop_usec > 0) - service_arm_timer(s, s->timeout_stop_usec); + service_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_stop_usec)); /* The service told us it's stopping, so it's as if we SIGTERM'd it. */ service_set_state(s, SERVICE_STOP_SIGTERM); @@ -1656,8 +1643,10 @@ static void service_enter_running(Service *s, ServiceResult f) { service_enter_reload_by_notify(s); else if (s->notify_state == NOTIFY_STOPPING) service_enter_stop_by_notify(s); - else + else { service_set_state(s, SERVICE_RUNNING); + service_arm_timer(s, usec_add(UNIT(s)->active_enter_timestamp.monotonic, s->runtime_max_usec)); + } } else if (s->remain_after_exit) service_set_state(s, SERVICE_EXITED); @@ -1711,6 +1700,7 @@ static void service_kill_control_processes(Service *s) { static void service_enter_start(Service *s) { ExecCommand *c; + usec_t timeout; pid_t pid; int r; @@ -1742,9 +1732,16 @@ static void service_enter_start(Service *s) { return; } + if (IN_SET(s->type, SERVICE_SIMPLE, SERVICE_IDLE)) + /* For simple + idle this is the main process. We don't apply any timeout here, but + * service_enter_running() will later apply the .runtime_max_usec timeout. */ + timeout = USEC_INFINITY; + else + timeout = s->timeout_start_usec; + r = service_spawn(s, c, - IN_SET(s->type, SERVICE_FORKING, SERVICE_DBUS, SERVICE_NOTIFY, SERVICE_ONESHOT) ? s->timeout_start_usec : 0, + timeout, true, true, true, @@ -1754,7 +1751,7 @@ static void service_enter_start(Service *s) { if (r < 0) goto fail; - if (s->type == SERVICE_SIMPLE || s->type == SERVICE_IDLE) { + if (IN_SET(s->type, SERVICE_SIMPLE, SERVICE_IDLE)) { /* For simple services we immediately start * the START_POST binaries. */ @@ -1769,9 +1766,7 @@ static void service_enter_start(Service *s) { s->control_pid = pid; service_set_state(s, SERVICE_START); - } else if (s->type == SERVICE_ONESHOT || - s->type == SERVICE_DBUS || - s->type == SERVICE_NOTIFY) { + } else if (IN_SET(s->type, SERVICE_ONESHOT, SERVICE_DBUS, SERVICE_NOTIFY)) { /* For oneshot services we wait until the start * process exited, too, but it is our main process. */ @@ -1840,7 +1835,7 @@ static void service_enter_restart(Service *s) { /* Don't restart things if we are going down anyway */ log_unit_info(UNIT(s), "Stop job pending for unit, delaying automatic restart."); - r = service_arm_timer(s, s->restart_usec); + r = service_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->restart_usec)); if (r < 0) goto fail; @@ -1870,9 +1865,7 @@ fail: static void service_enter_reload_by_notify(Service *s) { assert(s); - if (s->timeout_start_usec > 0) - service_arm_timer(s, s->timeout_start_usec); - + service_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_start_usec)); service_set_state(s, SERVICE_RELOAD); } @@ -1913,6 +1906,7 @@ fail: } static void service_run_next_control(Service *s) { + usec_t timeout; int r; assert(s); @@ -1924,9 +1918,14 @@ static void service_run_next_control(Service *s) { s->control_command = s->control_command->command_next; service_unwatch_control_pid(s); + if (IN_SET(s->state, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD)) + timeout = s->timeout_start_usec; + else + timeout = s->timeout_stop_usec; + r = service_spawn(s, s->control_command, - IN_SET(s->state, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD) ? s->timeout_start_usec : s->timeout_stop_usec, + timeout, false, !s->permissions_start_only, !s->root_directory_start_only, @@ -2882,6 +2881,11 @@ static int service_dispatch_timer(sd_event_source *source, usec_t usec, void *us service_enter_stop(s, SERVICE_FAILURE_TIMEOUT); break; + case SERVICE_RUNNING: + log_unit_warning(UNIT(s), "Service reached runtime time limit. Stopping."); + service_enter_stop(s, SERVICE_FAILURE_TIMEOUT); + break; + case SERVICE_RELOAD: log_unit_warning(UNIT(s), "Reload operation timed out. Stopping."); service_unwatch_control_pid(s); diff --git a/src/core/service.h b/src/core/service.h index 19efbccfc7..24408940d4 100644 --- a/src/core/service.h +++ b/src/core/service.h @@ -118,6 +118,7 @@ struct Service { usec_t restart_usec; usec_t timeout_start_usec; usec_t timeout_stop_usec; + usec_t runtime_max_usec; dual_timestamp watchdog_timestamp; usec_t watchdog_usec; diff --git a/src/core/socket.c b/src/core/socket.c index 2e4173aabc..740b748d65 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -170,29 +170,27 @@ static void socket_done(Unit *u) { s->timer_event_source = sd_event_source_unref(s->timer_event_source); } -static int socket_arm_timer(Socket *s) { +static int socket_arm_timer(Socket *s, usec_t usec) { int r; assert(s); - if (s->timeout_usec <= 0) { - s->timer_event_source = sd_event_source_unref(s->timer_event_source); - return 0; - } - if (s->timer_event_source) { - r = sd_event_source_set_time(s->timer_event_source, now(CLOCK_MONOTONIC) + s->timeout_usec); + r = sd_event_source_set_time(s->timer_event_source, usec); if (r < 0) return r; return sd_event_source_set_enabled(s->timer_event_source, SD_EVENT_ONESHOT); } + if (usec == USEC_INFINITY) + return 0; + r = sd_event_add_time( UNIT(s)->manager->event, &s->timer_event_source, CLOCK_MONOTONIC, - now(CLOCK_MONOTONIC) + s->timeout_usec, 0, + usec, 0, socket_dispatch_timer, s); if (r < 0) return r; @@ -1494,7 +1492,7 @@ static int socket_coldplug(Unit *u) { if (r < 0) return r; - r = socket_arm_timer(s); + r = socket_arm_timer(s, usec_add(u->state_change_timestamp.monotonic, s->timeout_usec)); if (r < 0) return r; } @@ -1507,6 +1505,7 @@ static int socket_coldplug(Unit *u) { SOCKET_STOP_PRE, SOCKET_STOP_PRE_SIGTERM, SOCKET_STOP_PRE_SIGKILL)) { + r = socket_open_fds(s); if (r < 0) return r; @@ -1548,15 +1547,15 @@ static int socket_spawn(Socket *s, ExecCommand *c, pid_t *_pid) { r = unit_setup_exec_runtime(UNIT(s)); if (r < 0) - goto fail; + return r; - r = socket_arm_timer(s); + r = socket_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_usec)); if (r < 0) - goto fail; + return r; r = unit_full_printf_strv(UNIT(s), c->argv, &argv); if (r < 0) - goto fail; + return r; exec_params.argv = argv; exec_params.environment = UNIT(s)->manager->environment; @@ -1573,26 +1572,22 @@ static int socket_spawn(Socket *s, ExecCommand *c, pid_t *_pid) { s->exec_runtime, &pid); if (r < 0) - goto fail; + return r; r = unit_watch_pid(UNIT(s), pid); if (r < 0) /* FIXME: we need to do something here */ - goto fail; + return r; *_pid = pid; return 0; - -fail: - s->timer_event_source = sd_event_source_unref(s->timer_event_source); - return r; } static int socket_chown(Socket *s, pid_t *_pid) { pid_t pid; int r; - r = socket_arm_timer(s); + r = socket_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_usec)); if (r < 0) goto fail; @@ -1735,7 +1730,7 @@ static void socket_enter_signal(Socket *s, SocketState state, SocketResult f) { goto fail; if (r > 0) { - r = socket_arm_timer(s); + r = socket_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_usec)); if (r < 0) goto fail; diff --git a/src/core/swap.c b/src/core/swap.c index 5568898bd7..d895e3ced1 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -160,29 +160,27 @@ static void swap_done(Unit *u) { s->timer_event_source = sd_event_source_unref(s->timer_event_source); } -static int swap_arm_timer(Swap *s) { +static int swap_arm_timer(Swap *s, usec_t usec) { int r; assert(s); - if (s->timeout_usec <= 0) { - s->timer_event_source = sd_event_source_unref(s->timer_event_source); - return 0; - } - if (s->timer_event_source) { - r = sd_event_source_set_time(s->timer_event_source, now(CLOCK_MONOTONIC) + s->timeout_usec); + r = sd_event_source_set_time(s->timer_event_source, usec); if (r < 0) return r; return sd_event_source_set_enabled(s->timer_event_source, SD_EVENT_ONESHOT); } + if (usec == USEC_INFINITY) + return 0; + r = sd_event_add_time( UNIT(s)->manager->event, &s->timer_event_source, CLOCK_MONOTONIC, - now(CLOCK_MONOTONIC) + s->timeout_usec, 0, + usec, 0, swap_dispatch_timer, s); if (r < 0) return r; @@ -552,7 +550,7 @@ static int swap_coldplug(Unit *u) { if (r < 0) return r; - r = swap_arm_timer(s); + r = swap_arm_timer(s, usec_add(u->state_change_timestamp.monotonic, s->timeout_usec)); if (r < 0) return r; } @@ -633,7 +631,7 @@ static int swap_spawn(Swap *s, ExecCommand *c, pid_t *_pid) { if (r < 0) goto fail; - r = swap_arm_timer(s); + r = swap_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_usec)); if (r < 0) goto fail; @@ -710,7 +708,7 @@ static void swap_enter_signal(Swap *s, SwapState state, SwapResult f) { goto fail; if (r > 0) { - r = swap_arm_timer(s); + r = swap_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_usec)); if (r < 0) goto fail; diff --git a/src/core/unit.c b/src/core/unit.c index 05466a84f9..0c1efc0e16 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -99,6 +99,7 @@ Unit *unit_new(Manager *m, size_t size) { u->unit_file_preset = -1; u->on_failure_job_mode = JOB_REPLACE; u->cgroup_inotify_wd = -1; + u->job_timeout = USEC_INFINITY; RATELIMIT_INIT(u->auto_stop_ratelimit, 10 * USEC_PER_SEC, 16); @@ -950,7 +951,7 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) { STRV_FOREACH(j, u->dropin_paths) fprintf(f, "%s\tDropIn Path: %s\n", prefix, *j); - if (u->job_timeout > 0) + if (u->job_timeout != USEC_INFINITY) fprintf(f, "%s\tJob Timeout: %s\n", prefix, format_timespan(timespan, sizeof(timespan), u->job_timeout, 0)); if (u->job_timeout_action != FAILURE_ACTION_NONE) diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 9bd7db9aaa..6df73c560a 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -1443,7 +1443,7 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen eq[0] == '-'); goto finish; - } else if (STR_IN_SET(field, "AccuracySec", "RandomizedDelaySec")) { + } else if (STR_IN_SET(field, "AccuracySec", "RandomizedDelaySec", "RuntimeMaxSec")) { char *n; usec_t t; size_t l; From fabab19068b6ccb894b0d2a6efbe0ad580771d05 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 21:55:39 +0100 Subject: [PATCH 19/22] core: no need to use unlink_noerrno() --- src/core/service.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/service.c b/src/core/service.c index edda02b8f3..a9345e38b9 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1448,7 +1448,7 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart) * out-of-date, and some software might be confused by it, so * let's remove it. */ if (s->pid_file) - unlink_noerrno(s->pid_file); + (void) unlink(s->pid_file); return; From 5c60db874d143bde29212d21b6d2af4da1a43555 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 22:13:45 +0100 Subject: [PATCH 20/22] sd-netlink: don't take possesion of netlink fd from caller on failure Fixes: #2338 --- src/libsystemd/sd-netlink/sd-netlink.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libsystemd/sd-netlink/sd-netlink.c b/src/libsystemd/sd-netlink/sd-netlink.c index 15d387df2c..ab42b96a2e 100644 --- a/src/libsystemd/sd-netlink/sd-netlink.c +++ b/src/libsystemd/sd-netlink/sd-netlink.c @@ -118,8 +118,10 @@ int sd_netlink_open_fd(sd_netlink **ret, int fd) { rtnl->fd = fd; r = socket_bind(rtnl); - if (r < 0) + if (r < 0) { + rtnl->fd = -1; /* on failure, the caller remains owner of the fd, hence don't close it here */ return r; + } *ret = rtnl; rtnl = NULL; From d6c16624bf417be67d62bbf53fa19d3ea44eaf5a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 22:17:35 +0100 Subject: [PATCH 21/22] sd-netlink: since whe acquire the netlink socket's sockaddr anyway, let's actually verify it --- src/libsystemd/sd-netlink/sd-netlink.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-netlink/sd-netlink.c b/src/libsystemd/sd-netlink/sd-netlink.c index ab42b96a2e..d3eb379c9a 100644 --- a/src/libsystemd/sd-netlink/sd-netlink.c +++ b/src/libsystemd/sd-netlink/sd-netlink.c @@ -44,11 +44,8 @@ static int sd_netlink_new(sd_netlink **ret) { return -ENOMEM; rtnl->n_ref = REFCNT_INIT; - rtnl->fd = -1; - rtnl->sockaddr.nl.nl_family = AF_NETLINK; - rtnl->original_pid = getpid(); LIST_HEAD_INIT(rtnl->match_callbacks); @@ -87,6 +84,9 @@ int sd_netlink_new_from_netlink(sd_netlink **ret, int fd) { if (r < 0) return -errno; + if (rtnl->sockaddr.nl.nl_family != AF_NETLINK) + return -EINVAL; + rtnl->fd = fd; *ret = rtnl; From 2c29d3324dbcab7720cc6bb5852b1a01daa6772c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 1 Feb 2016 22:37:58 +0100 Subject: [PATCH 22/22] man: document the new RuntimeMaxSec= setting --- man/systemd.service.xml | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/man/systemd.service.xml b/man/systemd.service.xml index d7b19ee27f..14f6cd6adc 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -470,7 +470,7 @@ configured time, the service will be considered failed and will be shut down again. Takes a unit-less value in seconds, or a time span value such as "5min 20s". Pass - 0 to disable the timeout logic. Defaults to + infinity to disable the timeout logic. Defaults to DefaultTimeoutStartSec= from the manager configuration file, except when Type=oneshot is used, in which case the @@ -489,7 +489,7 @@ KillMode= in systemd.kill5). Takes a unit-less value in seconds, or a time span value such - as "5min 20s". Pass 0 to disable the + as "5min 20s". Pass infinity to disable the timeout logic. Defaults to DefaultTimeoutStopSec= from the manager configuration file (see @@ -505,6 +505,16 @@ + + RuntimeMaxSec= + + Configures a maximum time for the service to run. If this is used and the service has been + active for longer than the specified time it is terminated and put into a failure state. Note that this setting + does not have any effect on Type=oneshot services, as they terminate immediately after + activation completed. Pass infinity (the default) to configure no runtime + limit. + + WatchdogSec= Configures the watchdog timeout for a service.