On 09/23/2014 04:23 AM, Wen Wang wrote:
On 09/19/2014 09:20 PM, Aline Manera wrote:
>
> On 09/19/2014 06:43 AM, Wen Wang wrote:
>>
>> On 09/19/2014 10:43 AM, Aline Manera wrote:
>>>
>>> On 09/18/2014 04:33 AM, Wen Wang wrote:
>>>> From: Wen Wang <wenwang(a)linux.vnet.ibm.com>
>>>>
>>>> This patch fix the bug that value of "Disk(GB)" input box is
changing
>>>> with the item of "Storage Pool" in "Templates" -->
"Actions" -->
>>>> "Edit"
>>>> -->"Edit Templates" dialogue. This might be comfusing to
user when
>>>> changing the "Storage Pool", "Disk(GB)" is changed
with it without
>>>> even
>>>> notice. This might confuse the users and is not necessary. Solved by
>>>> removing the change.
>>>>
>>>> Signed-off-by: Wen Wang <wenwang(a)linux.vnet.ibm.com>
>>>> ---
>>>> ui/js/src/kimchi.template_edit_main.js | 25
>>>> -------------------------
>>>> 1 files changed, 0 insertions(+), 25 deletions(-)
>>>>
>>>> diff --git a/ui/js/src/kimchi.template_edit_main.js
>>>> b/ui/js/src/kimchi.template_edit_main.js
>>>> index cb43091..61d23b4 100644
>>>> --- a/ui/js/src/kimchi.template_edit_main.js
>>>> +++ b/ui/js/src/kimchi.template_edit_main.js
>>>> @@ -117,31 +117,6 @@ kimchi.template_edit_main = function() {
>>>> });
>>>> });
>>>
>>>
>>>> - $('#template-edit-storagePool').change(function() {
>>>> - storagepool = $(this).val();
>>>> - var storageArray = storagepool.split("/");
>>>> - if (storageArray.length > 3) {
>>>> - volumeName = storageArray.pop();
>>>> - poolName = storageArray.pop();
>>>> - kimchi.getStoragePoolVolume(poolName, volumeName,
>>>> function(result) {
>>>> - $('input[name="disks"]',
>>>> templateEditForm).val(result.capacity / Math.pow(1024,3));
>>>> - $('input[name="disks"]',
>>>> templateEditForm).attr('disabled','disabled');
>>>> - return false;
>>>> - }, function (err) {
>>>> - kimchi.message.error(err.responseJSON.reason);
>>>> - });
>>>> - } else {
>>>> - if (origPool == storagepool) {
>>>> - // Previous disk size value
>>>> - $('input[name="disks"]',
>>>> templateEditForm).val(origDisks[0].size);
>>>> - } else {
>>>> - // Default disk size value
>>>> - $('input[name="disks"]',
templateEditForm).val(10);
>>>> - }
>>>> - $('input[name="disks"]',
>>>> templateEditForm).removeAttr('disabled');
>>>> - }
>>>> - });
>>>> -
>>>> $('#tmpl-edit-button-save').on('click', function()
{
>>>> var editableFields = [ 'name', 'cpus',
'memory',
>>>> 'storagepool', 'disks', 'graphics'];
>>>> var data = {};
>>>
>>> We can't remove this code.
>>> It is used when a iSCSI or SCSI storage pool is selected. In this
>>> case, the disk size comes from the volumes selected and can not be
>>> changed.
>>>
>>> iSCSI and SCSI pools are what we said as read-only pools which
>>> means we can't create new volumes in it as the volumes are
>>> pre-configured by system.
>>> So when user selects one of these pools, he/she needs also to point
>>> the volume because that we display <pool>/<volumes> in these
cases.
>>>
>>> What we can do is always display the previous disk size when
>>> changing from pools that are not iSCSI or SCSI.
>>> To do that we need to change the "else" statement like below:
>>>
>>>
>>> } else {
>>> - if (origPool == storagepool) {
>>> - // Previous disk size value
>>> - $('input[name="disks"]',
>>> templateEditForm).val(origDisks[0].size);
>>> - } else {
>>> - // Default disk size value
>>> - $('input[name="disks"]',
templateEditForm).val(10);
>>> - }
>>
>>> + // Previous disk size value
>>> + $('input[name="disks"]',
>>> templateEditForm).val(origDisks[0].size);
>>
>> I don't think these two line is necessary for this. I mean if we
>> detect no iSCSI nor SCSI device, The only thing we need to do is to
>> enable the input box. Having it changed without any notificaton is
>> not that good user experience. This input box has value when it
>> initiated and if changing to iSCSI or SCSI device, we just need to
>> enable the input box, if changing back, we disable the input box
>> again and refresh the value.
>
> If we don't update the disk size value when a no iSCSI nor SCSI
> device is selected, the input box will contain the previous value
> which can be not valid.
> Example.
>
> 1. Select a dir pool
> 2. Disk size is set to 10Gb
> 3. Select a iSCSI volume
> 4. Disk size is set to 4GB
> 5. Select back the dir pool
>
> 6.a Disk size is still set to 4GB (*without* those 2 lines)
> 6.b Disk size turns back to 4GB (*with* the 2 lines)
>
> From my view, the correct behavior is 6.b
I have tried your proposal, And there occurs one more problem just
according to this change. Think about the scenario below:
Current template is
"Storage Pool: default
Disk(GB): 15"
And user change the "Disk(GB)" value to 30( without saving)
then user choose the "Storage Pool" and decided to use "ISO", by
choosing that, the value of "Disk(GB)" changed to 15 which is the
value set for "default" and that value makes no sense to "ISO" so as
the change of this value.
So I recommend we just leave the user with the value that they edited
before and it will have a default value either from the server stored
or set by iSCSI & SCSI automatic settings.
Good point. So we should also save the inputed value
>
>>
>>> $('input[name="disks"]',
>>> templateEditForm).removeAttr('disabled');
>>> }
>>> });
>>>
>>>
>>>
>>>
>>
>