----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12941/#review23827 -----------------------------------------------------------
Heya, There is some feedback from reading the diffs. In general the patch looks good, i'll try and implement a setup based on this so i can test it. I'm assuming that with this patch we can get the L2 part of the GRE based SDN working? Can you explain what testing you did to prove that this code works? What did your setup look like and what tests did you execute? Cheers, Hugo api/src/com/cloud/network/Network.java <https://reviews.apache.org/r/12941/#comment47666> Please double check the code style. Not tabs, 4 spaces indent for example. The full style guide is available on confluence. plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java <https://reviews.apache.org/r/12941/#comment47667> Please check the style guide :-) No wild card imports. http://cloudstack.apache.org/develop/coding-conventions.html plugins/network-elements/ovs/src/com/cloud/network/element/OvsElement.java <https://reviews.apache.org/r/12941/#comment47668> You need the place holders, but to keep that patch clean it's often better to do that in an other patch. One patch with the functionality that is ready and can be used. Another one that will enabled place holders for future usage. Now we can't apply this patch to master because you advertise functionality that is not really implemented. plugins/network-elements/ovs/src/com/cloud/network/guru/OvsGuestNetworkGuru.java <https://reviews.apache.org/r/12941/#comment47670> This should be gone, determining if the GRE tunnel manager should be used must happen based on the settings of the physical network isolation type "GRE". setup/db/create-schema.sql <https://reviews.apache.org/r/12941/#comment47671> Please don't make changes to this file. Changes should be processed by the upgrade scripts. See http://markmail.org/message/uo7yq2qws2amjkfu - Hugo Trippaers On July 25, 2013, 9:05 a.m., tuna wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12941/ > ----------------------------------------------------------- > > (Updated July 25, 2013, 9:05 a.m.) > > > Review request for cloudstack, Sebastien Goasguen and Hugo Trippaers. > > > Repository: cloudstack-git > > > Description > ------- > > I made an update to refactor gre controller: > > + remove ovs_devices table, because we'll have an ODL plugin separately. > + move command/answer to new package: com.cloud.agent.api > + add Connectivity service checking > + add new NetworkProvider: Ovs > + add L3 services to Ovs Capabilities > + add L3 services prototype code. > > Next step: > + L3 services implement with VirtualRouter > + ODL plugin > > > Diffs > ----- > > api/src/com/cloud/network/Network.java a06208b > client/tomcatconf/applicationContext.xml.in 60f1e30 > > plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java > 30b0521 > > plugins/network-elements/ovs/src/com/cloud/agent/api/OvsCreateGreTunnelAnswer.java > PRE-CREATION > > plugins/network-elements/ovs/src/com/cloud/agent/api/OvsCreateGreTunnelCommand.java > PRE-CREATION > > plugins/network-elements/ovs/src/com/cloud/agent/api/OvsCreateTunnelAnswer.java > PRE-CREATION > > plugins/network-elements/ovs/src/com/cloud/agent/api/OvsCreateTunnelCommand.java > PRE-CREATION > > plugins/network-elements/ovs/src/com/cloud/agent/api/OvsDeleteFlowCommand.java > PRE-CREATION > > plugins/network-elements/ovs/src/com/cloud/agent/api/OvsDestroyBridgeCommand.java > PRE-CREATION > > plugins/network-elements/ovs/src/com/cloud/agent/api/OvsDestroyTunnelCommand.java > PRE-CREATION > > plugins/network-elements/ovs/src/com/cloud/agent/api/OvsFetchInterfaceAnswer.java > PRE-CREATION > > plugins/network-elements/ovs/src/com/cloud/agent/api/OvsFetchInterfaceCommand.java > PRE-CREATION > > plugins/network-elements/ovs/src/com/cloud/agent/api/OvsSetTagAndFlowAnswer.java > PRE-CREATION > > plugins/network-elements/ovs/src/com/cloud/agent/api/OvsSetTagAndFlowCommand.java > PRE-CREATION > > plugins/network-elements/ovs/src/com/cloud/agent/api/OvsSetupBridgeCommand.java > PRE-CREATION > plugins/network-elements/ovs/src/com/cloud/agent/api/StartupOvsCommand.java > PRE-CREATION > > plugins/network-elements/ovs/src/com/cloud/api/response/OvsDeviceResponse.java > c0901b2 > > plugins/network-elements/ovs/src/com/cloud/network/commands/AddOvsDeviceCmd.java > 1abc324 > > plugins/network-elements/ovs/src/com/cloud/network/commands/DeleteOvsDeviceCmd.java > 87eedfb > > plugins/network-elements/ovs/src/com/cloud/network/commands/ListOvsDevicesCmd.java > 2adb33a > plugins/network-elements/ovs/src/com/cloud/network/element/OvsElement.java > 0ea6b52 > > plugins/network-elements/ovs/src/com/cloud/network/element/OvsElementService.java > b55fe6b > > plugins/network-elements/ovs/src/com/cloud/network/guru/OvsGuestNetworkGuru.java > bbdf110 > plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsApi.java b533312 > plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsApiException.java > 20603e0 > > plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsCreateGreTunnelAnswer.java > 5f0f8c1 > > plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsCreateGreTunnelCommand.java > e2cd2d8 > > plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsCreateTunnelAnswer.java > fc2eb8a > > plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsCreateTunnelCommand.java > 1ececa0 > > plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsDeleteFlowCommand.java > 2a6d5d7 > > plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsDestroyBridgeCommand.java > 8be5586 > > plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsDestroyTunnelCommand.java > 4594d99 > > plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsFetchInterfaceAnswer.java > 1ee6606 > > plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsFetchInterfaceCommand.java > c27daf0 > > plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsSetTagAndFlowAnswer.java > ba16839 > > plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsSetTagAndFlowCommand.java > 17121a0 > > plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsSetupBridgeCommand.java > 29cce15 > > plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java > b1ecaac > > plugins/network-elements/ovs/src/com/cloud/network/ovs/StartupOvsCommand.java > b85331e > > plugins/network-elements/ovs/src/com/cloud/network/ovs/dao/OvsDeviceDao.java > 794e45e > > plugins/network-elements/ovs/src/com/cloud/network/ovs/dao/OvsDeviceDaoImpl.java > 11a4d48 > plugins/network-elements/ovs/src/com/cloud/network/ovs/dao/OvsDeviceVO.java > cab63f6 > > plugins/network-elements/ovs/src/com/cloud/network/resource/OvsResource.java > a94e4f8 > setup/db/create-schema.sql 143023a > > Diff: https://reviews.apache.org/r/12941/diff/ > > > Testing > ------- > > > Thanks, > > tuna > >
