oops.. sorry I missed that line..You are right, no impact on existing migrating logic at all.
Regards Mice 2012/8/17 Koushik Das <koushik....@citrix.com> > Mice, > > There is a line at the very beginning of in the 1st loop (before any > condition checks) > > AgentVmInfo info = infos.remove(vm.getId()); > > This will ensure that any VM that CS knows about doesn't get removed in > the 2nd loop. > > Thanks, > Koushik > > -----Original Message----- > From: Mice Xia [mailto:weiran.x...@gmail.com] > Sent: Friday, August 17, 2012 5:32 PM > To: cloudstack-dev@incubator.apache.org > Subject: Re: Review Request: Fix CS-15603 > > Koushik, > > Yes, there are two loops. > > The first loop does not take care of VMs in Migrating state, So VM in > Migrating state will be handled in the second loop, After removing the > first line in the second loop, the VM in Migrating state get stopped. > > How do you think adding another 'else if' in the first loop for migrating > state ? > > Regards > Mice > > 2012/8/17 Koushik Das <koushik....@citrix.com> > > > Mice, > > > > This change won't have any impact on VMs in migrating state. If you > > see any issue with VMs in migrating state raise a separate bug. > > If you look at fullSync code then it compares the db state (list of > > VMs from CS DB stored in variable 'set_vms') and actual state (list of > > VMs from the hosts stored in variable 'infos'). There are 2 for loops. > > In the first loop it processes VMs from 'set_vms' and in the process > > removes the entry from 'infos' if present. In the second loop it > > iterates over the remaining VMs in 'infos' which are not tracked by CS > and cleans them up. > > > > Thanks, > > Koushik > > > > -----Original Message----- > > From: Mice Xia [mailto:mice_...@tcloudcomputing.com] > > Sent: Tuesday, August 14, 2012 7:58 AM > > To: cloudstack-dev@incubator.apache.org > > Subject: RE: Review Request: Fix CS-15603 > > > > Koushik, > > > > I'm not very familiar with fullsync/deltaSync. I saw you removed > > following > > line: > > > > 'if (VirtualMachineName.isValidVmName(left.name)) continue; // if the > > vm follows cloudstack naming ignore it for stopping' > > > > What if someone migrates a VM, and restart management server > > immediately, then the VM is in migrating state in DB when mgmt-server > > is back, but fullSync does not take care of VMs in Migrating state. > > Removing above line leads the VM in migrating state get stopped, I'm > > not sure this is the desired behavior of fullSync, should this be > > handled in deltaSync or compareState? > > > > Regards > > Mice > > > > -----Original Message----- > > From: Koushik Das [mailto:koushik....@citrix.com] > > Sent: Tuesday, August 14, 2012 2:27 AM > > To: cloudstack-dev@incubator.apache.org > > Subject: RE: Review Request: Fix CS-15603 > > > > Matthew, > > > > You have a valid point and the regular delete operation does exactly > > what you have mentioned, in case the HV is not reachable it fails with > error. > > But in case of a forced delete, CS cleans up the db state even though > > it is unable to perform the actual delete. In this scenario when the > > HV connectivity resumes then CS syncs up its state with that of the HV. > > > > It may be argued if the option of forced delete be there or not. This > > feature has been there in CS for quite some time and I assume that > > there are users who find it useful. As far as CS is concerned it > > provides both options, it is up to the users what they want to choose. > > > > -Koushik > > > > -----Original Message----- > > From: Matthew Patton [mailto:mpat...@inforelay.com] > > Sent: Monday, August 13, 2012 7:06 PM > > To: cloudstack-dev@incubator.apache.org > > Subject: RE: Review Request: Fix CS-15603 > > > > This it's yet another case of misleading the user and lying about > > system state. The deletion flat out failed. CS should say so. > > > > if anything create a job in the queue and retry the operation a number > > of times if you want. But the state is "pending deletion" until it > > actually executes. > > >