cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nitin Mehta <Nitin.Me...@citrix.com>
Subject RE: Review Request: CS-15823 [VMware] failed snapshot entry should be removed automatically when multiple snopshots are taken on one volume simultaneously
Date Mon, 06 Aug 2012 04:54:00 GMT
But, then how would the users track what happened to their snapshots - they would see no record
of them at all ? And is this going to be done for recurring snapshots as well ?
I agree with Edison. Keeping a row of snapshot with Error state seems fine with me.

-----Original Message-----
From: Mice Xia [mailto:mice_xia@tcloudcomputing.com] 
Sent: Monday, August 06, 2012 8:47 AM
To: cloudstack-dev@incubator.apache.org
Cc: Edison Su; Koushik Das; Nitin Mehta
Subject: RE: Review Request: CS-15823 [VMware] failed snapshot entry should be removed automatically
when multiple snopshots are taken on one volume simultaneously

After reviewing the legacy code again, I suggest to expunge failed snapshot no matter what
reason. 
I think there is no need for users to see an Error snapshot, setting up a new thread for expunging
Error/Removed snapshots seem a bit heavy-weight and does not seem to improve much feasibility
for users.

I'll update the patch if everyone agrees on it.

Thanks & Regards
Mice

-----Original Message-----
From: Edison Su [mailto:Edison.su@citrix.com]
Sent: Saturday, August 04, 2012 2:12 AM
To: Mice Xia; Koushik Das; cloudstack-dev@incubator.apache.org; Nitin Mehta
Subject: RE: Review Request: CS-15823 [VMware] failed snapshot entry should be removed automatically
when multiple snopshots are taken on one volume simultaneously

I think if creating snapshot failed, no matter whatever reason, the snapshot should be marked
as "Error". Just like the VM, if it's failed to create, them marked as "Error", then a background
reclaim thread will expunge it.

> -----Original Message-----
> From: Mice Xia [mailto:mice_xia@tcloudcomputing.com]
> Sent: Friday, August 03, 2012 1:29 AM
> To: Koushik Das; cloudstack-dev@incubator.apache.org; Nitin Mehta
> Cc: Edison Su
> Subject: RE: Review Request: CS-15823 [VMware] failed snapshot entry 
> should be removed automatically when multiple snopshots are taken on 
> one volume simultaneously
> 
> Remove the wrong reviewer from loop.. sorry for that, rely on auto- 
> complete feature in review board but it turned out that was not 
> Anthony Xu..
> 
> Yes, handling it in finally block seems more neat, but I need some 
> background knowledge, i.e. in which situations failed snapshots should 
> be expunged, and in which should be marked as Error?
> From legacy code I assume that if it does not to pass the check before 
> createSnapshotOnPrimary, it is supposed to be expunged, otherwise 
> means something wrong occurs in resource side and snapshot was kept as Error.
> 
> And by the way, does VMs that failed to create get handled by the same 
> rule?
> 
> Regards
> Mice
> 
> -----Original Message-----
> From: Koushik Das [mailto:koushik.das@citrix.com]
> Sent: Friday, August 03, 2012 2:23 PM
> To: cloudstack-dev@incubator.apache.org; Mice Xia; Edison Su; Anthony 
> Urso
> Subject: RE: Review Request: CS-15823 [VMware] failed snapshot entry 
> should be removed automatically when multiple snopshots are taken on 
> one volume simultaneously
> 
> But even in generic cases (just look at the exception above the 
> changed one), the snapshot is marked as expunged. So I think it is 
> better to do it in finally block.
> 
> -----Original Message-----
> From: Nitin Mehta [mailto:Nitin.Mehta@citrix.com]
> Sent: Friday, August 03, 2012 11:11 AM
> To: cloudstack-dev@incubator.apache.org; mice xia; Edison Su; Anthony 
> Urso
> Subject: RE: Review Request: CS-15823 [VMware] failed snapshot entry 
> should be removed automatically when multiple snopshots are taken on 
> one volume simultaneously
> 
> Good point. But this is not failure per se in the snapshot creation 
> but more of a limitation of these hypervisors and probably we want to 
> keep that subtle difference by not putting it in Error but deleting 
> the row altogether and surfacing the limitation through the exception 
> to the end user.
> Satisfied :) ?
> 
> -----Original Message-----
> From: Koushik Das [mailto:koushik.das@citrix.com]
> Sent: Friday, August 03, 2012 10:38 AM
> To: cloudstack-dev@incubator.apache.org; mice xia; Edison Su; Anthony 
> Urso
> Subject: RE: Review Request: CS-15823 [VMware] failed snapshot entry 
> should be removed automatically when multiple snopshots are taken on 
> one volume simultaneously
> 
> Any reason to handle this specific case only? I see in the code there 
> are other places as well where snapshot creation may fail. Why not do 
> it in the finally block so that all failure cases get handled.
> 
> -Koushik
> 
> -----Original Message-----
> From: mice xia [mailto:noreply@reviews.apache.org] On Behalf Of mice 
> xia
> Sent: Friday, August 03, 2012 5:33 AM
> To: Edison Su; Anthony Urso
> Cc: cloudstack; mice xia
> Subject: Review Request: CS-15823 [VMware] failed snapshot entry 
> should be removed automatically when multiple snopshots are taken on 
> one volume simultaneously
> 
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6325/
> -----------------------------------------------------------
> 
> Review request for cloudstack, Anthony Urso and edison su.
> 
> 
> Description
> -------
> 
> changes:
> in SnapshotManagerImpl.java, expunge snapshot for this situation
> 
> 
> This addresses bug CS-15823.
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
> 6e3f9c1
> 
> Diff: https://reviews.apache.org/r/6325/diff/
> 
> 
> Testing
> -------
> 
> verified
> 
> 
> Thanks,
> 
> mice xia

Mime
View raw message