----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10970/#review21191 -----------------------------------------------------------
Daan, I noticed quite a few tabs in your patch. The CloudStack standard is to use spaces instead of tabs. Can you change the patch to fix this? Other comments below. Can you explain what tests you executed to test the existing vlan functionality (besides the unittests)? api/src/com/cloud/network/Networks.java <https://reviews.apache.org/r/10970/#comment44007> Why not use value.getScheme() != null ? core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java <https://reviews.apache.org/r/10970/#comment44008> Dead code can be removed. server/src/com/cloud/api/ApiResponseHelper.java <https://reviews.apache.org/r/10970/#comment44009> Does this have anything to do anything with your changes? server/src/com/cloud/api/ApiResponseHelper.java <https://reviews.apache.org/r/10970/#comment44010> Why did this need to go? server/src/com/cloud/configuration/ConfigurationManagerImpl.java <https://reviews.apache.org/r/10970/#comment44011> Is this split covered in your code? server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java <https://reviews.apache.org/r/10970/#comment44012> Did you answer this question? Why it it here? - Hugo Trippaers On May 28, 2013, 9:43 a.m., daan Hoogland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10970/ > ----------------------------------------------------------- > > (Updated May 28, 2013, 9:43 a.m.) > > > Review request for cloudstack, Murali Reddy, Hugo Trippaers, and Chiradeep > Vittal. > > > Description > ------- > > converting vlan id to uri to support a broader range of networks in for > instance vpc gateway connections > > > Diffs > ----- > > api/src/com/cloud/agent/api/to/IpAddressTO.java 82c7d99 > api/src/com/cloud/agent/api/to/NetworkTO.java 3edd4c0 > api/src/com/cloud/network/NetworkService.java 59702a2 > api/src/com/cloud/network/Networks.java 5aede05 > api/src/com/cloud/network/vpc/PrivateIp.java eb68433 > api/src/com/cloud/network/vpc/StaticRouteProfile.java 54aa6e4 > api/src/com/cloud/network/vpc/VpcGateway.java 5d278e9 > api/src/com/cloud/network/vpc/VpcService.java 7a444c0 > > api/src/org/apache/cloudstack/api/command/admin/vpc/CreatePrivateGatewayCmd.java > 22dfb9e > api/src/org/apache/cloudstack/api/response/PrivateGatewayResponse.java > c5c7df5 > > core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java > 8b996d1 > engine/schema/src/com/cloud/network/vpc/VpcGatewayVO.java 7df2dfd > > plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/BaremetaNetworkGuru.java > 6d14e3f > > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java > b897df2 > > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java > f979cfe > > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java > eac3248 > plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java > a626e31 > > plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java > 630d1b4 > > plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java > 4680fde > > plugins/network-elements/cisco-vnmc/test/com/cloud/network/element/CiscoVnmcElementTest.java > a16733b > > plugins/network-elements/f5/src/com/cloud/network/resource/F5BigIpResource.java > 1733712 > > plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java > fd065d5 > > plugins/network-elements/netscaler/src/com/cloud/network/resource/NetscalerResource.java > c0d4599 > > plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java > b1ecaac > server/src/com/cloud/api/ApiResponseHelper.java 89739cf > server/src/com/cloud/configuration/ConfigurationManagerImpl.java 214e292 > server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java 9d24e47 > server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java > f93bf7a > server/src/com/cloud/network/ExternalLoadBalancerUsageManagerImpl.java > 2c8031c > server/src/com/cloud/network/NetworkManager.java 05bc26e > server/src/com/cloud/network/NetworkManagerImpl.java 254510b > server/src/com/cloud/network/NetworkServiceImpl.java 1533ca9 > server/src/com/cloud/network/guru/DirectPodBasedNetworkGuru.java cf27986 > server/src/com/cloud/network/guru/PrivateNetworkGuru.java 2e266e7 > server/src/com/cloud/network/guru/PublicNetworkGuru.java a83cdb3 > > server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java > 9992b7c > server/src/com/cloud/network/vpc/PrivateGatewayProfile.java d6480cd > server/src/com/cloud/network/vpc/PrivateIpAddress.java 2f3cf53 > server/src/com/cloud/network/vpc/VpcManagerImpl.java 1aab732 > server/test/com/cloud/network/CreatePrivateNetworkTest.java PRE-CREATION > server/test/com/cloud/network/MockNetworkManagerImpl.java e5d34fb > server/test/com/cloud/vpc/MockNetworkManagerImpl.java 7129273 > server/test/com/cloud/vpc/MockVpcManagerImpl.java 921321f > setup/db/db/schema-410to420.sql 1c9a8c1 > > Diff: https://reviews.apache.org/r/10970/diff/ > > > Testing > ------- > > the NetworkTO is tested to accept uris with several initial states. > createPrivateNetwork in NetworkServiceImpl is tested (to accept only vlan or > lswitch based networks for now) > test code that used to use 'vlan://#' now uses 'vlan:#' > > > Thanks, > > daan Hoogland > >
