[GitHub] blueorangutan commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for XenServer, Vmware

[GitBox]

blueorangutan commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for 
XenServer, Vmware
URL: https://github.com/apache/cloudstack/pull/2507#issuecomment-376393285
 
 
   Trillian test result (tid-2412)
   Environment: vmware-55u3 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 151811 seconds
   Marvin logs: 
https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2507-t2412-vmware-55u3.zip
   Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermitten failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermitten failure detected: /marvin/tests/smoke/test_routers.py
   Intermitten failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermitten failure detected: /marvin/tests/smoke/test_templates.py
   Intermitten failure detected: /marvin/tests/smoke/test_usage.py
   Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Smoke tests completed. 63 look OK, 4 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 
2119.12 | test_routers_network_ops.py
   test_04_restart_network_wo_cleanup | `Failure` | 4.38 | test_routers.py
   ContextSuite context=TestCpuCapServiceOfferings>:teardown | `Error` | 0.00 | 
test_service_offerings.py
   test_04_rvpc_network_garbage_collector_nics | `Failure` | 664.10 | 
test_vpc_redundant.py
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime during network restart

[GitBox]

blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR 
downtime during network restart
URL: https://github.com/apache/cloudstack/pull/2508#issuecomment-376317190
 
 
   Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1835


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2512: Only use the host if its Resource State is Enabled.

[GitBox]

blueorangutan commented on issue #2512: Only use the host if its Resource State 
is Enabled.
URL: https://github.com/apache/cloudstack/pull/2512#issuecomment-376312149
 
 
   Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1834


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nitin-maharana commented on a change in pull request #2512: Only use the host if its Resource State is Enabled.

[GitBox]

