On 06/03/2012 05:04 AM, Itamar Heim wrote:
On 06/01/2012 08:16 PM, Amador Pahim wrote:
On 06/01/2012 12:39 PM, Itamar Heim wrote:
On 05/21/2012 09:37 PM, Gilad Chaplik wrote:
Hi Pahim,
Thanks for the input!
Comments inline.
Thanks,
Gilad.
----- Original Message -----
From: "Amador pahim"<apa...@redhat.com>
To: engine-devel@ovirt.org
Sent: Monday, May 21, 2012 5:52:34 PM
Subject: [Engine-devel] LOCALFS path validation
Hello,
I'm starting to know the engine code. I chose a little unstandardized
behaviour to follow through the devel process. I have a patch and
I'd like to know if you fell relevant to correct this issue:
- Description: Adding a LOCAL storage [1], webadmin does not validate
path against regex, sendind the invalid path (with final slash) to
vdsm [2] [3]. But, adding a NFS storage, the path is validated
before contacting vdsm [4] avoiding extra vdsm processing and
quickly/clearly informing user about what's wrong.
- Expected result: Same behaviour to NFS and LOCALFS storage path
validation. Validate LOCALFS path in webadmin before send it to vdsm
[5].
you may and should send a patch :)
- Newbie doubt: Wouldn't be better to validate the both local and nfs
path on the backend, achieving all user interfaces/APIs?
Because we have a rich client app (gwt), we can perform the
validation also in the client side very easily,
we do that to avoid unnecessary calls to the backend side, and to
have a better& responsive ui
(client side validation is performed instantly - without the need to
wait).
Anyway, every validation performed in the client side needs to be
performed also in backend side (for api, and other reasons
(security?)).
the client side can validate the format of the path, not if it really
exists.
do you mean local storage via the wizard, or just adding another
storage domain?
the 'configure local storage' wizard requires the host to be in
maintenance, which may or may not be a problem to implement this type
of validation.
I mean just adding a LOCALFS storage domain with an invalid path, like
this:
https://picasaweb.google.com/lh/photo/FWNiou2Y12GZO3AjfCH6K7QAv8cs6edaj3fEcMleB60
Currently we have this:
https://picasaweb.google.com/lh/photo/Pof6Z8ohgQAkRTDpEJKG-LQAv8cs6edaj3fEcMleB60
I sent a patch...
http://gerrit.ovirt.org/4857
... in order to do this:
https://picasaweb.google.com/lh/photo/PgzYrZHkkvm-WtFk_UFZLrQAv8cs6edaj3fEcMleB60
Just like "New NFS Domain" currently does:
https://picasaweb.google.com/lh/photo/Fd3zWegWE0T5C2tDo_tPZrQAv8cs6edaj3fEcMleB60
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
There is no validation in backend. I mean, the error "Error while
executing action AddLocalStorageDomain: Remote path is illegal" is
coming from VDSM:
https://gist.github.com/2762656
And is not caused by regex. VDSM is just comparing path /test/ with
os.path.abspath(), /test:
https://gist.github.com/2867874
_______________________________________________
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
