On Sat, 21.02.15 02:38, Dimitri John Ledkov (dimitri.j.led...@intel.com) wrote:
Sorry for the late review!
Can you please add a commit description to this, explaining the
precise rationale for this?
> ---
> src/core/main.c | 27 +++++++++++++++++++++++++++
> src/core/unit.c | 2 +-
> src/shared/install.c | 25 ++++++++++++++++++++-----
> src/shared/install.h | 2 +-
> 4 files changed, 49 insertions(+), 7 deletions(-)
>
> diff --git a/src/core/main.c b/src/core/main.c
> index 08f46f5..2656779 100644
> --- a/src/core/main.c
> +++ b/src/core/main.c
> @@ -1207,6 +1207,23 @@ static int write_container_id(void) {
> return write_string_file("/run/systemd/container", c);
> }
>
> +static int transient_presets(void) {
> + struct stat st;
> +
> + if (lstat("/usr/lib/systemd/system-preset-transient", &st) == 0)
> + return !!S_ISDIR(st.st_mode);
Please use is_dir() for this, it's slightly nicer to read.
> +#ifdef HAVE_SPLIT_USR
> + if (lstat("/lib/systemd/system-preset-transient", &st) == 0)
> + return !!S_ISDIR(st.st_mode);
> +#endif
> +
> + if (lstat("/etc/systemd/system-preset-transient", &st) == 0)
> + return !!S_ISDIR(st.st_mode);
> +
> + return 0;
> +}
Also, the function should probably return a proper "bool" instead of
an int. We use C99 bool heavily.
That said, maybe we shouldn't have this function at all, see below.
> +
> int main(int argc, char *argv[]) {
> Manager *m = NULL;
> int r, retval = EXIT_FAILURE;
> @@ -1619,6 +1636,16 @@ int main(int argc, char *argv[]) {
> if (arg_running_as == SYSTEMD_SYSTEM) {
> bump_rlimit_nofile(&saved_rlimit_nofile);
>
> + // NB! transient presets must be applied before
> normal
We try to stick to /* comments */ instead of // comments....
> + if (transient_presets()) {
> + r = unit_file_preset_all(UNIT_FILE_SYSTEM, true,
> NULL, UNIT_FILE_PRESET_ENABLE_ONLY, false, NULL, 0);
> + if (r < 0)
> + log_warning_errno(r, "Failed to populate
> transient preset unit settings, ignoring: %m");
> + else
> + log_info("Populated transient preset unit
> settings.");
> + }
Hmm, do we actually need the explicit check with transient_presets()
at all? I mean, it replicates the search path logic, and
unit_file_preset_all() should notice on its own that there are no
preset files in those dirs...
> diff --git a/src/shared/install.c b/src/shared/install.c
> index 65f1c24..7372e56 100644
> --- a/src/shared/install.c
> +++ b/src/shared/install.c
> @@ -1873,7 +1873,7 @@ UnitFileState unit_file_get_state(
> return r < 0 ? r : state;
> }
>
> -int unit_file_query_preset(UnitFileScope scope, const char
> -*root_dir, const char *name) {
> +int unit_file_query_preset(UnitFileScope scope, bool runtime, const
> -char *root_dir, const char *name) {
Hmm, "runtime", or "transient"? Pick one!
Lennart
--
Lennart Poettering, Red Hat
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
