Hey Guys, To give you an update, I've identified and categorized the functions in CitrixResourceBase into 4 categories
1) common: These deal with the host as a whole (example getHostInfo, callPlugin, connection pool etc) 2) compute: Dealing with operations on VMs (start,stop, reboot, update etc) 3) storage: Dealing with storage operations (creating VDI, attaching to VM, SR operations) 4) network: OVS / PIF / VIF operations I've created singleton classes for each and slowly moving the functionality there. These singletons don't have a base class nor do they implement an interface. I don't know what the best practice is. One question that I have is, should I create an interface for them or use some existing interface? Thanks, -Syed On Fri, May 20, 2016 at 9:58 AM, Syed Mushtaq <syed1.mush...@gmail.com> wrote: > Thanks guys for the Ideas. I will open a JIRA ticket and start working on > it. > > -Syed > > > On Thu, May 19, 2016 at 7:57 PM, Rafael Weingärtner < > rafaelweingart...@gmail.com> wrote: > >> Hi Syed, >> That is a great idea; however, it is a very hard task. >> The idea of Tim is great; actually, we already have some sort of hierarchy >> that is used in “CitrixResourceBase.java”. >> I would suggest you first removing the unused code, unused variable, and >> duplicate methods; that would be one PR. You can use a tool called >> UCdetector to find unused code. To find duplicated code you can use PMD. >> >> One very good example of code duplicity are the methods called >> >> “com.cloud.hypervisor.xenserver.resource.CitrixResourceBase.callHostPluginAsync(…)”. >> >> After you have cleaned the class, I suggest you analyzing where each >> remaining method is used and then look for the proper place to put them. >> It might be a good idea on separating between singletons that are >> responsible for well-defined tasks such as managing storage, networking, >> VMs creating and deletion, VMs monitoring and others. >> >> If you need any help, please do not hesitate on asking for our help. >> >> >> On Thu, May 19, 2016 at 4:50 PM, Daan Hoogland <daan.hoogl...@gmail.com> >> wrote: >> >> > Syed, >> > >> > gogogo. actually it has shrunk to 5k lines since 2012 ;) >> > >> > I like your initiative and initial direction. A lot of small steps to >> > improve the blob have been taken and I would sugest to keep going in >> small >> > steps. >> > >> > On Thu, May 19, 2016 at 9:44 PM, Tim Mackey <tmac...@gmail.com> wrote: >> > >> > > +1 >> > > >> > > When I went through this last time, not only was it hard to understand >> > the >> > > flows, but the XenServer version management was a pain. Would suggest >> > > creating a base class which always works (i.e. is independent of >> > XenServer >> > > version) for core functions. Then add in that which exists for a >> specific >> > > version. Should help greatly with testing IMO. >> > > >> > > -tim >> > > >> > > On Thu, May 19, 2016 at 2:37 PM, Syed Mushtaq < >> syed1.mush...@gmail.com> >> > > wrote: >> > > >> > > > Hi All, >> > > > >> > > > I would like to refactor CitrixResourceBase class which is >> responsible >> > > for >> > > > communicating with Xenserver. It has grown too long (>5K lines) and >> has >> > > > absolutely no testing. >> > > > >> > > > In my first pass I want to separate out the functionality buy the >> > > subsystem >> > > > it targets (compute, storage, network etc) and will go on from >> there. >> > > What >> > > > do you think? Is anyone working on this currently? >> > > > >> > > > Thanks, >> > > > -Syed >> > > > >> > > >> > >> > >> > >> > -- >> > Daan >> > >> >> >> >> -- >> Rafael Weingärtner >> > >
