On Wed, 2016-03-23 at 06:21 -0700, Dan Nicholson wrote:
> At Endless, we compose a system from debian packages which enable
> services based on package policy. However, when we compose our whole
> system, we run systemctl preset-all (in a chroot) with our preset file
> to get the services setup exactly as we want for images.
>
> After upgrading from 215 to 229, we're seeing some issues.
>
> 1. preset-all fails with ELOOP if there are any symlinks in
> /etc/systemd/system pointing to /usr/lib/systemd/system. However,
> systemd itself has created these symlinks when units define an Alias
> and have already been enabled. This seems like broken behavior as even
> if I forcefully clean things up, running preset-all again returns the
> same issue.
>
> # find /etc/systemd/system -maxdepth 1 -type l -delete
> # systemctl preset-all
> Created symlink /etc/systemd/system/display-manager.service, pointing
> to /usr/lib/systemd/system/gdm.service.
> Created symlink /etc/systemd/system/ctrl-alt-del.target, pointing to
> /usr/lib/systemd/system/exit.target.
> Created symlink
> /etc/systemd/system/dbus-org.freedesktop.ModemManager1.service,
> pointing to /usr/lib/systemd/system/ModemManager.service.
> Created symlink /etc/systemd/system/eos-updater.service, pointing to
> /usr/lib/systemd/system/eos-autoupdater.service.
> Created symlink
> /etc/systemd/system/dbus-org.freedesktop.nm-dispatcher.service,
> pointing to /usr/lib/systemd/system/NetworkManager-dispatcher.service.
> Created symlink /etc/systemd/system/kbrequest.target, pointing to
> /usr/lib/systemd/system/rescue.target.
> Created symlink /etc/systemd/system/dbus-org.bluez.service, pointing
> to /usr/lib/systemd/system/bluetooth.service.
> Created symlink /etc/systemd/system/eos-updater.timer, pointing to
> /usr/lib/systemd/system/eos-autoupdater.timer.
> Created symlink
> /etc/systemd/system/dbus-org.freedesktop.Avahi.service, pointing to
> /usr/lib/systemd/system/avahi-daemon.service.
> # systemctl preset-all
> Operation failed: Too many levels of symbolic links
>
> Weirdly, using a preset with one of the above real names sometimes
> works even though it would have the exact same issue, and sometimes it
> returns EEXIST for reasons I don't understand.
>
> I guess this all comes back to commit 0ec0deaa, which tries to
> consider syminks in /etc as user configuration. But then why are alias
> symlinks, which are not user configuration, created in /etc?
>
> https://github.com/systemd/systemd/issues/2298 has been open for a
> while with no comments.
> 2. Enable does not seem to be escaping the names of the wants
> directories correctly.
>
> # systemctl enable "dev-disk-by\x2dlabel-eos\x2dswap.swap"
> Created symlink
> /etc/systemd/system/dev-disk-byx2dlabel-eosx2dswap.device.wants/dev-
> disk-by\x2dlabel-eos\x2dswap.swap,
> pointing to /usr/lib/systemd/system/dev-disk-by\x2dlabel-
> eos\x2dswap.swap.
>
> The directory is created as
> dev-disk-byx2dlabel-eosx2dswap.device.wants, but I'm pretty sure the
> x's should be escaped with \. At least, that's what the unit names do,
> and that's how the directories used to be named by enable.
>
> Any ideas on these issues?
Seems to be fixed with the following 2 patches. Not sure if they have
wider impact, but they seem right to me.
From 55ae8b362f45d4869cafe876be8455996af3e520 Mon Sep 17 00:00:00 2001
From: Dan Nicholson <nichol...@endlessm.com>
Date: Wed, 23 Mar 2016 10:44:24 -0700
Subject: [PATCH 1/2] install: Allow existing /etc symlinks for preset
When running systemctl preset or preset-all, there may be existing symlinks in
/etc due to previously enabled units. Allow these rather than returning ELOOP.
---
src/shared/install.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/shared/install.c b/src/shared/install.c
index ef8f485..0e1c85c 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -2193,7 +2193,7 @@ static int execute_preset(
int q;
/* Returns number of symlinks that where supposed to be installed. */
- q = install_context_apply(scope, plus, paths, config_path, root_dir, force, SEARCH_LOAD, changes, n_changes);
+ q = install_context_apply(scope, plus, paths, config_path, root_dir, force, SEARCH_LOAD|SEARCH_FOLLOW_CONFIG_SYMLINKS, changes, n_changes);
if (r >= 0) {
if (q < 0)
r = q;
--
2.1.4
From e61b9ad8171165f8b1919967853c6430b2bfbaad Mon Sep 17 00:00:00 2001
From: Dan Nicholson <nichol...@endlessm.com>
Date: Wed, 23 Mar 2016 11:08:41 -0700
Subject: [PATCH 2/2] conf-parser: Set EXTRACT_RETAIN_ESCAPE when extracting
words
If you reference another unit with an escaped name, the escaped characters
should remain in the extracted word. This used to work correctly prior to
commit 34f253f0.
---
src/shared/conf-parser.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c
index bd0a1f4..1141f99 100644
--- a/src/shared/conf-parser.c
+++ b/src/shared/conf-parser.c
@@ -732,7 +732,7 @@ int config_parse_strv(const char *unit,
for (;;) {
char *word = NULL;
int r;
- r = extract_first_word(&rvalue, &word, WHITESPACE, EXTRACT_QUOTES);
+ r = extract_first_word(&rvalue, &word, WHITESPACE, EXTRACT_QUOTES|EXTRACT_RETAIN_ESCAPE);
if (r == 0)
break;
if (r == -ENOMEM)
--
2.1.4
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel
