You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Jayapal Reddy <ja...@citrix.com> on 2013/03/04 14:18:57 UTC

Review Request: multiple ip address per nic changes for basic zone

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

Review request for cloudstack, Abhinandan Prateek, Murali Reddy, and anthony xu.


Description
-------

In this patch changes are made for the basic zone and security groups


This addresses bug cloudstack-24.


Diffs
-----

  api/src/com/cloud/agent/api/SecurityGroupRulesCmd.java affad1f 
  api/src/com/cloud/agent/api/to/NicTO.java aa2aa19 
  api/src/com/cloud/network/security/SecurityGroupService.java c648032 
  api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java 0f99274 
  api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java cb5e085 
  core/src/com/cloud/agent/api/NetworkRulesVmSecondaryIpCommand.java PRE-CREATION 
  plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 1896b0e 
  scripts/vm/hypervisor/xenserver/vmops 92ed79a 
  server/src/com/cloud/hypervisor/HypervisorGuruBase.java efe9396 
  server/src/com/cloud/network/NetworkManager.java 8b6bf9a 
  server/src/com/cloud/network/NetworkManagerImpl.java a575183 
  server/src/com/cloud/network/NetworkModelImpl.java 0525888 
  server/src/com/cloud/network/NetworkServiceImpl.java 4c61aec 
  server/src/com/cloud/network/dao/IPAddressDao.java 1052639 
  server/src/com/cloud/network/dao/IPAddressDaoImpl.java 691e460 
  server/src/com/cloud/network/guru/DirectNetworkGuru.java 46a525e 
  server/src/com/cloud/network/security/SecurityGroupManagerImpl.java eafe88e 
  server/src/com/cloud/network/security/SecurityGroupManagerImpl2.java a3a0fc3 
  server/src/com/cloud/vm/UserVmManagerImpl.java 3fb5d92 
  server/src/com/cloud/vm/dao/NicDao.java 794bacc 
  server/src/com/cloud/vm/dao/NicDaoImpl.java 4491174 
  server/test/com/cloud/network/MockNetworkManagerImpl.java 8004310 
  server/test/com/cloud/vpc/MockNetworkManagerImpl.java 63ef874 

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


Testing
-------

Unit Tests done:
1. adding a secondary ip to nic
2. remove ip from nic
3. reaching public network from the VM.
4. security groups changes for the secondary ip are done.


Thanks,

Jayapal Reddy


Re: Review Request: multiple ip address per nic changes for basic zone

Posted by Abhinandan Prateek <ap...@apache.org>.

> On March 13, 2013, 4:59 a.m., Abhinandan Prateek wrote:
> > Ship It!

Shipped


- Abhinandan


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


