cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Murali Reddy" <muralimmre...@gmail.com>
Subject Re: Review Request: Nicira NVP integration for CloudStack
Date Thu, 05 Jul 2012 23:36:40 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5590/#review8903
-----------------------------------------------------------


Hugo, code in general looks good. Below are some minor comments. Is there is trial version
of NVP controller that can used for trying out integration or for testing?

- In CitrixResourceBase it seems to me that Nicira config params are added for every VIF that
is created. Should it be done only for the network's using STT isolation?

- In GuestNetworkGuru:

   + In isMyIsolationMethod(), if no isolation method is configured, its treated as acceptable
isolation by GuestNetworkGuru. I suggest a guru check only for what isolation it can support.

   + Whats is the rational for canHanlde() being implemented in each of the implementation
of GuestNetworkGuru? I think single abstract implementations should be fine. Do you see that
logic will change across the implementations?

- CloudStack Account names are not unique. So please use a combination of domain id + account
name or some other unique tag when creating a logical switch.

- I see that it is permitted to configure multiple NVP controllers per physical network? 
But always use the first controller available for the physical network when implementing the
network. Is it intended behavior?

- Not sure you mentioned this in the your earlier mails. Does this implementation work only
with Xen? or works with Kvm, Vmware as well?

- Murali Reddy


