cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chiradeep Vittal <>
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:
   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" <> wrote:

>Thanks for the heads-up, didn't notice.
>Here is a link to the file
>-----Original Message-----
>From: Chip Childers []
>Sent: Wednesday, June 13, 2012 2:27 PM
>Cc: Somik Behera (
>Subject: Re: Nicira integration pre-review
>Attachments are apparently being stripped out by the mailer daemon.
>On Wed, Jun 13, 2012 at 6:00 AM, Hugo Trippaers
><> wrote:
>> Actually attaching the patch should help I think.

View raw message