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" <[email protected]> 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:[email protected]] >Sent: Wednesday, June 13, 2012 2:27 PM >To: [email protected] >Cc: Somik Behera ([email protected]) >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 ><[email protected]> wrote: >> Actually attaching the patch should help I think.
