cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ritu Sabharwal" <rsabh...@brocade.com>
Subject Re: Review Request 22863: CLOUDSTACK-6823 : First code drop for Brocade Network plugin to orchestrate Brocade VDX switches for L2 connectivity.
Date Tue, 24 Jun 2014 16:18:36 GMT


> On June 24, 2014, 6:40 a.m., Hugo Trippaers wrote:
> > Hey,
> > 
> > There are a number of issues with this patch, i've made the commments on the lines
directly. Also where is the real functionality? The code in this patch seems to only be the
hook into the existing cloudstack code and not the brocade code itself?
> > 
> > Cheers,
> > 
> > Hugo

Hi Hugo,

I commented on the review comments directly. I included a patch file brocade-vcs.patch which
has Brocade code. 

I followed these steps to create the patch:

-git add plugins/network-elements/brocade-vcs
-git commit -m "Commit message“
-git format-patch master --stdout > ~/brocade-vcs.patch

Is there any other way to include Brocade code in the diffs? Please let me know. I will submit
that for review.

Thanks,
Ritu.


> On June 24, 2014, 6:40 a.m., Hugo Trippaers wrote:
> > build/replace.properties, line 20
> > <https://reviews.apache.org/r/22863/diff/1/?file=614967#file614967line20>
> >
> >     This change will break some developer setups. Can you remove this file from
the patch.

The changes in this file are for my local server. I will remove this file from the patch.


> On June 24, 2014, 6:40 a.m., Hugo Trippaers wrote:
> > api/src/com/cloud/network/Network.java, line 135
> > <https://reviews.apache.org/r/22863/diff/1/?file=614964#file614964line135>
> >
> >     Please remove tabs from the sources.

I will fix this.


> On June 24, 2014, 6:40 a.m., Hugo Trippaers wrote:
> > plugins/pom.xml, line 71
> > <https://reviews.apache.org/r/22863/diff/1/?file=614969#file614969line71>
> >
> >     Why do you need to include test in the main build process?

I created some test project for my testing. I will fix this.


> On June 24, 2014, 6:40 a.m., Hugo Trippaers wrote:
> > setup/db/create-schema.sql, line 148
> > <https://reviews.apache.org/r/22863/diff/1/?file=614970#file614970line148>
> >
> >     Don't make changes to this file. Add the new tables to the upgrade files. In
this case you need to make the changes in the 4.4 to 4.5 upgrade file.

I will fix this.


> On June 24, 2014, 6:40 a.m., Hugo Trippaers wrote:
> > utils/conf/db.properties, line 28
> > <https://reviews.apache.org/r/22863/diff/1/?file=614972#file614972line28>
> >
> >     Please don't commit any changes to this file.

This again is for my local server. I will remove this file from the patch.


- Ritu


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22863/#review46498
-----------------------------------------------------------


On June 23, 2014, 5:49 p.m., Ritu  Sabharwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22863/
> -----------------------------------------------------------
> 
> (Updated June 23, 2014, 5:49 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Bugs: CLOUDSTACK-6823
>     https://issues.apache.org/jira/browse/CLOUDSTACK-6823
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> First code drop for Brocade Network plugin to orchestrate Brocade VDX switches for L2
connectivity. Please create a new branch for Brocade plugin.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/network/Network.java b5e8173 
>   api/src/com/cloud/network/Networks.java 3159f6e 
>   api/src/com/cloud/network/PhysicalNetwork.java 5d38b50 
>   build/replace.properties 265f335 
>   client/pom.xml 3995f6e 
>   plugins/pom.xml 4f7805a 
>   setup/db/create-schema.sql 55cb4cc 
>   ui/scripts/system.js 0c56baf 
>   utils/conf/db.properties e1b5fe9 
> 
> Diff: https://reviews.apache.org/r/22863/diff/
> 
> 
> Testing
> -------
> 
> •	Create an isolated network; verify that the port-profile is created on the Brocade
switch.
> •	Attach a VM to the network; verify that the VMs MAC address is associated with the
port profile of the network on the Brocade switch.
> •	Delete VMs for an isolated network; verify that the VMs MAC address is disassociated
with the port profile of the network on the Brocade switch.
> •	Delete the isolated network; verify that the port-profile is deleted from the Brocade
switch.
> 
> 
> File Attachments
> ----------------
> 
> Diff for the existing cloudstack code
>   https://reviews.apache.org/media/uploaded/files/2014/06/23/8fc3cfb1-7a21-4714-98f3-6514cf54ba84__diff
> Patch for the plugin code
>   https://reviews.apache.org/media/uploaded/files/2014/06/23/fbb2b949-a42c-4b22-afa7-60e691c778df__brocade-vcs.patch
> 
> 
> Thanks,
> 
> Ritu  Sabharwal
> 
>


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