On 2019-12-12 09:25 Thomas Lamprecht wrote:
First, thanks for the patch!
Most seems to be adapted from surrounding code, which is OK.
Some comments inline.
Also, a pve-docs patch about this would be nice, to clear up things
like:
* MTU cannot be bigger than those used by the host in it's chain to the
outer network.
* All components of the network need to support this MTU
* one can test it under linux with `ping -M do -s $[9000 - 28] <addr>`
(where 9000 is the MTU and 28 is the ICMP echo packet overhead)
* That only VirtIO-net supports this at the moment (even if the UI
makes it
somewhat clear it's good to state that it's by design and not a bug
were
we just forgot to enable it for e1000e or the like :) )
* Intra-bridge communications can have 64k MTU if the bridge is
configured
to it
I.e., doesn't have to be much, but a few short sentences would be good
to have
for a new feature which can be a bit environment picky :)
I totally agree, but need to find time to do it.
On 12/11/19 6:03 PM, Paul Shepel wrote:
Signed-off-by: Paul Shepel <ta...@tacid.kiev.ua>
---
www/manager6/Parser.js | 5 +++++
www/manager6/qemu/NetworkEdit.js | 36
+++++++++++++++++++++++++++++---
2 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/www/manager6/Parser.js b/www/manager6/Parser.js
index 0790e683..6a9cbb78 100644
--- a/www/manager6/Parser.js
+++ b/www/manager6/Parser.js
@@ -131,6 +131,8 @@ Ext.define('PVE.Parser', { statics: {
res.disconnect = match_res[1];
} else if ((match_res = p.match(/^queues=(\d+)$/)) !== null) {
res.queues = match_res[1];
+ } else if ((match_res = p.match(/^mtu=(\d+)$/)) !== null) {
+ res.mtu = match_res[1];
} else if ((match_res =
p.match(/^trunks=(\d+(?:-\d+)?(?:;\d+(?:-\d+)?)*)$/)) !== null) {
res.trunks = match_res[1];
} else {
@@ -167,6 +169,9 @@ Ext.define('PVE.Parser', { statics: {
if (net.queues) {
netstr += ",queues=" + net.queues;
}
+ if (net.mtu) {
+ netstr += ",mtu=" + net.mtu;
+ }
if (net.disconnect) {
netstr += ",link_down=" + net.disconnect;
}
diff --git a/www/manager6/qemu/NetworkEdit.js
b/www/manager6/qemu/NetworkEdit.js
index abce4903..c66da59c 100644
--- a/www/manager6/qemu/NetworkEdit.js
+++ b/www/manager6/qemu/NetworkEdit.js
@@ -26,6 +26,12 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
delete me.network.rate;
}
+ if (values.mtu) {
+ me.network.mtu = values.mtu;
+ } else {
+ delete me.network.mtu;
+ }
+
var params = {};
params[me.confid] = PVE.Parser.printQemuNetwork(me.network);
@@ -92,7 +98,19 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
xtype: 'proxmoxcheckbox',
fieldLabel: gettext('Disconnect'),
name: 'disconnect'
- }
+ },
+ {
+ xtype: 'numberfield',
+ fieldLabel: gettext('MTU'),
+ minValue: 576,
+ maxValue: 65535,
+ value: '',
+ emptyText: '1500',
+ name: 'mtu',
+ disabled: true,
+ hidden: true,
+ allowBlank: true
+ }
];
if (me.insideWizard) {
@@ -111,7 +129,8 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
'model',
'macaddr',
'rate',
- 'queues'
+ 'queues',
+ 'mtu',
];
fields.forEach(function(fieldname) {
me.down('field[name='+fieldname+']').setDisabled(value);
@@ -150,7 +169,18 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
maxValue: 10*1024,
value: '',
emptyText: 'unlimited',
- allowBlank: true
+ allowBlank: true,
+ listeners: {
+ change: function(f, value) {
+ if (!me.rendered) {
+ return;
+ }
+ var mtuField = me.down('field[name=mtu]');
+ var isVirtioNet = ( value == 'virtio' );
+ mtuField.setDisabled(!isVirtioNet);
+ mtuField.setHidden(!isVirtioNet);
+ }
+ }
1. Indentation is completely off
2. This is on the Ratelimit numberfield, how can this possibly work?
My fault, misplace while adopting running patch to git code.
Of cause change listener should be on model select field.
3. I'd rather go for a viewModel + binding approach, i.e., the
following
change on top of this patch (I ommited the hiding on purpose, we
seldom do
this):
I'm fine with this, this is more elegant and correct way.
Actually, I just failed implement it with bind, so I did it with change
listener
----8<----
diff --git a/www/manager6/qemu/NetworkEdit.js
b/www/manager6/qemu/NetworkEdit.js
index c66da59c..b3801419 100644
--- a/www/manager6/qemu/NetworkEdit.js
+++ b/www/manager6/qemu/NetworkEdit.js
@@ -3,6 +3,14 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
alias: 'widget.pveQemuNetworkInputPanel',
onlineHelp: 'qm_network_device',
+ viewModel: {
+ data: {
+ cardModel: PVE.qemu.OSDefaults.generic.networkCard,
+ },
+ formulas: {
+ cardIsVirtIO: (get) => get('cardModel') === 'virtio',
+ }
+ },
insideWizard: false,
onGetValues: function(values) {
@@ -107,8 +115,9 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
value: '',
emptyText: '1500',
name: 'mtu',
- disabled: true,
- hidden: true,
+ bind: {
+ disabled: '{!cardIsVirtIO}',
+ },
allowBlank: true
}
];
@@ -150,6 +159,9 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
name: 'model',
fieldLabel: gettext('Model'),
value: PVE.qemu.OSDefaults.generic.networkCard,
+ bind: {
+ value: '{cardModel}',
+ },
allowBlank: false
},
{
@@ -170,17 +182,6 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
value: '',
emptyText: 'unlimited',
allowBlank: true,
- listeners: {
- change: function(f, value) {
- if (!me.rendered) {
- return;
- }
- var mtuField = me.down('field[name=mtu]');
- var isVirtioNet = ( value == 'virtio' );
- mtuField.setDisabled(!isVirtioNet);
- mtuField.setHidden(!isVirtioNet);
- }
- }
},
{
xtype: 'proxmoxintegerfield',
---
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
