----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13323/#review24762 -----------------------------------------------------------
server/src/com/cloud/network/NetworkManager.java <https://reviews.apache.org/r/13323/#comment48882> Seems not neceesary. server/src/com/cloud/network/NetworkManagerImpl.java <https://reviews.apache.org/r/13323/#comment48887> It's better to show the semantic of "if this is the last nic in the subnet" in code. Say, if (vm.getType() == Type.User && it's the last nic) server/src/com/cloud/network/NetworkManagerImpl.java <https://reviews.apache.org/r/13323/#comment48885> Uppercase "Subnet". server/src/com/cloud/vm/VirtualMachineManagerImpl.java <https://reviews.apache.org/r/13323/#comment48886> Why we still need this "if networkid is null then we remove all"? removeNic() for each network in network manager is not enough? - Sheng Yang On Aug. 6, 2013, 12:48 p.m., bharat kumar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13323/ > ----------------------------------------------------------- > > (Updated Aug. 6, 2013, 12:48 p.m.) > > > Review request for cloudstack, Alena Prokharchyk and Sheng Yang. > > > Bugs: Cloudstack-4083 > > > Repository: cloudstack-git > > > Description > ------- > > https://issues.apache.org/jira/browse/CLOUDSTACK-4083 > if a failure occurs while adding VM to another network (this should be the > first vm in the subnet). The ip alias created as a part of this process is > not removed. > > This occurred because we were not cleaning the alias ips in the event of a > failure. > > As a part of the fix. > 1.) moved the function removeDhcpServiceInsubnet and listLastNicsInSubnet to > NetworkManager (These were in VirtualMachineManagerImpl earlier.) > 2.) add the call to clean ipAlias in the remove nic function of the > networkManager. > 3.) Modified the removeDhcpServiceInsubnet to take network ad an argument. > This will help in removing only the ipAlias which belong to a particular > network. > > > Diffs > ----- > > server/src/com/cloud/network/NetworkManager.java dab7a13 > server/src/com/cloud/network/NetworkManagerImpl.java effee96 > server/src/com/cloud/vm/VirtualMachineManagerImpl.java b33ee49 > server/test/com/cloud/network/MockNetworkManagerImpl.java 7f34e55 > server/test/com/cloud/vpc/MockNetworkManagerImpl.java 178ebba > > Diff: https://reviews.apache.org/r/13323/diff/ > > > Testing > ------- > > Tested on 4.2. > created two guest networks guest1 and guest2. > created VMS in both the networks. > Added a VM(with no PV drivers) from guest1 to guest2. > this failed and on failure ipAlias configured as part of nic creation was > removed. > > Deleting the vm causes all the removal of all ipAliases from all the subnets > in which this is the lastvm. > > > Thanks, > > bharat kumar > >