[Kimchi-devel] [PATCH V2] UI-Template Edit: Enable user to change disk format
Rodrigo Trujillo
rodrigo.trujillo at linux.vnet.ibm.com
Mon Apr 27 20:16:09 UTC 2015
On 04/27/2015 12:13 PM, Aline Manera wrote:
>
>
> 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.
>
We had some conversations last week and I was coding the support to
multiple disks together with this patch,
then you asked me to stop and do later. So, this patch considered only 1
disk per template. In this case the 'first'
and 'last' selection work fine.
>> $.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,
Answer 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?
Because the "option" elements only have the text.
>
>
>> + $('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
>
Because the "option" elements only have the text.
>> + $('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?
This if statement do the following:
Suppose you have 2 storage pools of DIR type (dir1 and dir2), and a
storage pool of iSCSI type (iscsi-pool).
When you select 'iscsi-pool' it is going to set disk format as RAW, and
disable it. When you select a dir pool,
'dir1' for instance, it is going to enable and set disk format selection
as 'qcow2' which is the default format.
Now, suppose you have 'dir1' selected and selects format as "vmdk". Then
you change the pool to 'dir2',
it is NOT going to change the disk format to 'qcow2', keeping the
previous selected value.
In other words, it avoids change the format to qcow2 every time, it only
changes the format when changing
from a storage pool with fixed format type ('raw') to a storage pool
that accepts multiple formats.
>
>> + $('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