----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9898/#review17849 -----------------------------------------------------------
The patch set is so big that it is hard to review. It would be nice to separate it into management-server, plugin, agent plugin and UI. api/src/com/cloud/network/Network.java <https://reviews.apache.org/r/9898/#comment37807> IMHO, plugins may not have to create a Provider instance here. Instead, it can be created at plugin's configure method or getProvider, etc. server/src/com/cloud/network/NetworkManagerImpl.java <https://reviews.apache.org/r/9898/#comment37808> NetworkElement should be always called after NetworkGuru, isn't it? - Hiroaki Kawai On March 13, 2013, 9:32 a.m., Dave Cahill wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9898/ > ----------------------------------------------------------- > > (Updated March 13, 2013, 9:32 a.m.) > > > Review request for cloudstack, Hugo Trippaers and Chiradeep Vittal. > > > Description > ------- > > Feature spec: > https://cwiki.apache.org/confluence/display/CLOUDSTACK/Midokura+Networking+Plugin > > Jira ticket: > https://issues.apache.org/jira/browse/CLOUDSTACK-996 > > Notes: > > * Moved plugin to nonoss since the MidoNet client jar is not publicly > available > > * Documentation will follow as a separate commit > > * One main difference from existing networking plugins is the lack of a > Resource class; we didn't feel it was necessary in this case. As mentioned in > Extending CloudStack Networking [1]: > "Just like managers, resources are not strictly necessary. In theory a > Network Element could implement a client for the API of the new controller > and therefore be completely self-contained." > > * We allow overriding Public traffic via the MidoNetPublicNetworkGuru. We > checked this approach with the list [2] and received no comments, so we're > going with it for now. > > [1] https://cwiki.apache.org/CLOUDSTACK/extending-cloudstack-networking.html > [2] http://markmail.org/message/k5qse63eyylszm3i > > > This addresses bug CLOUDSTACK-996. > > > Diffs > ----- > > api/src/com/cloud/network/Network.java efed5cd > api/src/com/cloud/network/Networks.java e3d2158 > api/src/com/cloud/network/PhysicalNetwork.java 343a2b1 > api/src/org/apache/cloudstack/network/ExternalNetworkDeviceManager.java > bc22804 > client/pom.xml cda6ab8 > client/tomcatconf/nonossComponentContext.xml.in 20e0c32 > deps/install-non-oss.sh 74575a8 > plugins/hypervisors/kvm/pom.xml 013a58d > > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java > b622b6d > > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java > c93aeeb > plugins/network-elements/midokura-midonet/pom.xml 7f2e2d3 > plugins/network-elements/midonet/pom.xml PRE-CREATION > > plugins/network-elements/midonet/src/com/cloud/network/element/MidoNetElement.java > 48833b3 > > plugins/network-elements/midonet/src/com/cloud/network/element/SimpleFirewallRule.java > PRE-CREATION > > plugins/network-elements/midonet/src/com/cloud/network/guru/MidoNetGuestNetworkGuru.java > ed0eb3c > > plugins/network-elements/midonet/src/com/cloud/network/guru/MidoNetPublicNetworkGuru.java > PRE-CREATION > > plugins/network-elements/midonet/src/com/cloud/network/resource/MidoNetVifDriver.java > PRE-CREATION > > plugins/network-elements/midonet/test/com/cloud/network/element/MidoNetElementTest.java > PRE-CREATION > plugins/pom.xml 88f617b > server/src/com/cloud/configuration/Config.java 64465a2 > server/src/com/cloud/network/NetworkManagerImpl.java 3220c91 > ui/scripts/system.js 4d529ae > > Diff: https://reviews.apache.org/r/9898/diff/ > > > Testing > ------- > > Built and deployed, spun up Advanced Isolated network with two VMs, verified > internal and external connectivity via MidoNet. > > > Thanks, > > Dave Cahill > >
