Add truncation support to FileJob
ClosedPublic

Authored by feverfew on Sat, Dec 21, 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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
feverfew created this revision.Sat, Dec 21, 5:39 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptSat, Dec 21, 5:39 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
feverfew requested review of this revision.Sat, Dec 21, 5:39 PM
feverfew updated this revision to Diff 71990.Sun, Dec 22, 1:15 PM

Add unit test for truncation

apol added a subscriber: apol.Mon, Dec 23, 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)Mon, Dec 23, 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.Wed, Dec 25, 10:46 PM

Add feature to determine if protocol supports truncation

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

Missing @since 5.66

181

@since 5.66

src/core/kprotocolmanager.h
477

@since 5.66

src/core/slavebase.h
209

@since 5.66

493

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.Tue, Dec 31, 8:56 AM
feverfew updated this revision to Diff 72538.Wed, Jan 1, 2:59 PM

Fix introduction of BIC method

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

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

dfaure added a comment.Wed, Jan 1, 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.