Re: [PATCH pve-manager v2 03/10] fix #7294: ui: pool: add SDN VNets as pool members

[David Riley <d.riley@proxmox.com>]

some comments inline.

On 7/6/26 4:17 PM, Daniel Kral wrote:
> On Fri Jun 26, 2026 at 3:10 PM CEST, David Riley wrote:
>> Add user interface to manage SDN VNets inside resource pools.
>> In the dialog window users can select an SDN zone, pick an associated
>> VNet, and optionally specify a VLAN tag.
>>
>> The VNet selector only shows VNets of the selected zone to prevent
>> invalid configurations.
>>
>> Link:https://bugzilla.proxmox.com/show_bug.cgi?id=7294
>> Signed-off-by: David Riley<d.riley@proxmox.com>
>> ---
>>   www/css/ext6-pve.css             |  15 ++++
>>   www/manager6/Utils.js            |   1 +
>>   www/manager6/grid/PoolMembers.js | 123 +++++++++++++++++++++++++++++++
>>   3 files changed, 139 insertions(+)
>>
>> diff --git a/www/css/ext6-pve.css b/www/css/ext6-pve.css
>> index 5c37dd29..3c383dfa 100644
>> --- a/www/css/ext6-pve.css
>> +++ b/www/css/ext6-pve.css
>> @@ -464,6 +464,21 @@ div.right-aligned {
>>       content: " ";
>>   }
>>   
>> +.x-fa-pool-net:before {
>> +    width: 14px;
>> +    height: 14px;
>> +    position: absolute;
>> +    left: 1px;
>> +    top: 1px;
>> +    opacity: 0.45;
>> +}
>> +
>> +.x-fa-pool-net-grid:before {
>> +    left: 14px;
>> +    top: 6px;
>> +    opacity: 0.65;
>> +}
>> +
> The patch message should contain the information from patch #4, which
> explains why there is an opacity applied here.


Alright. I'll add an explanation.


