<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    On 06/03/2012 05:04 AM, Itamar Heim wrote:
    <blockquote cite="mid:4FCB1A94.8080201@redhat.com" type="cite">On
      06/01/2012 08:16 PM, Amador Pahim wrote:
      <br>
      <blockquote type="cite">On 06/01/2012 12:39 PM, Itamar Heim wrote:
        <br>
        <blockquote type="cite">On 05/21/2012 09:37 PM, Gilad Chaplik
          wrote:
          <br>
          <blockquote type="cite">Hi Pahim,
            <br>
            <br>
            Thanks for the input!
            <br>
            Comments inline.
            <br>
            <br>
            Thanks,
            <br>
            Gilad.
            <br>
            <br>
            ----- Original Message -----
            <br>
            <blockquote type="cite">From: "Amador
              pahim"<a class="moz-txt-link-rfc2396E" href="mailto:apahim@redhat.com">&lt;apahim@redhat.com&gt;</a>
              <br>
              To: <a class="moz-txt-link-abbreviated" href="mailto:engine-devel@ovirt.org">engine-devel@ovirt.org</a>
              <br>
              Sent: Monday, May 21, 2012 5:52:34 PM
              <br>
              Subject: [Engine-devel] LOCALFS path validation
              <br>
              <br>
              <br>
              Hello,
              <br>
              <br>
              I'm starting to know the engine code. I chose a little
              unstandardized
              <br>
              behaviour to follow through the devel process. I have a
              patch and
              <br>
              I'd like to know if you fell relevant to correct this
              issue:
              <br>
              <br>
              - Description: Adding a LOCAL storage [1], webadmin does
              not validate
              <br>
              path against regex, sendind the invalid path (with final
              slash) to
              <br>
              vdsm [2] [3]. But, adding a NFS storage, the path is
              validated
              <br>
              before contacting vdsm [4] avoiding extra vdsm processing
              and
              <br>
              quickly/clearly informing user about what's wrong.
              <br>
              <br>
              - Expected result: Same behaviour to NFS and LOCALFS
              storage path
              <br>
              validation. Validate LOCALFS path in webadmin before send
              it to vdsm
              <br>
              [5].
              <br>
            </blockquote>
            <br>
            you may and should send a patch :)
            <br>
            <br>
            <blockquote type="cite">
              <br>
              - Newbie doubt: Wouldn't be better to validate the both
              local and nfs
              <br>
              path on the backend, achieving all user interfaces/APIs?
              <br>
            </blockquote>
            <br>
            Because we have a rich client app (gwt), we can perform the
            <br>
            validation also in the client side very easily,
            <br>
            we do that to avoid unnecessary calls to the backend side,
            and to
            <br>
            have a better&amp; responsive ui
            <br>
            (client side validation is performed instantly - without the
            need to
            <br>
            wait).
            <br>
            <br>
            Anyway, every validation performed in the client side needs
            to be
            <br>
            performed also in backend side (for api, and other reasons
            (security?)).
            <br>
          </blockquote>
          <br>
          the client side can validate the format of the path, not if it
          really
          <br>
          exists.
          <br>
          do you mean local storage via the wizard, or just adding
          another
          <br>
          storage domain?
          <br>
          the 'configure local storage' wizard requires the host to be
          in
          <br>
          maintenance, which may or may not be a problem to implement
          this type
          <br>
          of validation.
          <br>
        </blockquote>
        <br>
        I mean just adding a LOCALFS storage domain with an invalid
        path, like
        <br>
        this:
        <br>
<a class="moz-txt-link-freetext" href="https://picasaweb.google.com/lh/photo/FWNiou2Y12GZO3AjfCH6K7QAv8cs6edaj3fEcMleB60">https://picasaweb.google.com/lh/photo/FWNiou2Y12GZO3AjfCH6K7QAv8cs6edaj3fEcMleB60</a>
        <br>
        <br>
        Currently we have this:
        <br>
<a class="moz-txt-link-freetext" href="https://picasaweb.google.com/lh/photo/Pof6Z8ohgQAkRTDpEJKG-LQAv8cs6edaj3fEcMleB60">https://picasaweb.google.com/lh/photo/Pof6Z8ohgQAkRTDpEJKG-LQAv8cs6edaj3fEcMleB60</a>
        <br>
        <br>
        I sent a patch...
        <br>
        <a class="moz-txt-link-freetext" href="http://gerrit.ovirt.org/4857">http://gerrit.ovirt.org/4857</a>
        <br>
        ... in order to do this:
        <br>
<a class="moz-txt-link-freetext" href="https://picasaweb.google.com/lh/photo/PgzYrZHkkvm-WtFk_UFZLrQAv8cs6edaj3fEcMleB60">https://picasaweb.google.com/lh/photo/PgzYrZHkkvm-WtFk_UFZLrQAv8cs6edaj3fEcMleB60</a>
        <br>
        <br>
        Just like "New NFS Domain" currently does:
        <br>
<a class="moz-txt-link-freetext" href="https://picasaweb.google.com/lh/photo/Fd3zWegWE0T5C2tDo_tPZrQAv8cs6edaj3fEcMleB60">https://picasaweb.google.com/lh/photo/Fd3zWegWE0T5C2tDo_tPZrQAv8cs6edaj3fEcMleB60</a>
        <br>
        <br>
      </blockquote>
      <br>
      ok - sounds basic enough of a change. I'd just note in the patch
      you are re-using same REGEX used for the previous validation
      happening in the backend (and actually try to re-use it, to not
      maintain a change in too many places0
      <br>
    </blockquote>
    There is no validation in backend. I mean, the error "Error while
    executing action AddLocalStorageDomain: Remote path is illegal" is
    coming from VDSM:<br>
    <meta http-equiv="content-type" content="text/html;
      charset=ISO-8859-1">
    <a href="https://gist.github.com/2762656">https://gist.github.com/2762656</a><br>
    And is not caused by regex. VDSM is just comparing path /test/ with
    os.path.abspath(), /test:<br>
    <meta http-equiv="content-type" content="text/html;
      charset=ISO-8859-1">
    <a href="https://gist.github.com/2867874">https://gist.github.com/2867874</a><br>
  </body>
</html>