Re: [pve-devel] [PATCH v2 manager] ui: vm network: allow to override MTU for virtio devices

[Aaron Lauterer <a.lauterer@proxmox.com>]

Looks good AFAICT and does what we want.

We could improve it further by adding a small validator. Otherwise the user will only notice their error once they get the error msg from the API.

A quick way that I came up with that could potentially be done better:

diff --git a/www/manager6/qemu/NetworkEdit.js b/www/manager6/qemu/NetworkEdit.js
index 1e34ad1c..2566d1a8 100644
--- a/www/manager6/qemu/NetworkEdit.js
+++ b/www/manager6/qemu/NetworkEdit.js
@@ -184,6 +184,13 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
                 bind: {
                     disabled: '{!isVirtio}',
                 },
+               validator: function(value) {
+                   if (value === "" || (value > 0 && value <= 65520)) {
+                       return true;
+                   } else {
+                       return gettext("must be between 0 and 65520");
+                   }
+               },
                 allowBlank: true,
             },
         ];


Besides this last improvement:

Reviewed-By: Aaron Lauterer <a.lauterer@proxmox.com>
Tested-By: Aaron Lauterer <a.lauterer@proxmox.com>



On 2/10/22 12:28, Oguz Bektas wrote:
> we already have the 'mtu' option inside the API, so we can just expose
> that option inside the 'Advanced' menu for virtio network interfaces.
> 
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
> v1->v2:
> * add viewmodel formula 'isVirtio', we use it to set the MTU field
> enabled only for virtio interfaces
> 
>   www/manager6/qemu/NetworkEdit.js | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
> 
> diff --git a/www/manager6/qemu/NetworkEdit.js b/www/manager6/qemu/NetworkEdit.js
> index b39cffdc..1e34ad1c 100644
> --- a/www/manager6/qemu/NetworkEdit.js
> +++ b/www/manager6/qemu/NetworkEdit.js
> @@ -19,6 +19,7 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>   	me.network.macaddr = values.macaddr;
>   	me.network.disconnect = values.disconnect;
>   	me.network.queues = values.queues;
> +	me.network.mtu = values.mtu;
>   
>   	if (values.rate) {
>   	    me.network.rate = values.rate;
> @@ -33,6 +34,16 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>   	return params;
>       },
>   
> +    viewModel: {
> +	data: {
> +	    network_model: '',
> +	},
> +	formulas: {
> +	    isVirtio: (get) => get('network_model') === 'virtio',
> +	},
> +    },
> +
> +
>       setNetwork: function(confid, data) {
>   	var me = this;
>   
> @@ -55,6 +66,7 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>   	me.bridgesel.setNodename(nodename);
>       },
>   
> +
>       initComponent: function() {
>   	var me = this;
>   
> @@ -112,6 +124,7 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>   			    'macaddr',
>   			    'rate',
>   			    'queues',
> +			    'mtu',
>   			];
>   			fields.forEach(function(fieldname) {
>   			    me.down('field[name='+fieldname+']').setDisabled(value);
> @@ -125,11 +138,13 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>   	    });
>   	}
>   
> +
>   	me.column2.push(
>   	    {
>   		xtype: 'pveNetworkCardSelector',
>   		name: 'model',
>   		fieldLabel: gettext('Model'),
> +		bind: '{network_model}',
>   		value: PVE.qemu.OSDefaults.generic.networkCard,
>   		allowBlank: false,
>   	    },
> @@ -161,6 +176,16 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>   		value: '',
>   		allowBlank: true,
>   	    },
> +	    {
> +		xtype: 'proxmoxintegerfield',
> +		name: 'mtu',
> +		fieldLabel: 'MTU',
> +		value: '',
> +		bind: {
> +		    disabled: '{!isVirtio}',
> +		},
> +		allowBlank: true,
> +	    },
>   	];
>   
>   	me.callParent();