You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@cloudstack.apache.org by Venkata Siva Vijayendra Bhamidipati <vi...@citrix.com> on 2013/05/09 04:33:36 UTC

Review Request: PVLAN provisioning support for vmware Distributed Virtual Switch deployments on cloudstack.

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

Review request for cloudstack, Chip Childers, Sateesh Chodapuneedi, and Kelven Yang.

Description
-------

Please find attached the diffs for pvlan support for vmware DVSwitch deployments on cloudstack. You will find two diffs - the parent diff is Sateesh's fix for CLOUSTACK-2316 which is needed to be cherry-picked on the pvlan branch from the master. The other diff contains the changes for pvlan support.

These diffs do not contain changes for pvlan provisioning on the Cisco Nexus 1000v distributed virtual switch.

This addresses bug CLOUDSTACK-1456.

Diffs
-----

plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 99ad1ca
server/src/com/cloud/network/NetworkManagerImpl.java 7a09eb5
server/src/com/cloud/network/NetworkModelImpl.java bd62886
server/src/com/cloud/vm/UserVmManagerImpl.java 683f0da
server/src/com/cloud/vm/VirtualMachineManagerImpl.java b0d6378
vmware-base/src/com/cloud/hypervisor/vmware/mo/DistributedVirtualSwitchMO.java 247be2a
vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java 7f323c5

Diff: https://reviews.apache.org/r/11019/diff/

Testing
-------

The code has been tested on the Vmware DVSwitch for advanced shared networks on vmware cluster deployments on cloudstack. Unit tests will be the same as those provided by Sheng as part of the overall PVLAN support for XenServer and KVM, and will exercise the vmware pvlan code path when user VMs are created with vNICs sitting on advanced shared networks that have the optional Private VLAN value set during their creation. VM live migration using vmware vMotion has also been tested with these changes on vmware and it works as expected.

Further testing will be carried out and this review request will be updated accordingly.

Thanks,

Venkata Siva Vijayendra Bhamidipati

Re: Review Request: PVLAN provisioning support for vmware Distributed Virtual Switch deployments on cloudstack.

Posted by Sheng Yang <sh...@yasker.org>.

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



server/src/com/cloud/vm/UserVmManagerImpl.java
<https://reviews.apache.org/r/11019/#comment42102>

    I meant, if you get the command executed in vmware resources, then you won't need hypervisor specify lines here.



server/src/com/cloud/vm/VirtualMachineManagerImpl.java
<https://reviews.apache.org/r/11019/#comment42103>

    Still here.


- Sheng Yang


On May 10, 2013, 2:32 a.m., Venkata Siva Vijayendra Bhamidipati wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11019/
> -----------------------------------------------------------
> 
> (Updated May 10, 2013, 2:32 a.m.)
> 
> 
> Review request for cloudstack, Chip Childers, Sheng Yang, Sateesh Chodapuneedi, Kelven Yang, and Animesh Chaturvedi.
> 
> 
> Description
> -------
> 
> Please find attached the diffs for pvlan support for vmware DVSwitch deployments on cloudstack. You will find two diffs - the parent diff is Sateesh's fix for CLOUSTACK-2316 which is needed to be cherry-picked on the pvlan branch from the master. The other diff contains the changes for pvlan support.
> 
> These diffs do not contain changes for pvlan provisioning on the Cisco Nexus 1000v distributed virtual switch.
> 
> 
> This addresses bug CLOUDSTACK-1456.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 99ad1ca 
>   server/src/com/cloud/network/NetworkManagerImpl.java 7a09eb5 
>   server/src/com/cloud/network/NetworkModelImpl.java bd62886 
>   server/src/com/cloud/vm/UserVmManagerImpl.java 683f0da 
>   server/src/com/cloud/vm/VirtualMachineManagerImpl.java b0d6378 
>   vmware-base/src/com/cloud/hypervisor/vmware/mo/DistributedVirtualSwitchMO.java 247be2a 
>   vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java 7f323c5 
> 
> Diff: https://reviews.apache.org/r/11019/diff/
> 
> 
> Testing
> -------
> 
> The code has been tested on the Vmware DVSwitch for advanced shared networks on vmware cluster deployments on cloudstack. Unit tests will be the same as those provided by Sheng as part of the overall PVLAN support for XenServer and KVM, and will exercise the vmware pvlan code path when user VMs are created with vNICs sitting on advanced shared networks that have the optional Private VLAN value set during their creation. VM live migration using vmware vMotion has also been tested with these changes on vmware and it works as expected.
> 
> Further testing will be carried out and this review request will be updated accordingly.
> 
> 
> Thanks,
> 
> Venkata Siva Vijayendra Bhamidipati
> 
>

