cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Hugo Trippaers" <htrippa...@schubergphilis.com>
Subject Re: Review Request 12941: [GSoC] refactor gre controller
Date Thu, 25 Jul 2013 09:57:32 GMT

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


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