core: do not attempt to add 'private' symlinks when RootImage/RootDirectory are used #22272
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There's an app0.service in the extension app0.raw, so don't use the same name for a unit in minimal.raw
Makes the setup idempotent, as mksquashfs by default attempts to append to an existing image
bluca
added
pid1
portable
yuwata
added
the
good-to-merge/waiting-for-ci 👍
|
I can confirm that the PR fixes #22264
|
gdamjan
approved these changes
Jan 27, 2022
bluca
force-pushed
the
state_dir_private_rootfs
branch
from
16c4617
to
1a8e4f4
Compare
|
TEST-29-PORTABLE failed. Is it related? |
|
Possibly but I don't quite understand why it's affecting the sanitizer build - @mrc0mmand any idea? |
yuwata
added
the
ci-fails/needs-rework 🔥
|
tentatively set the red label. |
|
I'm a bit confused now how this all should work, but from a quick glance it looks like a timing issue? The portable-related stuff is mostly dark magic for me, so I'm not sure what's going wrong there. |
|
There's two services running, and creating those files, by the time they are removed they should be all done, but maybe I need to add some looping |
I see. I'll do a couple of local tests just to confirm this, so we don't have to for CI confirmation in case it's something else. |
|
Indeed. I can reproduce the issue locally and applying: diff --git a/test/units/testsuite-29.sh b/test/units/testsuite-29.sh
index 3afdc44ff4..8cffb2e66d 100755
--- a/test/units/testsuite-29.sh
+++ b/test/units/testsuite-29.sh
@@ -112,8 +112,13 @@ portablectl detach --now --runtime --extension /usr/share/app1.raw /usr/share/mi
# Ensure that the combination of read-only images, state directory and dynamic user works, and that
# state is retained. Check after detaching, as on slow systems (eg: sanitizers) it might take a while
# after the service is attached before the file appears.
-grep -q -F bar /var/lib/private/app0/foo
-grep -q -F baz /var/lib/private/app1/foo
+for ((i = 0; i < 20; i++)); do
+ if grep -q -F bar /var/lib/private/app0/foo && grep -q -F baz /var/lib/private/app1/foo; then
+ break
+ fi
+
+ sleep 0.5
+done
# portablectl also works with directory paths rather than images
makes the test gods happy: |
|
wow that's really slow - ok thanks, this really helps, will fix it |
yeah, I guess bumping the sleep to 1s might make sense, given the loop runs for more than one iteration only in sanitizer runs |
|
After the loop, the |
bluca
force-pushed
the
state_dir_private_rootfs
branch
from
1a8e4f4
to
4127554
Compare
|
It's really weird, all the calls before the check should be blocking |
bluca
removed
the
ci-fails/needs-rework 🔥
…ectory are used A bind mount is added directly from private on the host to the actual destination directory, no need for the symlinks (which cannot be created as the bind mount happens first and creates the target as an actual directory) Fixes systemd#22264
bluca
force-pushed
the
state_dir_private_rootfs
branch
from
4127554
to
3fa80e5
Compare
|
Ah, I understand the issue - with sanitizers, the trusted profile is used, which means DynamicUser is disabled. But in that case the state directory storage is /var/lib/foo, not /var/lib/private/foo, but the test was hard-coded to check the latter only. Should be fixed now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A bind mount is added directly from private on the host to the actual
destination directory, no need for the symlinks (which cannot be created
as the bind mount happens first and creates the target as an actual directory)
Fixes #22264