[pve-devel] [PATCH manager v3 5/8] Add a field to show which SCSI controller type we are currently using

[Thomas Lamprecht]

On 09/28/2017 03:10 PM, Emmanuel Kasper wrote:
> On 09/27/2017 11:27 AM, Thomas Lamprecht wrote:
>> On 09/26/2017 02:17 PM, Emmanuel Kasper wrote:
>>> ---
>>>   www/manager6/qemu/HDEdit.js | 15 ++++++++++++---
>>>   1 file changed, 12 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/www/manager6/qemu/HDEdit.js b/www/manager6/qemu/HDEdit.js
>>> index 8a415d8b..075ad667 100644
>>> --- a/www/manager6/qemu/HDEdit.js
>>> +++ b/www/manager6/qemu/HDEdit.js
>>> @@ -24,11 +24,12 @@ Ext.define('PVE.qemu.HDInputPanel', {
>>>           this.lookup('iothread').setValue(false);
>>>           }
>>>   -        var allowDiscard = value.match(/^scsi/);
>>> -        this.lookup('discard').setDisabled(!allowDiscard);
>>> -        if (!allowDiscard) {
>>> +        var scsi = value.match(/^scsi/);
>>> +        this.lookup('discard').setDisabled(!scsi);
>>> +        if (!scsi) {
>>>           this.lookup('discard').setValue(false);
>>>           }
>>> +        this.lookup('scsiType').setVisible(scsi);
>>>       },
>>>         control: {
>>> @@ -141,6 +142,7 @@ Ext.define('PVE.qemu.HDInputPanel', {
>>>         if (me.bussel) {
>>>           me.bussel.setVMConfig(vmconfig);
>>> +        me.scsiType.setValue(vmconfig.scsihw);
>>>       }
>>>       if (me.unusedDisks) {
>>>           var disklist = [];
>>> @@ -195,6 +197,13 @@ Ext.define('PVE.qemu.HDInputPanel', {
>>>           vmconfig: me.insideWizard ? {ide2: 'cdrom'} : {}
>>>           });
>>>           me.column1.push(me.bussel);
>>> +        me.scsiType = Ext.create('Ext.form.field.Display', {>
>>> +        fieldLabel: gettext('Type'),
>>
>> Type is to general and not really true here (Type of what?).
>> 'SCSI Controller' would match the scsihw edit window better
>> and make it clearer why it disappears when something other
>> than SCSI is selected, what do you think?
> 
> The current gettext strings we have here are either 'SCSI Controller
> Type' need to be wrapped on two lines in the Edit Window, or 'Type'
> 

So my proposal would then be to change 'SCSI Controller Type'
to 'SCSI Controller'. 'Type' does not gives additional information for
this we select a SCSI controller here and we do not have type appended
to other fields, e.g., BIOS, Display.

> Since this 'Type' field is underneath the Bus Selector, appears only
> when the SCSI controller is selected in the Bus Selector, and include
> the string SCSI in all the cases that interess us (VirtIO SCSI), I think
> 'Type' is enough.
> 

The user does not knows the set of possible values though, so he sees
that SCSI appears with but there could be other options appearing to
as he does not sees a list of all possible options. IMHO, this is not
really user friendly. Also we would diverge from our strategy to avoid
calling the same thing two different names in the UI, with your version.

cheers,
Thomas

> For the rest, agree with your suggestions, I will integrate them in a V4.
> 
> Emmanuel
>