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


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


api/src/com/cloud/network/Network.java
<https://reviews.apache.org/r/22863/#comment81894>

    Please remove tabs from the sources.



build/replace.properties
<https://reviews.apache.org/r/22863/#comment81895>

    This change will break some developer setups. Can you remove this file from 
the patch.



plugins/pom.xml
<https://reviews.apache.org/r/22863/#comment81896>

    Why do you need to include test in the main build process?



setup/db/create-schema.sql
<https://reviews.apache.org/r/22863/#comment81897>

    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.



utils/conf/db.properties
<https://reviews.apache.org/r/22863/#comment81898>

    Please don't commit any changes to this file.


- Hugo Trippaers


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
> 
>

Reply via email to