cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "edison su" <edison...@citrix.com>
Subject Re: Review Request: Add the ability to specify different VIF drivers per traffic type in KVM
Date Mon, 04 Mar 2013 18:54:41 GMT


> On March 2, 2013, 1:10 a.m., edison su wrote:
> > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java,
line 802
> > <https://reviews.apache.org/r/9604/diff/1/?file=262223#file262223line802>
> >
> >     Wondering, what's the purpose of network.bridge.type used for? Can we treat
com.cloud.hypervisor.kvm.resource.OvsVifDriver as another vif driver? Then libvirt.vif.driver
is enough?
> 
> Dave Cahill wrote:
>     Hi Edison,
>     
>     That's a good point. I would guess that if we did that, for example we would move
the libvirt version check from LibvirtComputingResource into OvsVifDriver.configure(). I'll
leave the final word to Hugo though, there may be other reasons to keep bridge type.
>     
>     Thanks,
>     Dave.
> 
> Hugo Trippaers wrote:
>     I add the network.bridge.type to allow the libvirt resource to know what bridge it
should configure. Mainly because i didn't like to use classname (from libvirt.vif.driver)
in this check. It could theoretically be possible to have different vif drivers that use the
same configuration method in the resource. I think the configuration of the bridge should
actually be left to the vif driver as well, but i didn't want to implement this for this release.
>     
>     Is this causing any problems? Otherwise i would propose to use the code as is and
maybe fix in 4.2?

Not a problem at all, It's all up to you:) So the patch is ok to me.


- edison


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


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