----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8632/#review15753 -----------------------------------------------------------
I tried to apply your snapshot patches and test them, as they look reasonable, unfortunately I think something else is broken at the moment because snapshot cleanup is still not working. This patch may be useful but it's hard to tell with snapshot deletes still broken. We'll have to open a bug for it if there's not one already. at com.cloud.agent.api.SnapshotCommand.<init>(SnapshotCommand.java:56) at com.cloud.agent.api.DeleteSnapshotBackupCommand.<init>(DeleteSnapshotBackupCommand.java:89) at com.cloud.storage.snapshot.SnapshotManagerImpl.destroySnapshotBackUp(SnapshotManagerImpl.java:916) at com.cloud.utils.db.DatabaseCallback.intercept(DatabaseCallback.java:34) at com.cloud.storage.snapshot.SnapshotManagerImpl.deleteSnapshotInternal(SnapshotManagerImpl.java:847) at com.cloud.storage.snapshot.SnapshotManagerImpl.deleteSnapshot(SnapshotManagerImpl.java:794) at com.cloud.utils.component.ComponentLocator$InterceptorDispatcher.intercept(ComponentLocator.java:1262) at org.apache.cloudstack.api.command.user.snapshot.DeleteSnapshotCmd.execute(DeleteSnapshotCmd.java:92) at com.cloud.api.ApiDispatcher.dispatch(ApiDispatcher.java:168) at com.cloud.async.AsyncJobManagerImpl$1.run(AsyncJobManagerImpl.java:435) at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471) at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334) at java.util.concurrent.FutureTask.run(FutureTask.java:166) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603) at java.lang.Thread.run(Thread.java:722) This is against current master. I think the first big issue to tackle is to make it stop returning success when it obviously fails. - Marcus Sorensen On Dec. 18, 2012, 12:09 a.m., Prasanna Santhanam wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8632/ > ----------------------------------------------------------- > > (Updated Dec. 18, 2012, 12:09 a.m.) > > > Review request for cloudstack, Nitin Mehta, edison su, and Marcus Sorensen. > > > Description > ------- > > When CleanupSnapshotBackupCommand is sent to the SSVM agent because it needs > to perform storage GC - outdated snapshots remain undeleted. But the answer > returned to mgmt server is success. This is related to CLOUDSTACK-643 where > KVM uses a different path to its snapshot backups that includes the zoneid in > the path. > > > KVM uses /snapshots/zoneid/accountid/volumeid for its snapshot path. The > cleanupbackupsnapshotcommand which performs GC interprets an invalid path > (as > /snapshots/accountid/volumeid) to the snapshot and fails to delete them > from > sec. storage. > > Now GC path is corrected so snapshots removed fmor DB are also removed > from > secondary storage. > > > This addresses bug CLOUDSTACK-649. > > > Diffs > ----- > > core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java > d8fdc3a > > Diff: https://reviews.apache.org/r/8632/diff/ > > > Testing > ------- > > 1. Tested create snapshot, delete snapshot > 2. Tested recurring snapshot creation , deletion via storage gc > > Both scenarios pass > > > Thanks, > > Prasanna Santhanam > >