----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9604/#review17256 -----------------------------------------------------------
Dave, looks good. Couple of questions though, there is some bridge specific code in LibvirtComputingResource that checks existence of interfaces and such. Shouldn't that code also be modified to support multiple drivers per nic? Also currently there is only the native bridge and openvswitch in here. The actual recommendation is to use only one of them as far as i know. What is the compelling reason to allow for this difference? In most cases where openvswitch is used this means configuring vswitch either via the native interface or via the brcompat layer. While possible to combine the two it could lead to disastrous results when interfaces start overlapping. E.g. both openvswitch and bridge create an interface called br0 (i've been there ;-) ) - Hugo Trippaers On March 1, 2013, 7:46 a.m., Dave Cahill wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9604/ > ----------------------------------------------------------- > > (Updated March 1, 2013, 7:46 a.m.) > > > Review request for cloudstack, Hugo Trippaers, Wido den Hollander, and Marcus > Sorensen. > > > Description > ------- > > Feature spec: > https://cwiki.apache.org/confluence/display/CLOUDSTACK/Allow+specification+of+different+VIF+drivers+per+traffic+type+in+KVM > > Jira ticket: > https://issues.apache.org/jira/browse/CLOUDSTACK-1311 > > Adds the ability to specify different VIF drivers per traffic type in KVM. > > > This addresses bug CLOUDSTACK-1311. > > > Diffs > ----- > > > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java > 99b8723 > > plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtVifDriverTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/9604/diff/ > > > Testing > ------- > > Added unit tests to test all variations of this new configuration (no > configuration, defaults, override some traffic etc). > > Built and deployed, spun up Advanced Isolated network with two VMs, verified > internal and external connectivity. > > > Thanks, > > Dave Cahill > >
