> On Aug. 25, 2014, 8:56 p.m., Nitin Mehta wrote: > > plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java, > > line 95 > > <https://reviews.apache.org/r/24779/diff/1/?file=662277#file662277line95> > > > > Thanks for working on this patch. > > Any reason why you cant take the same approach as in master right now - > > that of adding a param - entityType in the method - can you please look at > > that code and fix it same way - keeping it consistent and more elegant ?
Because I'm new to Java and didn't know about the entityType method :) I'll clean it up and resumbit. While the fix is more elligant, it still feels like the correct fix is to remove all references to the UploadVO since the table is deprecated and should never have new data in it. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24779/#review51439 ----------------------------------------------------------- On Aug. 17, 2014, 3:02 a.m., David Bierce wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24779/ > ----------------------------------------------------------- > > (Updated Aug. 17, 2014, 3:02 a.m.) > > > Review request for cloudstack. > > > Bugs: CLOUDSTACK-6254 > https://issues.apache.org/jira/browse/CLOUDSTACK-6254 > > > Repository: cloudstack-git > > > Description > ------- > > PATCH] This is a quick stab at fixing a dataloss bug. The ultimate > solution is to refactor UploadManager to not use any deprecated code. It > appears there is still code left over that uses the UploadVO/Dao which no > long contains data about URL transfers. This method was hardcoded to always > pass Upload.Type.VOLUME as part of cleanup which was causing templates to be > removed entirely from secondary storage not just the symlink on secondary > storage. > > Rather than try to refactor all of it out, this puts > logic for determining if the cleanup task is for a volume or a template > by doing a lookup on the URL. It is a duplication of the same logic > from the calling method but is a very minimal code change until the > large problem is fixed. > > > Diffs > ----- > > > plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java > 4796653 > > Diff: https://reviews.apache.org/r/24779/diff/ > > > Testing > ------- > > On Cloudstack 4.2 4.3 > Set cleanupurl to 30 seconds. Downloaded a template, cleanup remvoed it from > database, didn't remove the template. > Downloaded Volume, volume was cleaned up from secondary stoage and database. > > > Thanks, > > David Bierce > >
