[pve-devel] applied: [PATCH pve-network 0/2] improvments

Thomas Lamprecht t.lamprecht at proxmox.com
Thu Jun 6 14:58:27 CEST 2019


On 6/6/19 10:16 AM, Alexandre DERUMIER wrote:
>>> On another note, is there any specific reason we have a double "Network"
>>> in the module namespace? Or could we change it to
>>>
>>> PVE/Network.pm
>>> PVE/Network/VlanPlugin.pm
>>> ...
> 
>>> and respective module name-path changes? As now, with not much external 
>>> code using this, it would be still relatively easy to change.. I mean
>>> it's nothing to important, but it irked me a little bit a few times ^^
>>> I could do the change, if you agree and have no objections :-)
> 
> This is because we already have a PVE/Network.pm.

ah yeah...

> 
> I wonder if I could rename this to PVE::Network::SDN  instead PVE::Network::Network, to avoid confusion ?

OK for me.

> 
> also, as I see that proxmox6 is now the master branch,
> 
> I think it could be great to move /PVE/API2/Network.pm  from pve-manager package
> and PVE/Network.pm from pve-common
> to this package.
> 
> (I would like to move the ifquery code to PVE/Network.pm, and begin to add SDN code in /PVE/API2/Network reload)
> 
> what do you think about this ?

As long as we have a clear dependency releationship between the
packages this could be OK. I.e., probably:

common <- network  <----------------- manager
            ^
             `- qemu-server/container <-/ 

Not sure what info / perl modules you all want to access from
pve-network, but guest will want to take use of pve-network and
manager too, so as long as you do not need to access any guest
(qemu-server, pve-container) and pve-manager modules from ther
we'd be good, pve-guest-common could probably be used.

looking at PVE::API2::Network's module usage it seems that it
would be movable without problems.

for PVE::Network we would need to either move PVE::INotify too,
or split it up and move the network part's from it only to
pve-network package, latter is IMO more reasonable..
Only problem here is that our Mailgateway uses INotify too quite
a bit so we'd need to take a closer look at this part...



 
> ----- Mail original -----
> De: "Thomas Lamprecht" <t.lamprecht at proxmox.com>
> À: "pve-devel" <pve-devel at pve.proxmox.com>, "aderumier" <aderumier at odiso.com>
> Envoyé: Jeudi 6 Juin 2019 08:32:24
> Objet: applied: [pve-devel] [PATCH pve-network 0/2] improvments
> 
> On 6/6/19 8:20 AM, Alexandre Derumier wrote: 
>> - fix multicast vxlan default mtu. (should be 50 bytes lower than physdev) 
>> - clean status sub from Thomas comments 
>>
>> Alexandre Derumier (2): 
>> vxlanmulticast : fix mtu 
>> cleanup status sub 
>>
>> PVE/Network/Network.pm | 13 ++++++++----- 
>> PVE/Network/Network/VlanPlugin.pm | 3 +-- 
>> PVE/Network/Network/VxlanMulticastPlugin.pm | 7 +++++-- 
>> test/generateconfig.pl | 3 ++- 
>> 4 files changed, 16 insertions(+), 10 deletions(-) 
>>
> 
> applied, thanks! 
> 
> On another note, is there any specific reason we have a double "Network" 
> in the module namespace? Or could we change it to 
> 
> PVE/Network.pm 
> PVE/Network/VlanPlugin.pm 
> ... 
> 
> and respective module name-path changes? As now, with not much external 
> code using this, it would be still relatively easy to change.. I mean 
> it's nothing to important, but it irked me a little bit a few times ^^ 
> I could do the change, if you agree and have no objections :-) 
> 
> cheers, 
> Thomas 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 






More information about the pve-devel mailing list