[Kimchi-devel] [PATCH V4 4/5] Storagepool SCSI/FC: Modifies UI flow to select a LUN to new VM

Aline Manera alinefm at linux.vnet.ibm.com
Wed Feb 12 12:13:20 UTC 2014


On 02/12/2014 03:01 AM, Rodrigo Trujillo wrote:
> On 02/10/2014 12:20 PM, Aline Manera wrote:
>>
>> When I select the text associated to the radio box it does not select 
>> the option. Need to fix it
> This is working in my tests
>

When creating a new vm from SCSI storage pool? Which browser are you 
using? I am on Firefox 26.

>> More comments below
>>
>> On 02/05/2014 12:18 PM, Rodrigo Trujillo wrote:
>>> This patch implements the UI functions and API calls to show to user
>>> the list of volumes (LUNs) of a SCSI FC storagepools. The user can then
>>> select the LUN when creating a new VM.
>>>
>>> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
>>> ---
>>>   ui/js/src/kimchi.api.js            | 13 +++++++
>>>   ui/js/src/kimchi.guest_add_main.js | 73 
>>> +++++++++++++++++++++++++++++++++++---
>>>   ui/pages/i18n.html.tmpl            |  1 +
>>>   3 files changed, 83 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/ui/js/src/kimchi.api.js b/ui/js/src/kimchi.api.js
>>> index 66fc41e..4597c5d 100644
>>> --- a/ui/js/src/kimchi.api.js
>>> +++ b/ui/js/src/kimchi.api.js
>>> @@ -155,6 +155,19 @@ var kimchi = {
>>>           });
>>>       },
>>>
>>> +    /*
>>> +    *   Retrieve the information of a storage pool by the given name.
>>> +    */
>>> +    retrieveStoragePool : function(storagePoolName, suc, err) {
>>> +        kimchi.requestJSON({
>>> +            url : kimchi.url + "storagepools/" +
>>> +                  encodeURIComponent(storagePoolName),
>>> +            type : 'GET',
>>> +            contentType : 'application/json',
>>> +            dataType : 'json'
>>> +        }).done(suc);
>>> +    },
>>> +
>>>       /**
>>>        * Retrieve the information of a template by the given name.
>>>        */
>>> diff --git a/ui/js/src/kimchi.guest_add_main.js 
>>> b/ui/js/src/kimchi.guest_add_main.js
>>> index 2085562..6b8fc38 100644
>>> --- a/ui/js/src/kimchi.guest_add_main.js
>>> +++ b/ui/js/src/kimchi.guest_add_main.js
>>> @@ -62,9 +62,7 @@ kimchi.guest_add_main = function() {
>>>           }
>>>       });
>>>
>>> -    var addGuest = function(event) {
>>> -        var formData = $('#form-vm-add').serializeObject();
>>> -
>>> +    var addGuest = function(formData) {
>>>           kimchi.createVM(formData, function() {
>>>               kimchi.listVmsAuto();
>>>               kimchi.window.close();
>>> @@ -79,8 +77,75 @@ kimchi.guest_add_main = function() {
>>>           return false;
>>>       };
>>>
>>> +    // This function is used to select a lun for new vm disk if 
>>> template has
>>> +    // a SCSI storagepool associated.
>>> +    function getLun() {
>>> +        var formData = $('#form-vm-add').serializeObject();
>>> +        var templateName = formData.template.substring(11);
>>
>>> + kimchi.retrieveTemplate(templateName,
>>
>>> function(templateInfo) {
>>> +            var poolName = templateInfo.storagepool.substring(14);
>>> +            kimchi.retrieveStoragePool(poolName, function(poolInfo){
>>> +                if (poolInfo.type === "scsi") {
>>> +                    kimchi.listStorageVolumes(poolInfo.name, 
>>> function(lunsList) {
>>> +                        if (lunsList.length == 0) {
>>> +                            kimchi.message.error('There are not 
>>> volumes for this pool');
>>
>> You need to add the message to i18n.html.tmpl and use it here, 
>> otherwise this message will not be translated.
> Done
>>
>>> +                            return false;
>>> +                        }
>>> +                        var popUpList = '<section 
>>> class="form-section">' +
>>> +                                           '<h2>1. Storage Pool: ' 
>>> +  poolInfo.name + '</h2>' +
>>> +                                           '<div class="lun_radios">';
>>
>> We need to improve the message to the user.
>>
>> "You select a Template associated to a SCSI Storage Pool so you need 
>> to select which LUN you wan to use as primary disk to guest."
>>
>> Or something like it
>
> Done
>>
>>> + $.each(lunsList, function(index, value) {
>>> +                            popUpList += '<div class="field">' +
>>> +                                             '<input type="radio" 
>>> id="lun-' + value.name + '" name="lun" value="' + value.name + '">' +
>>> +                                             '<label for="lun-' + 
>>> value.name + '">' + value.name + '</label></div>';
>>> +                        });
>>> +                        popUpList += '</div></section>';
>>> +                        console.log(popUpList)
>>> +                        var popup = $(popUpList);
>>> +                        popup.dialog({
>>> +                            autoOpen : true,
>>> +                            modal : true,
>>> +                            width : 400,
>>> +                            draggable : false,
>>> +                            resizable : false,
>>> +                            closeText: "X",
>>> +                            dialogClass : "network-config",
>>> +                            title: "Please, select a LUN",
>>> +                            close: function( event, ui ) { 
>>> $('input[name=lun]').attr('checked',false); },
>>> +                            buttons : [ {
>>> +                                text : i18n.action_select_lun,
>>> +                                class: "ui-button-primary",
>>> +                                click : function() {
>>> +                                    var lunName = 
>>> $('input:radio[name=lun]:checked').val();
>>> +                                    if (lunName === undefined) {
>>> + kimchi.message.error('You must select a LUN');
>>> +                                    } else {
>>> +                                        formData.volumes = new 
>>> Array(lunName);
>>> +                                        addGuest(formData);
>>> +                                    }
>>> +                                    $( this ).dialog( "close" );
>>> +                                }
>>> +                            }]
>>> +                        });
>>
>> What about split this big function into smaller ones?
>> It is hard to read it that way
>>
>> Example:
>>
>> 1) function to get template
>> 2) function to get storage associate to template
>> 3) function to get type of storage pool
>>
>> get template;
>>     get storage pool
>>         get storage pool type
>>
>> if pool type != scsi:
>>     return
>>
>> 4) function to display new dialog
> I am not 100% sure how to do this. JS functions are still complicated 
> for me, so it will take a while to code/test.
> As the code is working, I would rather ask some UI specialist to do 
> this refactoring in next days.
> Or I can try to do in future, when I have less things in my backlog.
>

Are you saying to I merge that in this way (which no one can read) and 
then other person redo the work?

>
>>
>>> +                    },function() {
>>> +                        // listStorageVolumes error handler
>>> + kimchi.message.error(i18n['msg.kimchi.list.volume.fail']);
>>> +                    });
>>> +                }
>>> +                else { addGuest(formData); }
>>> +            }, function() {
>>> +                // retrieveStoragePool error handler
>>> + kimchi.message.error(i18n['msg.kimchi.retrieve.pool.fail']);
>>> +            });
>>> +        }, function() {
>>> +            // retrieveStoragePool error handler
>>> + kimchi.message.error(i18n['msg.fail.template.retrieve']);
>>> +        });
>>> +        return false;
>>> +    }
>>> +
>>>       $('#form-vm-add').on('submit', addGuest);
>>> -    $('#vm-doAdd').on('click', addGuest);
>>> +    $('#vm-doAdd').on('click', getLun);
>>>
>>>       showTemplates();
>>>   };
>>> diff --git a/ui/pages/i18n.html.tmpl b/ui/pages/i18n.html.tmpl
>>> index a4c3ccb..f635fdf 100644
>>> --- a/ui/pages/i18n.html.tmpl
>>> +++ b/ui/pages/i18n.html.tmpl
>>> @@ -123,6 +123,7 @@ var i18n = {
>>>       'network_dialog_ok': "$_("OK")",
>>>       'network_dialog_cancel': "$_("Cancel")",
>>>       'action_create': "$_("Create")",
>>> +    'action_select_lun': "$_("Select")",
>>>       'msg_warning': "$_("Warning")",
>>>       'msg.logicalpool.confirm.delete': "$_("It will format your 
>>> disk and you will loose any data in"
>>>                                         " there, are you sure to 
>>> continue? ")",
>>
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>




More information about the Kimchi-devel mailing list