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

[Alexandre DERUMIER]

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

>>OK for me. 

Ok,I'll send a patch tomorrow

>>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) 
I have only 1 depend on qemu-server,pve-contairer with a dynamic 
include like pve-firewall

# dynamically include PVE::QemuServer and PVE::LXC
# to avoid dependency problems
my $have_qemu_server;
eval {
    require PVE::QemuServer;
    require PVE::QemuConfig;
    $have_qemu_server = 1;
};


It's only for 1 check on bridge removal. (to avoid removing a vnet 
where vm are running on it), but I think I can remove it now
than I have ifquery to verify status after reload.


>>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.. 

Yes, I was planning to move the network part too from Inotify to Network.pm.



----- Mail original -----
De: "Thomas Lamprecht" <t.lamprecht at proxmox.com>
À: "pve-devel" <pve-devel at pve.proxmox.com>, "aderumier" <aderumier at odiso.com>
Cc: "Fabian Grünbichler" <f.gruenbichler at proxmox.com>
Envoyé: Jeudi 6 Juin 2019 14:58:27
Objet: Re: [pve-devel] applied: [PATCH pve-network 0/2] improvments

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 
>