D17816: Support for xattrs on kio copy/move

Cochise César noreply at phabricator.kde.org
Thu Jan 17 12:59:14 GMT 2019


cochise marked an inline comment as done.
cochise added a comment.


  I'm not sure why the tests fail, and the tests failing are unrelated to xattrs. I think there is some problem in queuing the subjobs and ensuring they all have finished.
  After the copy job is finished, the `copyLocalDirectory` test can't find the file inside the copied test directory. Manually testing recursive copy I can't find problems, but the autotest fail. So, I think some parts of the copy job are executed after the copyjob finishes if I add a subjob. 
  As I said, we can use start, instead of exec, to be asynchronous, but using a subjob seems to need some refactor, maybe adding a new state for the xattr subjob.

INLINE COMMENTS

> cfeck wrote in copyjob.cpp:1119
> Here, too.

I tried various ways to call this as a subjob, but all of them leads to breakage of non xattr related tests. Maybe some major changes to the class are needed.

But the call can be asynchronous with little effort. All tests still pass if `start()` is called, instead of `exec()`.

> bruns wrote in copyjob.cpp:2199
> you can (typically) avoid the double (syscall, i.e. expensive) by preallocating the array and only reallocating if you get `errno == ERANGE`. Dito for getxattr.

In theory, xattrs have unlimited size on some filesystems. Ext limits it to a fs block (4 Kb on most end user setups). Allocating full 4 kb is a overkill, as most of xattr ae small.

But as most of files don't have xattr, and the function will return here, we have to pay the cost of the second call only if we will use it. If we preallocate memory, we have to pay a cost for every file.

> bruns wrote in copyjob.cpp:2221
> `... always ...`

On Linux, at least. Each item gets a `\0` terminator and the list itself too. Not tested on other systems.

> bruns wrote in copyjob.cpp:2225
> There should probably be a `if (!xattrkey.startsWith("user.")) continue;` here.

Doing some research and test...

> Currently, the security, system, trusted, and user extended attribute classes are defined as described below.  Additional classes may be added in the future.

If a copy a file with kioclient as user, any attribute outside `user.` is lost and I get a warning for the ones I'm unable to preserve, but if I copy as root, all are preserved.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D17816

To: cochise, dfaure
Cc: cfeck, bruns, phidrho, dhaumann, funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, nicolasfella, kde-frameworks-devel, michaelh
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190117/27c97558/attachment.html>


More information about the Kde-frameworks-devel mailing list