nitin-maharana commented on a change in pull request #2512: Only use the host 
if its Resource State is Enabled.
URL: https://github.com/apache/cloudstack/pull/2512#discussion_r177230153
 
 

 ##
 File path: 
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java
 ##
 @@ -474,7 +473,7 @@ else if (volumeInfo.getFormat() == ImageFormat.OVA || 
volumeInfo.getFormat() ==
 try {
 vmSnapshot = takeHypervisorSnapshot(volumeInfo);
 }
-catch (ResourceAllocationException ex) {
+catch (Exception ex) {
 
 Review comment:
   @mike-tutkowski, Thanks for the fix. I think it would be better to capture 
the required exceptions with multiple catches instead of the parent Exception 
class. We can always put this at the end.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nitin-maharana commented on a change in pull request #2512: Only use the host if its Resource State is Enabled.

[GitBox]

nitin-maharana commented on a change in pull request #2512: Only use the host 
if its Resource State is Enabled.
URL: https://github.com/apache/cloudstack/pull/2512#discussion_r177234196
 
 

 ##
 File path: 
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java
 ##
 @@ -484,7 +483,7 @@ else if (volumeInfo.getFormat() == ImageFormat.OVA || 
volumeInfo.getFormat() ==
 }
 
 SnapshotResult result = null;
-SnapshotInfo snapshotOnPrimary = null;
+SnapshotInfo snapshotOnPrimary;
 
 Review comment:
   I guess the removal of this may create NPE.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime during network restart

[GitBox]

blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR 
downtime during network restart
URL: https://github.com/apache/cloudstack/pull/2508#issuecomment-376121447
 
 
   Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1829


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime during network restart

[GitBox]

blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR 
downtime during network restart
URL: https://github.com/apache/cloudstack/pull/2508#issuecomment-376308863
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted 
as I make progress.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime during network restart

[GitBox]

blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR 
downtime during network restart
URL: https://github.com/apache/cloudstack/pull/2508#issuecomment-376158336
 
 
   Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1831


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime during network restart

[GitBox]

rhtyd commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime 
during network restart
URL: https://github.com/apache/cloudstack/pull/2508#issuecomment-376149921
 
 
   @blueorangutan package


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime during network restart

[GitBox]

blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR 
downtime during network restart
URL: https://github.com/apache/cloudstack/pull/2508#issuecomment-376113996
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted 
as I make progress.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime during network restart

[GitBox]

rhtyd commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime 
during network restart
URL: https://github.com/apache/cloudstack/pull/2508#issuecomment-376308836
 
 
   @blueorangutan package


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime during network restart

[GitBox]

rhtyd commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime 
during network restart
URL: https://github.com/apache/cloudstack/pull/2508#issuecomment-376113886
 
 
   @blueorangutan package


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime during network restart

[GitBox]

blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR 
downtime during network restart
URL: https://github.com/apache/cloudstack/pull/2508#issuecomment-376150035
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted 
as I make progress.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2512: Only use the host if its Resource State is Enabled.

[GitBox]

blueorangutan commented on issue #2512: Only use the host if its Resource State 
is Enabled.
URL: https://github.com/apache/cloudstack/pull/2512#issuecomment-376302242
 
 
   @mike-tutkowski a Jenkins job has been kicked to build packages. I'll keep 
you posted as I make progress.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] mike-tutkowski opened a new pull request #2512: Only use the host if its Resource State is Enabled.

[GitBox]

mike-tutkowski opened a new pull request #2512: Only use the host if its 
Resource State is Enabled.
URL: https://github.com/apache/cloudstack/pull/2512
 
 
   ## Description
   Update the StorageSystemSnapshotStrategy and StorageSystemDataMotionStrategy 
classes to only leverage a host when its Resource State is Enabled.
   
   https://issues.apache.org/jira/browse/CLOUDSTACK-10345
   
   ## Types of changes
   - [ ] Breaking change (fix or feature that would cause existing 
functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [x] Bug fix (non-breaking change which fixes an issue)
   - [ ] Enhancement (improves an existing feature and functionality)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   
   ## How Has This Been Tested?
   
   Checked to see if a host in maintenance mode is avoided
   
   ## Checklist:
   - [x] I have read the 
[CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md)
 document.
   - [x] My code follows the code style of this project.
   - [ ] My change requires a change to the documentation.
   - [ ] I have updated the documentation accordingly.
   - [ ] I have added tests to cover my changes.
   - [ ] All new and existing tests passed.
   
   
   @blueorangutan package
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2506: CLOUDSTACK-10341: Reduce systemvmtemplate size, install nftables

[GitBox]

blueorangutan commented on issue #2506: CLOUDSTACK-10341: Reduce 
systemvmtemplate size, install nftables
URL: https://github.com/apache/cloudstack/pull/2506#issuecomment-376286332
 
 
   @rhtyd a Trillian-Jenkins matrix job (centos6 mgmt + xs71, centos7 mgmt + 
vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2506: CLOUDSTACK-10341: Reduce systemvmtemplate size, install nftables

[GitBox]

rhtyd commented on issue #2506: CLOUDSTACK-10341: Reduce systemvmtemplate size, 
install nftables
URL: https://github.com/apache/cloudstack/pull/2506#issuecomment-376286140
 
 
   Manual tests have passed, let me kick a final run.
   @blueorangutan test matrix


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rafaelweingartner commented on issue #2465: CLOUDSTACK-10232: SystemVMs and VR to run as HVM on XenServer

[GitBox]

rafaelweingartner commented on issue #2465: CLOUDSTACK-10232: SystemVMs and VR 
to run as HVM on XenServer
URL: https://github.com/apache/cloudstack/pull/2465#issuecomment-376282411
 
 
   @khos2ow do you have a final feedback here?
   @borisstoyanov can you confirm that the tests were successful?
   
   Everything seems to be ready to merge this one.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2506: CLOUDSTACK-10341: Reduce systemvmtemplate size, install nftables

[GitBox]

blueorangutan commented on issue #2506: CLOUDSTACK-10341: Reduce 
systemvmtemplate size, install nftables
URL: https://github.com/apache/cloudstack/pull/2506#issuecomment-376271426
 
 
   Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1833


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2506: CLOUDSTACK-10341: Reduce systemvmtemplate size, install nftables

[GitBox]

blueorangutan commented on issue #2506: CLOUDSTACK-10341: Reduce 
systemvmtemplate size, install nftables
URL: https://github.com/apache/cloudstack/pull/2506#issuecomment-376262716
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted 
as I make progress.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2506: CLOUDSTACK-10341: Reduce systemvmtemplate size, install nftables

[GitBox]

rhtyd commented on issue #2506: CLOUDSTACK-10341: Reduce systemvmtemplate size, 
install nftables
URL: https://github.com/apache/cloudstack/pull/2506#issuecomment-376262516
 
 
   @blueorangutan package


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rafaelweingartner commented on issue #2438: [CLOUDSTACK-10307] Remove unused things from HostDaoImpl

[GitBox]

rafaelweingartner commented on issue #2438: [CLOUDSTACK-10307] Remove unused 
things from HostDaoImpl
URL: https://github.com/apache/cloudstack/pull/2438#issuecomment-376200545
 
 
   @DaanHoogland the test result came and everything seems to be ok.
   Do you want to do other tests?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2438: [CLOUDSTACK-10307] Remove unused things from HostDaoImpl

[GitBox]

blueorangutan commented on issue #2438: [CLOUDSTACK-10307] Remove unused things 
from HostDaoImpl
URL: https://github.com/apache/cloudstack/pull/2438#issuecomment-376194965
 
 
   Trillian test result (tid-2415)
   Environment: vmware-65 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 26863 seconds
   Marvin logs: 
https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2438-t2415-vmware-65.zip
   Smoke tests completed. 67 look OK, 0 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2449: WIP CLOUDSTACK-10278 idempotent column addition

[GitBox]

blueorangutan commented on issue #2449: WIP CLOUDSTACK-10278 idempotent column 
addition
URL: https://github.com/apache/cloudstack/pull/2449#issuecomment-376185656
 
 
   @ernjvr a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been 
kicked to run smoke tests


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2449: WIP CLOUDSTACK-10278 idempotent column addition

[GitBox]

blueorangutan commented on issue #2449: WIP CLOUDSTACK-10278 idempotent column 
addition
URL: https://github.com/apache/cloudstack/pull/2449#issuecomment-376185185
 
 
   Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1832


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] ernjvr commented on issue #2449: WIP CLOUDSTACK-10278 idempotent column addition

[GitBox]

ernjvr commented on issue #2449: WIP CLOUDSTACK-10278 idempotent column addition
URL: https://github.com/apache/cloudstack/pull/2449#issuecomment-376185304
 
 
   @blueorangutan test


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] lzh3636 commented on a change in pull request #2510: CLOUDSTACK-10334: Fix inadequate information for handling catch clauses

[GitBox]

lzh3636 commented on a change in pull request #2510: CLOUDSTACK-10334: Fix 
inadequate information for handling catch clauses
URL: https://github.com/apache/cloudstack/pull/2510#discussion_r177102655
 
 

 ##
 File path: server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker.java
 ##
 @@ -258,11 +258,11 @@ public void processParameters(final BaseCmd cmd, final 
Map params) {
 }
 
 } catch (final IllegalArgumentException e) {
-s_logger.error("Error initializing command " + 
cmd.getCommandName() + ", field " + field.getName() + " is not accessible.");
+s_logger.error("Error initializing command " + 
cmd.getCommandName() + ", field " + field.getName() + " is not accessible.", e);
 
 Review comment:
   Yes, thank you, it's done :)


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2449: WIP CLOUDSTACK-10278 idempotent column addition

[GitBox]

blueorangutan commented on issue #2449: WIP CLOUDSTACK-10278 idempotent column 
addition
URL: https://github.com/apache/cloudstack/pull/2449#issuecomment-376175585
 
 
   @ernjvr a Jenkins job has been kicked to build packages. I'll keep you 
posted as I make progress.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] ernjvr commented on issue #2449: WIP CLOUDSTACK-10278 idempotent column addition

[GitBox]

ernjvr commented on issue #2449: WIP CLOUDSTACK-10278 idempotent column addition
URL: https://github.com/apache/cloudstack/pull/2449#issuecomment-376175288
 
 
   @blueorangutan package


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on a change in pull request #2510: CLOUDSTACK-10334: Fix inadequate information for handling catch clauses

[GitBox]

DaanHoogland commented on a change in pull request #2510: CLOUDSTACK-10334: Fix 
inadequate information for handling catch clauses
URL: https://github.com/apache/cloudstack/pull/2510#discussion_r177097471
 
 

 ##
 File path: server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker.java
 ##
 @@ -258,11 +258,11 @@ public void processParameters(final BaseCmd cmd, final 
Map params) {
 }
 
 } catch (final IllegalArgumentException e) {
-s_logger.error("Error initializing command " + 
cmd.getCommandName() + ", field " + field.getName() + " is not accessible.");
+s_logger.error("Error initializing command " + 
cmd.getCommandName() + ", field " + field.getName() + " is not accessible.", e);
 
 Review comment:
   ok, so there is a misunderstanding here. The original exception is not 
rethrown, but a new one @lzh3636 . The question is can you add it to the throw 
and delete the error log instead?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rafaelweingartner commented on a change in pull request #2510: CLOUDSTACK-10334: Fix inadequate information for handling catch clauses

[GitBox]

rafaelweingartner commented on a change in pull request #2510: 
CLOUDSTACK-10334: Fix inadequate information for handling catch clauses
URL: https://github.com/apache/cloudstack/pull/2510#discussion_r177094723
 
 

 ##
 File path: server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker.java
 ##
 @@ -258,11 +258,11 @@ public void processParameters(final BaseCmd cmd, final 
Map params) {
 }
 
 } catch (final IllegalArgumentException e) {
-s_logger.error("Error initializing command " + 
cmd.getCommandName() + ", field " + field.getName() + " is not accessible.");
+s_logger.error("Error initializing command " + 
cmd.getCommandName() + ", field " + field.getName() + " is not accessible.", e);
 
 Review comment:
   That is what I am trying to say :)


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on a change in pull request #2510: CLOUDSTACK-10334: Fix inadequate information for handling catch clauses

[GitBox]

