From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <d.csapak@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id ECF1171641
 for <pve-devel@lists.proxmox.com>; Wed, 18 May 2022 15:13:48 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id DC4342CF2D
 for <pve-devel@lists.proxmox.com>; Wed, 18 May 2022 15:13:18 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS id 990D92CF1F
 for <pve-devel@lists.proxmox.com>; Wed, 18 May 2022 15:13:17 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 612EE436B9;
 Wed, 18 May 2022 15:13:17 +0200 (CEST)
Message-ID: <a55149f5-69d4-1d74-f0f0-5dc405cbef5a@proxmox.com>
Date: Wed, 18 May 2022 15:13:16 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:101.0) Gecko/20100101
 Thunderbird/101.0
Content-Language: en-US
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
 Oguz Bektas <o.bektas@proxmox.com>
References: <20220516093753.232985-1-o.bektas@proxmox.com>
From: Dominik Csapak <d.csapak@proxmox.com>
In-Reply-To: <20220516093753.232985-1-o.bektas@proxmox.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 1.178 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 NICE_REPLY_A           -2.119 Looks like a legit reply (A)
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 T_SCC_BODY_TEXT_LINE    -0.01 -
Subject: Re: [pve-devel] [PATCH v4 manager] ui: vm network: allow to
 override MTU for virtio devices
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Wed, 18 May 2022 13:13:49 -0000

ok so i noticed the following:

when i have a virtio-nic with no mtu set, we populate the field with
'1500' (even when we don't show the advanced fields) and pressing
ok, changes the mtu (to 1500)

what is the default for that value? none?
in any case, the default should be the emptyText, and we
should not set the field to a value when it's not set
in the config

i guess this would work if you set the 'mtu' to undefined/null/empty string in
the viewmodel, and set the emptyText appropriately

otherwise LGTM

On 5/16/22 11:37, 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.
> 
> Reviewed-By: Aaron Lauterer <a.lauterer@proxmox.com>
> Tested-By: Aaron Lauterer <a.lauterer@proxmox.com>
> Tested-By: Dylan Whyte <d.whyte@proxmox.com>
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
> 
> v3->v4:
> * show tooltip message about inheritance if mtu is 1
> 
> 
>   www/manager6/qemu/NetworkEdit.js | 34 ++++++++++++++++++++++++++++++++
>   1 file changed, 34 insertions(+)
> 
> diff --git a/www/manager6/qemu/NetworkEdit.js b/www/manager6/qemu/NetworkEdit.js
> index b39cffdc..6e56b73d 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: {
> +	    network_model: '',
> +	    mtu: 1500,
> +	},
> +	formulas: {
> +	    isVirtio: (get) => get('network_model') === 'virtio',
> +	    showMTUHint: (get) => get('mtu') === 1,
> +	},
> +    },
> +
>       setNetwork: function(confid, data) {
>   	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);
> @@ -130,6 +143,7 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>   		xtype: 'pveNetworkCardSelector',
>   		name: 'model',
>   		fieldLabel: gettext('Model'),
> +		bind: '{network_model}',
>   		value: PVE.qemu.OSDefaults.generic.networkCard,
>   		allowBlank: false,
>   	    },
> @@ -161,6 +175,26 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>   		value: '',
>   		allowBlank: true,
>   	    },
> +	    {
> +		xtype: 'proxmoxintegerfield',
> +		name: 'mtu',
> +		fieldLabel: 'MTU',
> +		bind: {
> +		    disabled: '{!isVirtio}',
> +		    value: '{mtu}',
> +		},
> +		minValue: 1,
> +		maxValue: 65520,
> +		allowBlank: true,
> +	    },
> +	    {
> +		xtype: 'displayfield',
> +		userCls: 'pmx-hint',
> +		value: gettext("mtu=1 is a special value, the MTU value will be inherited from the current bridge"),
> +		bind: {
> +		    hidden: '{!showMTUHint}',
> +		},
> +	    },
>   	];
>   
>   	me.callParent();