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.
> >
>

Reply via email to