<table><tr><td style="">cochise marked an inline comment as done.<br />cochise added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D17816">View Revision</a></tr></table><br /><div><div><p>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.<br />
After the copy job is finished, the <tt style="background: #ebebeb; font-size: 13px;">copyLocalDirectory</tt> 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. <br />
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.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D17816#inline-99767">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cfeck</span> wrote in <span style="color: #4b4d51; font-weight: bold;">copyjob.cpp:1119</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Here, too.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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.</p>

<p style="padding: 0; margin: 8px;">But the call can be asynchronous with little effort. All tests still pass if <tt style="background: #ebebeb; font-size: 13px;">start()</tt> is called, instead of <tt style="background: #ebebeb; font-size: 13px;">exec()</tt>.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D17816#inline-99107">View Inline</a><span style="color: #4b4d51; font-weight: bold;">bruns</span> wrote in <span style="color: #4b4d51; font-weight: bold;">copyjob.cpp:2199</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">you can (typically) avoid the double (syscall, i.e. expensive) by preallocating the array and only reallocating if you get <tt style="background: #ebebeb; font-size: 13px;">errno == ERANGE</tt>. Dito for getxattr.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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.</p>

<p style="padding: 0; margin: 8px;">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.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D17816#inline-99103">View Inline</a><span style="color: #4b4d51; font-weight: bold;">bruns</span> wrote in <span style="color: #4b4d51; font-weight: bold;">copyjob.cpp:2221</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;"><tt style="background: #ebebeb; font-size: 13px;">... always ...</tt></p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">On Linux, at least. Each item gets a <tt style="background: #ebebeb; font-size: 13px;">\0</tt> terminator and the list itself too. Not tested on other systems.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D17816#inline-99108">View Inline</a><span style="color: #4b4d51; font-weight: bold;">bruns</span> wrote in <span style="color: #4b4d51; font-weight: bold;">copyjob.cpp:2225</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">There should probably be a <tt style="background: #ebebeb; font-size: 13px;">if (!xattrkey.startsWith("user.")) continue;</tt> here.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Doing some research and test...</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p style="padding: 0; margin: 8px;">Currently, the security, system, trusted, and user extended attribute classes are defined as described below.  Additional classes may be added in the future.</p></blockquote>

<p style="padding: 0; margin: 8px;">If a copy a file with kioclient as user, any attribute outside <tt style="background: #ebebeb; font-size: 13px;">user.</tt> is lost and I get a warning for the ones I'm unable to preserve, but if I copy as root, all are preserved.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R241 KIO</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D17816">https://phabricator.kde.org/D17816</a></div></div><br /><div><strong>To: </strong>cochise, dfaure<br /><strong>Cc: </strong>cfeck, bruns, phidrho, dhaumann, funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, nicolasfella, kde-frameworks-devel, michaelh<br /></div>