[Errno 18] Invalid cross-device link

[codewithmichael]

Bug Description

Running multiple partitions and working with in-memory wine prefixes, I often run into Lutris errors like the following --

2022-09-03 13:52:56,212: Error while completing task <bound method wine.prelaunch of <lutris.runners.wine.wine object at 0x7f3fe3b13760>>: <class 'OSError'> [Errno 18] Invalid cross-device link: '/dev/shm/overlay/drive_c/users/username/Desktop' -> '/dev/shm/overlay/drive_c/users/username/Desktop.winecfg'
  File "/usr/lib/python3.10/site-packages/lutris/util/jobs.py", line 34, in target
    result = self.function(*args, **kwargs)
  File "/usr/lib/python3.10/site-packages/lutris/runners/wine.py", line 798, in prelaunch
    self.sandbox(prefix_manager)
  File "/usr/lib/python3.10/site-packages/lutris/runners/wine.py", line 924, in sandbox
    wine_prefix.desktop_integration(desktop_dir=self.runner_config.get("sandbox_dir"))
  File "/usr/lib/python3.10/site-packages/lutris/util/wine/prefix.py", line 166, in desktop_integration
    os.rename(path, old_path)
2022-09-03 13:52:56,212: [Errno 18] Invalid cross-device link: '/dev/shm/overlay/drive_c/users/username/Desktop' -> '/dev/shm/overlay/drive_c/users/username/Desktop.winecfg'
2022-09-03 13:53:01,752: Game prelaunch unsuccessful

Cause

These errors are due to Lutris still using Python's os.rename in a few places instead of shutil.move (which properly handles cross-device semantics).

Reproduction

In the case posted above, it was due to running on top of a pre-built mounted squashfs wine image with overlayfs, but it happens just as easily by symlinking dirs across my first and second drive if the "rename" requires moving the file to a location on a separate partition.

Multi-device/partition systems are the norm for *nix users, and it seems like this should be handled as a common case.

Potential Solution

shutil is already used in a good number of places, and it just looks like its implementation was incomplete.

I run a manually patched version of Lutris to get around the error, but every time there's an update I have to re-patch it. Here's a quick unified diff (against the lutris dir) of the swaps I usually make to make things play nice across partitions.

I really don't think noticing one bad call warrants contributor status, so if an existing contributor would like to pick it up, I'd appreciate it :)

--- ./util/wine/prefix.py.orig	2022-09-03 13:55:36.773578128 -0700
+++ ./util/wine/prefix.py	2022-09-03 15:11:04.517450303 -0700
@@ -1,5 +1,6 @@
 """Wine prefix management"""
 import os
+import shutil
 
 from lutris.util import joypad, system
 from lutris.util.display import DISPLAY_MANAGER
@@ -163,7 +164,7 @@
                         os.rmdir(path)
                     # We can't delete nonempty dir, so we rename as wine do.
                     except OSError:
-                        os.rename(path, old_path)
+                        shutil.move(path, old_path)
 
                 # if we want to create a symlink and one is already there, just
                 # skip to the next item.  this also makes sure the elif doesn't
@@ -193,7 +194,7 @@
                 else:
                     # We use first the renamed dir, otherwise we make it.
                     if os.path.isdir(old_path):
-                        os.rename(old_path, path)
+                        shutil.move(old_path, path)
                     else:
                         os.makedirs(path, exist_ok=True)
 
--- ./util/extract.py.orig	2022-09-03 14:16:30.387053375 -0700
+++ ./util/extract.py	2022-09-03 15:11:11.190759653 -0700
@@ -144,7 +144,7 @@
             logger.warning("Overwrite existing file %s", destination_path)
             os.remove(destination_path)
         if os.path.isdir(destination_path):
-            os.rename(destination_path, destination_path + random_id())
+            shutil.move(destination_path, destination_path + random_id())
 
         shutil.move(temp_path, to_directory)
         os.removedirs(temp_dir)
--- ./installer/commands.py.orig	2022-09-03 14:18:03.142728269 -0700
+++ ./installer/commands.py	2022-09-03 15:11:20.437394238 -0700
@@ -345,7 +345,7 @@
         os.makedirs(temp_dir)
         self._killable_process(shutil.move, src, temp_dir)
         src = os.path.join(temp_dir, os.path.basename(src))
