[systemd-devel] [PATCH] systemd-delta: add support for drop-in snippets
--- TODO | 3 - man/systemd-delta.xml | 7 ++ src/delta/delta.c | 184 -- 3 files changed, 170 insertions(+), 24 deletions(-) diff --git a/TODO b/TODO index 62016e1..3af9c33 100644 --- a/TODO +++ b/TODO @@ -89,9 +89,6 @@ Features: kmod static-nodes call kmod as an early service, and drop CAP_MKNOD from udevd.service -* systemd-delta needs to be made aware of *.d/*.conf drop-in files for - units. - * seems that when we follow symlinks to units we prefer the symlink destination path over /etc and /usr. We shouldn't do that. Instead /etc should always override /run+/usr and also any symlink diff --git a/man/systemd-delta.xml b/man/systemd-delta.xml index 9293c9b..0c7a54a 100644 --- a/man/systemd-delta.xml +++ b/man/systemd-delta.xml @@ -141,6 +141,13 @@ /varlistentry varlistentry + termvarnameextended/varname/term + +listitemparaShow *.conf files in drop-in +directories for units./para/listitem +/varlistentry + +varlistentry termvarnameunchanged/varname/term listitemparaShow unmodified diff --git a/src/delta/delta.c b/src/delta/delta.c index aec3dc8..aa42574 100644 --- a/src/delta/delta.c +++ b/src/delta/delta.c @@ -31,6 +31,7 @@ #include log.h #include pager.h #include build.h +#include strv.h static bool arg_no_pager = false; static int arg_diff = -1; @@ -41,9 +42,10 @@ static enum { SHOW_REDIRECTED = 1 2, SHOW_OVERRIDDEN = 1 3, SHOW_UNCHANGED = 1 4, +SHOW_EXTENDED = 1 5, SHOW_DEFAULTS = -(SHOW_MASKED | SHOW_EQUIVALENT | SHOW_REDIRECTED | SHOW_OVERRIDDEN) +(SHOW_MASKED | SHOW_EQUIVALENT | SHOW_REDIRECTED | SHOW_OVERRIDDEN | SHOW_EXTENDED) } arg_flags = 0; static int equivalent(const char *a, const char *b) { @@ -92,6 +94,14 @@ static int notify_override_overridden(const char *top, const char *bottom) { return 1; } +static int notify_override_extended(const char *top, const char *bottom) { +if (!(arg_flags SHOW_EXTENDED)) + return 0; + +printf(ANSI_HIGHLIGHT_ON [EXTENDED] ANSI_HIGHLIGHT_OFF%s → %s\n, top, bottom); +return 1; +} + static int notify_override_unchanged(const char *f) { if (!(arg_flags SHOW_UNCHANGED)) return 0; @@ -148,11 +158,114 @@ static int found_override(const char *top, const char *bottom) { return 0; } -static int enumerate_dir(Hashmap *top, Hashmap *bottom, const char *path) { +static int enumerate_dir_d(Hashmap *top, Hashmap *bottom, Hashmap *drops, const char *toppath, const char *drop) { +_cleanup_free_ char *conf = NULL; +_cleanup_free_ char *path = NULL; +_cleanup_strv_free_ char **list = NULL; +char **file; +char *c; +int r; + +path = strjoin(toppath, /, drop, NULL); +if (!path) +return -ENOMEM; + +path_kill_slashes(path); + +conf = strdup(drop); +if (!conf) +return -ENOMEM; + +c = strrchr(conf, '.'); +if(!c) +return -EINVAL; +*c = 0; + +r = get_files_in_directory(path, list); +if (r 0){ +log_error(Failed to enumerate %s: %s, path, strerror(-r)); +return r; +} + +STRV_FOREACH(file, list) { +Hashmap *h; +int k; +char *p; +char *d; + +if (!endswith(*file, .conf)) +continue; + +p = strjoin(path, /, *file, NULL); +if (!p) +return -ENOMEM; + +path_kill_slashes(p); + +d = strrchr(p, '/'); +if (!d || d == p) { +free(p); +return -EINVAL; +} +d --; +d = strrchr(p, '/'); + +if (!d || d == p) { +free(p); +return -EINVAL; +} + +k = hashmap_put(top, d, p); +if (k = 0) { +p = strdup(p); +if (!p) +return -ENOMEM; +d = strrchr(p, '/'); +d --; +d = strrchr(p, '/'); +} else if (k != -EEXIST) { +free(p); +return k; +
Re: [systemd-devel] [PATCH] systemd-delta: add support for drop-in snippets
Dne 16.5.2013 02:41, Zbigniew Jędrzejewski-Szmek napsal(a): On Tue, May 14, 2013 at 05:34:28PM +0200, Lukas Nykryn wrote: --- TODO | 3 - man/systemd-delta.xml | 7 ++ src/delta/delta.c | 182 -- 3 files changed, 170 insertions(+), 22 deletions(-) Hi Lukas, thank you for your patience and all those patch versions. Nevertheless, I still have some doubts: I created the following extended files, one pair which creates and override, and the third file which is not overriden, but is an extended configuration. == /run/systemd/system/dracut-pre-udev.service == [Unit] Description=dracut pre-udev hook Documentation=man:dracut-pre-udev.service(8) ... == /etc/systemd/system/dracut-pre-udev.service.d/0-descr.conf == [Unit] Description=foofoo == /run/systemd/system/dracut-pre-udev.service.d/0-descr.conf == [Unit] Description=foo == /run/systemd/system/dracut-pre-udev.service.d/0-descr2.conf == [Unit] Documentation=foo When I run systemd-delta I see: ./systemd-delta --diff=0 --no-pager systemd/system -t extended (nothing) ./systemd-delta --diff=0 --no-pager systemd/system [OVERRIDDEN] /etc/systemd/system/dracut-pre-udev.service.d/0-descr.conf → /run/systemd/system/dracut-pre-udev.service.d/0-descr.conf I would expect to see 0-descr.conf and 0-descr2.conf in the first case... And also 0-descr2.conf in the second case... Is it actually supposed to behave like that? It might be nice to extend the description in the manpage a bit. Zbyszek Hello, Thanks for noticing it and your patient. There was quite stupid mistake and I haven't noticed it because I was testing it on unit which was overridden. Lukas ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-delta: add support for drop-in snippets
On Tue, May 14, 2013 at 05:34:28PM +0200, Lukas Nykryn wrote: --- TODO | 3 - man/systemd-delta.xml | 7 ++ src/delta/delta.c | 182 -- 3 files changed, 170 insertions(+), 22 deletions(-) Hi Lukas, thank you for your patience and all those patch versions. Nevertheless, I still have some doubts: I created the following extended files, one pair which creates and override, and the third file which is not overriden, but is an extended configuration. == /run/systemd/system/dracut-pre-udev.service == [Unit] Description=dracut pre-udev hook Documentation=man:dracut-pre-udev.service(8) ... == /etc/systemd/system/dracut-pre-udev.service.d/0-descr.conf == [Unit] Description=foofoo == /run/systemd/system/dracut-pre-udev.service.d/0-descr.conf == [Unit] Description=foo == /run/systemd/system/dracut-pre-udev.service.d/0-descr2.conf == [Unit] Documentation=foo When I run systemd-delta I see: ./systemd-delta --diff=0 --no-pager systemd/system -t extended (nothing) ./systemd-delta --diff=0 --no-pager systemd/system [OVERRIDDEN] /etc/systemd/system/dracut-pre-udev.service.d/0-descr.conf → /run/systemd/system/dracut-pre-udev.service.d/0-descr.conf I would expect to see 0-descr.conf and 0-descr2.conf in the first case... And also 0-descr2.conf in the second case... Is it actually supposed to behave like that? It might be nice to extend the description in the manpage a bit. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] systemd-delta: add support for drop-in snippets
--- TODO | 3 - man/systemd-delta.xml | 7 ++ src/delta/delta.c | 182 -- 3 files changed, 170 insertions(+), 22 deletions(-) diff --git a/TODO b/TODO index 62016e1..3af9c33 100644 --- a/TODO +++ b/TODO @@ -89,9 +89,6 @@ Features: kmod static-nodes call kmod as an early service, and drop CAP_MKNOD from udevd.service -* systemd-delta needs to be made aware of *.d/*.conf drop-in files for - units. - * seems that when we follow symlinks to units we prefer the symlink destination path over /etc and /usr. We shouldn't do that. Instead /etc should always override /run+/usr and also any symlink diff --git a/man/systemd-delta.xml b/man/systemd-delta.xml index 9293c9b..0c7a54a 100644 --- a/man/systemd-delta.xml +++ b/man/systemd-delta.xml @@ -141,6 +141,13 @@ /varlistentry varlistentry + termvarnameextended/varname/term + +listitemparaShow *.conf files in drop-in +directories for units./para/listitem +/varlistentry + +varlistentry termvarnameunchanged/varname/term listitemparaShow unmodified diff --git a/src/delta/delta.c b/src/delta/delta.c index aec3dc8..c3cf1a9 100644 --- a/src/delta/delta.c +++ b/src/delta/delta.c @@ -31,6 +31,7 @@ #include log.h #include pager.h #include build.h +#include strv.h static bool arg_no_pager = false; static int arg_diff = -1; @@ -41,9 +42,10 @@ static enum { SHOW_REDIRECTED = 1 2, SHOW_OVERRIDDEN = 1 3, SHOW_UNCHANGED = 1 4, +SHOW_EXTENDED = 1 5, SHOW_DEFAULTS = -(SHOW_MASKED | SHOW_EQUIVALENT | SHOW_REDIRECTED | SHOW_OVERRIDDEN) +(SHOW_MASKED | SHOW_EQUIVALENT | SHOW_REDIRECTED | SHOW_OVERRIDDEN | SHOW_EXTENDED) } arg_flags = 0; static int equivalent(const char *a, const char *b) { @@ -92,6 +94,14 @@ static int notify_override_overridden(const char *top, const char *bottom) { return 1; } +static int notify_override_extended(const char *top, const char *bottom) { +if (!(arg_flags SHOW_EXTENDED)) + return 0; + +printf(ANSI_HIGHLIGHT_ON [EXTENDED] ANSI_HIGHLIGHT_OFF%s → %s\n, top, bottom); +return 1; +} + static int notify_override_unchanged(const char *f) { if (!(arg_flags SHOW_UNCHANGED)) return 0; @@ -148,11 +158,114 @@ static int found_override(const char *top, const char *bottom) { return 0; } -static int enumerate_dir(Hashmap *top, Hashmap *bottom, const char *path) { +static int enumerate_dir_d(Hashmap *top, Hashmap *bottom, Hashmap *drops, const char *toppath, const char *drop) { +_cleanup_free_ char *conf = NULL; +_cleanup_free_ char *path = NULL; +_cleanup_strv_free_ char **list = NULL; +char **file; +char *c; +int r; + +path = strjoin(toppath, /, drop, NULL); +if (!path) +return -ENOMEM; + +path_kill_slashes(path); + +conf = strdup(drop); +if (!conf) +return -ENOMEM; + +c = strrchr(conf, '.'); +if(!c) +return -EINVAL; +*c = 0; + +r = get_files_in_directory(path, list); +if (r 0){ +log_error(Failed to enumerate %s: %s, path, strerror(-r)); +return r; +} + +STRV_FOREACH(file, list) { +Hashmap *h; +int k; +char *p; +char *d; + +if (!endswith(*file, .conf)) +continue; + +p = strjoin(path, /, *file, NULL); +if (!p) +return -ENOMEM; + +path_kill_slashes(p); + +d = strrchr(p, '/'); +if (!d || d == p) { +free(p); +return -EINVAL; +} +d --; +d = strrchr(p, '/'); + +if (!d || d == p) { +free(p); +return -EINVAL; +} + +k = hashmap_put(top, d, p); +if (k = 0) { +p = strdup(p); +if (!p) +return -ENOMEM; +d = strrchr(p, '/'); +d --; +d = strrchr(p, '/'); +} else if (k != -EEXIST) { +free(p); +return k; +
Re: [systemd-devel] [PATCH] systemd-delta: add support for drop-in snippets
On Mon, May 06, 2013 at 12:17:45PM +0200, Lukas Nykryn wrote: +if (streq(suffix, systemd/system)) +units = true; Hi Lukas, this test looks fishy. It seems that systemd-delta has a fixed set of paths, const char prefixes[] = /etc\0 /run\0 /usr/local/lib\0 /usr/local/share\0 /usr/lib\0 /usr/share\0 #ifdef HAVE_SPLIT_USR /lib\0 #endif ; Shouldn't at least this be changed to strictly follow the code in systemd, with e.g. the first dir changed to $(sysconfdir)? Also, systemd-delta should support --user mode. The list of directories will become more complicated then. Those problems existed before your patch, but adding a fixed test for systemd/system might make them harder to fix. It seems to me that process_suffix and friends should be extended to take the parameter which specifies if .d directories should be honored. Otherwise patch looks OK. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] systemd-delta: add support for drop-in snippets
--- TODO | 3 - man/systemd-delta.xml | 7 +++ src/delta/delta.c | 170 ++ 3 files changed, 164 insertions(+), 16 deletions(-) diff --git a/TODO b/TODO index 84ede8c..eab5f87 100644 --- a/TODO +++ b/TODO @@ -74,9 +74,6 @@ Features: kmod static-nodes call kmod as an early service, and drop CAP_MKNOD from udevd.service -* systemd-delta needs to be made aware of *.d/*.conf drop-in files for - units. - * seems that when we follow symlinks to units we prefer the symlink destination path over /etc and /usr. We shouldn't do that. Instead /etc should always override /run+/usr and also any symlink diff --git a/man/systemd-delta.xml b/man/systemd-delta.xml index 9293c9b..0c7a54a 100644 --- a/man/systemd-delta.xml +++ b/man/systemd-delta.xml @@ -141,6 +141,13 @@ /varlistentry varlistentry + termvarnameextended/varname/term + +listitemparaShow *.conf files in drop-in +directories for units./para/listitem +/varlistentry + +varlistentry termvarnameunchanged/varname/term listitemparaShow unmodified diff --git a/src/delta/delta.c b/src/delta/delta.c index aec3dc8..49cfc00 100644 --- a/src/delta/delta.c +++ b/src/delta/delta.c @@ -31,6 +31,7 @@ #include log.h #include pager.h #include build.h +#include strv.h static bool arg_no_pager = false; static int arg_diff = -1; @@ -41,9 +42,10 @@ static enum { SHOW_REDIRECTED = 1 2, SHOW_OVERRIDDEN = 1 3, SHOW_UNCHANGED = 1 4, +SHOW_EXTENDED = 1 5, SHOW_DEFAULTS = -(SHOW_MASKED | SHOW_EQUIVALENT | SHOW_REDIRECTED | SHOW_OVERRIDDEN) +(SHOW_MASKED | SHOW_EQUIVALENT | SHOW_REDIRECTED | SHOW_OVERRIDDEN | SHOW_EXTENDED) } arg_flags = 0; static int equivalent(const char *a, const char *b) { @@ -92,6 +94,14 @@ static int notify_override_overridden(const char *top, const char *bottom) { return 1; } +static int notify_override_extended(const char *top, const char *bottom) { +if (!(arg_flags SHOW_EXTENDED)) + return 0; + +printf(ANSI_HIGHLIGHT_ON [EXTENDED] ANSI_HIGHLIGHT_OFF%s → %s\n, top, bottom); +return 1; +} + static int notify_override_unchanged(const char *f) { if (!(arg_flags SHOW_UNCHANGED)) return 0; @@ -148,11 +158,114 @@ static int found_override(const char *top, const char *bottom) { return 0; } -static int enumerate_dir(Hashmap *top, Hashmap *bottom, const char *path) { +static int enumerate_dir_d(Hashmap *top, Hashmap *bottom, Hashmap *drops, const char *toppath, const char *drop) { +_cleanup_free_ char *conf = NULL; +_cleanup_free_ char *path = NULL; +_cleanup_strv_free_ char **list = NULL; +char **file; +char *c; +int r; + +path = strjoin(toppath, /, drop, NULL); +if (!path) +return -ENOMEM; + +path_kill_slashes(path); + +conf = strdup(drop); +if (!conf) +return -ENOMEM; + +c = strrchr(conf, '.'); +if(!c) +return -EINVAL; +*c = 0; + +r = get_files_in_directory(path, list); +if (r 0){ +log_error(Failed to enumerate %s: %s, path, strerror(-r)); +return r; +} + +STRV_FOREACH(file, list) { +Hashmap *h; +int k; +char *p; +char *d; + +if (!endswith(*file, .conf)) +continue; + +p = strjoin(path, /, *file, NULL); +if (!p) +return -ENOMEM; + +path_kill_slashes(p); + +d = strrchr(p, '/'); +if (!d || d == p) { +free(p); +return -EINVAL; +} +d --; +d = strrchr(p, '/'); + +if (!d || d == p) { +free(p); +return -EINVAL; +} + +k = hashmap_put(top, d, p); +if (k = 0) { +p = strdup(p); +if (!p) +return -ENOMEM; +d = strrchr(p, '/'); +d --; +d = strrchr(p, '/'); +} else if (k != -EEXIST) { +free(p); +return k; +
Re: [systemd-devel] [PATCH] systemd-delta: add support for drop-in snippets
On Tue, 30.04.13 12:27, Lukas Nykryn (lnyk...@redhat.com) wrote: Looks mostly good. -static int enumerate_dir(Hashmap *top, Hashmap *bottom, const char *path) { +static int enumerate_dir_d(Hashmap *top, Hashmap *bottom, Hashmap *drops, const char *toppath, const char *drop) { Hmm, can we maybe use get_files_in_directory() for this? (Or to turn this around, maybe extend get_files_in_directory() sufficiently to support this scheme?) Lennart -- Lennart Poettering - Red Hat, Inc. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] systemd-delta: add support for drop-in snippets
--- TODO | 3 - src/delta/delta.c | 179 ++ 2 files changed, 166 insertions(+), 16 deletions(-) diff --git a/TODO b/TODO index 9adec5e..96d90d8 100644 --- a/TODO +++ b/TODO @@ -95,9 +95,6 @@ Features: kmod static-nodes call kmod as an early service, and drop CAP_MKNOD from udevd.service -* systemd-delta needs to be made aware of *.d/*.conf drop-in files for - units. - * seems that when we follow symlinks to units we prefer the symlink destination path over /etc and /usr. We shouldn't do that. Instead /etc should always override /run+/usr and also any symlink diff --git a/src/delta/delta.c b/src/delta/delta.c index aec3dc8..0ccb76c 100644 --- a/src/delta/delta.c +++ b/src/delta/delta.c @@ -41,9 +41,10 @@ static enum { SHOW_REDIRECTED = 1 2, SHOW_OVERRIDDEN = 1 3, SHOW_UNCHANGED = 1 4, +SHOW_EXTENDED = 1 5, SHOW_DEFAULTS = -(SHOW_MASKED | SHOW_EQUIVALENT | SHOW_REDIRECTED | SHOW_OVERRIDDEN) +(SHOW_MASKED | SHOW_EQUIVALENT | SHOW_REDIRECTED | SHOW_OVERRIDDEN | SHOW_EXTENDED) } arg_flags = 0; static int equivalent(const char *a, const char *b) { @@ -92,6 +93,14 @@ static int notify_override_overridden(const char *top, const char *bottom) { return 1; } +static int notify_override_extended(const char *top, const char *bottom) { +if (!(arg_flags SHOW_EXTENDED)) + return 0; + +printf(ANSI_HIGHLIGHT_ON [EXTENDED] ANSI_HIGHLIGHT_OFF%s → %s\n, top, bottom); +return 1; +} + static int notify_override_unchanged(const char *f) { if (!(arg_flags SHOW_UNCHANGED)) return 0; @@ -148,11 +157,124 @@ static int found_override(const char *top, const char *bottom) { return 0; } -static int enumerate_dir(Hashmap *top, Hashmap *bottom, const char *path) { +static int enumerate_dir_d(Hashmap *top, Hashmap *bottom, Hashmap *drops, const char *toppath, const char *drop) { +_cleanup_closedir_ DIR *dir; +_cleanup_free_ char *conf = NULL; +_cleanup_free_ char *path = NULL; +char *c; + +path = strjoin(toppath, /, drop, NULL); +if (!path) +return -ENOMEM; + +path_kill_slashes(path); + +conf = strdup(drop); +if (!conf) +return -ENOMEM; + +c = strrchr(conf, '.'); +if(!c) +return -EINVAL; +*c = 0; + +dir = opendir(path); +if (!dir) { +if (errno == ENOENT) +return 0; + +log_error(Failed to enumerate %s: %m, path); +return -errno; +} + +for (;;) { +Hashmap *h; +struct dirent *de; +union dirent_storage buf; +int k; +char *p; +char *d; + +k = readdir_r(dir, buf.de, de); +if (k != 0) +return -k; + +if (!de) +break; + +if (!dirent_is_file_with_suffix(de, .conf)) +continue; + +p = strjoin(path, /, de-d_name, NULL); +if (!p) +return -ENOMEM; + +path_kill_slashes(p); + +d = strrchr(p, '/'); +if (!d || d == p) { +free(p); +return -EINVAL; +} +d --; +d = strrchr(p, '/'); + +if (!d || d == p) { +free(p); +return -EINVAL; +} + +k = hashmap_put(top, d, p); +if (k = 0) { +p = strdup(p); +if (!p) +return -ENOMEM; +d = strrchr(p, '/'); +d --; +d = strrchr(p, '/'); +} else if (k != -EEXIST) { +free(p); +return k; +} + +free(hashmap_remove(bottom, d)); +k = hashmap_put(bottom, d, p); +if (k 0) { +free(p); +return k; +} + +h = hashmap_get(drops, conf); +if (!h) { +h = hashmap_new(string_hash_func, string_compare_func); +if (!h) +return -ENOMEM; +hashmap_put(drops, conf, h); +conf = strdup(conf); +if (!conf) +return -ENOMEM; +} + +p = strdup(p); +if (!p) +return -ENOMEM; + +k = hashmap_put(h,