[Engine-devel] LOCALFS path validation

[Amador Pahim]

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"<apahim at redhat.com>
>>> To: engine-devel at 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