On June 26, 2012, 4:56 p.m., Hugo Trippaers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5590/
> -----------------------------------------------------------
> 
> (Updated June 26, 2012, 4:56 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Patch to add Nicira NVP support to CloudStack. As discussed this patch is related to
phase 1, which is basic L2 connectivity. L3 connectivity and integration with the network
offering for SNAT will be in phase2.
> 
> 
> Diffs
> -----
> 
>   README.NiciraIntegration PRE-CREATION 
>   api/src/com/cloud/agent/api/CreateLogicalSwitchAnswer.java PRE-CREATION 
>   api/src/com/cloud/agent/api/CreateLogicalSwitchCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/CreateLogicalSwitchPortAnswer.java PRE-CREATION 
>   api/src/com/cloud/agent/api/CreateLogicalSwitchPortCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DeleteLogicalSwitchAnswer.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DeleteLogicalSwitchCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DeleteLogicalSwitchPortAnswer.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DeleteLogicalSwitchPortCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/StartupNiciraNvpCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/to/NicTO.java b65c61ee47c03bf33bcbf2ee10a2f8d6405538f0

>   api/src/com/cloud/agent/api/to/VirtualMachineTO.java 42d91626e35d20c960561ca1101aad17dc19febb

>   api/src/com/cloud/api/ApiConstants.java 00ec392d7b930a1ec0d2d77c2cccd035bc9f3321 
>   api/src/com/cloud/host/Host.java 0c9d06d48bfdaf1f491c71cda6e1d878214a2dd1 
>   api/src/com/cloud/network/Network.java 0443a0f41cc112105b48bb80dae58f6f3adc4b8d 
>   api/src/com/cloud/network/Networks.java 84135b8ee45489fb6c284c6cf3ae0356596cbc83 
>   api/src/com/cloud/network/PhysicalNetwork.java e54fe00bef0846ec96a3b3dde43ba433bab6f1ac

>   client/tomcatconf/components.xml.in 58541a59606aebb69c1e9566934736b1a6f86b21 
>   client/tomcatconf/nicira-nvp_commands.properties.in PRE-CREATION 
>   core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 39172423e36f06f3721a9b108f0f05f5008f5613

>   core/src/com/cloud/network/nicira/Attachment.java PRE-CREATION 
>   core/src/com/cloud/network/nicira/LogicalSwitch.java PRE-CREATION 
>   core/src/com/cloud/network/nicira/LogicalSwitchPort.java PRE-CREATION 
>   core/src/com/cloud/network/nicira/LogicalSwitchPortList.java PRE-CREATION 
>   core/src/com/cloud/network/nicira/NiciraNvpApi.java PRE-CREATION 
>   core/src/com/cloud/network/nicira/NiciraNvpApiException.java PRE-CREATION 
>   core/src/com/cloud/network/nicira/NiciraNvpTag.java PRE-CREATION 
>   core/src/com/cloud/network/nicira/TransportZoneBinding.java PRE-CREATION 
>   core/src/com/cloud/network/nicira/VifAttachment.java PRE-CREATION 
>   core/src/com/cloud/network/resource/NiciraNvpResource.java PRE-CREATION 
>   server/src/com/cloud/api/commands/AddNiciraNvpDeviceCmd.java PRE-CREATION 
>   server/src/com/cloud/api/commands/DeleteNiciraNvpDeviceCmd.java PRE-CREATION 
>   server/src/com/cloud/api/commands/ListNiciraNvpDeviceNetworksCmd.java PRE-CREATION

>   server/src/com/cloud/api/commands/ListNiciraNvpDevicesCmd.java PRE-CREATION 
>   server/src/com/cloud/api/response/NiciraNvpDeviceResponse.java PRE-CREATION 
>   server/src/com/cloud/configuration/DefaultComponentLibrary.java 3bfe84df25dd4027fdaad48b5ea2c24fff2ce432

>   server/src/com/cloud/host/dao/HostDaoImpl.java 745567dd7d260de5fab0945d72f89a8133cca4c4

>   server/src/com/cloud/hypervisor/HypervisorGuruBase.java d62e3800542e0394203506b25f5eb44c5fc90308

>   server/src/com/cloud/network/ExternalNetworkDeviceManager.java e09ff8e9580612ae010f550fc6f0a24b9cfd971d

>   server/src/com/cloud/network/NetworkManagerImpl.java 1f306ab07d136a785b9b433122fcd21418da12eb

>   server/src/com/cloud/network/NiciraNvpDeviceVO.java PRE-CREATION 
>   server/src/com/cloud/network/NiciraNvpNicMappingVO.java PRE-CREATION 
>   server/src/com/cloud/network/dao/NiciraNvpDao.java PRE-CREATION 
>   server/src/com/cloud/network/dao/NiciraNvpDaoImpl.java PRE-CREATION 
>   server/src/com/cloud/network/dao/NiciraNvpNicMappingDao.java PRE-CREATION 
>   server/src/com/cloud/network/dao/NiciraNvpNicMappingDaoImpl.java PRE-CREATION 
>   server/src/com/cloud/network/element/NiciraNvpElement.java PRE-CREATION 
>   server/src/com/cloud/network/element/NiciraNvpElementService.java PRE-CREATION 
>   server/src/com/cloud/network/guru/ExternalGuestNetworkGuru.java 6c381f2b18eb559e34b3d4d51329df0b7d8f5afb

>   server/src/com/cloud/network/guru/GuestNetworkGuru.java f9977102b1f4b7652a87f3b6c7e04e44aae6a2d8

>   server/src/com/cloud/network/guru/NiciraNvpGuestNetworkGuru.java PRE-CREATION 
>   server/src/com/cloud/network/guru/OvsGuestNetworkGuru.java d031feeb6ebb40155234d6b856746c8a5eab80de

>   setup/db/create-schema.sql afcee3f9f6f48d1e0da42f9f952018aec0904929 
>   ui/scripts/ui-custom/zoneWizard.js d46bad9800bac5b671fbe40c8456ab2da19e5531 
> 
> Diff: https://reviews.apache.org/r/5590/diff/
> 
> 
> Testing
> -------
> 
> Simple build check
>   clean-all build-all
> 
> Testing of all api calls
>  * addNiciraNvpDevice
>  * deleteNiciraNvpDevice		
>  * listNiciraNvpDevices
>  * listNiciraNvpDeviceNetwork
> 
> Functional testing using the following procedure
> * start from clean db and create zone (with guest traffic on a physical network with
stt isolation type)
> * add NiciraNvp network service provider and enable
> * add NiciraNcpDevice to physical network and configure using api
> * create guestnetwork
> * create instance linked to guest network
> * check existence of logical switch and logical ports for routervm and instance
> * check connectivity between routervm and instance
> * destroy host
> * after shutdown of routervm and network check logical switch and ports on nicira (should
be gone)
> 
> 
> Thanks,
> 
> Hugo Trippaers
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message