-        os.renames(src, dst)
+        shutil.move(src, dst)
 
     def _get_move_paths(self, params):
         """Process raw 'src' and 'dst' data."""
@@ -375,7 +375,7 @@
                     line = source_file.readline()
                     line = self._substitute(line)
                     dest_file.write(line)
-        os.rename(tmp_filename, filename)
+        shutil.move(tmp_filename, filename)
 
     def _get_task_runner_and_name(self, task_name):
         if "." in task_name:
--- ./migrations/migrate_banners.py.orig	2022-09-03 14:45:26.496386784 -0700
+++ ./migrations/migrate_banners.py	2022-09-03 15:11:28.944032543 -0700
@@ -1,5 +1,6 @@
 """Migrate banners from .local/share/lutris to .cache/lutris"""
 import os
+import shutil
 
 from lutris import settings
 from lutris.util.log import logger
@@ -17,7 +18,7 @@
                 dest_file = os.path.join(dest_dir, filename)
 
                 if not os.path.exists(dest_file):
-                    os.rename(src_file, dest_file)
+                    shutil.move(src_file, dest_file)
                 else:
                     os.unlink(src_file)

[danieljohnson2]

This doesn't look right to me. I do not understand how any of them can be across devices; almost all are renames within the same directory, and the one that isn't (migrate_banners.py) is still within the home directory of the user. How is it any of these are failing?

I'm also concerned that shutil.move can move a directory inside the destination directory, but os.rename will replace it or generate an error (depending on many details!); a simple search-replace seems unsafe to me.

[codewithmichael]

Good point, a move into the destination directory is a valid concern -- one I suppose I've ignored in my patches because I'm nearly always working with fresh prefixes and Lutris is typically moving its own directories. In case a user's working in a modified prefix that may have conflicts, it's likely safer to match os.rename() delete-if-empty-or-raise semantics -- something like:

if os.path.isdir(path):
    if len(os.listdir(path)) == 0:
        shutil.rmtree(path)
    else:
        raise OSError("Specified directory %s already exists" % path)
shutil.move(old_path, path)

[codewithmichael]

Regarding "renames within the same directory", running in a containerized or similar read-only/copy-on-write filesystem like a layered chroot environment makes "the same directory" an opaque concept, as the same directory may be read and written in different filesystems. Additionally, when the directory does change, as it does with the temp dir in installer/commands.py, in a modified prefix it could be as simple as crossing a symlinked device boundary.

As to "within the home directory of the user", I'm not the only one who runs an encrypted home directory that symlinks/bind-mounts in non-sensitive content directories like Games.

Edit: I should note, though, that I most commonly run into this while working under /dev/shm instead of my home dir

[danieljohnson2]

Hmm. Well banner migration could fail if ~/.local and ~/.cache are not on the same device, but this should be a temporary problem- Lutris will re-download the banners in 24 hours or so.

If your games are in a directory where 'foo' and 'foo.tmp' wind up on different devices, so that os.rename() fails, I think a lot of things are going to fail there. 'Safe-saving' by creating a temp file and renaming it over the original is common on Windows and Linux, and doing a copy-and-delete sequence would not be safe in the same way. I'd expect the games themselves to break right and left.

I'm not convinced this is a reasonable thing for Lutris to support.

[codewithmichael]

I haven't run into any game failures yet -- and I'm not sure I'd expect to. Presuming Wine is using standard linux move semantics, a mv between devices/filesystems is completely supported.

I'd have to look under the hood to see exactly what Python is doing with os.rename that causes the failure, but generally atomic copy-and-delete is pretty standard practice for cross-device moves.

If we're talking about safe-saving, I'd agree on a per-file basis -- but creating a temp directory and renaming it over an existing directory without an explicit delete is certainly not common practice, and I think that's primary what we're talking about in this case -- moving directories (aside from banners).

I won't presume to speak to what Lutris should and shouldn't support, though. Lutris is geared toward the gaming community, not power users, and the vast majority of users are likely on a single partition or a dedicated games partition.

But I would note that containerized filesystems and specialized chroots are becoming fairly standard practice for simple instancing and sandboxing. Wine prefixes -- as standalone working environments -- are a natural fit for that use case. So while it may not presently be the common case, I expect to see quite a bit more of it as general practice over the next few years.