Indeed, this is a non-trivial job, and it will take a pretty good amount of time and effort.
Sorry, I did not understand what you meant with "reference" file. Is that the base file that you are using to create a hierarchy? The solution you are creating looks promising, I am just wondering; you said that you are creating singletons, you probably are thinking about Spring singletons, right? There is only one problem, the “CitrixResourceBase” and its subclasses were not being wired using Spring-framework. Have you noticed that? To add those new classes (singletons) you are creating and wire them properly into the Spring life cycle may require some work, if you ran into a wall, please let us know, so we can help you. On Wed, May 25, 2016 at 5:09 PM, Syed Ahmed <sah...@cloudops.com> wrote: > Thanks Rafael, > > Here is how I am doing. You can see the tree in the screenshot attached. I > have started with storage and extracted the storage commands out of > CitrixResourceBase (I've renamed it to XenServerResourceBase). This is the > file for refrence. Let me know what you think. > > > https://github.com/syed/cloudstack/blob/ca2d1469f991727515d9b71a790411d36eb60fdc/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/storage/XenServerStorageResource.java > > It is a non-trivial refactor so may take some time but I think the end > result would be very good for everyone. > > > > On Wed, May 25, 2016 at 11:22 AM, Rafael Weingärtner < > rafaelweingart...@gmail.com> wrote: > > > 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 > > > -- Rafael Weingärtner
