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

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

[Oguz Bektas]

we already have the 'mtu' option in 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>
---
 www/manager6/qemu/NetworkEdit.js | 34 ++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

v5->v6:
* move the MTU bar to the left column, kept the message tooltip on the
right one
* minor syntax changes from thomas' review on v5

diff --git a/www/manager6/qemu/NetworkEdit.js b/www/manager6/qemu/NetworkEdit.js
index b39cffdc..d9f9b859 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,17 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
 	return params;
     },
 
+    viewModel: {
+	data: {
+	    networkModel: '',
+	    mtu: '',
+	},
+	formulas: {
+	    isVirtio: get => get('networkModel') === 'virtio',
+	    showMtuHint: get => get('mtu') === 1,
+	},
+    },
+
     setNetwork: function(confid, data) {
 	var me = this;
 
@@ -93,6 +105,18 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
 		fieldLabel: gettext('Disconnect'),
 		name: 'disconnect',
 	    },
+	    {
+		xtype: 'proxmoxintegerfield',
+		name: 'mtu',
+		fieldLabel: 'MTU',
+		bind: {
+		    disabled: '{!isVirtio}',
+		    value: '{mtu}',
+		},
+		minValue: 1,
+		maxValue: 65520,
+		allowBlank: true,
+	    },
 	];
 
 	if (me.insideWizard) {
@@ -112,6 +136,7 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
 			    'macaddr',
 			    'rate',
 			    'queues',
+			    'mtu',
 			];
 			fields.forEach(function(fieldname) {
 			    me.down('field[name='+fieldname+']').setDisabled(value);
@@ -130,6 +155,7 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
 		xtype: 'pveNetworkCardSelector',
 		name: 'model',
 		fieldLabel: gettext('Model'),
+		bind: '{networkModel}',
 		value: PVE.qemu.OSDefaults.generic.networkCard,
 		allowBlank: false,
 	    },
@@ -161,6 +187,14 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
 		value: '',
 		allowBlank: true,
 	    },
+	    {
+		xtype: 'displayfield',
+		userCls: 'pmx-hint',
+		value: gettext("Use the special value '1' to inherit the MTU value from the underlying bridge"),
+		bind: {
+		    hidden: '{!showMtuHint}',
+		},
+	    },
 	];
 
 	me.callParent();
-- 
2.30.2

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

[Daniel Tschlatscher]

This patch still applies well and works as intended.

There is one UI suggestion inline.


Tested-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>

On 5/20/22 10:03, Oguz Bektas wrote:
> we already have the 'mtu' option in 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>
> ---
>  www/manager6/qemu/NetworkEdit.js | 34 ++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> v5->v6:
> * move the MTU bar to the left column, kept the message tooltip on the
> right one> * minor syntax changes from thomas' review on v5
> 
> diff --git a/www/manager6/qemu/NetworkEdit.js b/www/manager6/qemu/NetworkEdit.js
> index b39cffdc..d9f9b859 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,17 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>  	return params;
>      },
>  
> +    viewModel: {
> +	data: {
> +	    networkModel: '',
> +	    mtu: '',
> +	},
> +	formulas: {
> +	    isVirtio: get => get('networkModel') === 'virtio',
> +	    showMtuHint: get => get('mtu') === 1,
> +	},
> +    },
> +
>      setNetwork: function(confid, data) {
>  	var me = this;
>  
> @@ -93,6 +105,18 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>  		fieldLabel: gettext('Disconnect'),
>  		name: 'disconnect',
>  	    },
> +	    {
> +		xtype: 'proxmoxintegerfield',
> +		name: 'mtu',
> +		fieldLabel: 'MTU',
> +		bind: {
> +		    disabled: '{!isVirtio}',
> +		    value: '{mtu}',
> +		},
> +		minValue: 1,
> +		maxValue: 65520,
> +		allowBlank: true,
> +	    },
>  	];
>  
>  	if (me.insideWizard) {
> @@ -112,6 +136,7 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>  			    'macaddr',
>  			    'rate',
>  			    'queues',
> +			    'mtu',
>  			];
>  			fields.forEach(function(fieldname) {
>  			    me.down('field[name='+fieldname+']').setDisabled(value);
> @@ -130,6 +155,7 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>  		xtype: 'pveNetworkCardSelector',
>  		name: 'model',
>  		fieldLabel: gettext('Model'),
> +		bind: '{networkModel}',
>  		value: PVE.qemu.OSDefaults.generic.networkCard,
>  		allowBlank: false,
>  	    },
> @@ -161,6 +187,14 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>  		value: '',
>  		allowBlank: true,
>  	    },
> +	    {
> +		xtype: 'displayfield',
> +		userCls: 'pmx-hint',
> +		value: gettext("Use the special value '1' to inherit the MTU value from the underlying bridge"),
> +		bind: {
> +		    hidden: '{!showMtuHint}',
> +		},
> +	    },

I think it might make a bit more sense to move this into the
advancedColumn1 as well, as now it is displayed in the opposite column,
which makes the UI flow kind of confusing.
Perhaps an even better option though, would be to move the displayfield
into an advancedColumnB, which would also remove the line width limit
and with it the somewhat ugly line break.

>  	];
>  
>  	me.callParent();

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

[Thomas Lamprecht]

Am 12/10/2022 um 10:57 schrieb Daniel Tschlatscher:
> This patch still applies well and works as intended.
> 
> There is one UI suggestion inline.
> 
> 
> Tested-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
> 

thx!
>> @@ -161,6 +187,14 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>>  		value: '',
>>  		allowBlank: true,
>>  	    },
>> +	    {
>> +		xtype: 'displayfield',
>> +		userCls: 'pmx-hint',
>> +		value: gettext("Use the special value '1' to inherit the MTU value from the underlying bridge"),
>> +		bind: {
>> +		    hidden: '{!showMtuHint}',
>> +		},
>> +	    },
> I think it might make a bit more sense to move this into the
> advancedColumn1 as well, as now it is displayed in the opposite column,
> which makes the UI flow kind of confusing.
> Perhaps an even better option though, would be to move the displayfield
> into an advancedColumnB, which would also remove the line width limit
> and with it the somewhat ugly line break.
> 

With Oguz not working here anymore, would you like to re-send this with that
change?