Re: Review Request: PVLAN provisioning support for vmware Distributed Virtual Switch deployments on cloudstack.

Posted by Sheng Yang <sh...@yasker.org>.

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

Ship it!


Applied to master.

Thanks!

commit 15be97772e1b41801867beef25ae66dfaf286458
Author: Vijayendra Bhamidipati <vi...@citrix.com>
Date:   Thu May 16 08:15:22 2013 -0700

    PVLAN : Implementing PVLAN deployment capability for VMware deployments in cloudstack.


- Sheng Yang


On May 16, 2013, 9:45 p.m., Venkata Siva Vijayendra Bhamidipati wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11019/
> -----------------------------------------------------------
> 
> (Updated May 16, 2013, 9:45 p.m.)
> 
> 
> Review request for cloudstack, Chip Childers, Sheng Yang, Sateesh Chodapuneedi, Kelven Yang, and Animesh Chaturvedi.
> 
> 
> Description
> -------
> 
> Please find attached the diffs for pvlan support for vmware DVSwitch deployments on cloudstack. You will find two diffs - the parent diff is Sateesh's fix for CLOUSTACK-2316 which is needed to be cherry-picked on the pvlan branch from the master. The other diff contains the changes for pvlan support.
> 
> These diffs do not contain changes for pvlan provisioning on the Cisco Nexus 1000v distributed virtual switch.
> 
> 
> This addresses bug CLOUDSTACK-1456.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/agent/api/PlugNicCommand.java b896e45 
>   plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 99ad1ca 
>   server/src/com/cloud/network/NetworkManagerImpl.java 7a09eb5 
>   server/src/com/cloud/network/NetworkModelImpl.java bd62886 
>   server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java bdfac06 
>   server/src/com/cloud/vm/UserVmManagerImpl.java 683f0da 
>   server/src/com/cloud/vm/VirtualMachineManagerImpl.java b0d6378 
>   vmware-base/src/com/cloud/hypervisor/vmware/mo/DistributedVirtualSwitchMO.java 247be2a 
>   vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java 7f323c5 
> 
> Diff: https://reviews.apache.org/r/11019/diff/
> 
> 
> Testing
> -------
> 
> The code has been tested on the Vmware DVSwitch for advanced shared networks on vmware cluster deployments on cloudstack. Unit tests will be the same as those provided by Sheng as part of the overall PVLAN support for XenServer and KVM, and will exercise the vmware pvlan code path when user VMs are created with vNICs sitting on advanced shared networks that have the optional Private VLAN value set during their creation. VM live migration using vmware vMotion has also been tested with these changes on vmware and it works as expected.
> 
> Further testing will be carried out and this review request will be updated accordingly.
> 
> 
> Thanks,
> 
> Venkata Siva Vijayendra Bhamidipati
> 
>

Re: Review Request: PVLAN provisioning support for vmware Distributed Virtual Switch deployments on cloudstack.

Posted by Venkata Siva Vijayendra Bhamidipati <vi...@citrix.com>.

(Updated May 16, 2013, 9:45 p.m.)

Review request for cloudstack, Chip Childers, Sheng Yang, Sateesh Chodapuneedi, Kelven Yang, and Animesh Chaturvedi.

Changes
-------

This is the latest PVLAN diff for vmware deployments on cloudstack. I run into the error "error: short SHA1 01a0384 is ambiguous. fatal: Not a valid object name 01a0384" when trying to update the diff. Looks like git is running into collisions with the SHA1 checksum of its commits between local and remote branches. Generating the patch in a clean new workspace did not fix this issue. I am not sure how to overcome this.

Please let me know if you are unable to apply this patch. You should be able to do a git apply at the least - please copy over the commit authorship from this patch and sign off on the patch.

Description
-------

These diffs do not contain changes for pvlan provisioning on the Cisco Nexus 1000v distributed virtual switch.

This addresses bug CLOUDSTACK-1456.

Diffs
-----

api/src/com/cloud/agent/api/PlugNicCommand.java b896e45
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 99ad1ca
server/src/com/cloud/network/NetworkManagerImpl.java 7a09eb5
server/src/com/cloud/network/NetworkModelImpl.java bd62886
server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java bdfac06
server/src/com/cloud/vm/UserVmManagerImpl.java 683f0da
server/src/com/cloud/vm/VirtualMachineManagerImpl.java b0d6378
vmware-base/src/com/cloud/hypervisor/vmware/mo/DistributedVirtualSwitchMO.java 247be2a
vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java 7f323c5

