Importing from path to library with limited access rights (e.g. CIFS share) fails
Submitted by an unknown user
Link to original bug (#719135)
Description
---- Reported by shotwell-maint@gnome.bugs 2013-09-26 16:26:00 -0700 ----
Original Redmine bug id: 7550
Original URL: http://redmine.yorba.org/issues/7550
Searchable id: yorba-bug-7550
Original author: Frenk X
Original description:
In the described case the image file itself is copied to the desired target
path, but the photo is not added to the database. Also the "Shotwell Import
Log.txt" shows a misleading error description stating something like "Could
not copy <SRC PATH>
to <SRC PATH>
."
In the particular case the library lies on a CIFS share where setting the owner is not allowed for the user of shotwell (it is correctly mapped by the mount itself).
IMO there are two ways to handle this case:
- Either remove the copied file and give a better error description or
- continue with the import since the file was copied correctly (and maybe post a warning).
I am not familiar with vala, so please improve the attached quick workaround for the second option.
---- Additional Comments From shotwell-maint@gnome.bugs 2013-10-31 02:01:00 -0700 ----
History
Comment 1
Updated by Jim Nelson about 1 month ago
- Category changed from import to network-storage
- Status changed from Open to Need Information
Is it possible you're seeing a similar problem as Thomas Novin (in this this thread: http://lists.yorba.org/pipermail/shotwell/2013-September/005014.html)?
One of the problems he encountered in his import log was "Operation not permitted" because the files on the attached storage had a group name that his account was not a member of.
If you could save the import log and attach it to this ticket, that might help diagnose this problem.
Comment 2
Updated by Thomas Novin about 1 month ago
This is my log:
http://paste.ubuntu.com/6177355/
As previously mentioned, the error is wrong. It has the src path two times.
After the failed import, all files are actually copied but Shotwell does not add them to the DB and says it just failed.
They are added later if you have watch enabled on the datastore (destination).
After my log, you can see that I am a member of the group that is in use. I cannot change this though, as this datastore is shared in my LAN.
The datastore is mounted via NFS.
Comment 3
Updated by Frenk X about 1 month ago
- File Shotwell Import Log.txt added
Yes, except that in my case there is a "Permission denied" instead of "Operation not permitted" when trying to change the owner of the file, it looks very similar. The difference in error descriptions might result from using NFS in Thomas Novins case and CIFS in my case.
Besides that it is not a solution to the problem itself, watching the library directory is not an option for me.
In ~/.cache/shotwell/shotwell.log
I found the line L 9379 2013-10-01 12:55:34 [DBG] Page.vala:133: Page Importing... Destroyed
. Starting to trace
the problem from there, I came across the try-finally-block in
src/LibraryFiles.vala:81. In case, copying the file throws an error, there is
only a finally block to unblacklist the destination file. This is alright if
the copy itself fails.
In the case of Thomas Novin and myself the file itself is copied correctly to
the destination, but setting the owner (as specified by ALL_METADATA
) fails.
Therefore File.copy
throws an error (obviously with the error code 14) which
is not caught, but only the finally block is executed. So an error with code
14 seems to mean, that the destination file exists, but attributes could not
be set. Tracing it back, this explains the output in shotwell.log
as well as
the existing destination file and the failed import into the database.
My initial workaround submitted in yorba.patch tries to catch the the error
from File.copy
. In case there is another error than with error code 14 and
the blacklist
flag is set, the same code is executed as before (but maybe
one need to throw another error here?). But if the error code is the (well)
understood 14, then it counts as a successful copy and the file can be added
to the database. Using the patch, I see no more error in the specified case
but instead have the photo imported correctly.
Another solution might be, to drop the ALL_METADATA
in favour of a better
description what to copy.
Comment 4
Updated by Thomas Novin about 1 month ago
- File 12.png added
Import to NFS, from NFS :/
All files copied OK though.
Comment 5
Updated by Jim Nelson about 1 month ago
- Status changed from Need Information to Review
Comment 6
Updated by Jim Nelson about 1 month ago
-
Target version deleted (
<strike>
_0.15.0_</strike>
)
Comment 7
Updated by Frenk X about 1 month ago
Can I do anything to help this patch get into the next release? I had to patch the 0.15 again to work in my environment...
Comment 8
Updated by Jim Nelson about 1 month ago
- Target version set to 0.16.0
This patch came in too late to make it into 0.15.
My concern looking at it is "err.code == 14", which doesn't help the next programmer who looks at this code. The Vala way of doing this would be something like
if ((err is IOError.PERMISSION_DENIED) && is_blacklist)
I also would like to test this in other permission-related situations (such as non-CIFS directories where the read bit has been turned off, etc.) to make sure it doesn't introduce breakage elsewhere.
Comment 9
Updated by Frenk X 26 days ago
- File issue7550_alternative.patch added
You are right. And there are also some logical issues with just handling this special error. There might be another reason why the PERMISSION_DENIED-Error is thrown which we do not know right now.
Therefore another idea to tackle the problem:
If the File.copy throws an error, the content of source and destination are compared. This is a more expensive operation, but it might still be better than having another user interaction and/or debugging the picture library.
In case the content is the same, I assume, that the copying itself worked. I suggest to print a warning instead of having a wrong error description as it is now. Only if the file content differs LibraryMonitor.unblacklist_file is called.
Find my suggestion attached.
Comment 10
Updated by Frenk X 22 days ago
- Status changed from Review to Fixed
Applied in changeset fd35f69e.
Comment 11
Updated by Jim Nelson 22 days ago
- Assignee set to Frenk X
Frenck, I've committed your patch with some minor changes. Since you changed
the finally
block to a catch
, the unblacklist path for the error-free case
still needs to execute. Otherwise, your change remains.
Can you pull from master and see if this still works for you?
Comment 12
Updated by Thomas Novin 22 days ago
The patch works fine, just imported as I normally do and no errors in the GUI this time, just reports that import went OK.
In the log, there is this MSG about it:
L 31914 2013-10-30 20:23:46 [MSG] main.vala:381: Shotwell Photo Manager 0.15.0+trunk (fd35f69e)
...
L 31914 2013-10-30 20:25:08 [MSG] LibraryFiles.vala:86: There was a problem copying /home/thnov/Dropbox/Camera Uploads/2013-10-30 16.15.00.jpg: Error setting owner: Operation not permitted
L 31914 2013-10-30 20:25:08 [DBG] BatchImport.vala:1967: Importing /media/mistik_shared/Photos/2013/10-30/2013-10-30 16.15.00.jpg
Comment 13
Updated by Frenk X 21 days ago
I just pulled fd35f69e from master. It worked fine including the expected addition to the log file from LibraryFiles.vala:86...
Thank you for considering this patch. It will help to finally import photos again on several environments I manage.
--- Bug imported by chaz@yorba.org 2013-11-25 21:59 UTC ---
This bug was previously known as bug 7550 at http://redmine.yorba.org/show_bug.cgi?id=7550 Imported an attachment (id=262710) Imported an attachment (id=262711) Imported an attachment (id=262712) Imported an attachment (id=262713)
Unknown milestone "unknown in product shotwell. Setting to default milestone for this product, "---". Setting qa contact to the default for this product. This bug either had no qa contact or an invalid one.
Version: 0.15.1
Resolution: RESOLVED FIXED