[Kimchi-devel] [PATCH V2] UI-Template Edit: Enable user to change disk format

Aline Manera alinefm at linux.vnet.ibm.com
Mon Apr 27 15:13:49 UTC 2015



On 17/04/2015 00:05, Rodrigo Trujillo wrote:
> This patch adds a new field (Disk Format) in Storage tab in Template
> edit window. Users will then be allowed to select any disk format
> supported by Libvirt and Kimchi backend.
> The default disk format is qcow2 (first option), so, for compatibility,
> if the template register in objecstore does not contain the disk format
> information, the new field is going to set 'qcow2' automatically.
> For iscsi storagepools, the new field is going to be disabled and the
> format is going to be set automatically as 'raw', just like the backend
> behaves currently, avoiding errors.
>
> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
> ---
>   ui/css/theme-default/template-edit.css |  8 ++++--
>   ui/js/src/kimchi.template_edit_main.js | 52 ++++++++++++++++++++++++++++++----
>   ui/pages/template-edit.html.tmpl       | 16 +++++++++++
>   3 files changed, 68 insertions(+), 8 deletions(-)
>
> diff --git a/ui/css/theme-default/template-edit.css b/ui/css/theme-default/template-edit.css
> index 7abee7c..ed9f02c 100644
> --- a/ui/css/theme-default/template-edit.css
> +++ b/ui/css/theme-default/template-edit.css
> @@ -95,7 +95,7 @@
>
>   #edit-template-tabs .template-storage-cell{
>       display: inline-block;
> -    width: 230px;
> +    width: 180px;
>   }
>
>   #edit-template-tabs .template-storage-cell label {
> @@ -114,7 +114,11 @@
>   }
>
>   #form-template-storage .template-tab-body .template-storage-name {
> -    width: 220px;
> +    width: 170px;
> +}
> +
> +#form-template-storage .template-tab-body .template-storage-disk-format {
> +    width: 160px;
>   }
>
>   #edit-template-tabs .template-tab-body input[readonly] {
> diff --git a/ui/js/src/kimchi.template_edit_main.js b/ui/js/src/kimchi.template_edit_main.js
> index 85f76cd..6029a95 100644
> --- a/ui/js/src/kimchi.template_edit_main.js
> +++ b/ui/js/src/kimchi.template_edit_main.js
> @@ -67,7 +67,7 @@ kimchi.template_edit_main = function() {
>                   $('.template-tab-body', '#form-template-storage').append(nodeStorage);
>                   var storageOptions = '';
>                   var scsiOptions = '';
> -                $('select', '#form-template-storage').find('option').remove();
> +                $('select:first', '#form-template-storage').find('option').remove();

It is really dangerous to do that (assuming that will always be the 
first select element in the form)
I suggest to add an id to the select elements in the form to distinguish 
them and then use the id to get the element.

>                   $.each(result, function(index, storageEntities) {
>                       if((storageEntities.state === 'active') && (storageEntities.type != 'kimchi-iso')) {
>                           if(storageEntities.type === 'iscsi' || storageEntities.type === 'scsi') {
> @@ -77,7 +77,7 @@ kimchi.template_edit_main = function() {
>                                       var isSlected = tmpPath === thisName ? ' selected' : '';
>                                       scsiOptions += '<option' + isSlected + '>' + tmpPath + '</option>';
>                                   });
> -                                $('select', '#form-template-storage').append(scsiOptions);
> +                                $('select:first', '#form-template-storage').append(scsiOptions);

Same as commented above,

>                               }, function() {});
>                           } else {
>                               var isSlected = storageEntities.name === thisName ? ' selected' : '';
> @@ -85,8 +85,20 @@ kimchi.template_edit_main = function() {
>                           }
>                       }
>                   });
> -                $('select', '#form-template-storage').append(storageOptions);
> -                $('select', '#form-template-storage').change(function() {
> +                $('select:first', '#form-template-storage').append(storageOptions);
> +
> +                // Set disk format
> +                $('select:last option', '#form-template-storage').each(function() {
> +                    if ($(this).text() == storageData.storageDiskFormat) {
> +                        $(this).prop('selected', true);
> +                    }
> +                });
> +
> +                $('select:last', '#form-template-storage').change(function() {
> +                    $('.template-storage-disk-format').val($(this).val());
> +                });
> +
> +                $('select:first', '#form-template-storage').change(function() {
>                       var selectedItem = $(this).parent().parent();
>                       var tempStorageNameFull = $(this).val();
>                       var tempName = tempStorageNameFull.split('/');
> @@ -99,9 +111,25 @@ kimchi.template_edit_main = function() {
>                               kimchi.getStoragePoolVolume(tempStorageName, tempName[tempName.length-1], function(info) {
>                                   volSize = info.capacity / Math.pow(1024, 3);
>                                   $('.template-storage-disk', selectedItem).attr('readonly', true).val(volSize);

> +                                $('select:last option', selectedItem).each(function() {
> +                                    this.selected = (this.text == 'raw');
> +                                });

Why do you need a loop to set the right value on select element?


> +                                $('select:last', selectedItem).prop('disabled', true).change();
>                               });
> +                        } else if (tempType === 'logical') {
> +                            $('.template-storage-disk', selectedItem).attr('readonly', false);

> +                            $('select:last option', selectedItem).each(function() {
> +                                this.selected = (this.text == 'raw');
> +                            });

Same question as above

> +                            $('select:last', selectedItem).prop('disabled', true).change();
>                           } else {
>                               $('.template-storage-disk', selectedItem).attr('readonly', false);


> +                            if ($('select:last', selectedItem).prop('disabled') == true) {

Does this 'if' statement is really needed?
I mean, the last case if for any read-only pool which means the format 
drop box will be always enabled.
Do you see an user case where it should be disabled?

> +                                $('select:last option', selectedItem).each(function() {
> +                                    this.selected = (this.text == 'qcow2');
> +                                });

Same question about the loop to set the right value on select element

> +                                $('select:last', selectedItem).prop('disabled', false).change();
> +                            }
>                           }
>                       });
>                   });
> @@ -120,7 +148,8 @@ kimchi.template_edit_main = function() {
>                               editMode : 'hide',
>                               storageName : defaultPool,
>                               storageType : defaultType,
> -                            storageDisk : diskEntities.size
> +                            storageDisk : diskEntities.size,
> +                            storageDiskFormat : diskEntities.format ? diskEntities.format : 'qcow2'
>                           }
>
>                           if (diskEntities.volume) {
> @@ -131,7 +160,17 @@ kimchi.template_edit_main = function() {
>                                   nodeData.storageDisk = volSize;
>                                   addStorageItem(nodeData);
>                                   $('.template-storage-disk').attr('readonly', true);
> +                                $('select:last option', '#form-template-storage').each(function() {
> +                                    this.selected = (this.text == 'raw');
> +                                });
> +                                $('select:last', '#form-template-storage').prop('disabled', true).change();
> +                            });
> +                        } else if (defaultType === 'logical') {
> +                            addStorageItem(storageNodeData);
> +                            $('select:last option', '#form-template-storage').each(function() {
> +                                this.selected = (this.text == 'raw');
>                               });
> +                            $('select:last', '#form-template-storage').prop('disabled', true).change();
>                           } else {
>                               addStorageItem(storageNodeData);
>                           }
> @@ -271,7 +310,8 @@ kimchi.template_edit_main = function() {
>                       origDisks[0]['volume'] && delete origDisks[0]['volume'];
>                       origDisks[0].size = Number($('.template-storage-disk', tmpItem).val());
>                   }
> -               data[field] = origDisks;
> +                origDisks[0].format = $('.template-storage-disk-format', tmpItem).val();
> +                data[field] = origDisks;
>               }
>               else if (field == 'graphics') {
>                  var type = $('#form-template-general [name="' + field + '"]').val();
> diff --git a/ui/pages/template-edit.html.tmpl b/ui/pages/template-edit.html.tmpl
> index c7832c9..e64a30f 100644
> --- a/ui/pages/template-edit.html.tmpl
> +++ b/ui/pages/template-edit.html.tmpl
> @@ -104,6 +104,7 @@
>                       <span class="template-storage-cell">$_("Storage Pool")</span>
>                       <span class="template-storage-cell">$_("Type")</span>
>                       <span class="template-storage-cell">$_("Disk(GB)")</span>
> +                    <span class="template-storage-cell">$_("Disk Format")</span>
>                       <button type="button" id="template-edit-storage-add-button" class="action-area"></button>
>                   </div>
>                   <div class="template-tab-body">
> @@ -160,6 +161,21 @@
>           <span class="template-storage-cell">
>               <input class="template-storage-disk" value={storageDisk} type="text" />
>           </span>
> +        <span class="template-storage-cell">
> +            <input class="template-storage-disk-format" value={storageDiskFormat} type="text" style="display:none" />
> +            <select>
> +               <option>qcow2</option>
> +               <option>raw</option>
> +               <option>bochs</option>
> +               <option>cloop</option>
> +               <option>cow</option>
> +               <option>dmg</option>
> +               <option>qcow</option>
> +               <option>qed</option>
> +               <option>vmdk</option>
> +               <option>vpc</option>
> +            </select>
> +        </span>
>       </div>
>   </script>
>   <script id="template-interface-tmpl" type="text/html">




More information about the Kimchi-devel mailing list