[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-09-02 Thread bhaisaab
Github user bhaisaab commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-136962823 @miguelaferreira @wilderrodrigues @wido @jburwell @abhinandanprateek @kishankavala hi, can you help review this, thanks --- If your project is set up for it, you

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-09-02 Thread karuturi
Github user karuturi commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-136981967 @bhaisaab There are 2 reviews already. @wido and @miguelaferreira gave :+1: I think you can merge this and #753 --- If your project is set up for it, you can

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-09-02 Thread bhaisaab
Github user bhaisaab commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-136995082 Thanks @karuturi merging now --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-09-02 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/753 --- 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-8762: Check to confirm disk ac...

2015-09-02 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/754 --- 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-8762: Check to confirm disk ac...

2015-09-01 Thread miguelaferreira
Github user miguelaferreira commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-136640381 When I build the entire project mvn always gets stuck running unit tests of some project (not always the same). I don't want to keep digging any further

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-09-01 Thread bhaisaab
Github user bhaisaab commented on the pull request: https://github.com/apache/cloudstack/pull/753#issuecomment-136662511 @miguelaferreira thanks :+1: --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-09-01 Thread miguelaferreira
Github user miguelaferreira commented on the pull request: https://github.com/apache/cloudstack/pull/753#issuecomment-136641962 @bhaisaab I'm testing this one now. --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-09-01 Thread miguelaferreira
Github user miguelaferreira commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-136635121 @bhaisaab I've created a totally new test environment, checked out your PR and started building it. I'm already past the point where it was failing before,

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-09-01 Thread miguelaferreira
Github user miguelaferreira commented on the pull request: https://github.com/apache/cloudstack/pull/753#issuecomment-136648011 Full maven build successful, with the exception of Console Proxy RDP client project that contains a flakey unit test. :+1: --- If your project is

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-31 Thread miguelaferreira
Github user miguelaferreira commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-136322596 Yes, I'm still getting the same error. It could very well be an environment issue. ping @wilderrodrigues @remibergsma can you guys give it a try?

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-30 Thread bhaisaab
Github user bhaisaab commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-136265583 @miguelaferreira I'm not sure, but this seems environment specific; travis is green and build also work in my environment; are you still able to reproduce.

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-28 Thread wido
Github user wido commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-135667196 Seems, good, but, all the logging is debug. Isn't there something which we have to print on info or error here? We want to make sure that we also print useful

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-28 Thread miguelaferreira
Github user miguelaferreira commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-135745806 I'm not able to build this branch with maven. ``` mvn clean install -P developer,systemvm ... [INFO]

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-28 Thread bhaisaab
Github user bhaisaab commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-135709691 @wido just checked again, most debug messages are in loop and changing them to info or errors would be unnecessary. In case of error, run time exceptions are

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-28 Thread bhaisaab
Github user bhaisaab commented on the pull request: https://github.com/apache/cloudstack/pull/753#issuecomment-135679096 @wido this is for master branch, that is for 4.5 branch (not cherry-pickable) --- If your project is set up for it, you can reply to this email and have your

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-28 Thread bhaisaab
Github user bhaisaab commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-135678520 @wido no need for info that, will fix to use error in case of errors --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-28 Thread wido
Github user wido commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-135745126 LGTM --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-28 Thread wido
Github user wido commented on the pull request: https://github.com/apache/cloudstack/pull/753#issuecomment-135665672 I think we can close this one? Since #754 is the same? --- 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-8762: Check to confirm disk ac...

2015-08-28 Thread miguelaferreira
Github user miguelaferreira commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-135712348 @bhaisaab It would be great if you could refactor the logic out, since it can be an object on its own, and that class is already huge as it is ( 5k LOC!).

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-28 Thread bhaisaab
Github user bhaisaab commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-135733090 @miguelaferreira have moved the code to a new util class (so can be reusable perhaps by other hypervisors) ^^ --- If your project is set up for it, you can reply

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-28 Thread miguelaferreira
Github user miguelaferreira commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-135756504 @wido can you build it? --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-28 Thread miguelaferreira
Github user miguelaferreira commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-135757828 @wido can you please try? --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-28 Thread wido
Github user wido commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-135757610 @miguelaferreira I didn't build it since all checks were green :) --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-28 Thread bhaisaab
Github user bhaisaab commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-135814417 @miguelaferreira from the logs, looks like you were building 4.5 branch (the 4.5.2 artifact); on my system the build passes as well, are you using Java8? --- If

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-28 Thread miguelaferreira
Github user miguelaferreira commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-135815149 @bhaisaab I've checked out this PR, which is your `shapeblue:4.5-CLOUDSTACK-8762` applied to `apache:4.5`. Yes, I do have Java8 installed, but the mvn

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-27 Thread bhaisaab
Github user bhaisaab commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-135483908 @miguelaferreira okay, I'll see what I can do. I'm also exploring other ways to detech if the vm disk file has changed, using an exhaustive checksum comparison for

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-27 Thread miguelaferreira
Github user miguelaferreira commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-135485255 @bhaisaab that's great. So if you think that in the future this implementation might change it would even be better to create an interface to use in the

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-27 Thread bhaisaab
GitHub user bhaisaab opened a pull request: https://github.com/apache/cloudstack/pull/754 CLOUDSTACK-8762: Check to confirm disk activity before starting a VM Implements a VM volume/disk file activity checker that checks if QCOW2 file has been changed before starting the VM.

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-27 Thread bhaisaab
GitHub user bhaisaab opened a pull request: https://github.com/apache/cloudstack/pull/753 CLOUDSTACK-8762: Check to confirm disk activity before starting a VM Implements a VM volume/disk file activity checker that checks if QCOW2 file has been changed before starting the VM.

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-27 Thread miguelaferreira
Github user miguelaferreira commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-135398434 I'll be happy to help with reviewing this PR, but I need help testing. Can you point me to a Marvin test that tests this functionality? --- If your

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-27 Thread bhaisaab
Github user bhaisaab commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-135398957 @miguelaferreira there is no marvin test written for this, this is a specific case of hosts fencing where two vms might be trying to write to the same disk, since

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-27 Thread miguelaferreira
Github user miguelaferreira commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/754#discussion_r38087198 --- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java --- @@ -3923,6 +3944,17 @@ public int

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-27 Thread miguelaferreira
Github user miguelaferreira commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-135400873 I'm reading through the code to see if I understand the change. The next thing is to find a way to test it. Might be difficult, but since it is a

[GitHub] cloudstack pull request: CLOUDSTACK-8762: Check to confirm disk ac...

2015-08-27 Thread miguelaferreira
Github user miguelaferreira commented on the pull request: https://github.com/apache/cloudstack/pull/754#issuecomment-135403411 It seems to me that the VM volume/disk file activity checker is a good candidate to become a new class that can be tested independent, and reused elsewhere.