Re: [Engine-devel] LOCALFS path validation

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(a)redhat.com> >>> To: engine-devel(a)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/FWNiou2Y12GZO3AjfCH6K7QAv8cs6edaj3f... Currently we have this: https://picasaweb.google.com/lh/photo/Pof6Z8ohgQAkRTDpEJKG-LQAv8cs6edaj3f... I sent a patch... http://gerrit.ovirt.org/4857 ... in order to do this: https://picasaweb.google.com/lh/photo/PgzYrZHkkvm-WtFk_UFZLrQAv8cs6edaj3f... Just like "New NFS Domain" currently does: https://picasaweb.google.com/lh/photo/Fd3zWegWE0T5C2tDo_tPZrQAv8cs6edaj3f...