core: log about /var/run/ prefix used in PIDFile=, patch it to be /run instead

In a way this is a follow-up for
a2d1fb882c, but adds a similar warning for
PIDFile=.

There's a much stronger case for doing this kind of notification in
tmpfiles.d (since it helps relating lines to each other for the purpose
of merging them). Doing this for PIDFile= is mostly about being
systematic and copying tmpfiles.d/ behaviour here.

While we are at it, let's also support relative filenames in PIDFile=
now, and prefix them with /run, to make them absolute.

Fixes: #10657

man/systemd.service.xml

@@ -272,14 +272,15 @@
       <varlistentry>
         <term><varname>PIDFile=</varname></term>
 
-        <listitem><para>Takes an absolute path referring to the PID file of the service. Usage of this option is
-        recommended for services where <varname>Type=</varname> is set to <option>forking</option>. The service manager
-        will read the PID of the main process of the service from this file after start-up of the service. The service
-        manager will not write to the file configured here, although it will remove the file after the service has shut
-        down if it still exists. The PID file does not need to be owned by a privileged user, but if it is owned by an
-        unprivileged user additional safety restrictions are enforced: the file may not be a symlink to a file owned by
-        a different user (neither directly nor indirectly), and the PID file must refer to a process already belonging
-        to the service.</para></listitem>
+        <listitem><para>Takes a path referring to the PID file of the service. Usage of this option is recommended for
+        services where <varname>Type=</varname> is set to <option>forking</option>. The path specified typically points
+        to a file below <filename>/run/</filename>. If a relative path is specified it is hence prefixed with
+        <filename>/run/</filename>. The service manager will read the PID of the main process of the service from this
+        file after start-up of the service. The service manager will not write to the file configured here, although it
+        will remove the file after the service has shut down if it still exists. The PID file does not need to be owned
+        by a privileged user, but if it is owned by an unprivileged user additional safety restrictions are enforced:
+        the file may not be a symlink to a file owned by a different user (neither directly nor indirectly), and the
+        PID file must refer to a process already belonging to the service.</para></listitem>
       </varlistentry>
 
       <varlistentry>

src/core/dbus-service.c

@@ -312,8 +312,40 @@ static int bus_service_set_transient_property(
         if (streq(name, "NotifyAccess"))
                 return bus_set_transient_notify_access(u, name, &s->notify_access, message, flags, error);
 
-        if (streq(name, "PIDFile"))
-                return bus_set_transient_path(u, name, &s->pid_file, message, flags, error);
+        if (streq(name, "PIDFile")) {
+                _cleanup_free_ char *n = NULL;
+                const char *v, *e;
+
+                r = sd_bus_message_read(message, "s", &v);
+                if (r < 0)
+                        return r;
+
+                n = path_make_absolute(v, u->manager->prefix[EXEC_DIRECTORY_RUNTIME]);
+                if (!n)
+                        return -ENOMEM;
+
+                path_simplify(n, true);
+
+                if (!path_is_normalized(n))
+                        return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "PIDFile= path '%s' is not valid", n);
+
+                e = path_startswith(n, "/var/run/");
+                if (e) {
+                        char *z;
+
+                        z = strjoin("/run/", e);
+                        if (!z)
+                                return log_oom();
+
+                        if (!UNIT_WRITE_FLAGS_NOOP(flags))
+                                log_unit_notice(u, "Transient unit's PIDFile= property references path below legacy directory /var/run, updating %s → %s; please update client accordingly.", n, z);
+
+                        free_and_replace(s->pid_file, z);
+                } else
+                        free_and_replace(s->pid_file, n);
+
+                return 1;
+        }
 
         if (streq(name, "USBFunctionDescriptors"))
                 return bus_set_transient_path(u, name, &s->usb_function_descriptors, message, flags, error);

src/core/load-fragment-gperf.gperf.m4

@@ -288,7 +288,7 @@ Unit.AssertControlGroupController,     config_parse_unit_condition_string, CONDI
 Unit.AssertNull,                 config_parse_unit_condition_null,   0,                             offsetof(Unit, asserts)
 Unit.CollectMode,                config_parse_collect_mode,          0,                             offsetof(Unit, collect_mode)
 m4_dnl
-Service.PIDFile,                 config_parse_unit_path_printf,      0,                             offsetof(Service, pid_file)
+Service.PIDFile,                 config_parse_pid_file,              0,                             offsetof(Service, pid_file)
 Service.ExecStartPre,            config_parse_exec,                  SERVICE_EXEC_START_PRE,        offsetof(Service, exec_command)
 Service.ExecStart,               config_parse_exec,                  SERVICE_EXEC_START,            offsetof(Service, exec_command)
 Service.ExecStartPost,           config_parse_exec,                  SERVICE_EXEC_START_POST,       offsetof(Service, exec_command)

src/core/load-fragment.c

@@ -4235,6 +4235,62 @@ int config_parse_emergency_action(
         return 0;
 }
 
+int config_parse_pid_file(
+                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) {
+
+        _cleanup_free_ char *k = NULL, *n = NULL;
+        Unit *u = userdata;
+        char **s = data;
+        const char *e;
+        int r;
+
+        assert(filename);
+        assert(lvalue);
+        assert(rvalue);
+        assert(u);
+
+        r = unit_full_printf(u, rvalue, &k);
+        if (r < 0) {
+                log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve unit specifiers in '%s', ignoring: %m", rvalue);
+                return 0;
+        }
+
+        /* If this is a relative path make it absolute by prefixing the /run */
+        n = path_make_absolute(k, u->manager->prefix[EXEC_DIRECTORY_RUNTIME]);
+        if (!n)
+                return log_oom();
+
+        /* Check that the result is a sensible path */
+        r = path_simplify_and_warn(n, PATH_CHECK_ABSOLUTE, unit, filename, line, lvalue);
+        if (r < 0)
+                return r;
+
+        e = path_startswith(n, "/var/run/");
+        if (e) {
+                char *z;
+
+                z = strjoin("/run/", e);
+                if (!z)
+                        return log_oom();
+
+                log_syntax(unit, LOG_NOTICE, filename, line, 0, "PIDFile= references path below legacy directory /var/run/, updating %s → %s; please update the unit file accordingly.", n, z);
+
+                free_and_replace(*s, z);
+        } else
+                free_and_replace(*s, n);
+
+        return 0;
+}
+
 #define FOLLOW_MAX 8
 
 static int open_follow(char **filename, FILE **_f, Set *names, char **_final) {

src/core/load-fragment.h

@@ -104,6 +104,7 @@ CONFIG_PARSER_PROTOTYPE(config_parse_job_timeout_sec);
 CONFIG_PARSER_PROTOTYPE(config_parse_job_running_timeout_sec);
 CONFIG_PARSER_PROTOTYPE(config_parse_log_extra_fields);
 CONFIG_PARSER_PROTOTYPE(config_parse_collect_mode);
+CONFIG_PARSER_PROTOTYPE(config_parse_pid_file);
 
 /* gperf prototypes */
 const struct ConfigPerfItem* load_fragment_gperf_lookup(const char *key, GPERF_LEN_TYPE length);