From: Daan De Meyer Date: Wed, 3 Apr 2024 14:06:14 +0000 (+0200) Subject: core: Serialize both pid and pidfd to keep downgrades working X-Git-Tag: v256-rc1~296 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=aaa872a71356a2599f028825125005f225384b95;p=thirdparty%2Fsystemd.git core: Serialize both pid and pidfd to keep downgrades working Currently, when downgrading from a version with pidfd support to a version without pidfd support, all information about running processes is lost as the newer systemd will serialized pidfds which are not recognized by the older systemd when deserializing. To improve the situation, let's serialize both the pid and the pidfd. This is safe because existing versions will either replace the first deserialized pidref with the second one or discard the second one in favor of the first one depending on the unit and field. Older versions that don't support pidfd's will silently discard any fields that contain a pidfd as those will try to parse the field as a pid and since a pidfd field will start with '@', those versions will debug error log and ignore the value. To make sure we reuse the existing pidfd as much as possible, the pidfd is serialized first. Both for scopes and service main pids, if the same pid is seen multiple times, the first pidref is kept. So by serializing the pidfd first we make sure the original pidfd is used instead of the new one which is opened when deserializing the first pid field. For other control units, older versions with pidfd support will discard the first pidfd and replace it with a new pidfd from the second pid field. This is a slight regression on downgrades, but we make sure it doesn't happen for future versions (and older versions when this commit is backported) by modifying the logic to only use the first successfully deserialized pidref so that the raw pid without pidfd is discarded instead of it replacing the existing pidfd. --- diff --git a/src/core/mount.c b/src/core/mount.c index b90b76d3ac0..886ef8974cb 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1438,8 +1438,8 @@ static int mount_deserialize_item(Unit *u, const char *key, const char *value, F } else if (streq(key, "control-pid")) { - pidref_done(&m->control_pid); - (void) deserialize_pidref(fds, value, &m->control_pid); + if (!pidref_is_set(&m->control_pid)) + (void) deserialize_pidref(fds, value, &m->control_pid); } else if (streq(key, "control-command")) { MountExecCommand id; diff --git a/src/core/service.c b/src/core/service.c index 0a3717e34e9..37025b4be27 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -3188,9 +3188,9 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value, s->reload_result = f; } else if (streq(key, "control-pid")) { - pidref_done(&s->control_pid); - (void) deserialize_pidref(fds, value, &s->control_pid); + if (!pidref_is_set(&s->control_pid)) + (void) deserialize_pidref(fds, value, &s->control_pid); } else if (streq(key, "main-pid")) { _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; diff --git a/src/core/socket.c b/src/core/socket.c index c9db6973f03..36d458f35c0 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -2631,8 +2631,9 @@ static int socket_deserialize_item(Unit *u, const char *key, const char *value, else s->n_refused += k; } else if (streq(key, "control-pid")) { - pidref_done(&s->control_pid); - (void) deserialize_pidref(fds, value, &s->control_pid); + + if (!pidref_is_set(&s->control_pid)) + (void) deserialize_pidref(fds, value, &s->control_pid); } else if (streq(key, "control-command")) { SocketExecCommand id; diff --git a/src/core/swap.c b/src/core/swap.c index 5601b832b2d..820dfdff81c 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -955,8 +955,8 @@ static int swap_deserialize_item(Unit *u, const char *key, const char *value, FD s->result = f; } else if (streq(key, "control-pid")) { - pidref_done(&s->control_pid); - (void) deserialize_pidref(fds, value, &s->control_pid); + if (!pidref_is_set(&s->control_pid)) + (void) deserialize_pidref(fds, value, &s->control_pid); } else if (streq(key, "control-command")) { SwapExecCommand id; diff --git a/src/shared/serialize.c b/src/shared/serialize.c index 483cbc7419b..d1f41ce4c82 100644 --- a/src/shared/serialize.c +++ b/src/shared/serialize.c @@ -180,7 +180,7 @@ int serialize_strv(FILE *f, const char *key, char **l) { } int serialize_pidref(FILE *f, FDSet *fds, const char *key, PidRef *pidref) { - int copy; + int r; assert(f); assert(fds); @@ -188,17 +188,21 @@ int serialize_pidref(FILE *f, FDSet *fds, const char *key, PidRef *pidref) { if (!pidref_is_set(pidref)) return 0; - /* If we have a pidfd we serialize the fd and encode the fd number prefixed by "@" in the - * serialization. Otherwise we serialize the numeric PID as it is. */ + /* We always serialize the pid, to keep downgrades mostly working (older versions will deserialize + * the pid and silently fail to deserialize the pidfd). If we also have a pidfd, we serialize it + * first and encode the fd number prefixed by "@" in the serialization. */ - if (pidref->fd < 0) - return serialize_item_format(f, key, PID_FMT, pidref->pid); + if (pidref->fd >= 0) { + int copy = fdset_put_dup(fds, pidref->fd); + if (copy < 0) + return log_error_errno(copy, "Failed to add file descriptor to serialization set: %m"); - copy = fdset_put_dup(fds, pidref->fd); - if (copy < 0) - return log_error_errno(copy, "Failed to add file descriptor to serialization set: %m"); + r = serialize_item_format(f, key, "@%i", copy); + if (r < 0) + return log_error_errno(r, "Failed to serialize PID file descriptor: %m"); + } - return serialize_item_format(f, key, "@%i", copy); + return serialize_item_format(f, key, PID_FMT, pidref->pid); } int serialize_ratelimit(FILE *f, const char *key, const RateLimit *rl) {