incubator-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: Add the ability to specify different VIF drivers per traffic type in KVM
Date Mon, 04 Mar 2013 08:37:33 GMT


> On March 1, 2013, 7:36 p.m., Hugo Trippaers wrote:
> > 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 wrote:
>     Scratch the first question, don't know what i was thinking... :-)
> 
> Dave Cahill wrote:
>     Hi Hugo,
>     
>     Thanks for the review and comments!
>     
>     The Motivation section of the FS gives some background on why we want to allow mixing
VIF drivers: 
>     https://cwiki.apache.org/confluence/display/CLOUDSTACK/Allow+specification+of+different+VIF+drivers+per+traffic+type+in+KVM
>     
>     <pasted>
>     "The MidoNet plugin intends to handle Guest traffic for Guest and System VMs, and
Public traffic for System VMs (Public traffic). 
>     
>     However, we wish to let Management / Control traffic be handled by the underlying
physical network using the default BridgeVifDriver. 
>     Currently, this is not possible without duplicating BridgeVifDriver code into the
MidonetVifDriver, as LibVirtComputingResource assumes there is only one VIF driver."
>     
>     Control traffic is probably the clearest example. This traffic is all local to the
host, so sending it through a virtual network is overkill. We could copy the control traffic
logic (setting up link local network etc) into the MidoNet VIF driver, but what if that logic
changes later? The duplication seems very undesirable to me.
>     
>     Since the MidoNet plugin is not checked in, I can see how the use case is less obvious.
However, I figured this is a fully backwards compatible change, and I wanted to propose and
commit it first, so that the plugin commit is self-contained.
>     
>     Hope that makes sense - let me know if you have any follow-ups. Also, I'm going to
reply to Edison's comment below, but I'll need your input too.
>     
>     Thanks,
>     Dave.
>

Hey Dave,

Makes sense now :-) I'm all ok of committing this to master, but i'll wait for the discussion
to finish on the issue below

What is the estimated time for the MidoNet plugin to hit the reviewboard? I think we don't
need to put this in 4.1?


- Hugo


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9604/#review17256
-----------------------------------------------------------


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


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