Diff: https://reviews.apache.org/r/11019/diff/

Testing
-------

Further testing will be carried out and this review request will be updated accordingly.

Thanks,

Venkata Siva Vijayendra Bhamidipati

Re: Review Request: PVLAN provisioning support for vmware Distributed Virtual Switch deployments on cloudstack.

Posted by Venkata Siva Vijayendra Bhamidipati <vi...@citrix.com>.

(Updated May 14, 2013, 10:35 p.m.)

Review request for cloudstack, Chip Childers, Sheng Yang, Sateesh Chodapuneedi, Kelven Yang, and Animesh Chaturvedi.

Changes
-------

Uploading fully tested vmware changes for pvlan support.

Tests carried out:
==================
Step 0. Provision PVLAN primary/isolated secondary VLAN on physical switch that the physical hosts will connect to, following appropriate manuals of the switch vendor.
Step 1. Set the vmware.use.dvswitch global flag to true, restart mgmt server.
Step 2. Create vmware cluster in vCenter. Enable cluster for vMotion.
Step 3. Create vmware DVS on the vmware datacenter that the cluster belongs to.
Step 4. Create cloudstack zone/vmware cluster on mgmt server.
Step 5. Login as admin to the mgmt server, and under - Infrastructure -> Zones view all -> your zone -> Physical network tab -> Physical network name -> Guest configure -> Network tab -> add Guest Network : create a pvlan enabled advanced shared network (you can use the "Offering for shared networks" network offering available in the drop down menu). Specify the primary (promiscuous vlan id) and the secondary isolated pvlan id. Specify valid routable IP gateway/ip range/netmask values.
Step 6. Create a guest VM having the above network. It should come up with an eth0 interface plumbed with a DHCP IP picked from the ip address range provided in step 5. Ping the router VM's ip (on the same subnet) - it should go through.
Step 7. Create another guest VM having the same network. It should also come up with an IP picked from the ip address range in step 5. This VM should be able to ping the router VM ip, but not the guest VM created in step 6.
Step 8. Create an isolated network under Network -> Add Guest network. Create a new guest VM having both this new isolated network and the shared pvlan network created above. The VM should come up fine with two eth interfaces, each having an ip range from the specified ranges. Same ping behavior must be observed for interfaces between guest VMs in the pvlan network.
Step 9. vMotion the guest VMs from one physical host to another. The migration must be smooth, the VM up, with no change seen in network configuration or behavior.

Description
-------

These diffs do not contain changes for pvlan provisioning on the Cisco Nexus 1000v distributed virtual switch.

This addresses bug CLOUDSTACK-1456.

Diffs (updated)
-----

Diff: https://reviews.apache.org/r/11019/diff/

Testing
-------

Further testing will be carried out and this review request will be updated accordingly.

Thanks,

Venkata Siva Vijayendra Bhamidipati

Re: Review Request: PVLAN provisioning support for vmware Distributed Virtual Switch deployments on cloudstack.

Posted by Venkata Siva Vijayendra Bhamidipati <vi...@citrix.com>.

(Updated May 10, 2013, 2:32 a.m.)

Review request for cloudstack, Chip Childers, Sheng Yang, Sateesh Chodapuneedi, Kelven Yang, and Animesh Chaturvedi.

Changes
-------

Incorporating changes following Sheng's comments.

Description
-------

These diffs do not contain changes for pvlan provisioning on the Cisco Nexus 1000v distributed virtual switch.

This addresses bug CLOUDSTACK-1456.

Diffs (updated)
-----

Diff: https://reviews.apache.org/r/11019/diff/

Testing
-------

Further testing will be carried out and this review request will be updated accordingly.

Thanks,

Venkata Siva Vijayendra Bhamidipati

Re: Review Request: PVLAN provisioning support for vmware Distributed Virtual Switch deployments on cloudstack.

Posted by Venkata Siva Vijayendra Bhamidipati <vi...@citrix.com>.

(Updated May 9, 2013, 7:09 p.m.)

Review request for cloudstack, Chip Childers, Sheng Yang, Sateesh Chodapuneedi, Kelven Yang, and Animesh Chaturvedi.

Changes
-------

+Sheng

Description
-------

These diffs do not contain changes for pvlan provisioning on the Cisco Nexus 1000v distributed virtual switch.

