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.me...@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....@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:nore...@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
