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

[Dominik Csapak]

looks mostly fine, one comment though:

i would put the hint either in advancedColumnB
so that the text spans the complete width of the window
(looks weird crammed to the left)

or even at the very top? if i have iothread on but the
advanced checkbox off, i do not see the hint ...
(ok this is probably not that bad, since the
other way the user would see a warning about iothreads
without actually seeing the option...)

we could do that in a follow up though

another semi-related note (nothing you should fix in this patch):
when we are in the wizard, the controller selection behaves very weird..
when i manually select a scsi controller (e.g. megaraid)
it sticks.. until i check/uncheck iothread (it switches
to virtio-scsi-single, and then back to virtio-scsi)

i'd rather have it either disabled in that case,
or just show the hint? we just have to check somehow if the users
selected a different controller manually (can we do that?)
@Thomas any different opinion?

also, the scsi controller display field is somehow broken
when one selectes the 'default' scsi controller (shows __default__)


On 4/27/20 11:15 AM, 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>
> ---
> 
> v2:
> * do all checks in updateIOThreadHint
> * check insideWizard as part of 'let visible' to avoid extra 'if'
> * view.drive is not set when adding a new disk to a VM, so get the controller
>    from 'controller' field directly then
> 
>   www/manager6/qemu/HDEdit.js | 25 ++++++++++++++++++++++++-
>   1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/www/manager6/qemu/HDEdit.js b/www/manager6/qemu/HDEdit.js
> index fd890600..4dc10dc6 100644
> --- a/www/manager6/qemu/HDEdit.js
> +++ b/www/manager6/qemu/HDEdit.js
> @@ -35,6 +35,20 @@ Ext.define('PVE.qemu.HDInputPanel', {
>   	    this.lookup('scsiController').setVisible(value.match(/^scsi/));
>   	},
>   
> +	updateIOThreadHint: function(iothread) {
> +	    let me = this;
> +	    let view = me.getView();
> +	    let hint = me.lookupReference('iothreadHint');
> +
> +	    let interface = view.drive.interface ||
> +		view.down('field[name=controller]').getValue();
> +	    let visible = !view.insideWizard && iothread &&
> +		interface === 'scsi' &&
> +		view.vmconfig.scsihw !== 'virtio-scsi-single';
> +
> +	    hint.setVisible(visible);
> +	},
> +
>   	control: {
>   	    'field[name=controller]': {
>   		change: 'onControllerChange',
> @@ -42,6 +56,8 @@ Ext.define('PVE.qemu.HDInputPanel', {
>   	    },
>   	    'field[name=iothread]' : {
>   		change: function(f, value) {
> +		    this.updateIOThreadHint(value);
> +
>   		    if (!this.getView().insideWizard) {
>   			return;
>   		    }
> @@ -285,7 +301,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(
>