<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <div class="moz-cite-prefix">Hi Neil,</div>
    <div class="moz-cite-prefix"><br>
    </div>
    <div class="moz-cite-prefix"><br>
    </div>
    <div class="moz-cite-prefix">On 31/08/2020 06:03, NeilBrown wrote:
    </div>
    <blockquote type="cite"
      cite="mid:87pn77qx60.fsf@notabene.neil.brown.name">
      <pre class="moz-quote-pre" wrap="">I have a question about llapi_hsm_action_progress().  The documentation
says that every interval sent "must" be unique, and must not overlap
(which not exactly the same as 'unique').  The code (on server side)
only partially enforces this.  It causes any request for an empty
interval (start>end) to fail, but otherwise accepts any interval.  If it
gets two identical intervals (not just overlapping, but identical), it
ignores the second.  This seems weird.

It would make some sense to just accept any interval - all it does is
sum the lengths, and use this to report status, so no corruption would
result.  It would also make sense to return an error if an interval
overlaps any previous interval, as this violates the spec.  It might
make sense to accept any interval, but only count the overlapped length
once.  But the current behaviour of only ignoring exact duplicates is
weird.  I tried removing that check, but there is a test (hsm_test 108)
which checks for repeating identical intervals.</pre>
    </blockquote>
    <p>test108 handles a "growing" extent (offset=0, length=1, 2, 3,
      ...).<br>
      test112 handles acknowledging non-overlapping extents twice each.</p>
    <p>I wrote a test to check what happens if you acknowledge
      overlapping extents:</p>
    <ul>
      <li>(offset=0, length=256)</li>
      <li>(offset=128, length=256)</li>
    </ul>
    <p>And surely enough mdt_cdt_get_work_done() returns "512" rather
      than the expected "384" (ie. 128 + 256).</p>
    <p>Even worse, when acknowledging a "shrinking" extent (offset=0,
      length=N, N - 1, N - 2, ...), only the last value is kept in
      store.</p>
    <p>From this, I think that exact duplicates are not really ignored,
      rather, intervals that share the same starting point overwrite one
      another, until only the last one remains.<br>
      Bug or feature? I don't really know.</p>
    <p><br>
    </p>
    <p>
    </p>
    <blockquote type="cite"
      cite="mid:87pn77qx60.fsf@notabene.neil.brown.name">
      <pre class="moz-quote-pre" wrap="">I want to clean up this code as I'm converting all users of the lustre
interval-tree to use the upstream-linux interval tree code.  What should
I do?

Should I remove test 108 because it is only testing one particular
corner case, or should I improve the code to handle all overlaps
consistently?  Would it be OK to fail an overlap (I'd need to change
test 108), it must they be quietly accepted?
</pre>
    </blockquote>
    <p>How does the upstream-linux interval tree compares to Lustre's?<br>
    </p>
    <p>If their behaviours match, there should not be any issue (so far
      as the current behaviour can be considered issue-free).</p>
    <p>Otherwise, I think it would be OK to just assume sending
      overlapping extents is a programming error and the server does not
      need to protect itself against it.<br>
      In terms of security, this isn't very good, but the problem
      already exists, and copytools are supposedly trusted binaries run
      as root.<br>
      You could then remove test108 as it is itself a programming error,
      test112 as well, and maybe others.<br>
    </p>
    <p>Just to be sure, you could open a new issue on Jira, and let
      others rule how much of a bug/feature the whole thing is.</p>
    <p>Cheers,<br>
      Quentin<br>
    </p>
  </body>
</html>