DaanHoogland commented on a change in pull request #2510: CLOUDSTACK-10334: Fix 
inadequate information for handling catch clauses
URL: https://github.com/apache/cloudstack/pull/2510#discussion_r177093840
 
 

 ##
 File path: server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker.java
 ##
 @@ -258,11 +258,11 @@ public void processParameters(final BaseCmd cmd, final 
Map params) {
 }
 
 } catch (final IllegalArgumentException e) {
-s_logger.error("Error initializing command " + 
cmd.getCommandName() + ", field " + field.getName() + " is not accessible.");
+s_logger.error("Error initializing command " + 
cmd.getCommandName() + ", field " + field.getName() + " is not accessible.", e);
 
 Review comment:
   I do not see that point, sorry. An unexpected error is caught and logged 
(not rethrown) and than a more generic message/exception is thrown. Other than 
unifying the two catch clauses in a catch 
(IllegalArgumentException|IllegalAccessException e), I don't see the harm here. 
We could improve by not logging at all and adding the original exception as a 
root cause to the CloudRuntimeException. Is that what you mean?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime during network restart

[GitBox]

blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR 
downtime during network restart
URL: https://github.com/apache/cloudstack/pull/2508#issuecomment-376158336
 
 
   Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1831


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime during network restart

[GitBox]

blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR 
downtime during network restart
URL: https://github.com/apache/cloudstack/pull/2508#issuecomment-376150035
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted 
as I make progress.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime during network restart

[GitBox]

rhtyd commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime 
during network restart
URL: https://github.com/apache/cloudstack/pull/2508#issuecomment-376149921
 
 
   @blueorangutan package


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rafaelweingartner commented on a change in pull request #2510: CLOUDSTACK-10334: Fix inadequate information for handling catch clauses

[GitBox]

rafaelweingartner commented on a change in pull request #2510: 
CLOUDSTACK-10334: Fix inadequate information for handling catch clauses
URL: https://github.com/apache/cloudstack/pull/2510#discussion_r177072218
 
 

 ##
 File path: server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker.java
 ##
 @@ -258,11 +258,11 @@ public void processParameters(final BaseCmd cmd, final 
Map params) {
 }
 
 } catch (final IllegalArgumentException e) {
-s_logger.error("Error initializing command " + 
cmd.getCommandName() + ", field " + field.getName() + " is not accessible.");
+s_logger.error("Error initializing command " + 
cmd.getCommandName() + ", field " + field.getName() + " is not accessible.", e);
 
 Review comment:
   Well, that was an answer. I would prefer to fix/improve the log messaging 
than to log twice the same stack trace. What do you think?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)

[GitBox]

rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving 
ACL rules (change order with drag and drop)
URL: https://github.com/apache/cloudstack/pull/2511#issuecomment-376148484
 
 
   @nitin-maharana thanks for your review.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rafaelweingartner commented on a change in pull request #2511: [CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)

[GitBox]

rafaelweingartner commented on a change in pull request #2511: 
[CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)
URL: https://github.com/apache/cloudstack/pull/2511#discussion_r177071282
 
 

 ##
 File path: 
api/src/main/java/org/apache/cloudstack/api/command/user/network/MoveNetworkAclItemCmd.java
 ##
 @@ -0,0 +1,96 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.network;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseAsyncCustomIdCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.NetworkACLItemResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.log4j.Logger;
