[Kimchi-devel] [PATCH] issue#382: Fix wrong default behavior of add guest cdrom

[Royce Lv]

On 2014年06月25日 22:12, Aline Manera wrote:
> On 06/24/2014 11:00 AM, Sheldon wrote:
>> Just a minor inline comment below
>>
>>
>> On 06/24/2014 04:52 PM, lvroyce at linux.vnet.ibm.com wrote:
>>> From: Royce Lv <lvroyce at linux.vnet.ibm.com>
>>>
>>> Add function for cdrom and disk form validation,
>>> so that cdrom will not be created when no path given.
>
> Question: does the backend already do that? Or we still need to add 
> some logic to it too?
Backend logic validated it. This is due to UI fills the first volume of 
default pool when path is blank, so this default behavior is wrong. I 
will update commit mesg to make it clear.
>
>>>
>>> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
>>> ---
>>>   ui/js/src/kimchi.guest_storage_add.main.js | 28 
>>> ++++++++++++++++++++++++++++
>>>   ui/pages/i18n.json.tmpl                    |  7 +++++--
>>>   2 files changed, 33 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/ui/js/src/kimchi.guest_storage_add.main.js 
>>> b/ui/js/src/kimchi.guest_storage_add.main.js
>>> index 6af00cc..20e81ca 100644
>>> --- a/ui/js/src/kimchi.guest_storage_add.main.js
>>> +++ b/ui/js/src/kimchi.guest_storage_add.main.js
>>> @@ -127,6 +127,25 @@ kimchi.guest_storage_add_main = function() {
>>>           });
>>>       });
>>>
>>> +    var validateCDROM = function(settings) {
>>> +        if (settings['path'] && settings['path'] != '')
>> other path, such as pool dir path are absolute path.
>> so should we keep consistency?
ACK
>>> +            return true;
>>> +        else {
>>> +            kimchi.message.error.code('KCHVMSTOR0001E');
>>> +            return false;
>>> +        }
>>> +    }
>>> +
>>> +    var validateDisk = function(settings) {
>>> +        if (settings['pool'] && settings['vol'])
>>> +            return true;
>>> +        else {
>>> +            kimchi.message.error.code('KCHVMSTOR0002E');
>>> +            return false;
>>> +        }
>>> +    }
>>> +
>>> +    validator = {cdrom: validateCDROM, disk: validateDisk};
>>>       var submitForm = function(event) {
>>>           if (submitButton.prop('disabled')) {
>>>               return false;
>>> @@ -147,6 +166,15 @@ kimchi.guest_storage_add_main = function() {
>>>                   settings[$(c).attr('name')] = $(c).val();
>>>               }
>>>           });
>>> +        // Validate form for cdrom and disk
>>> +        validateSpecifiedForm = validator[settings['type']];
>>> +        if (!validateSpecifiedForm(settings)) {
>>> +            $(submitButton).prop('disabled', false);
>>> +            $.each([submitButton, nameTextbox, pathTextbox, 
>>> poolTextbox, volTextbox], function(i, c) {
>>> +                $(c).prop('disabled', false);
>>> +            });
>>> +            return false;
>>> +        }
>>> $(submitButton).addClass('loading').text(i18n['KCHVMCD6003M']);
>>>
>>>           kimchi.addVMStorage(settings, function(result) {
>>> diff --git a/ui/pages/i18n.json.tmpl b/ui/pages/i18n.json.tmpl
>>> index ce23bc4..477733f 100644
>>> --- a/ui/pages/i18n.json.tmpl
>>> +++ b/ui/pages/i18n.json.tmpl
>>> @@ -162,5 +162,8 @@
>>>       "KCHPOOL6009E": "$_("This is not a valid Server Name or IP. 
>>> please, modify it.")",
>>>       "KCHPOOL6010M": "$_("Looking for available partitions ...")",
>>>       "KCHPOOL6011M": "$_("No available partitions found.")",
>>> -    "KCHPOOL6012M": "$_("This storage pool is not persistent. 
>>> Instead of deactivate, this action will permanently delete it. Would 
>>> you like to continue?")"
>>> -}
>>> \ No newline at end of file
>>> +    "KCHPOOL6012M": "$_("This storage pool is not persistent. 
>>> Instead of deactivate, this action will permanently delete it. Would 
>>> you like to continue?")",
>>> +
>>> +    "KCHVMSTOR0001E": "$_("CDROM path cannot be blank.")",
>>> +    "KCHVMSTOR0002E": "$_("Disk pool or volume cannot be blank.")"
>>> +}
>>
>>
>