incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Koushik Das <koushik....@citrix.com>
Subject RE: Review Request: Fix CS-15603
Date Fri, 17 Aug 2012 12:44:47 GMT
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.xia1@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.das@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_xia@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.das@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:mpatton@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.
>

Mime
View raw message