Fix: SMB/CIFS/CEPH mounted drive caused large file upload errors #17218
Conversation
On some installations using ceph/smb/cifs shares the protocol does not support renaming on the fly. This causes the issue as linked. This fix added a check for this and disables part-file if needed.
|
Would you mind to share some information about a setup where this change fixes the issue? Please sign off your commits. https://github.com/nextcloud/server/pull/17218/checks?check_run_id=228831643 for some details. |
|
#8942 (comment) there are also some positive reports about this code change. The suggested change makes sense to me in context of upload only (and therefore paths which are writeable but not readable) shares but I don't see how this fixes issues with large file uploads in general. |
|
Thx for your reply, forgot to change the testfile according to my little change. I will add this file to my PR so i think the tests will be successfull then. I will also provide a more detailed description on how i think this fixes the bug. |
| @@ -149,7 +149,7 @@ public function put($data) { | |||
| // mark file as partial while uploading (ignored by the scanner) | |||
| $partFilePath = $this->getPartFileBasePath($this->path) . '.ocTransferId' . rand() . '.part'; | |||
|
|
|||
| if (!$view->isCreatable($partFilePath) && $view->isUpdatable($this->path)) { | |||
| if ((!$view->isCreatable($partFilePath) && $view->isUpdatable($this->path)) || (!$view->isReadable($partFilePath))) { | |||
There was a problem hiding this comment.
@icewind1991 Souldn't all those checks run on the parent directory as the part file and path are the files that do ot exist yet, so isCreatable/isReadable will always return false.
There was a problem hiding this comment.
You're somewhat right I think, the isCreatable and isReadable check should be done on the parent, isUpdatable should be on the file
@stereu Any update on this? |
Was very busy the last time, i will have a look at it again in the next few days. Stay tuned. |
|
FYI: I stepped today into the same issue, this PR fixed the issue with 18.0.3 for me. Hope this get's atleast merged with 19.0? |
juliusknorr
added
2. developing
|
Not yet ready to be merged as it just disables part files in general as stated in #17218 (comment) |
|
As per #17218 (comment) this needs to be done slightly differently and the last activity of the author was more than a year ago. I'm closing this thus. Please open a new PR when you continue this work. Thanks a lot ✌️ |
|
Why do you close this? Issue is still present in 20.0.4. The fix from stereu helped us. If it is not state of the art, than please do it a better way, but do it. |
|
Did you actually read my comment? Read it again, it will tell you why. Also it links to a comment of the maintainer of this subsystem that has some concerns. |
Fix #8942
On some installations using ceph/smb/cifs shares the protocol does not support renaming on the fly. This causes the issue as linked. This fix added a check for this and disables part-file if needed.