----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11027/#review20416 -----------------------------------------------------------
core/src/com/cloud/agent/api/routing/createIpAliasCommand.java <https://reviews.apache.org/r/11027/#comment42065> Start of class name should be in caps. Same for file name. core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java <https://reviews.apache.org/r/11027/#comment42067> createIpAliasCmd->CreateIpAliasCmd core/src/com/cloud/network/DnsMasqConfigurator.java <https://reviews.apache.org/r/11027/#comment42064> It is better to move this to a separate file so that any changes to the content doesn't require a compilation core/src/com/cloud/network/DnsMasqConfigurator.java <https://reviews.apache.org/r/11027/#comment42066> You are appending all the dns entries without any separators engine/schema/src/com/cloud/network/dao/IPAddressDaoImpl.java <https://reviews.apache.org/r/11027/#comment42063> Fix spacing patches/systemvm/debian/config/root/createIpAlias.sh <https://reviews.apache.org/r/11027/#comment42060> Whats this comemnt about? patches/systemvm/debian/config/root/createIpAlias.sh <https://reviews.apache.org/r/11027/#comment42061> Make the logs useful ir remove them patches/systemvm/debian/config/root/deleteIpAlias.sh <https://reviews.apache.org/r/11027/#comment42059> Remove unwanted whitespaces patches/systemvm/debian/config/root/deleteIpAlias.sh <https://reviews.apache.org/r/11027/#comment42062> Some comment explaining why createIpAlias.sh is required would help patches/systemvm/debian/config/root/dnsmasq.sh <https://reviews.apache.org/r/11027/#comment42057> paqth -> path patches/systemvm/debian/config/root/dnsmasq.sh <https://reviews.apache.org/r/11027/#comment42058> This needs to be uncommented. Also remove unwanted whitespaces scripts/vm/hypervisor/xenserver/createipAlias.sh <https://reviews.apache.org/r/11027/#comment42068> Unwanted comment scripts/vm/hypervisor/xenserver/createipAlias.sh <https://reviews.apache.org/r/11027/#comment42069> Remove whitespaces server/src/com/cloud/configuration/ConfigurationManager.java <https://reviews.apache.org/r/11027/#comment42070> Is this file required to be modified? server/src/com/cloud/network/router/VirtualNetworkApplianceManager.java <https://reviews.apache.org/r/11027/#comment42071> configDhcp -> configDhcpForSubnet server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java <https://reviews.apache.org/r/11027/#comment42073> Again configDhcp, change it to correct name. - Koushik Das On May 10, 2013, 6:54 a.m., bharat kumar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11027/ > ----------------------------------------------------------- > > (Updated May 10, 2013, 6:54 a.m.) > > > Review request for cloudstack, Abhinandan Prateek and Koushik Das. > > > Description > ------- > > Cloudstack-702. Multiple ip ranges in different subnets. > > This feature enables adding of guest ip ranges (public ips) form different > subnets. > > In order to provide the dhcp service to a different subnet we create an > ipalias on the router. This allows the router to listen to the dhcp request > from the guest vms and respond accordingly. Every time a vm is deployed in > the new subnet we configure an ip alias on the router. Cloudstack uses > dnsmasq to provide dhcp service. We need to configure the dnsmasq to issue > ips on the new subnets. Added a new class dnsmasqconfigurator which generates > the dnsmasq confg file, this file replaces the old config in the router. > > The details of the alias ips are stored in db in the nic_ip_alias table. > Every time a new subnet is added one of the ip from the subnet is used to > configure the ip alias. > > I have pushed the code to > https://github.com/bvbharatk/cloud-stack/tree/Cloudstack-702 , also rebased > the code with master. > I need to test the code for advanced sg enabled network using kvm. > > This is duplicate of the pervious review request > https://reviews.apache.org/r/10948/. please ignore it. > > I have added the unit test > > Also accomodated some of the changes suggested by koushik. > corrected the import statements. renamed the IpAlias command to createIpAlias > command. > > > This addresses bug Cloudstack-702. > > > Diffs > ----- > > api/src/com/cloud/agent/api/to/DnsmasqTO.java PRE-CREATION > api/src/com/cloud/exception/MissingParameterValueException.java > PRE-CREATION > api/src/com/cloud/network/element/DhcpServiceProvider.java f73590c > api/src/com/cloud/vm/NicIpAlias.java PRE-CREATION > client/tomcatconf/applicationContext.xml.in 36f232c > core/src/com/cloud/agent/api/routing/DeleteIpAliasCommand.java PRE-CREATION > core/src/com/cloud/agent/api/routing/DnsMasqConfigCommand.java PRE-CREATION > core/src/com/cloud/agent/api/routing/IpAliasTO.java PRE-CREATION > core/src/com/cloud/agent/api/routing/createIpAliasCommand.java PRE-CREATION > > core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java > b9bda4d > core/src/com/cloud/network/DnsMasqConfigurator.java PRE-CREATION > engine/schema/src/com/cloud/dc/dao/VlanDao.java cc82632 > engine/schema/src/com/cloud/dc/dao/VlanDaoImpl.java 100295b > engine/schema/src/com/cloud/network/dao/IPAddressDao.java 3d588fa > engine/schema/src/com/cloud/network/dao/IPAddressDaoImpl.java 73f310f > engine/schema/src/com/cloud/vm/dao/NicDao.java 67c1e07 > engine/schema/src/com/cloud/vm/dao/NicDaoImpl.java c70d194 > patches/systemvm/debian/config/root/createIpAlias.sh PRE-CREATION > patches/systemvm/debian/config/root/deleteIpAlias.sh PRE-CREATION > patches/systemvm/debian/config/root/dnsmasq.sh PRE-CREATION > > plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/BaremetalDhcpElement.java > b72d1c8 > > plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java > d87da64 > > plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java > bac361d > > plugins/network-elements/midonet/src/com/cloud/network/element/MidoNetElement.java > 804e4a6 > scripts/vm/hypervisor/xenserver/createipAlias.sh PRE-CREATION > scripts/vm/hypervisor/xenserver/deleteipAlias.sh PRE-CREATION > scripts/vm/hypervisor/xenserver/vmops 30b5300 > scripts/vm/hypervisor/xenserver/xcpserver/patch b7961bb > scripts/vm/hypervisor/xenserver/xenserver56/patch 36dba3d > scripts/vm/hypervisor/xenserver/xenserver56fp1/patch d20e60f > scripts/vm/hypervisor/xenserver/xenserver60/patch c9125f4 > server/src/com/cloud/configuration/ConfigurationManager.java bce1776 > server/src/com/cloud/configuration/ConfigurationManagerImpl.java 37ca793 > server/src/com/cloud/network/NetworkManager.java 4af716c > server/src/com/cloud/network/NetworkManagerImpl.java 4fffbc1 > server/src/com/cloud/network/addr/PublicIp.java 25e9d30 > server/src/com/cloud/network/element/VirtualRouterElement.java f601f4f > server/src/com/cloud/network/router/VirtualNetworkApplianceManager.java > f49ab79 > server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java > 6620e0a > server/src/com/cloud/vm/dao/NicIpAliasDao.java PRE-CREATION > server/src/com/cloud/vm/dao/NicIpAliasDaoImpl.java PRE-CREATION > server/src/com/cloud/vm/dao/NicIpAliasVO.java PRE-CREATION > server/test/com/cloud/configuration/ValidateIpRangeTest.java PRE-CREATION > server/test/com/cloud/network/MockNetworkManagerImpl.java 2f717c8 > server/test/com/cloud/vpc/MockConfigurationManagerImpl.java 83b1924 > server/test/com/cloud/vpc/MockNetworkManagerImpl.java f884ba1 > server/test/com/cloud/vpc/MockVpcVirtualNetworkApplianceManager.java > ef5478b > setup/db/db/schema-410to420.sql c8ac1ec > > Diff: https://reviews.apache.org/r/11027/diff/ > > > Testing > ------- > > Tested this with xenserver in basic zone. > > > Thanks, > > bharat kumar > >