>>   .x-fa-treepanel:before {
>>       width: 16px;
>>       height: 24px;
>> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
>> index 040b5ae0..8f53dfb6 100644
>> --- a/www/manager6/Utils.js
>> +++ b/www/manager6/Utils.js
>> @@ -1322,6 +1322,7 @@ Ext.define('PVE.Utils', {
>>                   const networkTypeMapping = {
>>                       fabric: 'fa fa-road',
>>                       zone: 'fa fa-th',
>> +                    vnet: 'fa fa-network-wired x-fa-pool-net x-fa-pool-net-grid',
>>                   };
>>   
>>                   return networkTypeMapping[record['network-type']] ?? '';
>> diff --git a/www/manager6/grid/PoolMembers.js b/www/manager6/grid/PoolMembers.js
>> index 69f50e30..5aaa2b71 100644
>> --- a/www/manager6/grid/PoolMembers.js
>> +++ b/www/manager6/grid/PoolMembers.js
>> @@ -158,6 +158,108 @@ Ext.define('PVE.pool.AddStorage', {
>>       },
>>   });
>>   
>> +Ext.define('PVE.pool.AddVnet', {
>> +    extend: 'Proxmox.window.Edit',
>> +
>> +    viewModel: {
>> +        data: {
>> +            zone: '',
>> +        },
>> +        formulas: {
>> +            vnetDisabled: function (get) {
>> +                return !get('zone');
>> +            },
>> +        },
>> +    },
>> +
>> +    initComponent: function () {
>> +        let me = this;
>> +
>> +        if (!me.pool) {
>> +            throw 'no pool specified';
>> +        }
>> +
>> +        me.isCreate = true;
>> +        me.isAdd = true;
>> +        me.url = '/pools/';
>> +        me.method = 'PUT';
>> +        me.extraRequestParams.poolid = me.pool;
>> +
>> +        Ext.apply(me, {
>> +            subject: gettext('VNet'),
>> +            width: 350,
>> +            items: [
>> +                {
>> +                    xtype: 'inputpanel',
>> +                    onGetValues: function (values) {
>> +                        let network = `zone=${values.zone},vnet=${values.vnet}`;
>> +
>> +                        if (values.tag) {
>> +                            network += `,tag=${values.tag}`;
>> +                        }
>> +
>> +                        delete values.zone;
>> +                        delete values.vnet;
>> +                        delete values.tag;
>> +
>> +                        values.network = network;
>> +
>> +                        return values;
>> +                    },
>> +                    items: [
>> +                        {
>> +                            xtype: 'pveSDNZoneSelector',
>> +                            fieldLabel: gettext('Zone'),
>> +                            name: 'zone',
>> +                            allowBlank: false,
>> +                            bind: {
>> +                                value: '{zone}',
>> +                            },
>> +                            listeners: {
>> +                                change: function (_f, _value) {
>> +                                    let vnetField = me.down('field[name=vnet]');
>> +                                    if (vnetField) {
>> +                                        vnetField.setValue('');
>> +                                    }
> nit: might be nice to use a 'reference' on 'vnet' and then reference it
>       here with lookupReference('vnet'), but no hard feelings.


Would make the code a little cleaner. I'll refactored it.


>> +                                },
>> +                            },
>> +                        },
>> +                        {
>> +                            xtype: 'pveSDNVnetSelector',
>> +                            fieldLabel: gettext('VNet'),
>> +                            name: 'vnet',
>> +                            allowBlank: false,
>> +                            bind: {
>> +                                disabled: '{vnetDisabled}',
>> +                            },
>> +                            listeners: {
>> +                                beforequery: function (_queryPlan) {
>> +                                    let zone = me.getViewModel().get('zone');
>> +                                    let store = this.getStore();
>> +                                    store.clearFilter();
>> +                                    store.filter('zone', zone);
>> +                                    return true;
>> +                                },
>> +                            },
>> +                        },
>> +                        {
>> +                            xtype: 'proxmoxintegerfield',
>> +                            name: 'tag',
>> +                            fieldLabel: gettext('VLAN Tag'),
>> +                            minValue: 1,
>> +                            maxValue: 4094,
>> +                            allowBlank: true,
>> +                            emptyText: gettext('All'),
>> +                        },
>> +                    ],
>> +                },
>> +            ],
>> +        });
>> +
>> +        me.callParent();
>> +    },
>> +});
>> +
>>   Ext.define('PVE.grid.PoolMembers', {
>>       extend: 'Ext.grid.GridPanel',
>>       alias: ['widget.pvePoolMembers'],
>> @@ -223,6 +325,18 @@ Ext.define('PVE.grid.PoolMembers', {
>>                       rec.data.type === 'openvz'
>>                   ) {
>>                       params.vms = rec.data.vmid;
>> +                } else if (rec.get('type') === 'network') {
>> +                    if (rec.get('network-type') === 'vnet') {
>> +                        let [_type, zone, vnet, tag] = rec.data.id.split('/');
>> +
>> +                        let network = `zone=${zone},vnet=${vnet}`;
>> +
>> +                        if (tag) {
>> +                            network += `,tag=${tag}`;
>> +                        }
>> +
>> +                        params.network = network;
>> +                    }
> should maybe console.warn() or something similar if the network type is
> not recognized?


That should never be the case, but adding an else here might at least make it
immediately visible. So I'll consider adding it.


>>                   } else {
>>                       throw 'unknown resource type';
>>                   }
>> @@ -268,6 +382,15 @@ Ext.define('PVE.grid.PoolMembers', {
>>                                       win.show();
>>                                   },
>>                               },
>> +                            {
>> +                                text: gettext('VNet'),
>> +                                iconCls: 'fa fa-network-wired x-fa-pool-net',
>> +                                handler: function () {
>> +                                    let win = Ext.create('PVE.pool.AddVnet', { pool: me.pool });
>> +                                    win.on('destroy', reload);
>> +                                    win.show();
>> +                                },
>> +                            },
>>                           ],
>>                       }),
>>                   },
>