[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/1441 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-207452373 It's all set to go. :-) On Apr 8, 2016, at 7:09 AM, Rafael Weing?rtner mailto:notificati...@github.com>> wrote: @swill

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-207425226 @swill no final comments ;) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your projec

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-207424478 It looks like I have two LGTM on this PR and the tests are passing. This one is ready? Any final comments? --- If your project is set up for it, you can reply to t

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-207031562 @mike-tutkowski, @swill I am ok with that --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well.

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-207031227 Thanks for the comments @rafaelweingartner and @ustcweizhou. Since canHandle is part of the infrastructure, we decided this particular PR isn't the p

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-206981924 @swill, all of my enquiries here were answered. My only complaint was about the method name "canHandle"; it misled me during my first and second look at t

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-206981349 @DaanHoogland I have some issues with networking when using redundant VRs in my environment. I consistently have this type of issue, and it is unrelated to the actu

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-206978195 @swill you have a failure, why do you want an LGTM, seems to me it didn't pass the tests --- If your project is set up for it, you can reply to this email and

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-206932399 I think this is ready pending a code review. Can someone who has spent some time reviewing this PR with Mike please give me the go ahead on this PR. @rafaelweingart

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-206925982 ## CI RESULTS ### 84/85 TESTS PASSED The only failed test is a test that often fails in my environment and is unrelated to t

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-205949475 Thanks, Will! On Apr 5, 2016, at 12:35 PM, Will Stevens mailto:notificati...@github.com>> wrote: I will get CI run against this PR as s

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-205937640 I will get CI run against this PR as soon as I have my CI reinstalled. Thanks @mike-tutkowski --- If your project is set up for it, you can reply to this email and

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user rafaelweingartner commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1441#discussion_r56880597 --- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java --- @@ -279,7 +282,52 @@ public bo

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1441#discussion_r56801490 --- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java --- @@ -279,7 +282,52 @@ public boolean

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user alexandrelimassantana commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1441#discussion_r56768712 --- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java --- @@ -440,41 +400,28 @@

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-197942848 The canHandle method is used by CloudStack's snapshot infrastructure to determine which snapshot strategy to use. For example, StorageSystemSnapshotS

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-197952462 Maybe we're looking at different code? I see the canHandle method being used from StorageStrategyFactoryImpl.getSnapshotStrategy. It has a m

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-197948544 I understand that your point that some other classes can overwrite that method and may return other elements of the enum. However, I do not see anything e

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-197970382 Ok @mike-tutkowski, now I believe I understand a little bit more of the code. You are reusing a method that already exist and returns the Strateg

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-197935023 @mike-tutkowski thanks for the improvements on PR title and description. I understand the point of throwing an exception and I really like that approa

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-197992182 got it @mike-tutkowski --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project doe

[GitHub] cloudstack pull request: CLOUDSTACK-9297 - Reworked logic in Stora...

Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1441#issuecomment-197975991 CloudStack iterates over all known snapshot strategies and asks each if it can handle a particular situation (take, delete, revert) for a volume or volume sn