diff --git a/src/core/mount.c b/src/core/mount.c index f574e28ec47..175af5498af 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1778,7 +1778,7 @@ static int mount_setup_existing_unit( m->from_proc_self_mountinfo = true; - if (IN_SET(u->load_state, UNIT_NOT_FOUND, UNIT_BAD_SETTING, UNIT_ERROR)) { + if (UNIT_IS_LOAD_ERROR(u->load_state)) { /* The unit was previously not found or otherwise not loaded. Now that the unit shows up in * /proc/self/mountinfo we should reconsider it this, hence set it to UNIT_LOADED. */ u->load_state = UNIT_LOADED; diff --git a/src/core/service.c b/src/core/service.c index 38fb07d5948..953cb2934aa 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -4722,6 +4722,7 @@ int service_set_socket_fd( assert(s); assert(fd >= 0); + assert(sock); /* This is called by the socket code when instantiating a new service for a stream socket and the socket needs * to be configured. We take ownership of the passed fd on success. */ @@ -4753,12 +4754,13 @@ int service_set_socket_fd( return r; } - r = unit_add_two_dependencies(UNIT(sock), UNIT_BEFORE, UNIT_TRIGGERS, UNIT(s), false, UNIT_DEPENDENCY_IMPLICIT); + r = unit_add_two_dependencies(UNIT(s), UNIT_AFTER, UNIT_TRIGGERED_BY, UNIT(sock), false, UNIT_DEPENDENCY_IMPLICIT); if (r < 0) - return r; + return log_unit_debug_errno(UNIT(s), r, + "Failed to add After=/TriggeredBy= dependencies on socket unit: %m"); s->socket_fd = fd; - s->socket_peer = socket_peer_ref(peer); + s->socket_peer = peer; s->socket_fd_selinux_context_net = selinux_context_net; unit_ref_set(&s->accept_socket, UNIT(s), UNIT(sock)); diff --git a/src/core/socket.c b/src/core/socket.c index cc986ddd3fa..45656cbda77 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -1369,6 +1369,8 @@ clear: } int socket_load_service_unit(Socket *s, int cfd, Unit **ret) { + int r; + /* Figure out what the unit that will be used to handle the connections on the socket looks like. * * If cfd < 0, then we don't have a connection yet. In case of Accept=yes sockets, use a fake @@ -1388,7 +1390,6 @@ int socket_load_service_unit(Socket *s, int cfd, Unit **ret) { /* Build the instance name and load the unit */ _cleanup_free_ char *prefix = NULL, *instance = NULL, *name = NULL; - int r; r = unit_name_to_prefix(UNIT(s)->id, &prefix); if (r < 0) @@ -2295,8 +2296,8 @@ static void socket_enter_running(Socket *s, int cfd_in) { if (!pending) { if (!UNIT_ISSET(s->service)) { - r = log_unit_warning_errno(UNIT(s), SYNTHETIC_ERRNO(ENOENT), - "Service to activate vanished, refusing activation."); + log_unit_warning(UNIT(s), + "Service to activate vanished, refusing activation."); goto fail; } @@ -2340,18 +2341,15 @@ static void socket_enter_running(Socket *s, int cfd_in) { } r = socket_load_service_unit(s, cfd, &service); - if (r < 0) { - if (ERRNO_IS_DISCONNECT(r)) - return; - - log_unit_warning_errno(UNIT(s), r, "Failed to load connection service unit: %m"); + if (ERRNO_IS_NEG_DISCONNECT(r)) + return; + if (r < 0 || UNIT_IS_LOAD_ERROR(service->load_state)) { + log_unit_warning_errno(UNIT(s), r < 0 ? r : service->load_error, + "Failed to load connection service unit: %m"); goto fail; } - - r = unit_add_two_dependencies(UNIT(s), UNIT_BEFORE, UNIT_TRIGGERS, service, - false, UNIT_DEPENDENCY_IMPLICIT); - if (r < 0) { - log_unit_warning_errno(UNIT(s), r, "Failed to add Before=/Triggers= dependencies on connection unit: %m"); + if (service->load_state == UNIT_MASKED) { + log_unit_warning(UNIT(s), "Connection service unit is masked, refusing."); goto fail; } @@ -2366,7 +2364,10 @@ static void socket_enter_running(Socket *s, int cfd_in) { goto fail; } - TAKE_FD(cfd); /* We passed ownership of the fd to the service now. Forget it here. */ + /* We passed ownership of the fd and socket peer to the service now. */ + TAKE_FD(cfd); + TAKE_PTR(p); + s->n_connections++; r = manager_add_job(UNIT(s)->manager, JOB_START, service, JOB_REPLACE, NULL, &error, NULL); @@ -2388,13 +2389,9 @@ refuse: return; queue_error: - if (ERRNO_IS_RESOURCE(r)) - log_unit_warning(UNIT(s), "Failed to queue service startup job: %s", - bus_error_message(&error, r)); - else - log_unit_warning(UNIT(s), "Failed to queue service startup job (Maybe the service file is missing or not a %s unit?): %s", - cfd >= 0 ? "template" : "non-template", - bus_error_message(&error, r)); + log_unit_warning_errno(UNIT(s), r, "Failed to queue service startup job%s: %s", + cfd >= 0 && !ERRNO_IS_RESOURCE(r) ? " (Maybe the service is missing or is a template unit?)" : "", + bus_error_message(&error, r)); fail: socket_enter_stop_pre(s, SOCKET_FAILURE_RESOURCES); @@ -3259,12 +3256,11 @@ static int socket_dispatch_timer(sd_event_source *source, usec_t usec, void *use return 0; } -int socket_collect_fds(Socket *s, int **fds) { - size_t k = 0, n = 0; - int *rfds; +int socket_collect_fds(Socket *s, int **ret) { + size_t n = 0, k = 0; assert(s); - assert(fds); + assert(ret); /* Called from the service code for requesting our fds */ @@ -3274,25 +3270,25 @@ int socket_collect_fds(Socket *s, int **fds) { n += p->n_auxiliary_fds; } - if (n <= 0) { - *fds = NULL; + if (n == 0) { + *ret = NULL; return 0; } - rfds = new(int, n); - if (!rfds) + int *fds = new(int, n); + if (!fds) return -ENOMEM; LIST_FOREACH(port, p, s->ports) { if (p->fd >= 0) - rfds[k++] = p->fd; - for (size_t i = 0; i < p->n_auxiliary_fds; ++i) - rfds[k++] = p->auxiliary_fds[i]; + fds[k++] = p->fd; + FOREACH_ARRAY(i, p->auxiliary_fds, p->n_auxiliary_fds) + fds[k++] = *i; } assert(k == n); - *fds = rfds; + *ret = fds; return (int) n; } diff --git a/src/core/socket.h b/src/core/socket.h index 5efe01d2bf0..973a697f861 100644 --- a/src/core/socket.h +++ b/src/core/socket.h @@ -171,7 +171,7 @@ int socket_acquire_peer(Socket *s, int fd, SocketPeer **p); DEFINE_TRIVIAL_CLEANUP_FUNC(SocketPeer*, socket_peer_unref); /* Called from the service code when collecting fds */ -int socket_collect_fds(Socket *s, int **fds); +int socket_collect_fds(Socket *s, int **ret); /* Called from the service code when a per-connection service ended */ void socket_connection_unref(Socket *s); diff --git a/src/core/swap.c b/src/core/swap.c index afd067f0e0d..9e200086f37 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -422,7 +422,7 @@ static int swap_setup_unit( /* The unit is definitely around now, mark it as loaded if it was previously referenced but * could not be loaded. After all we can load it now, from the data in /proc/swaps. */ - if (IN_SET(u->load_state, UNIT_NOT_FOUND, UNIT_BAD_SETTING, UNIT_ERROR)) { + if (UNIT_IS_LOAD_ERROR(u->load_state)) { u->load_state = UNIT_LOADED; u->load_error = 0; } @@ -438,6 +438,8 @@ static int swap_setup_unit( p->priority_set = true; unit_add_to_dbus_queue(u); + TAKE_PTR(new); + return 0; } diff --git a/src/core/unit.h b/src/core/unit.h index 7134d362fe1..0cfd90b3d4c 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -67,6 +67,10 @@ static inline bool UNIT_IS_LOAD_COMPLETE(UnitLoadState t) { return t >= 0 && t < _UNIT_LOAD_STATE_MAX && t != UNIT_STUB && t != UNIT_MERGED; } +static inline bool UNIT_IS_LOAD_ERROR(UnitLoadState t) { + return IN_SET(t, UNIT_NOT_FOUND, UNIT_BAD_SETTING, UNIT_ERROR); +} + /* Stores the 'reason' a dependency was created as a bit mask, i.e. due to which configuration source it came to be. We * use this so that we can selectively flush out parts of dependencies again. Note that the same dependency might be * created as a result of multiple "reasons", hence the bitmask. */