cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Syed Mushtaq <syed1.mush...@gmail.com>
Subject Re: Refactoring CitrixResourceBase
Date Wed, 25 May 2016 15:10:41 GMT
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.mushtaq@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 <
> rafaelweingartner@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.hoogland@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 <tmackey@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.mushtaq@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
>>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message