> On Aug. 7, 2013, 12:03 a.m., Sheng Yang wrote: > > server/src/com/cloud/network/NetworkManagerImpl.java, line 2433 > > <https://reviews.apache.org/r/13323/diff/1/?file=337776#file337776line2433> > > > > 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)
Hi sheng, removeDhcpServiceInsubnet functon calls the function listLastNicsInSubnet to get the last nics in subnet and then it removes the DhcpService based on the networkId. So I mean we do not know if it is last nic in the subnet until we call the listLastNicsInsubnet so we cannot put this check before hand. > On Aug. 7, 2013, 12:03 a.m., Sheng Yang wrote: > > server/src/com/cloud/vm/VirtualMachineManagerImpl.java, line 479 > > <https://reviews.apache.org/r/13323/diff/1/?file=337777#file337777line479> > > > > Why we still need this "if networkid is null then we remove all"? > > removeNic() for each network in network manager is not enough? Hi sheng, In cases when a VM is getting expunged we need to remove the Dhcpservice in all the networks in which this vm has a last nic. so we pass a null as networkId to do this. - bharat ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13323/#review24762 ----------------------------------------------------------- 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 > >
