public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH v6 manager] ui: vm network: allow to override MTU for virtio devices
@ 2022-05-20  8:03 Oguz Bektas
  2022-10-12  8:57 ` Daniel Tschlatscher
  0 siblings, 1 reply; 3+ messages in thread
From: Oguz Bektas @ 2022-05-20  8:03 UTC (permalink / raw)
  To: pve-devel

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





^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pve-devel] [PATCH v6 manager] ui: vm network: allow to override MTU for virtio devices
  2022-05-20  8:03 [pve-devel] [PATCH v6 manager] ui: vm network: allow to override MTU for virtio devices Oguz Bektas
@ 2022-10-12  8:57 ` Daniel Tschlatscher
  2022-11-08 16:47   ` Thomas Lamprecht
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Tschlatscher @ 2022-10-12  8:57 UTC (permalink / raw)
  To: pve-devel

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();




^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pve-devel] [PATCH v6 manager] ui: vm network: allow to override MTU for virtio devices
  2022-10-12  8:57 ` Daniel Tschlatscher
@ 2022-11-08 16:47   ` Thomas Lamprecht
  0 siblings, 0 replies; 3+ messages in thread
From: Thomas Lamprecht @ 2022-11-08 16:47 UTC (permalink / raw)
  To: Proxmox VE development discussion, Daniel Tschlatscher

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?




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-11-08 16:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-20  8:03 [pve-devel] [PATCH v6 manager] ui: vm network: allow to override MTU for virtio devices Oguz Bektas
2022-10-12  8:57 ` Daniel Tschlatscher
2022-11-08 16:47   ` Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal