Add truncation support to FileJob
ClosedPublic

Authored by feverfew on Dec 21 2019, 5:39 PM.

Details

Summary

This patch adds support for truncation for FileJob via creation of a
FileJob->truncate(KIO::filesize_t length) method.
It also implements this function in the file slave.
The patch also adds a "truncating" field to the protocol file, thus making it
possible to determine if a protocol supports this new feature.

Currently within the FileJob class there is no native way to truncate an
open file. To achieve truncation, one has to do a FileJob->read() until the
truncation point and then do a KIO::put() to truncate accordingly. This brings
two undesirable issues:

  1. Incredibly wasteful and non-performant.
  2. One has to do IO outside of the FileJob interface. One has to make sure that

during the KIO::put(), there are no open file descriptors (created via
KIO::open()), as they would be invalidated by the KIO::put().

Adding native supports allows cleaner, more performant code and means that
the FileJob class is functionally complete in providing random-access I/O.

Test Plan

Tested in conjunction with KIOFuse, does truncation
correctly.

Diff Detail

Repository
R241 KIO
Branch
TruncateSupport
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 20146
Build 20164: arc lint + arc unit
feverfew created this revision.Dec 21 2019, 5:39 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptDec 21 2019, 5:39 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
feverfew requested review of this revision.Dec 21 2019, 5:39 PM
feverfew updated this revision to Diff 71990.Dec 22 2019, 1:15 PM

Add unit test for truncation

apol added a subscriber: apol.Dec 23 2019, 5:40 PM

It would be interesting to explain why it's important or how it's to be used in the commit message.

feverfew edited the summary of this revision. (Show Details)Dec 23 2019, 7:03 PM
feverfew edited the summary of this revision. (Show Details)
In D26148#582365, @apol wrote:

It would be interesting to explain why it's important or how it's to be used in the commit message.

Done

feverfew updated this revision to Diff 72180.Dec 25 2019, 10:46 PM

Add feature to determine if protocol supports truncation

feverfew edited the summary of this revision. (Show Details)Dec 25 2019, 10:47 PM
dfaure requested changes to this revision.Dec 31 2019, 8:56 AM
dfaure added inline comments.
src/core/filejob.h
109

Missing @since 5.66

180

@since 5.66

src/core/kprotocolmanager.h
477 ↗(On Diff #72180)

@since 5.66

src/core/slavebase.h
209

@since 5.66

481

WARNING RED ALERT.... adding a virtual method is a binary incompatible change. This cannot be done in the 5.x timeframe.

Use a different solution, like special() or virtual_hook(). I think virtual_hook is better if the idea is to turn it into a virtual method in KF6.

This revision now requires changes to proceed.Dec 31 2019, 8:56 AM
feverfew updated this revision to Diff 72538.Jan 1 2020, 2:59 PM

Fix introduction of BIC method

dfaure accepted this revision.Jan 1 2020, 6:02 PM
This revision is now accepted and ready to land.Jan 1 2020, 6:02 PM

Thanks @dfaure! Would you be able to also review D26191?

dfaure added a comment.Jan 1 2020, 6:35 PM

Thanks @dfaure! Would you be able to also review D26191?

Yes, once it has been ported to virtual_hook as well.

Thanks @dfaure! Would you be able to also review D26191?

Yes, once it has been ported to virtual_hook as well.

Will do that soon, but D23384 is more important to get reviewed as tagging is in a couple of days. D26191 is going to be in 20.04 anyway so not the biggest of rushes in that regard.

This revision was automatically updated to reflect the committed changes.