[GitHub] cloudstack pull request: resolving a conflict due to reformatting

2016-01-17 Thread thecodeassassin
Github user thecodeassassin commented on the pull request: https://github.com/apache/cloudstack/pull/1344#issuecomment-172453755 for what it's worth: 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 do

[GitHub] cloudstack pull request: CLOUDSTACK-9231: Root volume migration fr...

2016-01-17 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1336#issuecomment-172449988 [1336.vpc.results.txt](https://github.com/apache/cloudstack/files/93940/1336.vpc.results.txt) [1336.network.results.txt](https://github.com/apache/cloudsta

Build failed in Jenkins: build-master-slowbuild #3013

2016-01-17 Thread jenkins
See -- [...truncated 28733 lines...] [INFO] [INFO] --- findbugs-maven-plugin:3.0.1:findbugs (findbugs) @ cloud-quickcloud --- [INFO] [INFO] <<< findbugs-maven-plugin:3.0.1:check (cloudstack

[GitHub] cloudstack pull request: CLOUDSTACK-9195: Cancelled/failed async j...

2016-01-17 Thread koushik-das
Github user koushik-das commented on the pull request: https://github.com/apache/cloudstack/pull/1272#issuecomment-172421544 @DaanHoogland Thanks for reviewing the code. On MS shutdown/restart the incomplete job entry will always get marked as cancelled in DB. You are right that an

[GitHub] cloudstack pull request: Quota: findbug fixes

2016-01-17 Thread agneya2001
Github user agneya2001 commented on the pull request: https://github.com/apache/cloudstack/pull/1289#issuecomment-172418090 @remibergsma @bhaisaab made review changes --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If you

Build failed in Jenkins: build-master-slowbuild #3012

2016-01-17 Thread jenkins
See -- [...truncated 28733 lines...] [INFO] [INFO] --- findbugs-maven-plugin:3.0.1:findbugs (findbugs) @ cloud-quickcloud --- [INFO] [INFO] <<< findbugs-maven-plugin:3.0.1:check (cloudstack

Build failed in Jenkins: build-master-slowbuild #3011

2016-01-17 Thread jenkins
See Changes: [Remi Bergsma] Revert "Merge pull request #1228 from borisroman/CLOUDSTACK-9149" -- [...truncated 28743 lines...] [INFO] [INFO] --- findbugs-maven-plugin:3.0.1:findbugs

[GitHub] cloudstack pull request: BUG-ID: CLOUDSTACK-8849: Usage job should...

2016-01-17 Thread rodrigo93
Github user rodrigo93 commented on the pull request: https://github.com/apache/cloudstack/pull/827#issuecomment-172394161 Hi @yvsubhash, could you make a Jira? Sorry to ask you, but, why did you remove the tries and catches? It should throw exceptions if that part of the code can b

[GitHub] cloudstack pull request: Fixed return type Void to void in DataMot...

2016-01-17 Thread rodrigo93
Github user rodrigo93 commented on the pull request: https://github.com/apache/cloudstack/pull/969#issuecomment-172391494 @DaanHoogland I see... Well, thanks for your effort on helping in this PR. :) --- If your project is set up for it, you can reply to this email and have your repl

[GitHub] cloudstack pull request: BUG-ID CLOUDSTACK-8939

2016-01-17 Thread rodrigo93
Github user rodrigo93 commented on the pull request: https://github.com/apache/cloudstack/pull/914#issuecomment-172391340 @remibergsma The author seems to be inactive in github. His last public activity was on Oct 20, 2015... --- If your project is set up for it, you can reply to thi

[GitHub] cloudstack pull request: Removed unused code from com.cloud.api.Ap...

2016-01-17 Thread GabrielBrascher
Github user GabrielBrascher commented on the pull request: https://github.com/apache/cloudstack/pull/1263#issuecomment-172386387 @rodrigo93 This problem is not from the Jenkins. I am aware with the real problem (APIChecker bean) just haven't forced to commit again with the corrections

[GitHub] cloudstack pull request: CLOUDSTACK-9160: Remove unused folder(s)/...

2016-01-17 Thread rodrigo93
Github user rodrigo93 commented on the pull request: https://github.com/apache/cloudstack/pull/1238#issuecomment-172384379 LGTM based on the files, no tests were made. What is happening with Jenkins? I noticed that many PRs are flagged with error because Jenkins did not work. I

Build failed in Jenkins: build-master-slowbuild #3010

2016-01-17 Thread jenkins
See -- [...truncated 28733 lines...] [INFO] [INFO] --- findbugs-maven-plugin:3.0.1:findbugs (findbugs) @ cloud-quickcloud --- [INFO] [INFO] <<< findbugs-maven-plugin:3.0.1:check (cloudstack

[GitHub] cloudstack pull request: Removed unused variables from "NetworkSta...

2016-01-17 Thread rodrigo93
Github user rodrigo93 commented on the pull request: https://github.com/apache/cloudstack/pull/1261#issuecomment-172383990 LGMT. Can I give you a suggestion? You could remove those variable initializing with "_" (underscore) seems that is a common practice in C++ programming. --- If

[GitHub] cloudstack pull request: Fix cloud-cli references after PR 1228

2016-01-17 Thread remibergsma
Github user remibergsma closed the pull request at: https://github.com/apache/cloudstack/pull/1345 --- 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 featu

[GitHub] cloudstack pull request: Removed unused code from com.cloud.api.Ap...

2016-01-17 Thread rodrigo93
Github user rodrigo93 commented on the pull request: https://github.com/apache/cloudstack/pull/1263#issuecomment-172383741 @GabrielBrascher Nice change. It is always good to keep the good programming practices, especially in a project of this size. Can you commit it again? It seem

[GitHub] cloudstack pull request: Fix cloud-cli references after PR 1228

2016-01-17 Thread remibergsma
Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/1345#issuecomment-172383714 Found even more. Decided to revert PR 1228 instead. Closing this one. --- If your project is set up for it, you can reply to this email and have your reply appe

[GitHub] cloudstack pull request: Fix cloud-cli references after PR 1228

2016-01-17 Thread remibergsma
GitHub user remibergsma opened a pull request: https://github.com/apache/cloudstack/pull/1345 Fix cloud-cli references after PR 1228 After merging PR #1228, building RPM packages results in problems: ``` + cp -r cloud-cli/cloudtool /data/git/cs2/cloudstack/dist/rpmbuild

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

2016-01-17 Thread nitin-maharana
Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1319#issuecomment-172376856 Sure @remi --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

2016-01-17 Thread remibergsma
Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/1319#issuecomment-172376749 @nitin-maharana We can merge this once the integration tests have run. Will put it on my list. --- If your project is set up for it, you can reply to this emai

[GitHub] cloudstack pull request: Implement a NSX API request execution cou...

2016-01-17 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1294#issuecomment-172375026 makes sense @miguelaferreira . I have no use to this but I think it needs to go in LGTM @remibergsma --- If your project is set up for it, you can reply

[GitHub] cloudstack pull request: CLOUDSTACK-9230: Remove unnecessary retur...

2016-01-17 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/1335 --- 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-9230: Remove unnecessary retur...

2016-01-17 Thread remibergsma
Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/1335#issuecomment-172369877 LGTM, makes sense. Checked syntax and that is also still OK. https://cloud.githubusercontent.com/assets/1630096/12379391/223bdafa-bd5a-11e5-9795-1252ed58

[GitHub] cloudstack pull request: engine/schema: Use semantically correct u...

2016-01-17 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1279#issuecomment-172369667 I don't see the semantic necessity for this. doe upgrades work different if Upgrade461to470() is used insted of an empty Upgrade462to470()? It is just adding

[GitHub] cloudstack pull request: resolving a conflict due to reformatting

2016-01-17 Thread remibergsma
Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/1344#issuecomment-172368421 Thanks @DaanHoogland FYI this was the conflict after fwd-merge: ``` rbergsma[~/git/apache/cloudstack](master)$ git diff diff --cc engi

[GitHub] cloudstack pull request: resolving a conflict due to reformatting

2016-01-17 Thread DaanHoogland
GitHub user DaanHoogland opened a pull request: https://github.com/apache/cloudstack/pull/1344 resolving a conflict due to reformatting You can merge this pull request into a Git repository by running: $ git pull https://github.com/DaanHoogland/cloudstack master Alternatively

[GitHub] cloudstack pull request: [4.7] Critical VPCVR issues fixed: CLOUDS...

2016-01-17 Thread wilderrodrigues
Github user wilderrodrigues closed the pull request at: https://github.com/apache/cloudstack/pull/1277 --- 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 f

[GitHub] cloudstack pull request: [4.7] Critical VPCVR issues fixed: CLOUDS...

2016-01-17 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/1277#issuecomment-172362547 This PR has already been merged by @remibergsma . For same weird reason it remained open here and now says it has conflicts. Closing it. Thanks, Rem

[GitHub] cloudstack pull request: [4.7] Critical VPCVR issues fixed: CLOUDS...

2016-01-17 Thread remibergsma
Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/1277#issuecomment-172361223 PR is merged but not properly picked up by Github or so. Checked all commits, they are in 4.7 just fine. @wilderrodrigues please close this PR, thanks! --- If

Build failed in Jenkins: build-master-slowbuild #3009

2016-01-17 Thread jenkins
See -- [...truncated 28733 lines...] [INFO] [INFO] --- findbugs-maven-plugin:3.0.1:findbugs (findbugs) @ cloud-quickcloud --- [INFO] [INFO] <<< findbugs-maven-plugin:3.0.1:check (cloudstack

[GitHub] cloudstack pull request: [4.7] Critical VPCVR issues fixed: CLOUDS...

2016-01-17 Thread remibergsma
Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/1277#issuecomment-172359841 Run the tests again, all fine. ``` nosetests --with-marvin --marvin-config=${marvinCfg} -s -a tags=advanced,required_hardware=true \ component/t

[GitHub] cloudstack pull request: SecurityGroupRulesCmd code cleanup

2016-01-17 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1287#issuecomment-172354935 What I did not understand is that when I read I think, represent what? Into what? What format? Reading the method call I know it is doing somethin

[GitHub] cloudstack pull request: SecurityGroupRulesCmd code cleanup

2016-01-17 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1287#issuecomment-172353941 Thanks @rafaelweingartner. as for your last remark ``` Your code as always is great, I would just suggest some more improvement at the new method you c

[GitHub] cloudstack pull request: SecurityGroupRulesCmd code cleanup

2016-01-17 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1287#issuecomment-172347935 @DaanHoogland, that is great. Those points I lifted up were merely suggestions on how to improve the code a little bit more. Actually, in mos

[GitHub] cloudstack pull request: SecurityGroupRulesCmd code cleanup

2016-01-17 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1287#issuecomment-172341007 @rafaelweingartner I gave you your way more then I intended to with respect to comments. Please have a look if this satisfies your need/requirements. --- If y

Build failed in Jenkins: build-master-slowbuild #3008

2016-01-17 Thread jenkins
See Changes: [priti.sarap] CLOUDSTACK-9001: Modifying snapshot results validation in [priti.sarap] CLOUDSTACK-9005: Modifying tearDown function [priti.sarap] CLOUDSTACK-9041: Modifying template creation from snapshot func

[GitHub] cloudstack pull request: CLOUDSTACK-9231: Root volume migration fr...

2016-01-17 Thread cristofolini
Github user cristofolini commented on the pull request: https://github.com/apache/cloudstack/pull/1336#issuecomment-172333464 Fix looks simple enough. Code LGTM. Although I would usually suggest adding a javadoc the new `MigrateWithStorageCommand` method you've created, this just chan

[GitHub] cloudstack pull request: Cloudstack-8885 added blocked connection ...

2016-01-17 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/857#issuecomment-172329794 one small remark on the comment but code 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

[GitHub] cloudstack pull request: Cloudstack-8885 added blocked connection ...

2016-01-17 Thread DaanHoogland
Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/857#discussion_r49947012 --- Diff: plugins/event-bus/rabbitmq/src/org/apache/cloudstack/mom/rabbitmq/RabbitMQEventBus.java --- @@ -507,6 +512,21 @@ public synchronized boole

[GitHub] cloudstack pull request: CLOUDSTACK-9120 READ.ME files describing ...

2016-01-17 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1202#issuecomment-172328911 @remibergsma good point, will do --- 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

Re: [GitHub] cloudstack pull request: SecurityGroupRulesCmd code cleanup

2016-01-17 Thread Daan Hoogland
On Sat, Jan 16, 2016 at 12:57 PM, rafaelweingartner wrote: > Github user rafaelweingartner commented on the pull request: > > https://github.com/apache/cloudstack/pull/1287#issuecomment-172196150 > > @DaanHoogland, > I reviewed your code and I have a few suggestions/questions. > >

[GitHub] cloudstack pull request: CLOUDSTACK-9054 use of google-optional as...

2016-01-17 Thread DaanHoogland
Github user DaanHoogland closed the pull request at: https://github.com/apache/cloudstack/pull/1060 --- 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 feat

[GitHub] cloudstack pull request: CLOUDSTACK-9054 use of google-optional as...

2016-01-17 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1060#issuecomment-172326528 discussions to dev@ as indicated by @jburwell . this PR losts its purpose. --- If your project is set up for it, you can reply to this email and have your repl

Re: [DISCUSS][PROPOSE] use of optional instead of null

2016-01-17 Thread Daan Hoogland
Thanks John, I agree that the solution from your gist is a good one in several circumstances as well. It doesn't completely invalidate both other solutions completely, though. so now we have three 1. exceptions instead of nulls 2. optionals 3. nullables Not sure how to continue from here, except t

[GitHub] cloudstack pull request: CLOUDSTACK-9195: Cancelled/failed async j...

2016-01-17 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1272#issuecomment-172323097 @koushik-das your change makes perfect sense to me from a db housekeeping point of view. I have one (little) doubt; what are the semantic ramifications. An asy

[GitHub] cloudstack pull request: Remove template ulimit from createtmplt.s...

2016-01-17 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/1223 --- 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-8885 added blocked connection ...

2016-01-17 Thread remibergsma
Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/857#issuecomment-172321551 Ping @borisroman @DaanHoogland @wilderrodrigues Can you guys review this please? Seems like a good fix. --- If your project is set up for it, you can reply to t

[GitHub] cloudstack pull request: CLOUDSTACK-9001: Modifying snapshot resul...

2016-01-17 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/994 --- 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-9005: Modifying tearDown funct...

2016-01-17 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/1000 --- 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-9041: Modifying template creat...

2016-01-17 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/1041 --- 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-9240 remove 40GB filesize limi...

2016-01-17 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/1343 --- 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-9240 remove 40GB filesize limi...

2016-01-17 Thread remibergsma
Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/1343#issuecomment-172320121 Will merge this based on the LGTMs in #1223 as the only difference is the branch. --- If your project is set up for it, you can reply to this email and have yo

[GitHub] cloudstack pull request: Remove template ulimit from createtmplt.s...

2016-01-17 Thread remibergsma
Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/1223#issuecomment-172320075 For the record, LGTM from me. I'll merge the other PR (against 4.7) and close this one. --- If your project is set up for it, you can reply to this email and h

[GitHub] cloudstack pull request: CLOUDSTACK-9240 remove 40GB filesize limi...

2016-01-17 Thread remibergsma
GitHub user remibergsma opened a pull request: https://github.com/apache/cloudstack/pull/1343 CLOUDSTACK-9240 remove 40GB filesize limit from SSVM scripts Both createvolume.sh and createtmplt.sh have a 40GB hardcoded limit for the size of the template that gets created. I could not

[GitHub] cloudstack pull request: test: Fix Libvirt test so that it works o...

2016-01-17 Thread remibergsma
Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/1242#issuecomment-172318654 Pinging @bheuvel to have a look. --- 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-9195: Cancelled/failed async j...

2016-01-17 Thread remibergsma
Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/1272#issuecomment-172318575 Ping @DaanHoogland @borisroman @wilderrodrigues Can you guys have a look at this one-line change please? Thanks! --- If your project is set up for it, you can

Build failed in Jenkins: build-master-slowbuild #3007

2016-01-17 Thread jenkins
See -- [...truncated 28733 lines...] [INFO] [INFO] --- findbugs-maven-plugin:3.0.1:findbugs (findbugs) @ cloud-quickcloud --- [INFO] [INFO] <<< findbugs-maven-plugin:3.0.1:check (cloudstack

[GitHub] cloudstack pull request: Removed .pydevproject from plugin kvm hyp...

2016-01-17 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/1226 --- 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: Removed cloud-cli folder and contents, as...

2016-01-17 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/1228 --- 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-9210: Pass secondary IPs to de...

2016-01-17 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/1309 --- 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: Implement a NSX API request execution cou...

2016-01-17 Thread remibergsma
Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/1294#issuecomment-172309823 @borisroman said he had an improvement to the unit test. Let's discuss and add it to the PR so we can merge it. --- If your project is set up for it, you can r

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add VirtIO channel to al...

2016-01-17 Thread wido
Github user wido commented on the pull request: https://github.com/apache/cloudstack/pull/985#issuecomment-172309253 @nislim No, not really. The people at libvirt-java aren't the fastest. Thinking about forking it into CloudStack itself. --- If your project is set up for it, you can

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add VirtIO channel to al...

2016-01-17 Thread nislim
Github user nislim commented on the pull request: https://github.com/apache/cloudstack/pull/985#issuecomment-172307572 Any progress on getting the patches into upstream libvirt-java? @wido --- If your project is set up for it, you can reply to this email and have your reply appear on

Build failed in Jenkins: build-master-slowbuild #3006

2016-01-17 Thread jenkins
See -- [...truncated 28733 lines...] [INFO] [INFO] --- findbugs-maven-plugin:3.0.1:findbugs (findbugs) @ cloud-quickcloud --- [INFO] [INFO] <<< findbugs-maven-plugin:3.0.1:check (cloudstack