[pve-devel] [PATCH manager] HDEdit: warn when IO threads are enabled with incompatible controller

[Dominik Csapak]

Looks good one comment inline:

On 4/16/20 5:15 PM, Stefan Reiter wrote:
> The only warning displayed before was in the "VM Start" task log, rather
> hidden. In the wizard we already auto-selected the correct controller, but
> not when modifying a disk on an existing VM.
> 
> Don't break existing behaviour (to allow users to disable IO threads for
> VMs that currently have it set but an incompatible controller), but do warn
> them that the setting will be ignored.
> 
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
>   www/manager6/qemu/HDEdit.js | 28 +++++++++++++++++++++++++++-
>   1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/www/manager6/qemu/HDEdit.js b/www/manager6/qemu/HDEdit.js
> index fd890600..d50fe077 100644
> --- a/www/manager6/qemu/HDEdit.js
> +++ b/www/manager6/qemu/HDEdit.js
> @@ -35,6 +35,19 @@ Ext.define('PVE.qemu.HDInputPanel', {
>   	    this.lookup('scsiController').setVisible(value.match(/^scsi/));
>   	},
>   
> +	updateIOThreadHint: function() {
> +	    let me = this;
> +	    let view = me.getView();
> +	    if (view.insideWizard) {
> +		// we autoselect a compatible controller in the wizard
> +		return;
> +	    }
> +
> +	    let visible = view.drive.interface === 'scsi' &&
> +		view.vmconfig.scsihw !== 'virtio-scsi-single';
> +	    me.lookupReference('iothreadHint').setVisible(visible);
> +	},
> +
>   	control: {
>   	    'field[name=controller]': {
>   		change: 'onControllerChange',
> @@ -42,6 +55,12 @@ Ext.define('PVE.qemu.HDInputPanel', {
>   	    },
>   	    'field[name=iothread]' : {
>   		change: function(f, value) {
> +		    if (value) {
> +			this.updateIOThreadHint();
> +		    } else {
> +			this.lookupReference('iothreadHint').setVisible(false);
> +		    }
> +

i would rather move this check inside updateIOThreadHint (and give value 
as parameter)

then the whole show/hide logic is constrained to that function

alternatively you could use some kind of viewModel formula/values
and bind the 'hidden' property to that (we already have a viewmodel)
e.g bind the iothread and controller value to the viewmodel
and have a formula that checks those values

(after shortly looking over the whole panel, we could probably do a lot
more via the viewmodel here, would probably save a few lines)

>   		    if (!this.getView().insideWizard) {
>   			return;
>   		    }
> @@ -285,7 +304,14 @@ Ext.define('PVE.qemu.HDInputPanel', {
>   		fieldLabel: gettext('Write limit') + ' (ops/s)',
>   		labelWidth: labelWidth,
>   		emptyText: gettext('unlimited')
> -	    }
> +	    },
> +	    {
> +		xtype: 'displayfield',
> +		userCls: 'pmx-hint',
> +		reference: 'iothreadHint',
> +		value: gettext("IO threads will only be used with VirtIO Block disks or when using the 'VirtIO SCSI single' controller model!"),
> +		hidden: true,
> +	    },
>   	);
>   
>   	me.advancedColumn2.push(
>