cool. :) *Will STEVENS* Lead Developer
*CloudOps* *| *Cloud Solutions Experts 420 rue Guy *|* Montreal *|* Quebec *|* H3J 1S6 w cloudops.com *|* tw @CloudOps_ On Wed, May 25, 2016 at 9:45 PM, Syed Mushtaq <syed1.mush...@gmail.com> wrote: > Nope. I think they should all be XenServer (they were Citrix earlier) I > plan to clean that up too. > > -Syed > > On Wed, May 25, 2016 at 9:43 PM, Will Stevens <williamstev...@gmail.com> > wrote: > > > Is there a reason some are prefixed with 'Xs' and others are prefixed > with > > 'XenServer'? > > On May 25, 2016 9:41 PM, "Will Stevens" <williamstev...@gmail.com> > wrote: > > > > > Ya. I didn't know either and made the same mistake. :) > > > On May 25, 2016 9:40 PM, "Syed Mushtaq" <syed1.mush...@gmail.com> > wrote: > > > > > >> Aah I did not know that. Thanks Will .. here is the link > > >> http://i.imgur.com/5B55XMB.png > > >> > > >> On Wed, May 25, 2016 at 6:37 PM, Will Stevens < > williamstev...@gmail.com > > > > > >> wrote: > > >> > > >> > Attachments don't work. You will have to host it publicly and then > > post > > >> a > > >> > link to it. > > >> > On May 25, 2016 4:28 PM, "Syed Ahmed" <sah...@cloudops.com> wrote: > > >> > > > >> > > Forgot to attach screenshot > > >> > > > > >> > > On Wed, May 25, 2016 at 4: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 > > >> > >>> > > >> > >> > > >> > >> > > >> > > > > >> > > > >> > > > > > >
