Hi Syed, That is a great job. I would only suggest breaking the commons a little bit more between “monitoring” and “common”. On the monitoring side, we could have host monitoring, VMs monitoring, VMs' status checks (running, stopped, and others) and maybe other tasks that aim to monitor/check a resource state/information.
About the singletons you extracted, I do not see a need for an interface right now (maybe I am overlooking the need); in the future if the need for interfaces appears, we can create some interfaces and use them into the singletons you created. On Wed, May 25, 2016 at 12:10 PM, Syed Mushtaq <syed1.mush...@gmail.com> wrote: > 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 > >> > > > > > -- Rafael Weingärtner