This addresses bug CLOUDSTACK-1456.

Diffs
-----

Diff: https://reviews.apache.org/r/11019/diff/

Testing
-------

Further testing will be carried out and this review request will be updated accordingly.

Thanks,

Venkata Siva Vijayendra Bhamidipati

Re: Review Request: PVLAN provisioning support for vmware Distributed Virtual Switch deployments on cloudstack.

Posted by Sheng Yang <sh...@yasker.org>.


> On May 9, 2013, 6:54 p.m., Sheng Yang wrote:
> > vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java, line 746
> > <https://reviews.apache.org/r/11019/diff/1/?file=289105#file289105line746>
> >
> >     Reason to remove type converting?
> 
> Venkata Siva Vijayendra Bhamidipati wrote:
>     Again, eclipse generated, but why would we need that cast? I would remove it.

The original value is int, and the maximum of int is 2,147,483,647, in other word, the maximum bandwidth here would be 2G, then overflow. That's why you need convert it to long type here.


> On May 9, 2013, 6:54 p.m., Sheng Yang wrote:
> > vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java, line 485
> > <https://reviews.apache.org/r/11019/diff/1/?file=289105#file289105line485>
> >
> >     better using e.g. secondary vlan id. pvlanid is confusing.
> 
> Venkata Siva Vijayendra Bhamidipati wrote:
>     I'll change that to isolatedpvlanid, should be clearer that way.

Could you check if it's possible to represent a community secondary vlan in the future? If so, we'd better name it e.g. secondary pvlan.


- Sheng


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


On May 9, 2013, 7:09 p.m., Venkata Siva Vijayendra Bhamidipati wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11019/
> -----------------------------------------------------------
> 
> (Updated May 9, 2013, 7:09 p.m.)
> 
> 
> Review request for cloudstack, Chip Childers, Sheng Yang, Sateesh Chodapuneedi, Kelven Yang, and Animesh Chaturvedi.
> 
> 
> Description
> -------
> 
> Please find attached the diffs for pvlan support for vmware DVSwitch deployments on cloudstack. You will find two diffs - the parent diff is Sateesh's fix for CLOUSTACK-2316 which is needed to be cherry-picked on the pvlan branch from the master. The other diff contains the changes for pvlan support.
> 
> These diffs do not contain changes for pvlan provisioning on the Cisco Nexus 1000v distributed virtual switch.
> 
> 
> This addresses bug CLOUDSTACK-1456.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 99ad1ca 
>   server/src/com/cloud/network/NetworkManagerImpl.java 7a09eb5 
>   server/src/com/cloud/network/NetworkModelImpl.java bd62886 
>   server/src/com/cloud/vm/UserVmManagerImpl.java 683f0da 
>   server/src/com/cloud/vm/VirtualMachineManagerImpl.java b0d6378 
>   vmware-base/src/com/cloud/hypervisor/vmware/mo/DistributedVirtualSwitchMO.java 247be2a 
>   vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java 7f323c5 
> 
> Diff: https://reviews.apache.org/r/11019/diff/
> 
> 
> Testing
> -------
> 
> The code has been tested on the Vmware DVSwitch for advanced shared networks on vmware cluster deployments on cloudstack. Unit tests will be the same as those provided by Sheng as part of the overall PVLAN support for XenServer and KVM, and will exercise the vmware pvlan code path when user VMs are created with vNICs sitting on advanced shared networks that have the optional Private VLAN value set during their creation. VM live migration using vmware vMotion has also been tested with these changes on vmware and it works as expected.
> 
> Further testing will be carried out and this review request will be updated accordingly.
> 
> 
> Thanks,
> 
> Venkata Siva Vijayendra Bhamidipati
> 
>

Re: Review Request: PVLAN provisioning support for vmware Distributed Virtual Switch deployments on cloudstack.

Posted by Venkata Siva Vijayendra Bhamidipati <vi...@citrix.com>.


> On May 9, 2013, 6:54 p.m., Sheng Yang wrote:
> >

Thanks for the review Sheng. Please find my comments below. I'll upload newer diffs with some of the suggested changes shortly.


> On May 9, 2013, 6:54 p.m., Sheng Yang wrote:
> > server/src/com/cloud/network/NetworkModelImpl.java, line 933
> > <https://reviews.apache.org/r/11019/diff/1/?file=289101#file289101line933>
> >
> >     What's this for?

