[pve-devel] [PATCH pve-manager 1/1] Close #2443: gui: qemu/NetworkEdit: add network mtu advanced option
tacid at tacid.kiev.ua
tacid at tacid.kiev.ua
Thu Dec 12 17:13:47 CET 2019
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 <tacid at 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',
> ---
More information about the pve-devel
mailing list