----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19460/#review37881 -----------------------------------------------------------
It looks like VolumeServiceImpl.takeSnapshot() is the method who isn't properly handling the exception. It is also called from multiple places (namely VolumeApiServiceImpl.takeSnapshot() and VolumeApiServiceImpl.orchestrateTakeVolumeSnapshot()). It looks like your fix is only handling one of those cases. If you were to put the throw statement into VolumeServiceImpl.takeSnapshot()'s catch instead of just ignoring the exception it would be handled for anyone who tries to take a snapshot. - Chris Suich On March 20, 2014, noon, Alex Hitchins wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19460/ > ----------------------------------------------------------- > > (Updated March 20, 2014, noon) > > > Review request for cloudstack. > > > Repository: cloudstack-git > > > Description > ------- > > https://issues.apache.org/jira/browse/CLOUDSTACK-5825 > > I've added a check for null returned. Previously it would return even though > there were errors. If null is returned, the takeSnapshot raised and caught an > exception. I check for null and if received throw a new exception rather than > continue the flow. > > > Diffs > ----- > > server/src/com/cloud/storage/VolumeApiServiceImpl.java 5ffa99b > > Diff: https://reviews.apache.org/r/19460/diff/ > > > Testing > ------- > > Compiled and deployed on a new test system, completed steps and snapshots > created successfully. > > I could not re-create a snapshot error, however normal operation is as > expected. It's better it throws an exception rather than continue to log it's > successful when it's not. > > > Thanks, > > Alex Hitchins > >