I hit an NPE at com.cloud.network.NetworkModelImpl.getNetworkRate(NetworkModelImpl.java:929) because the reference to VMInstanceVO would be null since the vm (router VM in this case) wouldn't exist yet. getNetworkOfferingNetworkRate() internally returns -1 to indicate unlimited network rate if it cannot obtain the networkRate from the provided networkofferingid and dcId. So I put in a check for null values and returned -1 if the call cannot be made. But I think it's better I pass null values instead if either is null, and let getNetworkOfferingNetworkRate() take care of the values passed in. Will make the required change.


> On May 9, 2013, 6:54 p.m., Sheng Yang wrote:
> > server/src/com/cloud/vm/UserVmManagerImpl.java, line 2848
> > <https://reviews.apache.org/r/11019/diff/1/?file=289102#file289102line2848>
> >
> >     Can VMware just ignore the command? Rather than put hypervisor specific code here.

Yes VMWare can ignore it because the we configure the port groups in the start command path. The migration code path works differently with DVS since we don't have to plumb it on each host again after the first time when we configure the DVS (which pushes out the required config to each host it manages). The vmwareresource will still implement a stub of this method to not cause warnings when the VM is destroyed.


> On May 9, 2013, 6:54 p.m., Sheng Yang wrote:
> > server/src/com/cloud/vm/VirtualMachineManagerImpl.java, line 56
> > <https://reviews.apache.org/r/11019/diff/1/?file=289103#file289103line56>
> >
> >     What's the change in this file for? Not used anywhere.

Eclipse generated. Will remove it if it isn't being used anywhere.


> On May 9, 2013, 6:54 p.m., Sheng Yang wrote:
> > vmware-base/src/com/cloud/hypervisor/vmware/mo/DistributedVirtualSwitchMO.java, line 74
> > <https://reviews.apache.org/r/11019/diff/1/?file=289104#file289104line74>
> >
> >     I am confused by pvlanid existed with vlanid here. Do you mean "isolated pvlan"(which is secondary vlan)?

Yes


> On May 9, 2013, 6:54 p.m., Sheng Yang wrote:
> > vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java, line 485
> > <https://reviews.apache.org/r/11019/diff/1/?file=289105#file289105line485>
> >
> >     better using e.g. secondary vlan id. pvlanid is confusing.

I'll change that to isolatedpvlanid, should be clearer that way.


> On May 9, 2013, 6:54 p.m., Sheng Yang wrote:
> > vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java, line 746
> > <https://reviews.apache.org/r/11019/diff/1/?file=289105#file289105line746>
> >
> >     Reason to remove type converting?

Again, eclipse generated, but why would we need that cast? I would remove it.


- Venkata Siva Vijayendra


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


On May 9, 2013, 7:09 p.m., Venkata Siva Vijayendra Bhamidipati wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11019/
> -----------------------------------------------------------
> 
> (Updated May 9, 2013, 7:09 p.m.)
> 
> 
> Review request for cloudstack, Chip Childers, Sheng Yang, Sateesh Chodapuneedi, Kelven Yang, and Animesh Chaturvedi.
> 
> 
> Description
> -------
> 
> Please find attached the diffs for pvlan support for vmware DVSwitch deployments on cloudstack. You will find two diffs - the parent diff is Sateesh's fix for CLOUSTACK-2316 which is needed to be cherry-picked on the pvlan branch from the master. The other diff contains the changes for pvlan support.
> 
> These diffs do not contain changes for pvlan provisioning on the Cisco Nexus 1000v distributed virtual switch.
> 
> 
> This addresses bug CLOUDSTACK-1456.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 99ad1ca 
>   server/src/com/cloud/network/NetworkManagerImpl.java 7a09eb5 
>   server/src/com/cloud/network/NetworkModelImpl.java bd62886 
>   server/src/com/cloud/vm/UserVmManagerImpl.java 683f0da 
>   server/src/com/cloud/vm/VirtualMachineManagerImpl.java b0d6378 
>   vmware-base/src/com/cloud/hypervisor/vmware/mo/DistributedVirtualSwitchMO.java 247be2a 
>   vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java 7f323c5 
> 
> Diff: https://reviews.apache.org/r/11019/diff/
> 
> 
> Testing
> -------
> 
> The code has been tested on the Vmware DVSwitch for advanced shared networks on vmware cluster deployments on cloudstack. Unit tests will be the same as those provided by Sheng as part of the overall PVLAN support for XenServer and KVM, and will exercise the vmware pvlan code path when user VMs are created with vNICs sitting on advanced shared networks that have the optional Private VLAN value set during their creation. VM live migration using vmware vMotion has also been tested with these changes on vmware and it works as expected.
> 
> Further testing will be carried out and this review request will be updated accordingly.
> 
> 
> Thanks,
> 
> Venkata Siva Vijayendra Bhamidipati
> 
>