+
+import com.cloud.event.EventTypes;
+import com.cloud.network.vpc.NetworkACLItem;
+import com.cloud.user.Account;
+
+@APICommand(name = "moveNetworkAclItem", description = "Move an ACL rule to a 
position bettwen two other ACL rules of the same ACL network list", 
responseObject = NetworkACLItemResponse.class, requestHasSensitiveInfo = false, 
responseHasSensitiveInfo = false)
+public class MoveNetworkAclItemCmd extends BaseAsyncCustomIdCmd {
+
+public static final Logger s_logger = 
Logger.getLogger(MoveNetworkAclItemCmd.class.getName());
+private static final String s_name = "moveNetworkAclItemResponse";
+
+@Parameter(name = ApiConstants.ID, type = CommandType.STRING, required = 
true, description = "The ID of the network ACL rule that is being moved to a 
new position.")
+private String uuidRuleBeingMoved;
+
+@Parameter(name = ApiConstants.ID_PREVIOUS_ACL_RULE, type = 
CommandType.STRING, description = "The ID of the first rule that is right 
before the new position where the rule being moved is going to be placed. This 
value can be 'NULL' if the rule is being moved to the first position of the 
network ACL list.")
+private String previousAclRuleUuid;
+
+@Parameter(name = ApiConstants.ID_NEXT_ACL_RULE, type = 
CommandType.STRING, description = "The ID of the rule that is right after the 
new position where the rule being moved is going to be placed. This value can 
be 'NULL' if the rule is being moved to the last position of the network ACL 
list.")
+private String nextAclRuleUuid;
 
 Review comment:
   I thought about that; in practice, we do not need to externalize (show users 
the ‘number’ field). However, if we remove this option now, this would 
break/affect some API methods. That is why I preferred to first introduce this 
new method, and maybe later (ACS 5?!) we can start braking some API methods and 
remove the ‘number’ field from them.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rafaelweingartner commented on a change in pull request #2511: [CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)

[GitBox]

rafaelweingartner commented on a change in pull request #2511: 
[CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)
URL: https://github.com/apache/cloudstack/pull/2511#discussion_r177070454
 
 

 ##
 File path: 
server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java
 ##
 @@ -922,4 +925,170 @@ public NetworkACL updateNetworkACL(final Long id, final 
String customId, final B
 return _networkACLDao.findById(id);
 }
 
+@Override
+public NetworkACLItem 
moveNetworkAclRuleToNewPosition(MoveNetworkAclItemCmd moveNetworkAclItemCmd) {
+String uuidRuleBeingMoved = 
moveNetworkAclItemCmd.getUuidRuleBeingMoved();
+String nextAclRuleUuid = moveNetworkAclItemCmd.getNextAclRuleUuid();
+String previousAclRuleUuid = 
moveNetworkAclItemCmd.getPreviousAclRuleUuid();
+
+if (StringUtils.isBlank(previousAclRuleUuid) && 
StringUtils.isBlank(nextAclRuleUuid)) {
+throw new InvalidParameterValueException("Both previous and next 
ACL rule IDs cannot be blank.");
+}
+
+NetworkACLItemVO ruleBeingMoved = 
_networkACLItemDao.findByUuid(uuidRuleBeingMoved);
+if (ruleBeingMoved == null) {
+throw new InvalidParameterValueException(String.format("Could not 
find a rule with ID[%s]", uuidRuleBeingMoved));
+}
+NetworkACLItemVO previousRule = 
retrieveAndValidateAclRule(previousAclRuleUuid);
+NetworkACLItemVO nextRule = 
retrieveAndValidateAclRule(nextAclRuleUuid);
+
+validateMoveAclRulesData(ruleBeingMoved, previousRule, nextRule);
+
+List allAclRules = 
getAllAclRulesSortedByNumber(ruleBeingMoved.getAclId());
+if (previousRule == null) {
+return moveRuleToTheTop(ruleBeingMoved, allAclRules);
+}
+if (nextRule == null) {
+return moveRuleToTheBottom(ruleBeingMoved, allAclRules);
+}
+
+return moveRuleBetweenAclRules(ruleBeingMoved, allAclRules, 
previousRule, nextRule);
+}
+
+/**
+ * Loads all ACL rules from given network ACL list. Then, the ACL rules 
will be sorted according to the 'number' field in ascending order.
+ */
+protected List getAllAclRulesSortedByNumber(long aclId) {
+List allAclRules = 
_networkACLItemDao.listByACL(aclId);
+Collections.sort(allAclRules, new Comparator() {
+@Override
+public int compare(NetworkACLItemVO o1, NetworkACLItemVO o2) {
+return o1.number - o2.number;
+}
+});
+return allAclRules;
+}
+
+/**
+ * Mover an ACL to the space between to other rules. If there is already 
enough room to accommodate the ACL rule being moved, we simply get the 'number' 
field from the previous ACL rule and add one, and then define this new value as 
the 'number' value for the ACL rule being moved.
 
 Review comment:
   Done


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rafaelweingartner commented on a change in pull request #2511: [CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)

[GitBox]

rafaelweingartner commented on a change in pull request #2511: 
[CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)
URL: https://github.com/apache/cloudstack/pull/2511#discussion_r177070367
 
 

 ##
 File path: api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
 ##
 @@ -152,6 +152,8 @@
 public static final String ICMP_TYPE = "icmptype";
 public static final String ID = "id";
 public static final String IDS = "ids";
+public static final String ID_PREVIOUS_ACL_RULE = "previousaclruleid";
+public static final String ID_NEXT_ACL_RULE = "nextaclruleid";
 
 Review comment:
   That is a good idea. 
   Done.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] wido commented on issue #2505: CLOUDSTACK-10333: Secure Live VM Migration for KVM

[GitBox]

wido commented on issue #2505: CLOUDSTACK-10333: Secure Live VM Migration for 
KVM
URL: https://github.com/apache/cloudstack/pull/2505#issuecomment-376132182
 
 
   Although the iptables changes have been removed for Ubuntu/Debian I think we 
should also remove them from the RPM packages.
   
   In my opinion a package should never be allowed to touch firewalls without 
the operating knowing it.
   
   In the documentation we already tell users to open ports: 
http://docs.cloudstack.apache.org/projects/cloudstack-installation/en/4.11/hypervisor/kvm.html#configuring-the-firewall
   
   If additional ports need to be opened we should put them in there, but not 
just open them in a RPM or DEB package.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2505: CLOUDSTACK-10333: Secure Live VM Migration for KVM

[GitBox]

blueorangutan commented on issue #2505: CLOUDSTACK-10333: Secure Live VM 
Migration for KVM
URL: https://github.com/apache/cloudstack/pull/2505#issuecomment-376131034
 
 
   Packaging result: ✖centos6 ✖centos7 ✖debian. JID-1830


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2505: CLOUDSTACK-10333: Secure Live VM Migration for KVM

[GitBox]

blueorangutan commented on issue #2505: CLOUDSTACK-10333: Secure Live VM 
Migration for KVM
URL: https://github.com/apache/cloudstack/pull/2505#issuecomment-376126719
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted 
as I make progress.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2505: CLOUDSTACK-10333: Secure Live VM Migration for KVM

[GitBox]

rhtyd commented on issue #2505: CLOUDSTACK-10333: Secure Live VM Migration for 
KVM
URL: https://github.com/apache/cloudstack/pull/2505#issuecomment-376126542
 
 
   @blueorangutan package


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime during network restart

[GitBox]

blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR 
downtime during network restart
URL: https://github.com/apache/cloudstack/pull/2508#issuecomment-376121447
 
 
   Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1829


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime during network restart

[GitBox]

blueorangutan commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR 
downtime during network restart
URL: https://github.com/apache/cloudstack/pull/2508#issuecomment-376113996
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted 
as I make progress.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime during network restart

[GitBox]

rhtyd commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime 
during network restart
URL: https://github.com/apache/cloudstack/pull/2508#issuecomment-376113886
 
 
   @blueorangutan package


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for XenServer, Vmware

[GitBox]

blueorangutan commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for 
XenServer, Vmware
URL: https://github.com/apache/cloudstack/pull/2507#issuecomment-376100098
 
 
   Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1828


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] olivierlemasle commented on a change in pull request #2498: CLOUDSTACK-10327: Do not invalidate the session when API command not found

[GitBox]

olivierlemasle commented on a change in pull request #2498: CLOUDSTACK-10327: 
Do not invalidate the session when API command not found
URL: https://github.com/apache/cloudstack/pull/2498#discussion_r177023277
 
 

 ##
 File path: server/src/com/cloud/api/ApiServer.java
 ##
 @@ -958,6 +959,9 @@ private boolean commandAvailable(final InetAddress 
remoteAddress, final String c
 } catch (final RequestLimitException ex) {
 s_logger.debug(ex.getMessage());
 throw new ServerApiException(ApiErrorCode.API_LIMIT_EXCEED, 
ex.getMessage());
+}  catch (final UnavailableCommandException ex) {
+s_logger.debug(ex.getMessage());
+throw new 
ServerApiException(ApiErrorCode.UNSUPPORTED_ACTION_ERROR, ex.getMessage());
 
 Review comment:
   @rhtyd I'm not sure I understand your suggestion. The Http error code is 
specified here, in line 964). The `ServerApiException` is caught on 
`ApiServlet` which generates the HTTP response.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[cloudstack] 01/01: [Merge 4.11] CLOUDSTACK-10319: Allow TLSv1, v1.1 for XenServer, Vmware (#2507)

[rohit]

This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git

commit 9222da2d625e4d85889c7d8bc1c5f0caea448129
Merge: 9733a10 c4cc679
Author: Rohit Yadav 
AuthorDate: Mon Mar 26 14:17:27 2018 +0530

[Merge 4.11] CLOUDSTACK-10319: Allow TLSv1, v1.1 for XenServer, Vmware 
(#2507)

Signed-off-by: Rohit Yadav 

 client/conf/java.security.ciphers.in  | 2 +-
 .../src/main/java/org/apache/cloudstack/utils/security/SSLUtils.java  | 4 ++--
 utils/src/test/java/com/cloud/utils/security/SSLUtilsTest.java| 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

-- 
To stop receiving notification emails like this one, please contact
ro...@apache.org.

[cloudstack] branch master updated (9733a10 -> 9222da2)

[rohit]

This is an automated email from the ASF dual-hosted git repository.

rohit pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git.


from 9733a10  CLOUDSTACK-10221: Allow IPv6 when creating a Basic Network 
(#2397)
 add c4cc679  CLOUDSTACK-10319: Allow TLSv1, v1.1 for XenServer, Vmware 
(#2507)
 new 9222da2  [Merge 4.11] CLOUDSTACK-10319: Allow TLSv1, v1.1 for 
XenServer, Vmware (#2507)

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 client/conf/java.security.ciphers.in  | 2 +-
 .../src/main/java/org/apache/cloudstack/utils/security/SSLUtils.java  | 4 ++--
 utils/src/test/java/com/cloud/utils/security/SSLUtilsTest.java| 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

-- 
To stop receiving notification emails like this one, please contact
ro...@apache.org.

[cloudstack] branch 4.11 updated: CLOUDSTACK-10319: Allow TLSv1, v1.1 for XenServer, Vmware (#2507)

[rohit]

This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch 4.11
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/4.11 by this push:
 new c4cc679  CLOUDSTACK-10319: Allow TLSv1, v1.1 for XenServer, Vmware 
(#2507)
c4cc679 is described below

commit c4cc679c3b34a5f38cc17a01a96e9d69aa370641
Author: Rohit Yadav 
AuthorDate: Mon Mar 26 14:16:49 2018 +0530

CLOUDSTACK-10319: Allow TLSv1, v1.1 for XenServer, Vmware (#2507)

This reverts changes from #2480, instead moves TLS settings to
java ciphers settings config file. It should be sufficient to enforce
TLS v1.2 on public facing CloudStack services:
- CloudStack webserver (Jetty based)
- Apache2 for secondary storage VM
- CPVM HTTPs server

Signed-off-by: Rohit Yadav 
---
 client/conf/java.security.ciphers.in  | 2 +-
 .../src/main/java/org/apache/cloudstack/utils/security/SSLUtils.java  | 4 ++--
 utils/src/test/java/com/cloud/utils/security/SSLUtilsTest.java| 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/client/conf/java.security.ciphers.in 
b/client/conf/java.security.ciphers.in
index 986abf6..27e2d69 100644
--- a/client/conf/java.security.ciphers.in
+++ b/client/conf/java.security.ciphers.in
@@ -15,4 +15,4 @@
  # specific language governing permissions and limitations
  # under the License.
 
-jdk.tls.disabledAlgorithms=DH keySize < 128, RSA keySize < 128, DES keySize < 
128, SHA1 keySize < 128, MD5 keySize < 128, RC4
\ No newline at end of file
+jdk.tls.disabledAlgorithms=SSLv2Hello, SSLv3, TLSv1, TLSv1.1, DH keySize < 
128, RSA keySize < 128, DES keySize < 128, SHA1 keySize < 128, MD5 keySize < 
128, RC4
diff --git 
a/utils/src/main/java/org/apache/cloudstack/utils/security/SSLUtils.java 
b/utils/src/main/java/org/apache/cloudstack/utils/security/SSLUtils.java
index 9fbdb4a..8016f5a 100644
--- a/utils/src/main/java/org/apache/cloudstack/utils/security/SSLUtils.java
+++ b/utils/src/main/java/org/apache/cloudstack/utils/security/SSLUtils.java
@@ -34,7 +34,7 @@ public class SSLUtils {
 public static String[] getSupportedProtocols(String[] protocols) {
 Set set = new HashSet();
 for (String s : protocols) {
-if (s.equals("TLSv1") || s.equals("TLSv1.1") || s.equals("SSLv3") 
|| s.equals("SSLv2Hello")) {
+if (s.equals("SSLv3") || s.equals("SSLv2Hello")) {
 continue;
 }
 set.add(s);
@@ -46,7 +46,7 @@ public class SSLUtils {
  * It returns recommended protocols that are considered secure.
  */
 public static String[] getRecommendedProtocols() {
-return new String[] { "TLSv1.2" };
+return new String[] { "TLSv1", "TLSv1.1", "TLSv1.2" };
 }
 
 /**
diff --git a/utils/src/test/java/com/cloud/utils/security/SSLUtilsTest.java 
b/utils/src/test/java/com/cloud/utils/security/SSLUtilsTest.java
index 6c66dcd..625b538 100644
--- a/utils/src/test/java/com/cloud/utils/security/SSLUtilsTest.java
+++ b/utils/src/test/java/com/cloud/utils/security/SSLUtilsTest.java
@@ -69,9 +69,9 @@ public class SSLUtilsTest {
 }
 
 private void verifyProtocols(ArrayList protocolsList) {
+Assert.assertTrue(protocolsList.contains("TLSv1"));
+Assert.assertTrue(protocolsList.contains("TLSv1.1"));
 Assert.assertTrue(protocolsList.contains("TLSv1.2"));
-Assert.assertFalse(protocolsList.contains("TLSv1"));
-Assert.assertFalse(protocolsList.contains("TLSv1.1"));
 Assert.assertFalse(protocolsList.contains("SSLv3"));
 Assert.assertFalse(protocolsList.contains("SSLv2Hello"));
 }

-- 
To stop receiving notification emails like this one, please contact
ro...@apache.org.

[GitHub] rhtyd commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for XenServer, Vmware

[GitBox]

rhtyd commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for 
XenServer, Vmware
URL: https://github.com/apache/cloudstack/pull/2507#issuecomment-376091371
 
 
   Checked test results, test LGTM (the failures are all env issues). I'll 
merge this to unblock other PRs who are failing testing on older 
xenserver/vmware based environments.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for XenServer, Vmware

[GitBox]

blueorangutan commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for 
XenServer, Vmware
URL: https://github.com/apache/cloudstack/pull/2507#issuecomment-376091570
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted 
as I make progress.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd closed pull request #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for XenServer, Vmware

[GitBox]

rhtyd closed pull request #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for 
XenServer, Vmware
URL: https://github.com/apache/cloudstack/pull/2507
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/client/conf/java.security.ciphers.in 
b/client/conf/java.security.ciphers.in
index 986abf61e71..27e2d690ee6 100644
--- a/client/conf/java.security.ciphers.in
+++ b/client/conf/java.security.ciphers.in
@@ -15,4 +15,4 @@
  # specific language governing permissions and limitations
  # under the License.
 
-jdk.tls.disabledAlgorithms=DH keySize < 128, RSA keySize < 128, DES keySize < 
128, SHA1 keySize < 128, MD5 keySize < 128, RC4
\ No newline at end of file
+jdk.tls.disabledAlgorithms=SSLv2Hello, SSLv3, TLSv1, TLSv1.1, DH keySize < 
128, RSA keySize < 128, DES keySize < 128, SHA1 keySize < 128, MD5 keySize < 
128, RC4
diff --git 
a/utils/src/main/java/org/apache/cloudstack/utils/security/SSLUtils.java 
b/utils/src/main/java/org/apache/cloudstack/utils/security/SSLUtils.java
index 9fbdb4aa553..8016f5a1916 100644
--- a/utils/src/main/java/org/apache/cloudstack/utils/security/SSLUtils.java
+++ b/utils/src/main/java/org/apache/cloudstack/utils/security/SSLUtils.java
@@ -34,7 +34,7 @@
 public static String[] getSupportedProtocols(String[] protocols) {
 Set set = new HashSet();
 for (String s : protocols) {
-if (s.equals("TLSv1") || s.equals("TLSv1.1") || s.equals("SSLv3") 
|| s.equals("SSLv2Hello")) {
+if (s.equals("SSLv3") || s.equals("SSLv2Hello")) {
 continue;
 }
 set.add(s);
@@ -46,7 +46,7 @@
  * It returns recommended protocols that are considered secure.
  */
 public static String[] getRecommendedProtocols() {
-return new String[] { "TLSv1.2" };
+return new String[] { "TLSv1", "TLSv1.1", "TLSv1.2" };
 }
 
 /**
diff --git a/utils/src/test/java/com/cloud/utils/security/SSLUtilsTest.java 
b/utils/src/test/java/com/cloud/utils/security/SSLUtilsTest.java
index 6c66dcd1bd0..625b538d7f2 100644
--- a/utils/src/test/java/com/cloud/utils/security/SSLUtilsTest.java
+++ b/utils/src/test/java/com/cloud/utils/security/SSLUtilsTest.java
@@ -69,9 +69,9 @@ public void getSupportedProtocolsTest() {
 }
 
 private void verifyProtocols(ArrayList protocolsList) {
+Assert.assertTrue(protocolsList.contains("TLSv1"));
+Assert.assertTrue(protocolsList.contains("TLSv1.1"));
 Assert.assertTrue(protocolsList.contains("TLSv1.2"));
-Assert.assertFalse(protocolsList.contains("TLSv1"));
-Assert.assertFalse(protocolsList.contains("TLSv1.1"));
 Assert.assertFalse(protocolsList.contains("SSLv3"));
 Assert.assertFalse(protocolsList.contains("SSLv2Hello"));
 }


 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for XenServer, Vmware

[GitBox]

blueorangutan commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for 
XenServer, Vmware
URL: https://github.com/apache/cloudstack/pull/2507#issuecomment-376090147
 
 
   Packaging result: ✖centos6 ✔centos7 ✔debian. JID-1827


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for XenServer, Vmware

[GitBox]

blueorangutan commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for 
XenServer, Vmware
URL: https://github.com/apache/cloudstack/pull/2507#issuecomment-376090147
 
 
   Packaging result: ✖centos6 ✔centos7 ✔debian. JID-1827


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for XenServer, Vmware

[GitBox]

blueorangutan commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for 
XenServer, Vmware
URL: https://github.com/apache/cloudstack/pull/2507#issuecomment-376082971
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted 
as I make progress.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR downtime during network restart

[GitBox]

DaanHoogland commented on issue #2508: WIP - CLOUDSTACK-9114: Reduce VR 
downtime during network restart
URL: https://github.com/apache/cloudstack/pull/2508#issuecomment-376075487
 
 
   looks good @rhtyd 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] borisstoyanov commented on issue #2496: [CLOUDSTACK-10332] Users are not able to change/edit the protocol of an ACL rule

[GitBox]

borisstoyanov commented on issue #2496: [CLOUDSTACK-10332] Users are not able 
to change/edit the protocol of an ACL rule 
URL: https://github.com/apache/cloudstack/pull/2496#issuecomment-376074793
 
 
   @rafaelweingartner we've seen these errors with other PRs, I think they are 
not related


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland closed pull request #2397: CLOUDSTACK-10221: Allow IPv6 when creating a Basic Network

[GitBox]

DaanHoogland closed pull request #2397: CLOUDSTACK-10221: Allow IPv6 when 
creating a Basic Network
URL: https://github.com/apache/cloudstack/pull/2397
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git 
a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
 
b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
index cec2e5926c1..1b707c3979d 100644
--- 
a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
+++ 
b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
@@ -38,6 +38,7 @@
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.utils.StringUtils;
 import org.apache.log4j.Logger;
 
 import org.apache.cloudstack.acl.ControlledEntity.ACLType;
@@ -2116,16 +2117,12 @@ public Network createGuestNetwork(final long 
networkOfferingId, final String nam
 
 boolean ipv6 = false;
 
-if (ip6Gateway != null && ip6Cidr != null) {
+if (StringUtils.isNotBlank(ip6Gateway) && 
StringUtils.isNotBlank(ip6Cidr)) {
 ipv6 = true;
 }
 // Validate zone
 final DataCenterVO zone = _dcDao.findById(zoneId);
 if (zone.getNetworkType() == NetworkType.Basic) {
-if (ipv6) {
-throw new InvalidParameterValueException("IPv6 is not 
supported in Basic zone");
-}
-
 // In Basic zone the network should have aclType=Domain, 
domainId=1, subdomainAccess=true
 if (aclType == null || aclType != ACLType.Domain) {
 throw new InvalidParameterValueException("Only AclType=Domain 
can be specified for network creation in Basic zone");
@@ -2188,6 +2185,10 @@ public Network createGuestNetwork(final long 
networkOfferingId, final String nam
 }
 }
 
+if (ipv6 && !NetUtils.isValidIp6Cidr(ip6Cidr)) {
+throw new InvalidParameterValueException("Invalid IPv6 cidr 
specified");
+}
+
 //TODO(VXLAN): Support VNI specified
 // VlanId can be specified only when network offering supports it
 final boolean vlanSpecified = vlanId != null;
@@ -2328,7 +2329,7 @@ public Network doInTransaction(final TransactionStatus 
status) {
 userNetwork.setGateway(gateway);
 }
 
-if (ip6Cidr != null && ip6Gateway != null) {
+if (StringUtils.isNotBlank(ip6Gateway) && 
StringUtils.isNotBlank(ip6Cidr)) {
 userNetwork.setIp6Cidr(ip6Cidr);
 userNetwork.setIp6Gateway(ip6Gateway);
 }


 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[cloudstack] branch master updated: CLOUDSTACK-10221: Allow IPv6 when creating a Basic Network (#2397)

[dahn]

This is an automated email from the ASF dual-hosted git repository.

dahn pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/master by this push:
 new 9733a10  CLOUDSTACK-10221: Allow IPv6 when creating a Basic Network 
(#2397)
9733a10 is described below

commit 9733a10ecda5f1af0f2c0fa863fc976a3e710946
Author: Wido den Hollander 
AuthorDate: Mon Mar 26 09:36:57 2018 +0200

CLOUDSTACK-10221: Allow IPv6 when creating a Basic Network (#2397)

Since CloudStack 4.10 Basic Networking supports IPv6 and thus
should be allowed to be specified when creating a network.

Signed-off-by: Wido den Hollander 
---
 .../engine/orchestration/NetworkOrchestrator.java   | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git 
a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
 
b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
index cec2e59..1b707c3 100644
--- 
a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
+++ 
b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
@@ -38,6 +38,7 @@ import java.util.stream.Collectors;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.utils.StringUtils;
 import org.apache.log4j.Logger;
 
 import org.apache.cloudstack.acl.ControlledEntity.ACLType;
@@ -2116,16 +2117,12 @@ public class NetworkOrchestrator extends ManagerBase 
implements NetworkOrchestra
 
 boolean ipv6 = false;
 
-if (ip6Gateway != null && ip6Cidr != null) {
+if (StringUtils.isNotBlank(ip6Gateway) && 
StringUtils.isNotBlank(ip6Cidr)) {
 ipv6 = true;
 }
 // Validate zone
 final DataCenterVO zone = _dcDao.findById(zoneId);
 if (zone.getNetworkType() == NetworkType.Basic) {
-if (ipv6) {
-throw new InvalidParameterValueException("IPv6 is not 
supported in Basic zone");
-}
-
 // In Basic zone the network should have aclType=Domain, 
domainId=1, subdomainAccess=true
 if (aclType == null || aclType != ACLType.Domain) {
 throw new InvalidParameterValueException("Only AclType=Domain 
can be specified for network creation in Basic zone");
@@ -2188,6 +2185,10 @@ public class NetworkOrchestrator extends ManagerBase 
implements NetworkOrchestra
 }
 }
 
+if (ipv6 && !NetUtils.isValidIp6Cidr(ip6Cidr)) {
+throw new InvalidParameterValueException("Invalid IPv6 cidr 
specified");
+}
+
 //TODO(VXLAN): Support VNI specified
 // VlanId can be specified only when network offering supports it
 final boolean vlanSpecified = vlanId != null;
@@ -2328,7 +2329,7 @@ public class NetworkOrchestrator extends ManagerBase 
implements NetworkOrchestra
 userNetwork.setGateway(gateway);
 }
 
-if (ip6Cidr != null && ip6Gateway != null) {
+if (StringUtils.isNotBlank(ip6Gateway) && 
StringUtils.isNotBlank(ip6Cidr)) {
 userNetwork.setIp6Cidr(ip6Cidr);
 userNetwork.setIp6Gateway(ip6Gateway);
 }

-- 
To stop receiving notification emails like this one, please contact
d...@apache.org.

[GitHub] ernjvr commented on issue #2449: WIP CLOUDSTACK-10278 idempotent column addition

[GitBox]

ernjvr commented on issue #2449: WIP CLOUDSTACK-10278 idempotent column addition
URL: https://github.com/apache/cloudstack/pull/2449#issuecomment-376073260
 
 
   @blueorangutan test


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on a change in pull request #2510: CLOUDSTACK-10334: Fix inadequate information for handling catch clauses

[GitBox]

DaanHoogland commented on a change in pull request #2510: CLOUDSTACK-10334: Fix 
inadequate information for handling catch clauses
URL: https://github.com/apache/cloudstack/pull/2510#discussion_r177001234
 
 

 ##
 File path: server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker.java
 ##
 @@ -258,11 +258,11 @@ public void processParameters(final BaseCmd cmd, final 
Map params) {
 }
 
 } catch (final IllegalArgumentException e) {
-s_logger.error("Error initializing command " + 
cmd.getCommandName() + ", field " + field.getName() + " is not accessible.");
+s_logger.error("Error initializing command " + 
cmd.getCommandName() + ", field " + field.getName() + " is not accessible.", e);
 
 Review comment:
   @rafaelweingartner are you satisfied with this answer?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2449: WIP CLOUDSTACK-10278 idempotent column addition

[GitBox]

blueorangutan commented on issue #2449: WIP CLOUDSTACK-10278 idempotent column 
addition
URL: https://github.com/apache/cloudstack/pull/2449#issuecomment-376073404
 
 
   @ernjvr a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been 
kicked to run smoke tests


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2438: [CLOUDSTACK-10307] Remove unused things from HostDaoImpl

[GitBox]

blueorangutan commented on issue #2438: [CLOUDSTACK-10307] Remove unused things 
from HostDaoImpl
URL: https://github.com/apache/cloudstack/pull/2438#issuecomment-376065645
 
 
   @DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + vmware-65) has 
been kicked to run smoke tests


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on issue #2438: [CLOUDSTACK-10307] Remove unused things from HostDaoImpl

[GitBox]

DaanHoogland commented on issue #2438: [CLOUDSTACK-10307] Remove unused things 
from HostDaoImpl
URL: https://github.com/apache/cloudstack/pull/2438#issuecomment-376065498
 
 
   @blueorangutan test centos7 vmware-65


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for XenServer, Vmware

[GitBox]

rhtyd commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for 
XenServer, Vmware
URL: https://github.com/apache/cloudstack/pull/2507#issuecomment-376065281
 
 
   @blueorangutan test centos7 vmware-55u3
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] ernjvr commented on issue #2449: WIP CLOUDSTACK-10278 idempotent column addition

[GitBox]

ernjvr commented on issue #2449: WIP CLOUDSTACK-10278 idempotent column addition
URL: https://github.com/apache/cloudstack/pull/2449#issuecomment-376065403
 
 
   @blueorangutan package


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2449: WIP CLOUDSTACK-10278 idempotent column addition

[GitBox]

blueorangutan commented on issue #2449: WIP CLOUDSTACK-10278 idempotent column 
addition
URL: https://github.com/apache/cloudstack/pull/2449#issuecomment-376065431
 
 
   @ernjvr a Jenkins job has been kicked to build packages. I'll keep you 
posted as I make progress.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for XenServer, Vmware

[GitBox]

blueorangutan commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for 
XenServer, Vmware
URL: https://github.com/apache/cloudstack/pull/2507#issuecomment-375957838
 
 
   @rhtyd a Trillian-Jenkins test job (centos7 mgmt + vmware-55u3) has been 
kicked to run smoke tests


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on issue #2438: [CLOUDSTACK-10307] Remove unused things from HostDaoImpl

[GitBox]

DaanHoogland commented on issue #2438: [CLOUDSTACK-10307] Remove unused things 
from HostDaoImpl
URL: https://github.com/apache/cloudstack/pull/2438#issuecomment-376065359
 
 
   sorry for the late response, @rafaelweingartner . I have no idea what is 
keeping them, I'll look.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for XenServer, Vmware

[GitBox]

rhtyd commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for 
XenServer, Vmware
URL: https://github.com/apache/cloudstack/pull/2507#issuecomment-375957793
 
 
   @blueorangutan test centos7 vmware-55u3


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for XenServer, Vmware

[GitBox]

rhtyd commented on issue #2507: CLOUDSTACK-10319: Allow TLSv1, v1.1 for 
XenServer, Vmware
URL: https://github.com/apache/cloudstack/pull/2507#issuecomment-376065281
 
 
   @blueorangutan test centos7 vmware-55u3
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2505: CLOUDSTACK-10333: Secure Live VM Migration for KVM

[GitBox]

rhtyd commented on a change in pull request #2505: CLOUDSTACK-10333: Secure 
Live VM Migration for KVM
URL: https://github.com/apache/cloudstack/pull/2505#discussion_r176993902
 
 

 ##
 File path: debian/cloudstack-agent.postinst
 ##
 @@ -50,6 +50,13 @@ case "$1" in
 mkdir /etc/libvirt/hooks
 fi
 cp -a /usr/share/cloudstack-agent/lib/libvirtqemuhook 
/etc/libvirt/hooks/qemu
+
+# Enable TLS enabled VM migration for libvirtd
+if ! iptables-save | grep -- "-A INPUT -p tcp -m tcp --dport 16514 -j 
ACCEPT" > /dev/null; then
+iptables -t filter -A INPUT -p tcp -m tcp --dport 16514 -j ACCEPT
+iptables-save > /etc/iptables/rules.v4
+fi
+
 
 Review comment:
   Alright @wido, perhaps we can remove this for Ubuntu (debian pkg). On both 
CentOS 6 and 7, iptables service is indeed available that is used to save 
existing rules, firewalld is not used here. It is likely that things may break 
for el6/7 users. I'm okay to document the change in release notes docs as well. 
Let's ask others for their thoughts - @DaanHoogland @rafaelweingartner @resmo 
@ustcweizhou @nvazquez @mlsorensen ?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] wido commented on a change in pull request #2505: CLOUDSTACK-10333: Secure Live VM Migration for KVM

[GitBox]

wido commented on a change in pull request #2505: CLOUDSTACK-10333: Secure Live 
VM Migration for KVM
URL: https://github.com/apache/cloudstack/pull/2505#discussion_r176992891
 
 

 ##
 File path: debian/cloudstack-agent.postinst
 ##
 @@ -50,6 +50,13 @@ case "$1" in
 mkdir /etc/libvirt/hooks
 fi
 cp -a /usr/share/cloudstack-agent/lib/libvirtqemuhook 
/etc/libvirt/hooks/qemu
+
+# Enable TLS enabled VM migration for libvirtd
+if ! iptables-save | grep -- "-A INPUT -p tcp -m tcp --dport 16514 -j 
ACCEPT" > /dev/null; then
+iptables -t filter -A INPUT -p tcp -m tcp --dport 16514 -j ACCEPT
+iptables-save > /etc/iptables/rules.v4
+fi
+
 
 Review comment:
   If users are using firewalld or ufw on their CentOS/Ubuntu system this may 
break things. And like I said, /etc/iptables does not exist on Ubuntu systems 
by default, you need the iptables-persistent package for that.
   
   I wouldn't touch the firewall in a postinst of a package. The package should 
not touch parts of the system it's not configuring.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2505: CLOUDSTACK-10333: Secure Live VM Migration for KVM

[GitBox]

rhtyd commented on a change in pull request #2505: CLOUDSTACK-10333: Secure 
Live VM Migration for KVM
URL: https://github.com/apache/cloudstack/pull/2505#discussion_r176992386
 
 

 ##
 File path: debian/cloudstack-agent.postinst
 ##
 @@ -50,6 +50,13 @@ case "$1" in
 mkdir /etc/libvirt/hooks
 fi
 cp -a /usr/share/cloudstack-agent/lib/libvirtqemuhook 
/etc/libvirt/hooks/qemu
+
+# Enable TLS enabled VM migration for libvirtd
+if ! iptables-save | grep -- "-A INPUT -p tcp -m tcp --dport 16514 -j 
ACCEPT" > /dev/null; then
+iptables -t filter -A INPUT -p tcp -m tcp --dport 16514 -j ACCEPT
+iptables-save > /etc/iptables/rules.v4
+fi
+
 
 Review comment:
   The `cloudstack-setup-agent` reconfigures libvirtd config and also adds 
iptables rules for several ports, the post-install script (both rpm+deb) does a 
test if expected iptables rules are in place and adds the ACCEPT rule only if 
needed.
   
   Given not all users may use a config mgmt system such as 
chef/puppet/ansible, running the commands as part of post-install script will 
save the additional work they may need to do themselves (manually or 
automated). @wido I'm okay to advise users via release/admin docs, but I don't 
see any negative/side-effects with the change.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] wido commented on a change in pull request #2505: CLOUDSTACK-10333: Secure Live VM Migration for KVM

[GitBox]

wido commented on a change in pull request #2505: CLOUDSTACK-10333: Secure Live 
VM Migration for KVM
URL: https://github.com/apache/cloudstack/pull/2505#discussion_r176988145
 
 

 ##
 File path: packaging/centos63/cloud.spec
 ##
 @@ -493,6 +493,12 @@ if [ -f 
"%{_sysconfdir}/cloud.rpmsave/agent/agent.properties" ]; then
 mv %{_sysconfdir}/cloud.rpmsave/agent/agent.properties 
%{_sysconfdir}/cloud.rpmsave/agent/agent.properties.rpmsave
 fi
 
+# Enable TLS enabled VM migration for libvirtd
+if ! iptables-save | grep -- "-A INPUT -p tcp -m tcp --dport 16514 -j ACCEPT" 
> /dev/null; then
+iptables -t filter -A INPUT -p tcp -m tcp --dport 16514 -j ACCEPT
+iptables-save > /etc/iptables/rules.v4
+fi
+
 
 Review comment:
   Same here as in the DEB package, I'm not in favor of this


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] wido commented on a change in pull request #2505: CLOUDSTACK-10333: Secure Live VM Migration for KVM

[GitBox]

wido commented on a change in pull request #2505: CLOUDSTACK-10333: Secure Live 
VM Migration for KVM
URL: https://github.com/apache/cloudstack/pull/2505#discussion_r176988097
 
 

 ##
 File path: debian/cloudstack-agent.postinst
 ##
 @@ -50,6 +50,13 @@ case "$1" in
 mkdir /etc/libvirt/hooks
 fi
 cp -a /usr/share/cloudstack-agent/lib/libvirtqemuhook 
/etc/libvirt/hooks/qemu
+
+# Enable TLS enabled VM migration for libvirtd
+if ! iptables-save | grep -- "-A INPUT -p tcp -m tcp --dport 16514 -j 
ACCEPT" > /dev/null; then
+iptables -t filter -A INPUT -p tcp -m tcp --dport 16514 -j ACCEPT
+iptables-save > /etc/iptables/rules.v4
+fi
+
 
 Review comment:
   I am not to keen on this one. Do we really want our packages to start 
configuring a firewall on a host?
   
   This should be on the docs to tell people to open the port(s), not having 
packages doing it manually.
   
   In addition, the directory /etc/iptables does not exist by default on Ubuntu.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] wido commented on a change in pull request #2505: CLOUDSTACK-10333: Secure Live VM Migration for KVM

[GitBox]

wido commented on a change in pull request #2505: CLOUDSTACK-10333: Secure Live 
VM Migration for KVM
URL: https://github.com/apache/cloudstack/pull/2505#discussion_r176988178
 
 

 ##
 File path: packaging/centos7/cloud.spec
 ##
 @@ -437,6 +437,12 @@ if [ -f 
"%{_sysconfdir}/cloud.rpmsave/agent/agent.properties" ]; then
 mv %{_sysconfdir}/cloud.rpmsave/agent/agent.properties 
%{_sysconfdir}/cloud.rpmsave/agent/agent.properties.rpmsave
 fi
 
+# Enable TLS enabled VM migration for libvirtd
+if ! iptables-save | grep -- "-A INPUT -p tcp -m tcp --dport 16514 -j ACCEPT" 
> /dev/null; then
+iptables -t filter -A INPUT -p tcp -m tcp --dport 16514 -j ACCEPT
+iptables-save > /etc/iptables/rules.v4
+fi
 
 Review comment:
   And here again


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services