On March 6, 2013, 12:45 p.m., Jayapal Reddy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9730/
> -----------------------------------------------------------
> 
> (Updated March 6, 2013, 12:45 p.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek, Murali Reddy, and anthony xu.
> 
> 
> Description
> -------
> 
> In this patch changes are made for the basic zone and security groups
> 
> 
> This addresses bug cloudstack-24.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/agent/api/SecurityGroupRulesCmd.java affad1f 
>   api/src/com/cloud/agent/api/to/NicTO.java aa2aa19 
>   api/src/com/cloud/network/security/SecurityGroupService.java c648032 
>   api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java 0f99274 
>   api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java cb5e085 
>   core/src/com/cloud/agent/api/NetworkRulesVmSecondaryIpCommand.java PRE-CREATION 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 4a89806 
>   scripts/vm/hypervisor/xenserver/vmops db6f6d6 
>   server/src/com/cloud/hypervisor/HypervisorGuruBase.java efe9396 
>   server/src/com/cloud/network/NetworkManager.java 8b6bf9a 
>   server/src/com/cloud/network/NetworkManagerImpl.java ba5ab5d 
>   server/src/com/cloud/network/NetworkModelImpl.java e83c0d2 
>   server/src/com/cloud/network/NetworkServiceImpl.java 821aa6d 
>   server/src/com/cloud/network/dao/IPAddressDao.java 1052639 
>   server/src/com/cloud/network/dao/IPAddressDaoImpl.java b0eef4f 
>   server/src/com/cloud/network/guru/DirectNetworkGuru.java 46a525e 
>   server/src/com/cloud/network/security/SecurityGroupManagerImpl.java eafe88e 
>   server/src/com/cloud/network/security/SecurityGroupManagerImpl2.java a3a0fc3 
>   server/src/com/cloud/vm/UserVmManagerImpl.java ca9c13f 
>   server/src/com/cloud/vm/dao/NicDao.java 794bacc 
>   server/src/com/cloud/vm/dao/NicDaoImpl.java 4491174 
>   server/test/com/cloud/network/MockNetworkManagerImpl.java 8004310 
>   server/test/com/cloud/vpc/MockNetworkManagerImpl.java 63ef874 
> 
> Diff: https://reviews.apache.org/r/9730/diff/
> 
> 
> Testing
> -------
> 
> Unit Tests done:
> 1. adding a secondary ip to nic
> 2. remove ip from nic
> 3. reaching public network from the VM.
> 4. security groups changes for the secondary ip are done.
> 
> 
> Thanks,
> 
> Jayapal Reddy
> 
>


Re: Review Request: multiple ip address per nic changes for basic zone

Posted by Abhinandan Prateek <ap...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9730/#review17783
-----------------------------------------------------------

Ship it!


Ship It!

- Abhinandan Prateek


On March 6, 2013, 12:45 p.m., Jayapal Reddy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9730/
> -----------------------------------------------------------
> 
> (Updated March 6, 2013, 12:45 p.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek, Murali Reddy, and anthony xu.
> 
> 
> Description
> -------
> 
> In this patch changes are made for the basic zone and security groups
> 
> 
> This addresses bug cloudstack-24.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/agent/api/SecurityGroupRulesCmd.java affad1f 
>   api/src/com/cloud/agent/api/to/NicTO.java aa2aa19 
>   api/src/com/cloud/network/security/SecurityGroupService.java c648032 
>   api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java 0f99274 
>   api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java cb5e085 
>   core/src/com/cloud/agent/api/NetworkRulesVmSecondaryIpCommand.java PRE-CREATION 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 4a89806 
>   scripts/vm/hypervisor/xenserver/vmops db6f6d6 
>   server/src/com/cloud/hypervisor/HypervisorGuruBase.java efe9396 
>   server/src/com/cloud/network/NetworkManager.java 8b6bf9a 
>   server/src/com/cloud/network/NetworkManagerImpl.java ba5ab5d 
>   server/src/com/cloud/network/NetworkModelImpl.java e83c0d2 
>   server/src/com/cloud/network/NetworkServiceImpl.java 821aa6d 
>   server/src/com/cloud/network/dao/IPAddressDao.java 1052639 
>   server/src/com/cloud/network/dao/IPAddressDaoImpl.java b0eef4f 
>   server/src/com/cloud/network/guru/DirectNetworkGuru.java 46a525e 
>   server/src/com/cloud/network/security/SecurityGroupManagerImpl.java eafe88e 
>   server/src/com/cloud/network/security/SecurityGroupManagerImpl2.java a3a0fc3 
>   server/src/com/cloud/vm/UserVmManagerImpl.java ca9c13f 
>   server/src/com/cloud/vm/dao/NicDao.java 794bacc 
>   server/src/com/cloud/vm/dao/NicDaoImpl.java 4491174 
>   server/test/com/cloud/network/MockNetworkManagerImpl.java 8004310 
>   server/test/com/cloud/vpc/MockNetworkManagerImpl.java 63ef874 
> 
> Diff: https://reviews.apache.org/r/9730/diff/
> 
> 
> Testing
> -------
> 
> Unit Tests done:
> 1. adding a secondary ip to nic
> 2. remove ip from nic
> 3. reaching public network from the VM.
> 4. security groups changes for the secondary ip are done.
> 
> 
> Thanks,
> 
> Jayapal Reddy
> 
>


Re: Review Request: multiple ip address per nic changes for basic zone

Posted by anthony xu <an...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9730/#review17736
-----------------------------------------------------------

Ship it!


Ship It!

- anthony xu


On March 6, 2013, 12:45 p.m., Jayapal Reddy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9730/
> -----------------------------------------------------------
> 
> (Updated March 6, 2013, 12:45 p.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek, Murali Reddy, and anthony xu.
> 
> 
> Description
> -------
> 
> In this patch changes are made for the basic zone and security groups
> 
> 
> This addresses bug cloudstack-24.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/agent/api/SecurityGroupRulesCmd.java affad1f 
>   api/src/com/cloud/agent/api/to/NicTO.java aa2aa19 
>   api/src/com/cloud/network/security/SecurityGroupService.java c648032 
>   api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java 0f99274 
>   api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java cb5e085 
>   core/src/com/cloud/agent/api/NetworkRulesVmSecondaryIpCommand.java PRE-CREATION 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 4a89806 
>   scripts/vm/hypervisor/xenserver/vmops db6f6d6 
>   server/src/com/cloud/hypervisor/HypervisorGuruBase.java efe9396 
>   server/src/com/cloud/network/NetworkManager.java 8b6bf9a 
>   server/src/com/cloud/network/NetworkManagerImpl.java ba5ab5d 
>   server/src/com/cloud/network/NetworkModelImpl.java e83c0d2 
>   server/src/com/cloud/network/NetworkServiceImpl.java 821aa6d 
>   server/src/com/cloud/network/dao/IPAddressDao.java 1052639 
>   server/src/com/cloud/network/dao/IPAddressDaoImpl.java b0eef4f 
>   server/src/com/cloud/network/guru/DirectNetworkGuru.java 46a525e 
>   server/src/com/cloud/network/security/SecurityGroupManagerImpl.java eafe88e 
>   server/src/com/cloud/network/security/SecurityGroupManagerImpl2.java a3a0fc3 
>   server/src/com/cloud/vm/UserVmManagerImpl.java ca9c13f 
>   server/src/com/cloud/vm/dao/NicDao.java 794bacc 
>   server/src/com/cloud/vm/dao/NicDaoImpl.java 4491174 
>   server/test/com/cloud/network/MockNetworkManagerImpl.java 8004310 
>   server/test/com/cloud/vpc/MockNetworkManagerImpl.java 63ef874 
> 
> Diff: https://reviews.apache.org/r/9730/diff/
> 
> 
> Testing
> -------
> 
> Unit Tests done:
> 1. adding a secondary ip to nic
> 2. remove ip from nic
> 3. reaching public network from the VM.
> 4. security groups changes for the secondary ip are done.
> 
> 
> Thanks,
> 
> Jayapal Reddy
> 
>


Re: Review Request: multiple ip address per nic changes for basic zone

Posted by Abhinandan Prateek <ap...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9730/#review17685
-----------------------------------------------------------


ipv6 support needs to be added in future.

- Abhinandan Prateek


On March 6, 2013, 12:45 p.m., Jayapal Reddy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9730/
> -----------------------------------------------------------
> 
> (Updated March 6, 2013, 12:45 p.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek, Murali Reddy, and anthony xu.
> 
> 
> Description
> -------
> 
> In this patch changes are made for the basic zone and security groups
> 
> 
> This addresses bug cloudstack-24.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/agent/api/SecurityGroupRulesCmd.java affad1f 
>   api/src/com/cloud/agent/api/to/NicTO.java aa2aa19 
>   api/src/com/cloud/network/security/SecurityGroupService.java c648032 
>   api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java 0f99274 
>   api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java cb5e085 
>   core/src/com/cloud/agent/api/NetworkRulesVmSecondaryIpCommand.java PRE-CREATION 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 4a89806 
>   scripts/vm/hypervisor/xenserver/vmops db6f6d6 
>   server/src/com/cloud/hypervisor/HypervisorGuruBase.java efe9396 
>   server/src/com/cloud/network/NetworkManager.java 8b6bf9a 
>   server/src/com/cloud/network/NetworkManagerImpl.java ba5ab5d 
>   server/src/com/cloud/network/NetworkModelImpl.java e83c0d2 
>   server/src/com/cloud/network/NetworkServiceImpl.java 821aa6d 
>   server/src/com/cloud/network/dao/IPAddressDao.java 1052639 
>   server/src/com/cloud/network/dao/IPAddressDaoImpl.java b0eef4f 
>   server/src/com/cloud/network/guru/DirectNetworkGuru.java 46a525e 
>   server/src/com/cloud/network/security/SecurityGroupManagerImpl.java eafe88e 
>   server/src/com/cloud/network/security/SecurityGroupManagerImpl2.java a3a0fc3 
>   server/src/com/cloud/vm/UserVmManagerImpl.java ca9c13f 
>   server/src/com/cloud/vm/dao/NicDao.java 794bacc 
>   server/src/com/cloud/vm/dao/NicDaoImpl.java 4491174 
>   server/test/com/cloud/network/MockNetworkManagerImpl.java 8004310 
>   server/test/com/cloud/vpc/MockNetworkManagerImpl.java 63ef874 
> 
> Diff: https://reviews.apache.org/r/9730/diff/
> 
> 
> Testing
> -------
> 
> Unit Tests done:
> 1. adding a secondary ip to nic
> 2. remove ip from nic
> 3. reaching public network from the VM.
> 4. security groups changes for the secondary ip are done.
> 
> 
> Thanks,
> 
> Jayapal Reddy
> 
>


Re: Review Request: multiple ip address per nic changes for basic zone

Posted by Jayapal Reddy <ja...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9730/
-----------------------------------------------------------

(Updated March 6, 2013, 12:45 p.m.)


Review request for cloudstack, Abhinandan Prateek, Murali Reddy, and anthony xu.


Changes
-------

updated patch minor chagnes


Description
-------

In this patch changes are made for the basic zone and security groups


This addresses bug cloudstack-24.


Diffs (updated)
-----

  api/src/com/cloud/agent/api/SecurityGroupRulesCmd.java affad1f 
  api/src/com/cloud/agent/api/to/NicTO.java aa2aa19 
  api/src/com/cloud/network/security/SecurityGroupService.java c648032 
  api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java 0f99274 
  api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java cb5e085 
  core/src/com/cloud/agent/api/NetworkRulesVmSecondaryIpCommand.java PRE-CREATION 
  plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 4a89806 
  scripts/vm/hypervisor/xenserver/vmops db6f6d6 
  server/src/com/cloud/hypervisor/HypervisorGuruBase.java efe9396 
  server/src/com/cloud/network/NetworkManager.java 8b6bf9a 
  server/src/com/cloud/network/NetworkManagerImpl.java ba5ab5d 
  server/src/com/cloud/network/NetworkModelImpl.java e83c0d2 
  server/src/com/cloud/network/NetworkServiceImpl.java 821aa6d 
  server/src/com/cloud/network/dao/IPAddressDao.java 1052639 
  server/src/com/cloud/network/dao/IPAddressDaoImpl.java b0eef4f 
  server/src/com/cloud/network/guru/DirectNetworkGuru.java 46a525e 
  server/src/com/cloud/network/security/SecurityGroupManagerImpl.java eafe88e 
  server/src/com/cloud/network/security/SecurityGroupManagerImpl2.java a3a0fc3 
  server/src/com/cloud/vm/UserVmManagerImpl.java ca9c13f 
  server/src/com/cloud/vm/dao/NicDao.java 794bacc 
  server/src/com/cloud/vm/dao/NicDaoImpl.java 4491174 
  server/test/com/cloud/network/MockNetworkManagerImpl.java 8004310 
  server/test/com/cloud/vpc/MockNetworkManagerImpl.java 63ef874 

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


Testing
-------

Unit Tests done:
1. adding a secondary ip to nic
2. remove ip from nic
3. reaching public network from the VM.
4. security groups changes for the secondary ip are done.


Thanks,

Jayapal Reddy


Re: Review Request: multiple ip address per nic changes for basic zone

Posted by Jayapal Reddy <ja...@citrix.com>.

> On March 5, 2013, 7:28 p.m., anthony xu wrote:
> > - log file for SG is used to restore SG when VM reboot, and do SG sync, I saw log for second ip is added, I didn't see code to use the log to restore SG, can you check it SG rules is correct after you reboot VM through xencener? after VM reboot, the NIC interface name changes, CS uses log to rebuild SG rules for this VM. can you check if VM SG rules are deleted after you shutdown VM through xencenter?
> > - I didn't see the change for KVM, is this feature only for XS.
> 
> Jayapal Reddy wrote:
>     I tested the vm reboot case from xencenter. The rules (SG) rules for secondary ips are added correctly.
>     
>     When vm is stopped the rules for the secondary ips are deleted.
>       1.destroy_arptables_rules - deletes all the the arprules 
>       2.The existing code flush and delete ipset with name VMNAME, this causes the deletion of 
>         ipset with vmname i created.
>     
>     
>     
>     The secondary ip log file is read using the get_vm_sec_ips 
>     When vm is rebooted all the arprules for the secondary ips are added using the secondary ip log file.
>     
>     The ipset is update will not change on reboot. 
>     The update of ipset happen only when the vm secondary ip added/deleted and ipset is deleted when vm is destroyed.
>     
>     get_vm_sec_ips - get the vm secondary ips from the log file
>     arp_rules_vmip - adds the arp rules for the vm ips
>     
>     
>     In vm reboot we destroy the SG rules and add rules again. 
>     In network_rules_for_rebooted_vm we have below code for secondary ips
>     
>         #check wether the vm has secondary ips
>         if is_secondary_ips_set(vm_name) == True:
>             vmips = get_vm_sec_ips(vm_name)
>             #add arp rules for the secondaryp ip
>     	for ip in vmips:
>                 arp_rules_vmip(vmchain, vifs, [ip], vm_mac, "-A")

The changes in the patch are only for xenserver, for kvm yet to submit the patch


- Jayapal


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


On March 4, 2013, 1:18 p.m., Jayapal Reddy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9730/
> -----------------------------------------------------------
> 
> (Updated March 4, 2013, 1:18 p.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek, Murali Reddy, and anthony xu.
> 
> 
> Description
> -------
> 
> In this patch changes are made for the basic zone and security groups
> 
> 
> This addresses bug cloudstack-24.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/agent/api/SecurityGroupRulesCmd.java affad1f 
>   api/src/com/cloud/agent/api/to/NicTO.java aa2aa19 
>   api/src/com/cloud/network/security/SecurityGroupService.java c648032 
>   api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java 0f99274 
>   api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java cb5e085 
>   core/src/com/cloud/agent/api/NetworkRulesVmSecondaryIpCommand.java PRE-CREATION 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 1896b0e 
>   scripts/vm/hypervisor/xenserver/vmops 92ed79a 
>   server/src/com/cloud/hypervisor/HypervisorGuruBase.java efe9396 
>   server/src/com/cloud/network/NetworkManager.java 8b6bf9a 
>   server/src/com/cloud/network/NetworkManagerImpl.java a575183 
>   server/src/com/cloud/network/NetworkModelImpl.java 0525888 
>   server/src/com/cloud/network/NetworkServiceImpl.java 4c61aec 
>   server/src/com/cloud/network/dao/IPAddressDao.java 1052639 
>   server/src/com/cloud/network/dao/IPAddressDaoImpl.java 691e460 
>   server/src/com/cloud/network/guru/DirectNetworkGuru.java 46a525e 
>   server/src/com/cloud/network/security/SecurityGroupManagerImpl.java eafe88e 
>   server/src/com/cloud/network/security/SecurityGroupManagerImpl2.java a3a0fc3 
>   server/src/com/cloud/vm/UserVmManagerImpl.java 3fb5d92 
>   server/src/com/cloud/vm/dao/NicDao.java 794bacc 
>   server/src/com/cloud/vm/dao/NicDaoImpl.java 4491174 
>   server/test/com/cloud/network/MockNetworkManagerImpl.java 8004310 
>   server/test/com/cloud/vpc/MockNetworkManagerImpl.java 63ef874 
> 
> Diff: https://reviews.apache.org/r/9730/diff/
> 
> 
> Testing
> -------
> 
> Unit Tests done:
> 1. adding a secondary ip to nic
> 2. remove ip from nic
> 3. reaching public network from the VM.
> 4. security groups changes for the secondary ip are done.
> 
> 
> Thanks,
> 
> Jayapal Reddy
> 
>


Re: Review Request: multiple ip address per nic changes for basic zone

Posted by Jayapal Reddy <ja...@citrix.com>.

> On March 5, 2013, 7:28 p.m., anthony xu wrote:
> > - log file for SG is used to restore SG when VM reboot, and do SG sync, I saw log for second ip is added, I didn't see code to use the log to restore SG, can you check it SG rules is correct after you reboot VM through xencener? after VM reboot, the NIC interface name changes, CS uses log to rebuild SG rules for this VM. can you check if VM SG rules are deleted after you shutdown VM through xencenter?
> > - I didn't see the change for KVM, is this feature only for XS.

I tested the vm reboot case from xencenter. The rules (SG) rules for secondary ips are added correctly.

When vm is stopped the rules for the secondary ips are deleted.
  1.destroy_arptables_rules - deletes all the the arprules 
  2.The existing code flush and delete ipset with name VMNAME, this causes the deletion of 
    ipset with vmname i created.


The secondary ip log file is read using the get_vm_sec_ips 
When vm is rebooted all the arprules for the secondary ips are added using the secondary ip log file.

The ipset is update will not change on reboot. 
The update of ipset happen only when the vm secondary ip added/deleted and ipset is deleted when vm is destroyed.

get_vm_sec_ips - get the vm secondary ips from the log file
arp_rules_vmip - adds the arp rules for the vm ips


In vm reboot we destroy the SG rules and add rules again. 
In network_rules_for_rebooted_vm we have below code for secondary ips

    #check wether the vm has secondary ips
    if is_secondary_ips_set(vm_name) == True:
        vmips = get_vm_sec_ips(vm_name)
        #add arp rules for the secondaryp ip
	for ip in vmips:
            arp_rules_vmip(vmchain, vifs, [ip], vm_mac, "-A")


- Jayapal


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


On March 4, 2013, 1:18 p.m., Jayapal Reddy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9730/
> -----------------------------------------------------------
> 
> (Updated March 4, 2013, 1:18 p.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek, Murali Reddy, and anthony xu.
> 
> 
> Description
> -------
> 
> In this patch changes are made for the basic zone and security groups
> 
> 
> This addresses bug cloudstack-24.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/agent/api/SecurityGroupRulesCmd.java affad1f 
>   api/src/com/cloud/agent/api/to/NicTO.java aa2aa19 
>   api/src/com/cloud/network/security/SecurityGroupService.java c648032 
>   api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java 0f99274 
>   api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java cb5e085 
>   core/src/com/cloud/agent/api/NetworkRulesVmSecondaryIpCommand.java PRE-CREATION 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 1896b0e 
>   scripts/vm/hypervisor/xenserver/vmops 92ed79a 
>   server/src/com/cloud/hypervisor/HypervisorGuruBase.java efe9396 
>   server/src/com/cloud/network/NetworkManager.java 8b6bf9a 
>   server/src/com/cloud/network/NetworkManagerImpl.java a575183 
>   server/src/com/cloud/network/NetworkModelImpl.java 0525888 
>   server/src/com/cloud/network/NetworkServiceImpl.java 4c61aec 
>   server/src/com/cloud/network/dao/IPAddressDao.java 1052639 
>   server/src/com/cloud/network/dao/IPAddressDaoImpl.java 691e460 
>   server/src/com/cloud/network/guru/DirectNetworkGuru.java 46a525e 
>   server/src/com/cloud/network/security/SecurityGroupManagerImpl.java eafe88e 
>   server/src/com/cloud/network/security/SecurityGroupManagerImpl2.java a3a0fc3 
>   server/src/com/cloud/vm/UserVmManagerImpl.java 3fb5d92 
>   server/src/com/cloud/vm/dao/NicDao.java 794bacc 
>   server/src/com/cloud/vm/dao/NicDaoImpl.java 4491174 
>   server/test/com/cloud/network/MockNetworkManagerImpl.java 8004310 
>   server/test/com/cloud/vpc/MockNetworkManagerImpl.java 63ef874 
> 
> Diff: https://reviews.apache.org/r/9730/diff/
> 
> 
> Testing
> -------
> 
> Unit Tests done:
> 1. adding a secondary ip to nic
> 2. remove ip from nic
> 3. reaching public network from the VM.
> 4. security groups changes for the secondary ip are done.
> 
> 
> Thanks,
> 
> Jayapal Reddy
> 
>


Re: Review Request: multiple ip address per nic changes for basic zone

Posted by anthony xu <an...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9730/#review17417
-----------------------------------------------------------


- log file for SG is used to restore SG when VM reboot, and do SG sync, I saw log for second ip is added, I didn't see code to use the log to restore SG, can you check it SG rules is correct after you reboot VM through xencener? after VM reboot, the NIC interface name changes, CS uses log to rebuild SG rules for this VM. can you check if VM SG rules are deleted after you shutdown VM through xencenter?
- I didn't see the change for KVM, is this feature only for XS.

- anthony xu


On March 4, 2013, 1:18 p.m., Jayapal Reddy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9730/
> -----------------------------------------------------------
> 
> (Updated March 4, 2013, 1:18 p.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek, Murali Reddy, and anthony xu.
> 
> 
> Description
> -------
> 
> In this patch changes are made for the basic zone and security groups
> 
> 
> This addresses bug cloudstack-24.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/agent/api/SecurityGroupRulesCmd.java affad1f 
>   api/src/com/cloud/agent/api/to/NicTO.java aa2aa19 
>   api/src/com/cloud/network/security/SecurityGroupService.java c648032 
>   api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java 0f99274 
>   api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java cb5e085 
>   core/src/com/cloud/agent/api/NetworkRulesVmSecondaryIpCommand.java PRE-CREATION 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 1896b0e 
>   scripts/vm/hypervisor/xenserver/vmops 92ed79a 
>   server/src/com/cloud/hypervisor/HypervisorGuruBase.java efe9396 
>   server/src/com/cloud/network/NetworkManager.java 8b6bf9a 
>   server/src/com/cloud/network/NetworkManagerImpl.java a575183 
>   server/src/com/cloud/network/NetworkModelImpl.java 0525888 
>   server/src/com/cloud/network/NetworkServiceImpl.java 4c61aec 
>   server/src/com/cloud/network/dao/IPAddressDao.java 1052639 
>   server/src/com/cloud/network/dao/IPAddressDaoImpl.java 691e460 
>   server/src/com/cloud/network/guru/DirectNetworkGuru.java 46a525e 
>   server/src/com/cloud/network/security/SecurityGroupManagerImpl.java eafe88e 
>   server/src/com/cloud/network/security/SecurityGroupManagerImpl2.java a3a0fc3 
>   server/src/com/cloud/vm/UserVmManagerImpl.java 3fb5d92 
>   server/src/com/cloud/vm/dao/NicDao.java 794bacc 
>   server/src/com/cloud/vm/dao/NicDaoImpl.java 4491174 
>   server/test/com/cloud/network/MockNetworkManagerImpl.java 8004310 
>   server/test/com/cloud/vpc/MockNetworkManagerImpl.java 63ef874 
> 
> Diff: https://reviews.apache.org/r/9730/diff/
> 
> 
> Testing
> -------
> 
> Unit Tests done:
> 1. adding a secondary ip to nic
> 2. remove ip from nic
> 3. reaching public network from the VM.
> 4. security groups changes for the secondary ip are done.
> 
> 
> Thanks,
> 
> Jayapal Reddy
> 
>