Re: Review Request: PVLAN provisioning support for vmware Distributed Virtual Switch deployments on cloudstack.

Posted by Sheng Yang <sh...@yasker.org>.

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



server/src/com/cloud/network/NetworkModelImpl.java
<https://reviews.apache.org/r/11019/#comment41965>

    What's this for?



server/src/com/cloud/vm/UserVmManagerImpl.java
<https://reviews.apache.org/r/11019/#comment41966>

    Can VMware just ignore the command? Rather than put hypervisor specific code here.



server/src/com/cloud/vm/VirtualMachineManagerImpl.java
<https://reviews.apache.org/r/11019/#comment41967>

    What's the change in this file for? Not used anywhere.



vmware-base/src/com/cloud/hypervisor/vmware/mo/DistributedVirtualSwitchMO.java
<https://reviews.apache.org/r/11019/#comment41972>

    I am confused by pvlanid existed with vlanid here. Do you mean "isolated pvlan"(which is secondary vlan)? 



vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java
<https://reviews.apache.org/r/11019/#comment41973>

    better using e.g. secondary vlan id. pvlanid is confusing.



vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java
<https://reviews.apache.org/r/11019/#comment41974>

    Reason to remove type converting?


- Sheng Yang


On May 9, 2013, 5:47 p.m., Venkata Siva Vijayendra Bhamidipati wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11019/
> -----------------------------------------------------------
> 
> (Updated May 9, 2013, 5:47 p.m.)
> 
> 
> Review request for cloudstack, Chip Childers, Sateesh Chodapuneedi, Kelven Yang, and Animesh Chaturvedi.
> 
> 
> Description
> -------
> 
> Please find attached the diffs for pvlan support for vmware DVSwitch deployments on cloudstack. You will find two diffs - the parent diff is Sateesh's fix for CLOUSTACK-2316 which is needed to be cherry-picked on the pvlan branch from the master. The other diff contains the changes for pvlan support.
> 
> These diffs do not contain changes for pvlan provisioning on the Cisco Nexus 1000v distributed virtual switch.
> 
> 
> This addresses bug CLOUDSTACK-1456.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 99ad1ca 
>   server/src/com/cloud/network/NetworkManagerImpl.java 7a09eb5 
>   server/src/com/cloud/network/NetworkModelImpl.java bd62886 
>   server/src/com/cloud/vm/UserVmManagerImpl.java 683f0da 
>   server/src/com/cloud/vm/VirtualMachineManagerImpl.java b0d6378 
>   vmware-base/src/com/cloud/hypervisor/vmware/mo/DistributedVirtualSwitchMO.java 247be2a 
>   vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java 7f323c5 
> 
> Diff: https://reviews.apache.org/r/11019/diff/
> 
> 
> Testing
> -------
> 
> The code has been tested on the Vmware DVSwitch for advanced shared networks on vmware cluster deployments on cloudstack. Unit tests will be the same as those provided by Sheng as part of the overall PVLAN support for XenServer and KVM, and will exercise the vmware pvlan code path when user VMs are created with vNICs sitting on advanced shared networks that have the optional Private VLAN value set during their creation. VM live migration using vmware vMotion has also been tested with these changes on vmware and it works as expected.
> 
> Further testing will be carried out and this review request will be updated accordingly.
> 
> 
> Thanks,
> 
> Venkata Siva Vijayendra Bhamidipati
> 
>

Re: Review Request: PVLAN provisioning support for vmware Distributed Virtual Switch deployments on cloudstack.

Posted by Venkata Siva Vijayendra Bhamidipati <vi...@citrix.com>.

(Updated May 9, 2013, 5:47 p.m.)

Review request for cloudstack, Chip Childers, Sateesh Chodapuneedi, Kelven Yang, and Animesh Chaturvedi.

Changes
-------

+Animesh to list of reviewers.

Description
-------

These diffs do not contain changes for pvlan provisioning on the Cisco Nexus 1000v distributed virtual switch.

This addresses bug CLOUDSTACK-1456.

Diffs
-----

Diff: https://reviews.apache.org/r/11019/diff/

Testing
-------

Further testing will be carried out and this review request will be updated accordingly.

Thanks,

Venkata Siva Vijayendra Bhamidipati