-
-
Notifications
You must be signed in to change notification settings - Fork 115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New custom-persist feature #551
Conversation
When the custom-persist feature is enabled, we no longer need to worry about the bind directories configured in /rw/config/qubes-bind-dirs.d.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #551 +/- ##
==========================================
- Coverage 70.57% 70.46% -0.12%
==========================================
Files 3 3
Lines 469 474 +5
==========================================
+ Hits 331 334 +3
- Misses 138 140 +2 ☔ View full report in Codecov by Sentry. |
987de8f
to
3583fca
Compare
Happy to see this getting implemented. And @Guiiix happy to see a new face on the Qubes team :) Here's some feedback from giving this and it's sister PR a spin. Although I haven't been able to get this working yet (but I could be setting it up wrongly). What I did:
Expected:
What happened instead:
So I'm probably doing something wrong. Is this the expected usage of this feature or am I messing something in the process? |
Yes, I think bind-dirs does not create dirs on its own, they need to exist beforehand. This is especially relevant since by looking at just the path, you don't have enough info to create it - it can be either a file or a directory, you don't know what permissions to use, owner etc... But maybe this can be extended with some defaults? For example if doesn't exist, then create as a dir, with owner same as the parent directory, and default mode of 755? Or extend the syntax to allow specify those details... |
Can you check why? The default /home from the template should be good enough for normal applications to start... |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025022819-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025021804-4.3&flavor=update
Failed tests25 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/129058#dependencies 14 fixed
Unstable testsPerformance TestsPerformance degradation:29 performance degradations
Remaining performance tests:43 tests
|
It seems to be some some permission / SELinux issue. The following is the tail of the console log for
And this is what happens in a working template (without custom-persist), which I called
If I then enable the |
The selinux messages are red herring, it's related to GUI session failing to start, so application is started outside of it. The actual problem is somewhere about GUI agent - see either qubes-gui-agent logs in journal or /home/user/.xsession-errors. |
Hi @deeplow! Thank you for your message. I'm very happy to be part to this adventure with you all :) I tried to reproduce on release r4.2 (commits backported to branch release 4.2) but I didn't get any error on the template:
Did both |
FWIW I also didn't reproduced the GUI apps failure, but I tried it in AppVM, not the template itself. I added integration tests here: https://github.com/QubesOS/qubes-core-admin/pull/654/files#diff-19c2f2ac398e72a651b363ef17c05d7f0348b8bed0cbbf5c938265e09c76e463R57-R118 There are a couple of things going on. First, it does fail (as expected) on directories that don't exist initially (one may expect /home/user/Downloads to exists already, but in practice it's created on first boot - and that happens after bind-dirs call). But there is another thing - |
I tried many things to reproduce but I was not able to. Maybe I'm doing wrong. I did the following (I'm running a 4.2-stable version of Qubes):
Rootfs is not mounted for
Order seems to be good in logs:
|
Could be non-deterministic. What happens if you set vcpus to 1 for this qube? |
Or maybe it's more likely to fail if you add more custom-persist dirs? |
In any case, I don't see anything that would guarantee /usr/local and /home being mounted only after bind-dirs is called. I think explicit |
You're right, it's probably non-deterministic. Even with 1vcpu and a lot of custom persist directories, it's working well on my VM but something is missing in the If we want to keep the same behavior as before when this feature is disabled, we don't want
I guess this is why you see |
I added a sleep at the beginning of |
This should do the job if my understanding of |
But at least tests are green now :) |
Thanks for testing. I think this only happens on files / directories under I have followed your steps, although when updating to the step "update qvm-features
I do still see the mounts: mount
So I think what's happening here is that when the mounts happen in home, they should up in thunar. You tested with a file in Also, in the journalctl -u qubes-bind-dirs.service -u qubes-mount-dirs.service
|
The "eject" icon there is especially confusing. Will it actually unmount the thing if you click it? Maybe for some reason Thunar thinks it's a removable device... I wonder how it gets that, there is no device attached after all (so it can't be udev, and probably not udisks either). |
Ah yes I got the eject button too with a bind mount in https://github.com/GNOME/gvfs/blob/master/monitor/udisks2/what-is-shown.txt If we add
This is related to the second problem
Duplicates mounts that might be caused by mount propagation: As the second entry is automatically generated, we can't add the By overriding
It tries but got permission denied |
This makes sense. So, it's basically caused by having |
Ah yes!! I was thinking So the winning combination is:
|
When using custom-persist to pre-create the resource before bind mounting it, we might have to create its parents too. That was done using mkdir --parents that was causing parents to be created with root:root ownership which can leads to errors if, for example, a user wants to bind mount a directory inside its home dir. With this fix, parents are created with the same ownership as the resource.
…nting When disabling persistent /home or /usr/local, custom-persist was using a systemd drop-in to override the What= option and set it to the same value as the Where= one. This bind mount is unnecessary and was causing trouble when bind mounting other resources in /home or /usr/local. Instead, a ConditionPathExists= option is added to control whether this mount happens.
This allows to hide mountpoints from Thunar sidebar (happens when bind mounting a file or dir in $HOME).
A minor notes, potentially for the future:
|
Noticed, thank you for the clarification! |
vm-systemd/bind-dirs.sh
Outdated
test -d "$fso_rw" && mkdir --parents "$fso_ro" | ||
test -f "$fso_rw" && touch "$fso_ro" | ||
if [ -f "$fso_rw" ]; then | ||
parent_directory="$(dirname "$fso_ro")" | ||
test -d "$parent_directory" || mkdir --parents "$parent_directory" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those two mkdirs are the ones that matter for owner/mode. But here you don't have the settings readily available... maybe you can read them $fso_rw
parents (which is already created at this point)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes my commit is a nonsense...
What about letting this part intact, using mk_parent_dirs
to create $path
($fso_ro
) parents and mkdir -p
to create $rw_path
($fso_rw
) parents here:
qubes-core-agent-linux/vm-systemd/bind-dirs.sh
Lines 200 to 217 in f18831c
echo "custom-persist: pre-creating file ${rw_path} with rights ${owner}:${group} ${mode}" | |
if [ ! -d "$parent_directory" ]; then | |
if ! mk_parent_dirs "$parent_directory" "$owner" "$group"; then | |
echo "Unable to create ${rw_path} parent dirs, skipping" | |
continue | |
fi | |
fi | |
touch "${rw_path}" | |
elif [ "$resource_type" = "dir" ]; then | |
echo "custom-persist: pre-creating directory ${rw_path} with rights ${owner}:${group} ${mode}" | |
if ! mk_parent_dirs "$rw_path" "$owner" "$group"; then | |
echo "Unable to create ${rw_path} parent dirs, skipping" | |
continue | |
fi | |
else | |
echo "Invalid entry ${target}, skipping" | |
continue | |
fi |
EDIT: no, that would only work the first time when $fso_rw doesn't exist...
Thunar now don't show those dirs anymore :) Generally I think it's all good now. I'll take a look at the whole diff one more, but should be okay. I noticed one more potential issue - SELinux labels. new dirs have |
This PR adds a new feature
custom-persist
described in QubesOS/qubes-issues#1006