[pve-devel] [PATCH manager 4/4] fix #973: disable iothread for non-virtio drives

[Dietmar Maurer]

comment inline:

> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  www/manager6/form/ControllerSelector.js |  2 ++
>  www/manager6/qemu/HDEdit.js             | 14 ++++++++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/www/manager6/form/ControllerSelector.js
> b/www/manager6/form/ControllerSelector.js
> index 72c4767..f323963 100644
> --- a/www/manager6/form/ControllerSelector.js
> +++ b/www/manager6/form/ControllerSelector.js
> @@ -67,6 +67,7 @@ Ext.define('PVE.form.ControllerSelector', {
>  		{
>  		    xtype: 'PVE.form.BusTypeSelector',
>  		    name: 'controller',
> +		    itemId: 'controllerbox',

What for? You do not use it anywhere inside this file (see further
comments below).

>  		    value: 'ide',
>  		    noVirtIO: me.noVirtIO,
>  		    noScsi: me.noScsi,
> @@ -79,6 +80,7 @@ Ext.define('PVE.form.ControllerSelector', {
>  			    var field = me.down('field[name=deviceid]');
>  			    field.setMaxValue(PVE.form.ControllerSelector.maxIds[value]);
>  			    field.validate();
> +			    me.fireEvent('change');

I thought 'Ext.form.FieldContainer' handles this?

>  			}
>  		    }
>  		},
> diff --git a/www/manager6/qemu/HDEdit.js b/www/manager6/qemu/HDEdit.js
> index a360f23..5a0f851 100644
> --- a/www/manager6/qemu/HDEdit.js
> +++ b/www/manager6/qemu/HDEdit.js
> @@ -105,6 +105,16 @@ Ext.define('PVE.qemu.HDInputPanel', {
>  	me.hdfilesel.setStorage(undefined, nodename);
>      },
>  
> +    controllerChange: function() {
> +	var me = this;
> +	var iothreadbox = me.down('#iothreadbox');
> +	var controllerbox = me.down('#controllerbox');

not nice...

> +	if (iothreadbox && controllerbox) {
> +	    // disable iothread box when controller != virtio
> +
>     iothreadbox.setDisabled(!me.down('#controllerbox').value.match(/^virtio/));
> +	}
> +    },

what about virtio-scsi (as mentioned by alexandre)?

>      initComponent : function() {
>  	var me = this;
>  
> @@ -118,6 +128,8 @@ Ext.define('PVE.qemu.HDInputPanel', {
>  		vmconfig: me.insideWizard ? {ide2: 'cdrom'} : {}
>  	    });
>  	    me.column1.push(me.bussel);
> +	    me.mon(me.bussel, 'change', me.controllerChange, me);
> +	    me.mon(me.bussel, 'afterrender', me.controllerChange, me);
>  	}
>  
>  	if (me.unused) {
> @@ -240,7 +252,9 @@ Ext.define('PVE.qemu.HDInputPanel', {
>  	});
>  
>  	me.column2.push({
> +	    itemId: 'iothreadbox',
>  	    xtype: 'pvecheckbox',
> +	    disabled: me.insideWizard || (me.confid && !me.confid.match(/^virtio/)),
>  	    fieldLabel: gettext('IO thread'),
>  	    name: 'iothread'

Why you you introduce new itemIds here? You can simply 
find a field using:

me.down('field[name=iothread]')

I also think we should use view controllers when possible, and thus
use 'reference' instead of itemIDs?