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
