GNOME Bugzilla – Bug 748248
Fix trashing on overlayfs
Last modified: 2018-05-24 17:47:23 UTC
In order to determine whether to trash a file to the home directory, we compare its st_dev to our home directory's st_dev field. This is the wrong thing to do on overlayfs when deleting files, because st_dev contains the ID of the filesystem providing the file (which can be the lower or upper filesystem), but directories always return the ID of the overlayfs. Thus the comparison fails and we are unable to trash the file. Fix this by checking st_dev of the parent directory when we are deleting a file.
Created attachment 302071 [details] [review] Fix trashing on overlayfs
Review of attachment 302071 [details] [review]: You might consider doing the parent check unconditionally -- probably the usual case is trashing is files, so we'll end up taking this 'fallback' more often than not. ::: gio/glocalfile.c @@ +1936,3 @@ + { + parent = g_file_get_parent (file); + path = g_file_get_path (parent); You could just use ->filename. You could also just use g_path_get_dirname() to avoid the new GFile.
Created attachment 302080 [details] [review] Fix trashing on overlayfs In order to determine whether to trash a file to the home directory, we compare its st_dev to our home directory's st_dev field. This is the wrong thing to do on overlayfs when deleting files, because st_dev contains the ID of the filesystem providing the file (which can be the lower or upper filesystem), but directories always return the ID of the overlayfs. Thus the comparison fails and we are unable to trash the file. Fix this by checking st_dev of the parent directory when we are deleting a file.
This patch seems to have side-effects, see bug #748629.
FYI I experience bug #748629 with libglib2.0-0 version 2.44.0-1ubuntu3, and not 2.44.0-2 or 2.44.0-3 (assuming that Ubuntu package corresponds to upstream version 2.44.0-1).
(In reply to Sadi from comment #5) > FYI I experience bug #748629 with libglib2.0-0 version 2.44.0-1ubuntu3, and > not 2.44.0-2 or 2.44.0-3 (assuming that Ubuntu package corresponds to > upstream version 2.44.0-1). It includes this same patch. http://changelogs.ubuntu.com/changelogs/pool/main/g/glib2.0/glib2.0_2.44.0-1ubuntu3/changelog
I've just been looking at that regression introduced by this patch. I can't reproduce the previous behaviour though. Can someone help? - Install an Ubuntu 14.04 VM and give it an additional disk - Format this disk and mount it at /srv - mkdir /srv/foo; ln -s /srv/foo foo; touch foo/a - gvfs-trash foo/a Doesn't work. I read the reports as saying that this used to trash into the home directory. Is that right? Can someone help me out?
(In reply to Iain Lane from comment #7) > Doesn't work. As in it's trying to trash to /srv which I thought people were saying was not right. If I make /srv/.Trash/uid then it trashes into there with or without this patch.
A similar bug report at Ubuntu's launchpad might perhaps be helpful: https://bugs.launchpad.net/ubuntu/+source/glib2.0/+bug/1495781 Or the fix mentioned there might even be more helpful: https://gist.github.com/CzBiX/e64256b23687bb13da02
(In reply to Iain Lane from comment #7) > I've just been looking at that regression introduced by this patch. I can't > reproduce the previous behaviour though. Can someone help? I should be able to test it again. Got other stuff to do now, but maybe next week or so.
(In reply to Iain Lane from comment #7) > - Install an Ubuntu 14.04 VM and give it an additional disk > - Format this disk and mount it at /srv > - mkdir /srv/foo; ln -s /srv/foo foo; touch foo/a > - gvfs-trash foo/a I tried this (changing the path to /mnt/hd1, which is what I have). I get $ gvfs-trash foo/a Error trashing file: Unable to trash file: Invalid cross-device link This is with libglib2.0-0 2.46.2-1 on Debian unstable, which contains the patch in this bug report even though I have tried to tell them not to include it yet. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=800047 > Doesn't work. I read the reports as saying that this used to trash into the > home directory. Is that right? Can someone help me out? Without this patch it works fine as it always has and trashes to the correct location, in my case /mnt/hd1/.Trash-1000. > As in it's trying to trash to /srv which I thought people were saying was > not right. If I make /srv/.Trash/uid then it trashes into there with or > without this patch. I don't follow. For me, without this patch, it trashes to /mnt/hd1/.Trash-1000, which works fine, and there's no uid file. I don't expect to have to create one.
Ubuntu 14.04 is a release which doesn't have the patch in question. The above steps didn't work there. You're saying they do, but that's not what I am seeing. I have simply never managed to get trashing like that to work. It would be helpful if you could try the steps that I gave, not a different environment, and tell me what to do differently to get trashing to work.
Sorry, I don't have the time to test Ubuntu 14.04 VM. I can only tell you what I see on Debian unstable, which is that I don't remember ever having trouble trashing files on symlinked folders before this patch was included in the Debian package.
Removing the NEEDINFO status... Iain, I recommend only using NEEDINFO when you want info from the bug reporter
Hi, FYI I do not experience the bug in Ubuntu 14.04 with libglib2.0-0 version 2.40.2-0ubuntu1 Yet I do experience the bug in Ubuntu 16.04.1 (and Xubuntu 16.04).
in Ubuntu 16.04.1 libglib2.0-0 version is 2.48.1-1ubuntu16.04.1 it has the bug. By the way I confirm that subfolders can be trashed. Files inside subfolders can be trashed. Not files located directly inside linked folder.
I am running ubuntu 16.04 and started seeing this bug only after moving my data disk to LVM. My original setup was an ssd for operating system and home directory and a 1 TB SCSI drive for data. I had several folders such as Downloads symlinked from home directory to data drive. The data drive was originally set up without a partition. it was raw /dev/sdb. I had no issues with this bug. I recently added a new 2 TB data drive to my system and set it up with LVM. I copied the contents of the 1TB drive to the 2 TB drive using cp -aur I edited fstab using UUID to mount the new 2 TB drive in the location of the old 1TB drive. I rebooted and all symlinks worked as expected, pointing to the new 2 TB drive. Now I get the error "file can't be put in the trash. do you want to delete it immediately" with any file in the top level of the Downloads directory when selecting the Downloads symlink in home. I can delete files that are in subdirectories within Downloads directory. I can delete files from /mnt/storage/Downloads which is the origin for the symlink in the home directory of the same name. All drives are formatted ext4. I found the same error using both nemo and nautilus I have write permission for all folders including: .local/share/Trash /mnt/storage/.Trash-1000
After upgrading from Ubuntu 16.04 to 16.10 (new libglib 2.0-0 version 2.50.0-1) this problem got even worse: Now neither items in a symlinked subdirectory nor those accessed directly (not via symlink) in that drive/partition can be moved to Trash (now called Rubbish Bin here wastebasket there)... It's incredible that this problem has been around for so long while there's a patch here (https://gist.github.com/CzBiX/e64256b23687bb13da02) which just needs updating every time the libglib package concerned (apparently the culprit) gets updated :-(
I'm not familiar with C, GLib or OverlayFS, but the patch looks simple, so maybe I can ask some perhaps stupid questions. + if (!S_ISDIR (file_stat.st_mode)) + { + path = g_path_get_dirname (local->filename); + g_lstat (path, &file_stat); + g_free (path); + } Assume /home/osmo/Documents is a symlink and I have a file /home/osmo/Documents/test.txt. * Does path in the code evaluate to /home/osmo/Documents? My quick testing at a Python prompt at least tells me that g_path_get_dirname doesn't evaluate the symlink. * Is it useful with OverlayFS to stat a symlink (regarding the st_dev problem the patch is trying to solve)? Or could you just skip the stat if path is a symlink? * If no to the previous, could you evaluate the symlink (like readlink does) before doing the stat? I can test some simple changes on my system (again not Ubuntu 14.04!), but not being familiar with C and GLib, I'd need some pointers on what to try, preferrably even written out for me.
Created attachment 340594 [details] [review] This new version uses g_stat instead of g_lstat, to follow symlinks. If you're trashing a file from within a symlink to another device, you want to trash on the other device and not the device the symlink is on.
Thanks, Iain. I tested the 2.50.2-2 Debian package, which includes your new patch -- it works fine for me.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/1027.