3:48 p.m.
On 27/04/2015 17:16, Rodrigo Trujillo wrote:
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(a)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.
I am reviewing only this patch I want to have *this patch* in good shape
to have it merged.
If you want to make it dependent of other patch, it is up to you, but in
that case this patch will be on hold.
>> $.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
Same I said before.
>
>
>> }, 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.
I don't think you need a loop to do that:
https://api.jquery.com/attribute-equals-selector/
>
>
>> + $('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.
Same 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?
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.
Got it. Thanks.
>
>> + $('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">
>