On 8/3/12 7:29 PM, "Tomoe Sugihara" <to...@midokura.com> wrote:
> > >> On Aug. 3, 2012, 10:03 p.m., Chiradeep Vittal wrote: >> > >>plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVifD >>river.java, line 35 >> > >><https://reviews.apache.org/r/6285/diff/3/?file=133630#file133630line35> >> > >> > This should be an Interface rather than an abstract class >> > The interface should belong in com.cloud.resource in core and >>should be called VifDriver. You can have a subclass that is abstract >>(VifDriverBase) as well. The constructor should not throw exceptions, >>instead you should have an abstract configure() method that throws a >>ConfigurationException > >Thanks for the review. Sure, I can do that. >Note that VifDriver will be hypervisor specific (signature differes), so >maybe I should create a interface com.cloud.hypervisor.kvm.VifDriver for >KVM, com.cloud.hypervisor.xen.VifDriver for Xen. What do you think? Hopefully you can make it more generic than that. It seems that the main problem is the plug method. If not, that is OK > > >- Tomoe > > >----------------------------------------------------------- >This is an automatically generated e-mail. To reply, visit: >https://reviews.apache.org/r/6285/#review9837 >----------------------------------------------------------- > > >On Aug. 2, 2012, 11:18 p.m., Tomoe Sugihara wrote: >> >> ----------------------------------------------------------- >> This is an automatically generated e-mail. To reply, visit: >> https://reviews.apache.org/r/6285/ >> ----------------------------------------------------------- >> >> (Updated Aug. 2, 2012, 11:18 p.m.) >> >> >> Review request for cloudstack, edison su and Chiradeep Vittal. >> >> >> Description >> ------- >> >> Remove methods that are no longer needed in >>LibvirtComputingResource.java >> >> Signed-off-by: Tomoe Sugihara <to...@midokura.com> >> >> Add unplug() hook >> >> Signed-off-by: Tomoe Sugihara <to...@midokura.com> >> >> Add vif driver support >> >> - define abstract class named LibvirtVifDriver >> - add LibvirtBridgeVifDriver to support current network implementation >> - unplug() will be added >> >> Signed-off-by: Tomoe Sugihara <to...@midokura.com> >> >> >> Diffs >> ----- >> >> >>plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtBrid >>geVifDriver.java PRE-CREATION >> >>plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComp >>utingResource.java 73101a9896c4863bc75502ff8b0b6daceff52dc4 >> >>plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVifD >>river.java PRE-CREATION >> >> Diff: https://reviews.apache.org/r/6285/diff/ >> >> >> Testing >> ------- >> >> Manually tested launching VM on all-in-one KVM box with Advanced >>networking. >> >> >> Thanks, >> >> Tomoe Sugihara >> >> >
