xdg-desktop-portal
xdg-desktop-portal copied to clipboard
flatpak
xdg-document-portal: implement flock
I implemented flock operation of FUSE.
This is needed for Steam client.
- https://github.com/ValveSoftware/steam-for-linux/issues/10829
Tested
I tested whether the new xdg-document-portal accepts flock(2) syscall with flock(1).
$ umount /run/user/60331/doc
$ meson compile -C _build
$ ./_build/document-portal/xdg-document-portal &
$ cd /run/user/60331/doc
$ touch lock.L
$ flock -n lock.L sleep 100
Then, within 100 seconds, I ran flock(1) in other terminal session.
$ flock -n lock.L ls || echo BAD
BAD
I checked blocking operations are rejected.
$ flock lock.L ls
flock: lock.L: Function not implemented
$ flock -s lock.L ls
flock: lock.L: Function not implemented
Looks like this will block the entire document portal when any file is blocked due to flock.
Hi, @swick I mitigated not to block entire document-portal by rejecting blocking operations of flock. Steam client works well in my Arch Linux box.
@hfiguiere Thanks for your suggestion!
Thanks for the patch. Please squash the suggestion commit into a single commit.
Supporting flock only for certain option is very suspicious to me. This needs review from someone who knows about fuse and filesystems.
Hello, I'm wondering if someone can again take a look at this branch and merge it in. Without this addition, the Steam flatpak is unable to handle adding another filesystem.
Without this addition, the Steam flatpak is unable to handle adding another filesystem
You can give the unofficial Steam Flatpak app direct access to locations where you want to add an extra Steam library by completely exiting from Steam, and then using something like:
$ mkdir /media/other-disk/steam-library
$ flatpak override --user --filesystem=/media/other-disk/steam-library com.valvesoftware.Steam
before starting Steam again and selecting /media/other-disk/steam-library as your new Steam library. This avoids the performance overhead of going through a FUSE filesystem, as well as avoiding any limitations like the one fixed in this branch.
Without this addition, the Steam flatpak is unable to handle adding another filesystem
You can give the unofficial Steam Flatpak app direct access to locations where you want to add an extra Steam library by completely exiting from Steam, and then using something like:
$ mkdir /media/other-disk/steam-library $ flatpak override --user --filesystem=/media/other-disk/steam-library com.valvesoftware.Steambefore starting Steam again and selecting
/media/other-disk/steam-libraryas your new Steam library. This avoids the performance overhead of going through a FUSE filesystem, as well as avoiding any limitations like the one fixed in this branch.
Upon using the override I precisely get the original errorno 38 (function not implemented) that lufia mentions here, and I believe that the additions are need to allow the line to work, unless I am misunderstanding something.
Upon using the override I precisely get the original
errorno 38(function not implemented) that lufia mentions here, and I believe that the additions are need to allow the line to work, unless I am misunderstanding something.
No, the override is sufficient by itself. You do need to restart Steam, and you may need to remove and re-add the library in Steam so it doesn't continue to use the portal path. That error means it's probably still going through the document portal.
Also, just to be clear, you need to modify the path to match your actual Steam library location; don't use literally /media/other-disk/steam-library.
Upon using the override I precisely get the original
errorno 38(function not implemented) that lufia mentions here, and I believe that the additions are need to allow the line to work, unless I am misunderstanding something.No, the override is sufficient by itself. You do need to restart Steam, and you may need to remove and re-add the library in Steam so it doesn't continue to use the portal path. That error means it's probably still going through the document portal.
Also, just to be clear, you need to modify the path to match your actual Steam library location; don't use literally
/media/other-disk/steam-library.
Really? I'll take a look and try again in a few days (occupied with moving right now).
I actually recall that the override was working for me perfectly fine on the Fedora KDE spin, but somehow it has not been working for me on the Fedora Sway spin. Both installations should have me running on Wayland.
I appreciate the input.
Do you understand the implications of supporting a weird subset of flags for flock?
flock does something on a per-process basis but any flock call on fuse will result in flock being called in the docuement portal process. Do we know what the results are of this?
Do we know what the results are of this?
I suspect the risk is some app does flock(file) will cause other apps that flock() the same file to not actually receive said lock, meaning the lock is useless.
There also seems to be missing "cleanup" of files left locked by some app, meaning an app doing flock() and then crashes before flock(LOCK_UN) will mean the lock will remain indefinitely.
Do you understand the implications of supporting a weird subset of flags for flock?
The implication is that xdg-document-portal won't support flock() with a particular subset of flags.
I also want to ask you to please stop calling things "weird". It doesn't set a productive tone to this conversation.
flock does something on a per-process basis but any flock call on fuse will result in flock being called in the docuement portal process. Do we know what the results are of this?
I do not know. If you do, please inform us.
I'll go ahead and merge this, as a stop-gap solution. It'll be interesting to be able to support blocking flock() calls somehow, maybe by spawning a thread for each blocking call? @lufia are you interested in exploring this next step?
I guess one could implement file locking on behalf of document portal owners in the document portal, i.e. as long as there is more than one lock owner, we call flock(), and then handle LOCK_SH vs LOCK_EX on a per app basis. But it does mean apps will be able to "lock" files it got shared, for better or worse. I guess as long as they already have write access, it isn't that bad.
Sorry, but I just don't think it makes a lot of sense to merge things that we don't fully understand even if they seem important to some apps. I'd equally appreciate if you'd stop assuming that everything I say is an attack on you. Calling it weird that just the nonblocking flock variants are supported isn't far fetched because AFAICS when nonblock is supported, so is the blocking variant. What we have here definitely isn't something that apps are used to seeing which can easily introduce issues in some interesting code paths.
@lufia
- Is the game library correctly detected and added in Steam (while making sure that no static permission was added to add the library, i.e. no
flatpak override ...)? - Are the games playable?
@Mikenux I can play games on the external drive with Steam correctly; no overrides.
$ ls .local/share/flatpak/
app appstream db exports repo runtime # no overrides directory
When I created this PR, I thought "This is not good but Steam works and it does not make document-portal more bad."
However, along with I'm reading this thread, I felt it is better to close this PR then to file an issue that is described same problem that this tried to fix.
I guess if "non-blocking flock operations" means that other apps accessing the file locked by an app can still access and write to it, then that's ok?
Only locking read and write access to a file is problematic and should probably be either allowed automatically for known cases or subject to permission.
Given that the PR only handles non block requests and does not handle cleaning up locks when the fuse fd gets dropped, it isn't ready to be merged. I'm going to close it because there was no activity in a year.
Feel free to re-open this PR or open a new PR when a version exists with those problems fixed.