cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chiradeep Vittal <Chiradeep.Vit...@citrix.com>
Subject Re: Nicira integration pre-review
Date Fri, 15 Jun 2012 06:55:52 GMT
That's a big patch set. You should split it up, e.g.,
Patch Set #1: Nicira API glue
Patch Set #2: Guru and Element
Patch Set #3: etc.

Since this is a pre-review, I will give general comments:
 - the coding guide is here:
http://docs.cloud.com/CloudStack_Documentation/Design_Documents/Coding_Conv
entions
   Note variable conventions, class and interface javadoc etc.
 - NiciraElement is incomplete it appears, so at least a //TODO:(Hugo) do
blah here would be nice. Also, NiciraNvpElement would be better since they
probably will have other products
 - It is a convention that actual API calls to external elements are made
from within a Resource. The NiciraElement would be expected to call an API
on the NiciraManager. The NiciraManager would construct relevant messages,
for example: CreateAndAttachLogicalPortCommand and dispatch it to the
NiciraNvpResource. The convention ensures that the decision to run these
components as standalone processes can be a late binding decision.
 - Not sure that you need to modify any base classes at all. For example
NetworkGuruBase -- the modifications can be in the subclass. Not sure I
understand the purpose of 'isolationMethods' either
 - It is unfortunate that you had to modify enums. We should probably move
to a static list initialized at startup (like Network.Service) that others
can extend in their own class.


On 6/13/12 6:00 AM, "Hugo Trippaers" <HTrippaers@schubergphilis.com> wrote:

>Thanks for the heads-up, didn't notice.
>
>Here is a link to the file
>
>http://dl.dropbox.com/u/70226362/nicira-phase1.patch
>
>Cheers,
>
>Hugo
>
>-----Original Message-----
>From: Chip Childers [mailto:chip.childers@sungard.com]
>Sent: Wednesday, June 13, 2012 2:27 PM
>To: cloudstack-dev@incubator.apache.org
>Cc: Somik Behera (somik@nicira.com)
>Subject: Re: Nicira integration pre-review
>
>Attachments are apparently being stripped out by the mailer daemon.
>
>-chip
>
>On Wed, Jun 13, 2012 at 6:00 AM, Hugo Trippaers
><HTrippaers@schubergphilis.com> wrote:
>> Actually attaching the patch should help I think.


Mime
View raw message