-
Notifications
You must be signed in to change notification settings - Fork 305
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
drop FUSE detection workaround for flatpak #2900
Comments
I've bisected this down to 59a653c, reverting it on |
Interesting. Can you tell exactly where in flatpak this starts to fail? Flatpak does some interesting things with temporary and parent/child repos that might not get good test coverage in ostree. |
I have no experience with all the different glib abstractions so I haven't found anything yet. The best I have found so far is that it happens somewhere in or after |
The error comes from GZlibDecompressor. Why that would happen from creating the file in the repo's It would be interesting to run flatpak with |
running with both Partial Log
the log includes a different error message which is from
ostree/src/libostree/ostree-lzma-common.c Line 60 in 363a1f1
I think the error message might be misleading, at least with LZMA.
Both the only difference that is obvious is that one uses the dfd and a relative path while the other just uses whatever is in $TMPDIR or /var/tmp. |
Exactly. Do you have the full log or at least the earlier parts? I don't know if it will help, but I'd like to see exactly where the repo is. In order to use the flatpak system repo, there's a funky 2 stage setup where (I believe) it uses a temporary repo writable by the unprivileged user as the intermediary. I'd like to know what's special about this configuration because I don't think the patch breaks typical ostree usage. |
BTW, the 2 errors are essentially the same thing. In one you're getting a In both cases, though, the decompression library is saying it's reached the end of the stream without seeing the expected end. I don't know why that's happening, but I think both errors likely come from the same cause. |
I've alsog one ahead and captured the output of (all anon inodes and and sockets removed for clarity)
|
Another Fedora user, azarok, has also mentioned that they can install and update Flatpak apps by using sudo privileges. I have also tested this and can confirm that using sudo with Flatpak install and update commands works in my case. While I understand that Flatpak shouldn't normally require sudo, I thought it worth mentioning this observation. |
Thats interesting. running flatpak in the same way I did before, but this time as root, showed different path being used.
|
@Jan200101 Thanks for all that info. I'm starting to suspect the error is actually in the system helper that pulls the objects from the user's temporary repo used for downloading into the I'd bet if you do a |
Can you run the system helper directly with |
I can confirm that using
Sure thing
|
I actually think that's expected. At the end of your earlier verbose log you could see the user side instructing the system helper to cancel because of failures. So, I think it is the user side in this interaction that's failing like was shown in your earlier logs. There must be something different about the temporary proxy repo the user sets up in @alexlarsson do you have some time to look at this? I think you probably understand the interaction with the flatpak system helper better than anyone. |
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
@leafi thanks for the ping. I'll revert the commit mentioned in #2900 (comment) in the hopes that it'll fix this for Arch Linux. Please check whether 2023.4-2 in extra-testing fixes this for you. |
This comment was marked as duplicate.
This comment was marked as duplicate.
😉 |
@dvzrv Hey, thanks. I just enabled extra-testing, updated, and |
@dvzrv From your arch change, it appears reverting f7f6f87 fixes the problem, right? If so, care to turn it into a PR? I think that's the wise thing to do for now while the root cause is investigated. |
@dvzrv Also confirming 2023.4-2 in extra-testing fixed the issue for me, FWIW. |
Sure, I can do that today!
Thanks for verifying! Moved to the stable repos. |
This comment was marked as duplicate.
This comment was marked as duplicate.
This reverts commit f7f6f87. This seems to have broken flatpak, so we'll revert and then investigate. Closes: ostreedev#2900
Revert PR up in #2901 |
This comment was marked as duplicate.
This comment was marked as duplicate.
Oh yeah, |
This comment was marked as duplicate.
This comment was marked as duplicate.
Fedora 38 update (revert) to test: https://bodhi.fedoraproject.org/updates/FEDORA-2023-464fae1680 |
I mangled together a setup with revokefs-fuse and got the following interesting responses when trying to use
So libglnx tries to fall back to a regular temporary file and gets
Similarly, if I use I'm not sure if I'm driving it right yet. I'm also on linux 6.1, which is a bit old at this point. I might have to try this out in a VM with a newer kernel. |
I haven't figured out what's going on yet, but something is going wrong as the data is being read from the fd. Here the LZMA decompressor is being given some data to convert but being told the size is 0:
The Teasing out the actual fd and current position:
Taking a look at the temporary file outside gdb it looks like it was downloaded successfully and is definitely larger than 24576 bytes.
|
This reverts commit f7f6f87. This seems to have broken flatpak, so we'll revert and then investigate. Closes: ostreedev#2900
It appears that this is a kind of race or at least the reader is going too fast for fuse. When I was breaking in So, I'm starting to suspect this is bug in a corner case in The simple reproducer I'm using is to install a flatpak runtime that has no dependencies. I've settled on the openh264 runtime from flathub since it's pretty small:
You could probably go even smaller with one of the theme extensions. In my earlier "hacked up revokefs-fuse" test, I was trying to use the demo, but it didn't appear to be doing the right thing since any IO on the mounted target was failing. |
I had tried doing an update in a loop and parts of it succeeded after a while, which indeed points to a race. |
Let's keep this open until we figure out how we can safely re-land 59a653c because we do still want to make that change. Probably to be conservative we may need to do something hacky like detecting whether we're being executed by flatpak, or maybe if we're writing to a FUSE mount? |
I think detecting if we're going through FUSE. I think that should be pretty straightforward with an |
One thing dawned on me about the Before this change, the flow was 99% write based from the unprivileged user's perspective. It was either downloading metadata objects to memory and then writing them into the temporary repo's objects directory, or it was downloading file or delta parts to |
I was finally able to get the demo going after having it not close the socket the frontend needs. I thought you'd always end up in the |
Found the problem. Since revokefs-fuse is using FUSE's default cached mode, you sometimes get page cache results from |
Is this issue releated to |
I don't know what that issue is, but I doubt it's related. Please open a separate issue with details about it. |
FYI, I think I fixed the actual bug in flatpak/flatpak#5454. As requested there, the preference is not to reinstate f7f6f87 (or an equivalent) until flatpak releases with that fix have been made. |
This reverts commit 4e61e6f and re-instates the fix for ensuring that we download temporary files into the repository location. However in order to ensure we don't re-introduce ostreedev#2900 we detect the case where we're writing to a FUSE mount and keep the prior behavior. I've verified that this works with flatpak. Note a downside of this is the change needs to be triplicated across the 3 http backends. This then again Closes: ostreedev#2571
#2913 is queued; going to retitle this issue to track when we can drop the workaround...since it's not high priority I'd give it say 6 months. |
This reverts commit 4e61e6f and re-instates the fix for ensuring that we download temporary files into the repository location. However in order to ensure we don't re-introduce ostreedev#2900 we detect the case where we're writing to a FUSE mount and keep the prior behavior. I've verified that this works with flatpak. Note a downside of this is the change needs to be triplicated across the 3 http backends. This then again Closes: ostreedev#2571
As reported on flatpak/flatpak#5452, multiple Fedora users seem to be seeing Flatpak update failures since their OS was upgraded from libostree 2023.3 to 2023.4. @yuntaz0 reports in flatpak/flatpak#5452 (comment) that rolling back to a previous OS image resolves this for them; only a few packages were involved in that rollback, of which libostree seems like the most likely cause for a Flatpak regression.
I haven't packaged 2023.4 for my OS (Debian) yet, so I have no particular insight into this myself.
The text was updated successfully, but these errors were encountered: