<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"><apahim@redhat.com></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& 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>