[GitHub] mrunalinikankariya commented on issue #2242: CLOUDSTACK-9958:Include tags of resources in listUsageRecords API

[git]

mrunalinikankariya commented on issue #2242: CLOUDSTACK-9958:Include tags of 
resources in listUsageRecords API
URL: https://github.com/apache/cloudstack/pull/2242#issuecomment-336345491
 
 
   tag:This is Ready to Merge
 

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 #1558: CLOUDSTACK-10072: Remove unused code from "com.cloud.vm.UserVmManagerImpl"

[git]

rhtyd commented on issue #1558: CLOUDSTACK-10072: Remove unused code from 
"com.cloud.vm.UserVmManagerImpl"
URL: https://github.com/apache/cloudstack/pull/1558#issuecomment-336125805
 
 
   @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 #2258: Cloudstack 10064: Secondary storage Usage for uploadedVolume is not collected

[git]

rhtyd commented on issue #2258: Cloudstack 10064: Secondary storage Usage for 
uploadedVolume is not collected
URL: https://github.com/apache/cloudstack/pull/2258#issuecomment-336125672
 
 
   @PranaliM the new test has failed, please see the logs and fix the failure
 

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 #1558: CLOUDSTACK-10072: Remove unused code from "com.cloud.vm.UserVmManagerImpl"

[git]

blueorangutan commented on issue #1558: CLOUDSTACK-10072: Remove unused code 
from "com.cloud.vm.UserVmManagerImpl"
URL: https://github.com/apache/cloudstack/pull/1558#issuecomment-336125905
 
 
   @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 #2231: [CLOUDSTACK-10039] Adding IOPS/GB offering

[git]

rhtyd commented on issue #2231: [CLOUDSTACK-10039] Adding IOPS/GB offering
URL: https://github.com/apache/cloudstack/pull/2231#issuecomment-336125424
 
 
   @syed looks like we have new conflicts
 

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 #2208: CLOUDSTACK-9542 make listNics and ListUserVms return uniform NIC data

[git]

DaanHoogland commented on issue #2208: CLOUDSTACK-9542 make listNics and 
ListUserVms return uniform NIC data
URL: https://github.com/apache/cloudstack/pull/2208#issuecomment-336070793
 
 
   two runs with the same version led to different fails as I suspected. I 
don't see direct relation to the code but don't dare to merge. I will rebase 
and go from there.
 

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 #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

borisstoyanov commented on issue #2293: CLOUDSTACK-10047: DVSwitch fixes and 
improvements
URL: https://github.com/apache/cloudstack/pull/2293#issuecomment-336061995
 
 
   @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 #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

blueorangutan commented on issue #2293: CLOUDSTACK-10047: DVSwitch fixes and 
improvements
URL: https://github.com/apache/cloudstack/pull/2293#issuecomment-336068676
 
 
   Packaging result: ?centos6 ?centos7 ?debian. JID-1152
 

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 #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

blueorangutan commented on issue #2293: CLOUDSTACK-10047: DVSwitch fixes and 
improvements
URL: https://github.com/apache/cloudstack/pull/2293#issuecomment-336076082
 
 
   @borisstoyanov 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] agx commented on issue #2083: Iptables speedup

[git]

agx commented on issue #2083: Iptables speedup
URL: https://github.com/apache/cloudstack/pull/2083#issuecomment-336047386
 
 
   @DaanHoogland from what I can see to use iptables-conv verbatim we'd mostly 
need to 
   
   - [x] not sys.exit(1) from the module
   - [x] move to using logger instead of print (default output can remain 
unchanged by configuring the logger appropriately)
   - [ ] be able to write a file instead of stdout (stdout can stay the default 
when run as a script)
   
   Did I miss s.th.? @sl0 does this sound reasonable?
 

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] agx commented on issue #2083: Iptables speedup

[git]

agx commented on issue #2083: Iptables speedup
URL: https://github.com/apache/cloudstack/pull/2083#issuecomment-336047386
 
 
   @DaanHoogland from what I can see to use iptables-conv verbatim we'd mostly 
need to 
   
   - [x] not sys.exit(1) from the module
   - [ ] move to using logger instead of print (default output can remain 
unchanged by configuring the logger appropriately)
   - [ ] be able to write a file instead of stdout (stdout can stay the default 
when run as a script)
   
   Did I miss s.th.? @sl0 does this sound reasonable?
 

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 #2181: CLOUDSTACK-9957 Annotations

[git]

blueorangutan commented on issue #2181: CLOUDSTACK-9957 Annotations
URL: https://github.com/apache/cloudstack/pull/2181#issuecomment-336071131
 
 
   @DaanHoogland 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] DaanHoogland commented on issue #2181: CLOUDSTACK-9957 Annotations

[git]

DaanHoogland commented on issue #2181: CLOUDSTACK-9957 Annotations
URL: https://github.com/apache/cloudstack/pull/2181#issuecomment-336070998
 
 
   @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] agx commented on issue #2083: Iptables speedup

[git]

agx commented on issue #2083: Iptables speedup
URL: https://github.com/apache/cloudstack/pull/2083#issuecomment-336047386
 
 
   @DaanHoogland from what I can see to use iptables-conv verbatim we'd mostly 
need to 
   
   * not sys.exit(1) from the module (done)
   * move to using logger instead of print
   * be able to write a file instead of stdout
   
   Did I miss s.th.?
 

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 #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

blueorangutan commented on issue #2293: CLOUDSTACK-10047: DVSwitch fixes and 
improvements
URL: https://github.com/apache/cloudstack/pull/2293#issuecomment-336062186
 
 
   @borisstoyanov 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] borisstoyanov commented on issue #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

borisstoyanov commented on issue #2293: CLOUDSTACK-10047: DVSwitch fixes and 
improvements
URL: https://github.com/apache/cloudstack/pull/2293#issuecomment-336076028
 
 
   @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] agx commented on issue #2083: Iptables speedup

[git]

agx commented on issue #2083: Iptables speedup
URL: https://github.com/apache/cloudstack/pull/2083#issuecomment-336047386
 
 
   @DaanHoogland from what I can see to use iptables-conv verbatim we'd mostly 
need to 
   
   - [x] not sys.exit(1) from the module
   - [ ] move to using logger instead of print (default output can remain 
unchanged by configuring the logger appropriately)
   - [ ] be able to write a file instead of stdout (stdout can stay the default 
when run as a script)
   
   Did I miss s.th.? @sl0 does this sound reasonable?
 

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 #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

rafaelweingartner commented on a change in pull request #2293: 
CLOUDSTACK-10047: DVSwitch fixes and improvements
URL: https://github.com/apache/cloudstack/pull/2293#discussion_r144250258
 
 

 ##
 File path: server/src/com/cloud/hypervisor/HypervisorGuruBase.java
 ##
 @@ -138,7 +143,24 @@ protected VirtualMachineTO 
toVirtualMachineTO(VirtualMachineProfile vmProfile) {
 if(vm.getType() == VirtualMachine.Type.NetScalerVm) {
 nicProfile.setBroadcastType(BroadcastDomainType.Native);
 }
-nics[i++] = toNicTO(nicProfile);
+NicTO nicTo = toNicTO(nicProfile);
+final NetworkVO network = 
_networkDao.findByUuid(nicTo.getNetworkUuid());
+if (network != null) {
+final Map details = 
networkOfferingDetailsDao.getNtwkOffDetails(network.getNetworkOfferingId());
+if (details != null) {
 
 Review comment:
   You are right about the details, but I think I clicked on the wrong line 
(sorry for the mistake). I was talking about the ` if (network != null) ` 
check. if ` if (network == null)` you do not do anything, then it is the same 
as using a continue. I mean, you do a `nics[i++] = nicTo;`, but that can go 
inside the condition as well. Duplicated lines are not that great, so let's see 
what else could be done...
   
   We can do something else, we could extract lines 149-160 to a method; this 
would improve the readability of the code and enable unit tests and Java docs.
 

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] SowjanyaPatha commented on issue #2242: CLOUDSTACK-9958:Include tags of resources in listUsageRecords API

[git]

SowjanyaPatha commented on issue #2242: CLOUDSTACK-9958:Include tags of 
resources in listUsageRecords API
URL: https://github.com/apache/cloudstack/pull/2242#issuecomment-336088845
 
 
   LGTM for Testing
 

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] vedulasantosh commented on issue #2244: CLOUDSTACK-10054:Volume download times out in 3600 seconds

[git]

vedulasantosh commented on issue #2244: CLOUDSTACK-10054:Volume download times 
out in 3600 seconds
URL: https://github.com/apache/cloudstack/pull/2244#issuecomment-336090373
 
 
   Test LGTM
   
   SSVM log:
   
   2017-10-12 09:23:16,435 WARN  [vmware.manager.VmwareStorageManagerImpl] 
(Script-1:null) Interrupting script.
   2017-10-12 09:23:16,439 WARN  [vmware.manager.VmwareStorageManagerImpl] 
(agentRequest-Handler-8:job-200/job-201, cmd: CreateEntityDownloadURLCommand) 
Timed out: tar -cf 2363a57d7f624661832fe41e83c109d1.ova 
2363a57d7f624661832fe41e83c109d1.ovf 
2363a57d7f624661832fe41e83c109d1-disk0.vmdk .  Output is:
   2017-10-12 09:23:16,440 DEBUG 
[storage.resource.VmwareSecondaryStorageResourceHandler] 
(agentRequest-Handler-8:job-200/job-201, cmd: CreateEntityDownloadURLCommand) 
Command execution answer: {"result":false,"details":"Failed to 
download","wait":0}
   
   
   Management-server log:
   
   2017-10-12 14:53:16,884 ERROR [c.c.v.VmWorkJobDispatcher] 
(Work-Job-Executor-1:ctx-da437f5a job-200/job-201) (logid:18ef6817) Unable to 
complete AsyncJobVO {id:201, userId: 2, accountId: 2, instanceType: null, 
instanceId: null, cmd: com.cloud.vm.VmWorkExtractVolume, cmdInfo: 
rO0ABXNyACBjb20uY2xvdWQudm0uVm1Xb3JrRXh0cmFjdFZvbHVtZfgl82-871PmAgACSgAIdm9sdW1lSWRKAAZ6b25lSWR4cgATY29tLmNsb3VkLnZtLlZtV29ya5-ZtlbwJWdrAgAESgAJYWNjb3VudElkSgAGdXNlcklkSgAEdm1JZEwAC2hhbmRsZXJOYW1ldAASTGphdmEvbGFuZy9TdHJpbmc7eHAAAgACAAN0ABRWb2x1bWVBcGlTZXJ2aWNlSW1wbAAEAAE,
 cmdVersion: 0, status: IN_PROGRESS, processStatus: 0, resultCode: 0, result: 
null, initMsid: 193904265554077, completeMsid: null, lastUpdated: null, 
lastPolled: null, created: Thu Oct 12 13:47:51 IST 2017}, job origin:200
   com.cloud.utils.exception.CloudRuntimeException: Unable to create a link for 
entity at volumes/2/4/2363a57d7f624661832fe41e83c109d1 on ssvm,Failed to 
download
   
   2017-10-12 14:47:57,677 ERROR [c.c.a.ApiAsyncJobDispatcher] 
(API-Job-Executor-1:ctx-493b9dfa job-200) (logid:18ef6817) Unexpected exception 
while executing org.apache.cloudstack.api.command.user.volume.ExtractVolumeCmd
   com.cloud.utils.exception.CloudRuntimeException: Unable to serialize: Job is 
cancelled as it has been blocking others for too long
   at 
org.apache.cloudstack.framework.jobs.impl.JobSerializerHelper.fromObjectSerializedString(JobSerializerHelper.java:134)
   at 
org.apache.cloudstack.framework.jobs.impl.AsyncJobManagerImpl.unmarshallResultObject(AsyncJobManagerImpl.java:726)
   at 
com.cloud.storage.VolumeApiServiceImpl.extractVolume(VolumeApiServiceImpl.java:2352)
   at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
   at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   at java.lang.reflect.Method.invoke(Method.java:498)
   at 
org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:333)
   at 
org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:190)
   at 
org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:157)
   at 
org.apache.cloudstack.network.contrail.management.EventUtils$EventInterceptor.invoke(EventUtils.java:107)
   at 
org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:168)
   at 
com.cloud.event.ActionEventInterceptor.invoke(ActionEventInterceptor.java:51)
   at 
org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:168)
   at 
org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:92)
   at 
org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
   at 
org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:213)
   at com.sun.proxy.$Proxy166.extractVolume(Unknown Source)
   at 
org.apache.cloudstack.api.command.user.volume.ExtractVolumeCmd.execute(ExtractVolumeCmd.java:137)
   at com.cloud.api.ApiDispatcher.dispatch(ApiDispatcher.java:150)
   at 
com.cloud.api.ApiAsyncJobDispatcher.runJob(ApiAsyncJobDispatcher.java:108)
   at 
org.apache.cloudstack.framework.jobs.impl.AsyncJobManagerImpl$5.runInContext(AsyncJobManagerImpl.java:561)
   at 
org.apache.cloudstack.managed.context.ManagedContextRunnable$1.run(ManagedContextRunnable.java:49)
   at 
org.apache.cloudstack.managed.context.impl.DefaultManagedContext$1.call(DefaultManagedContext.java:56)
   at 
org.apache.cloudstack.managed.context.impl.DefaultManagedContext.callWithContext(DefaultManagedContext.java:103)
   at 

[GitHub] rhtyd commented on a change in pull request #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

rhtyd commented on a change in pull request #2293: CLOUDSTACK-10047: DVSwitch 
fixes and improvements
URL: https://github.com/apache/cloudstack/pull/2293#discussion_r144259524
 
 

 ##
 File path: utils/src/main/java/com/cloud/utils/UriUtils.java
 ##
 @@ -391,4 +391,57 @@ public static InputStream getInputStreamFromUrl(String 
url, String user, String
 return null;
 }
 }
+
+/**
+ * Expands a given vlan URI to a list of vlan IDs
+ * @param vlanAuthority the URI part without the vlan:// scheme
+ * @return returns list of vlan integer ids
+ */
+public static List expandVlanUri(final String vlanAuthority) {
+final List expandedVlans = new ArrayList<>();
+if (vlanAuthority == null || vlanAuthority.isEmpty()) {
+return expandedVlans;
+}
+for (final String vlanPart: vlanAuthority.split(",")) {
+if (vlanPart == null || vlanPart.isEmpty()) {
+continue;
+}
+final String[] range = vlanPart.split("-");
+if (range.length == 2) {
+Integer start = NumbersUtil.parseInt(range[0], -1);
+Integer end = NumbersUtil.parseInt(range[1], -1);
+if (start <= end && end > -1 && start > -1) {
+while (start <= end) {
+expandedVlans.add(start++);
+}
+}
+} else {
+final Integer value = NumbersUtil.parseInt(range[0], -1);
+if (value > -1) {
+expandedVlans.add(value);
+}
+}
+}
+return expandedVlans;
+}
+
+/**
+ * Checks if given vlan URI authorities overlap
+ * @param vlanRange1
+ * @param vlanRange2
+ * @return true if they overlap
+ */
+public static boolean checkVlanUriOverlap(final String vlanRange1, final 
String vlanRange2) {
+final List vlans1 = expandVlanUri(vlanRange1);
+final List vlans2 = expandVlanUri(vlanRange2);
+if (vlans1 == null || vlans2 == null) {
 
 Review comment:
   In general, this case won't be hit but the implementation was done to be 
defensive of all sorts of inputs, including nulls. If vlanRange provided to 
`expandVlanUri` had issues such as non-integers or negative values, then you'll 
get empty list. Again, this was defensive programming.
 

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 #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

rhtyd commented on a change in pull request #2293: CLOUDSTACK-10047: DVSwitch 
fixes and improvements
URL: https://github.com/apache/cloudstack/pull/2293#discussion_r144263042
 
 

 ##
 File path: utils/src/test/java/com/cloud/utils/UriUtilsTest.java
 ##
 @@ -57,4 +60,45 @@ public void getUpdateUri() {
 //XXX: Interesting cases not covered:
 // * port is ignored and left out from the return value
 }
+
+@Test
+public void expandVlanEmpty() {
+List vlans = UriUtils.expandVlanUri("");
+Assert.assertTrue(vlans.size() == 0);
+}
+
+@Test
+public void expandVlanSingleValue() {
+List vlans = UriUtils.expandVlanUri("10");
+Assert.assertTrue(vlans.size() == 1);
+Assert.assertEquals(vlans, Collections.singletonList(10));
+}
+
+@Test
+public void expandVlanValidRange() {
+List vlans = UriUtils.expandVlanUri("10-12,14,17,40-43");
+Assert.assertEquals(vlans, Arrays.asList(10,11,12,14,17,40,41,42,43));
+}
+
+@Test
+public void expandVlanInvalidRange() {
+List vlans = UriUtils.expandVlanUri("10-,12-14,-4,5-2");
+Assert.assertEquals(vlans, Arrays.asList(10,12,13,14));
+}
+
+@Test
+public void testVlanUriOverlap() {
+Assert.assertTrue(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
"10"));
+Assert.assertTrue(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
"32,33-44,30-31"));
+Assert.assertTrue(UriUtils.checkVlanUriOverlap("10-30", "25-35"));
+}
+
+@Test
+public void testVlanUriNoOverlap() {
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
null));
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
""));
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
"32"));
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("10,22,111", "12"));
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("100-200", 
"30-40,50,201-250"));
 
 Review comment:
   Sure, sometimes we do want to over-engineer? :)
 

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 #2048: CLOUDSTACK-9880: Expansion of Management IP Range.

[git]

borisstoyanov commented on issue #2048: CLOUDSTACK-9880: Expansion of 
Management IP Range.
URL: https://github.com/apache/cloudstack/pull/2048#issuecomment-336103107
 
 
   @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] blueorangutan commented on issue #2048: CLOUDSTACK-9880: Expansion of Management IP Range.

[git]

blueorangutan commented on issue #2048: CLOUDSTACK-9880: Expansion of 
Management IP Range.
URL: https://github.com/apache/cloudstack/pull/2048#issuecomment-336103201
 
 
   @borisstoyanov 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] rhtyd commented on issue #2250: CLOUDSTACK-10057: listNetworkOfferings now returns the correct number of offerings.

[git]

rhtyd commented on issue #2250: CLOUDSTACK-10057: listNetworkOfferings now 
returns the correct number  of offerings.
URL: https://github.com/apache/cloudstack/pull/2250#issuecomment-336104911
 
 
   @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 a change in pull request #2281: CLOUDSTACK-10102: New network type (L2)

[git]

rhtyd commented on a change in pull request #2281: CLOUDSTACK-10102: New 
network type (L2)
URL: https://github.com/apache/cloudstack/pull/2281#discussion_r144247590
 
 

 ##
 File path: engine/schema/src/com/cloud/vm/dao/UserVmDaoImpl.java
 ##
 @@ -301,13 +306,16 @@ public void updateVM(long id, String displayName, 
boolean enable, Long osTypeId,
 
 @Override
 public List listByNetworkIdAndStates(long networkId, State... 
states) {
-if (UserVmSearch == null) {
+NetworkVO network = networkDao.findById(networkId);
+if (UserVmSearch == null && network != null) {
 SearchBuilder nicSearch = _nicDao.createSearchBuilder();
 nicSearch.and("networkId", nicSearch.entity().getNetworkId(), 
SearchCriteria.Op.EQ);
 nicSearch.and("removed", nicSearch.entity().getRemoved(), 
SearchCriteria.Op.NULL);
-nicSearch.and().op("ip4Address", 
nicSearch.entity().getIPv4Address(), SearchCriteria.Op.NNULL);
-nicSearch.or("ip6Address", nicSearch.entity().getIPv6Address(), 
SearchCriteria.Op.NNULL);
-nicSearch.cp();
+if (! network.getGuestType().equals(Network.GuestType.L2)) {
 
 Review comment:
   Space after `!` may be removed
 

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 #2281: CLOUDSTACK-10102: New network type (L2)

[git]

rhtyd commented on a change in pull request #2281: CLOUDSTACK-10102: New 
network type (L2)
URL: https://github.com/apache/cloudstack/pull/2281#discussion_r144248244
 
 

 ##
 File path: ui/scripts/configuration.js
 ##
 @@ -2766,10 +2776,13 @@
 
$form.find('.form-item[rel=isPersistent]').find('input[type=checkbox]').attr("disabled",
 "disabled");
 
 
-} else { //$(this).val() == 
"Isolated"
+} else if ($(this).val() == 
"Isolated") {
 
 Review comment:
   Can both the cases for L2 and Isolated be collapsed, as both are using same 
code?
 

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 #2281: CLOUDSTACK-10102: New network type (L2)

[git]

rhtyd commented on a change in pull request #2281: CLOUDSTACK-10102: New 
network type (L2)
URL: https://github.com/apache/cloudstack/pull/2281#discussion_r144248102
 
 

 ##
 File path: ui/scripts/configuration.js
 ##
 @@ -2766,10 +2776,13 @@
 
$form.find('.form-item[rel=isPersistent]').find('input[type=checkbox]').attr("disabled",
 "disabled");
 
 
-} else { //$(this).val() == 
"Isolated"
+} else if ($(this).val() == 
"Isolated") {
 
$form.find('.form-item[rel=specifyVlan]').find('input[type=checkbox]').removeAttr("disabled");
 //make it editable
 
$form.find('.form-item[rel=isPersistent]').find('input[type=checkbox]').removeAttr("disabled");
 
 
 Review comment:
   Remove extra new line
 

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 #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

rafaelweingartner commented on a change in pull request #2293: 
CLOUDSTACK-10047: DVSwitch fixes and improvements
URL: https://github.com/apache/cloudstack/pull/2293#discussion_r144250838
 
 

 ##
 File path: utils/src/main/java/com/cloud/utils/UriUtils.java
 ##
 @@ -391,4 +391,57 @@ public static InputStream getInputStreamFromUrl(String 
url, String user, String
 return null;
 }
 }
+
+/**
+ * Expands a given vlan URI to a list of vlan IDs
+ * @param vlanAuthority the URI part without the vlan:// scheme
+ * @return returns list of vlan integer ids
+ */
+public static List expandVlanUri(final String vlanAuthority) {
+final List expandedVlans = new ArrayList<>();
+if (vlanAuthority == null || vlanAuthority.isEmpty()) {
+return expandedVlans;
+}
+for (final String vlanPart: vlanAuthority.split(",")) {
+if (vlanPart == null || vlanPart.isEmpty()) {
+continue;
+}
+final String[] range = vlanPart.split("-");
+if (range.length == 2) {
+Integer start = NumbersUtil.parseInt(range[0], -1);
+Integer end = NumbersUtil.parseInt(range[1], -1);
+if (start <= end && end > -1 && start > -1) {
+while (start <= end) {
+expandedVlans.add(start++);
+}
+}
+} else {
+final Integer value = NumbersUtil.parseInt(range[0], -1);
+if (value > -1) {
+expandedVlans.add(value);
+}
+}
+}
+return expandedVlans;
+}
+
+/**
+ * Checks if given vlan URI authorities overlap
+ * @param vlanRange1
+ * @param vlanRange2
+ * @return true if they overlap
+ */
+public static boolean checkVlanUriOverlap(final String vlanRange1, final 
String vlanRange2) {
+final List vlans1 = expandVlanUri(vlanRange1);
+final List vlans2 = expandVlanUri(vlanRange2);
+if (vlans1 == null || vlans2 == null) {
 
 Review comment:
   I did not understand the explanations :(
   
   Anyways, looking at the code of `expandVlanUri`, I did not see a way for it 
to return null. Worst case scenario it returns an empty list. Do we need this 
check?
 

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 issue #2048: CLOUDSTACK-9880: Expansion of Management IP Range.

[git]

nitin-maharana commented on issue #2048: CLOUDSTACK-9880: Expansion of 
Management IP Range.
URL: https://github.com/apache/cloudstack/pull/2048#issuecomment-336095574
 
 
   @borisstoyanov, DONE!!
   
   Thanks,
   Nitin
 

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 #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

rafaelweingartner commented on a change in pull request #2293: 
CLOUDSTACK-10047: DVSwitch fixes and improvements
URL: https://github.com/apache/cloudstack/pull/2293#discussion_r144259343
 
 

 ##
 File path: server/src/com/cloud/hypervisor/HypervisorGuruBase.java
 ##
 @@ -138,7 +143,24 @@ protected VirtualMachineTO 
toVirtualMachineTO(VirtualMachineProfile vmProfile) {
 if(vm.getType() == VirtualMachine.Type.NetScalerVm) {
 nicProfile.setBroadcastType(BroadcastDomainType.Native);
 }
-nics[i++] = toNicTO(nicProfile);
+NicTO nicTo = toNicTO(nicProfile);
+final NetworkVO network = 
_networkDao.findByUuid(nicTo.getNetworkUuid());
+if (network != null) {
+final Map details = 
networkOfferingDetailsDao.getNtwkOffDetails(network.getNetworkOfferingId());
+if (details != null) {
 
 Review comment:
   Thanks for both the explanations and for the effort!
   
   BTW: your explanation is great to enrich our code base. This explanation 
would be awesome in a Javadoc, so people in the future can understand why we 
are doing this without needing to deeply inspect the code.
 

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 #2048: CLOUDSTACK-9880: Expansion of Management IP Range.

[git]

borisstoyanov commented on issue #2048: CLOUDSTACK-9880: Expansion of 
Management IP Range.
URL: https://github.com/apache/cloudstack/pull/2048#issuecomment-336097755
 
 
   thanks @nitin-maharana 
   @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 #2048: CLOUDSTACK-9880: Expansion of Management IP Range.

[git]

blueorangutan commented on issue #2048: CLOUDSTACK-9880: Expansion of 
Management IP Range.
URL: https://github.com/apache/cloudstack/pull/2048#issuecomment-336097815
 
 
   @borisstoyanov 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 #2048: CLOUDSTACK-9880: Expansion of Management IP Range.

[git]

blueorangutan commented on issue #2048: CLOUDSTACK-9880: Expansion of 
Management IP Range.
URL: https://github.com/apache/cloudstack/pull/2048#issuecomment-336102745
 
 
   Packaging result: ?centos6 ?centos7 ?debian. JID-1153
 

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] fmaximus commented on issue #2250: CLOUDSTACK-10057: listNetworkOfferings now returns the correct number of offerings.

[git]

fmaximus commented on issue #2250: CLOUDSTACK-10057: listNetworkOfferings now 
returns the correct number  of offerings.
URL: https://github.com/apache/cloudstack/pull/2250#issuecomment-336103994
 
 
   I've added an extra unit test for the changed code, to keep the history 
intact in relation to the Trillian Tests, I have not squashed yet. I will 
squash once we have approval.
 

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 #2294: Adding allocated IOPS to storage pool response

[git]

nitin-maharana commented on a change in pull request #2294: Adding allocated 
IOPS to storage pool response
URL: https://github.com/apache/cloudstack/pull/2294#discussion_r144201108
 
 

 ##
 File path: api/src/org/apache/cloudstack/api/response/StoragePoolResponse.java
 ##
 @@ -288,6 +292,10 @@ public void setCapacityIops(Long capacityIops) {
 this.capacityIops = capacityIops;
 }
 
+public void setAllocatedIops(Long allocatedIops) {
 
 Review comment:
   It's good if you would add the getter method as well. or Do you think it is 
not needed?
 

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 #2284: CLOUDSTACK-10103: Cloudian Connector for CloudStack

[git]

rafaelweingartner commented on a change in pull request #2284: 
CLOUDSTACK-10103: Cloudian Connector for CloudStack
URL: https://github.com/apache/cloudstack/pull/2284#discussion_r144254195
 
 

 ##
 File path: 
plugins/integrations/cloudian/src/org/apache/cloudstack/cloudian/client/CloudianClient.java
 ##
 @@ -0,0 +1,349 @@
+// 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.cloudian.client;
+
+import java.io.IOException;
+import java.net.SocketTimeoutException;
+import java.security.KeyManagementException;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.SecureRandom;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.X509TrustManager;
+
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.utils.security.SSLUtils;
+import org.apache.http.HttpHost;
+import org.apache.http.HttpResponse;
+import org.apache.http.HttpStatus;
+import org.apache.http.auth.AuthScope;
+import org.apache.http.auth.Credentials;
+import org.apache.http.auth.UsernamePasswordCredentials;
+import org.apache.http.client.AuthCache;
+import org.apache.http.client.CredentialsProvider;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.config.RequestConfig;
+import org.apache.http.client.methods.HttpDelete;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.client.methods.HttpPut;
+import org.apache.http.client.protocol.HttpClientContext;
+import org.apache.http.conn.ConnectTimeoutException;
+import org.apache.http.conn.ssl.NoopHostnameVerifier;
+import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
+import org.apache.http.entity.StringEntity;
+import org.apache.http.impl.auth.BasicScheme;
+import org.apache.http.impl.client.BasicAuthCache;
+import org.apache.http.impl.client.BasicCredentialsProvider;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.log4j.Logger;
+
+import com.cloud.utils.nio.TrustAllManager;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Strings;
+
+public class CloudianClient {
+private static final Logger LOG = Logger.getLogger(CloudianClient.class);
+
+private final HttpClient httpClient;
+private final HttpClientContext httpContext;
+private final String adminApiUrl;
+
+public CloudianClient(final String host, final Integer port, final String 
scheme, final String username, final String password, final boolean 
validateSSlCertificate, final int timeout) throws KeyStoreException, 
NoSuchAlgorithmException, KeyManagementException {
+final CredentialsProvider provider = new BasicCredentialsProvider();
+provider.setCredentials(AuthScope.ANY, new 
UsernamePasswordCredentials(username, password));
+final HttpHost adminHost = new HttpHost(host, port, scheme);
+final AuthCache authCache = new BasicAuthCache();
+authCache.put(adminHost, new BasicScheme());
+
+this.adminApiUrl = adminHost.toURI();
+this.httpContext = HttpClientContext.create();
+this.httpContext.setCredentialsProvider(provider);
+this.httpContext.setAuthCache(authCache);
+
+final RequestConfig config = RequestConfig.custom()
+.setConnectTimeout(timeout * 1000)
+.setConnectionRequestTimeout(timeout * 1000)
+.setSocketTimeout(timeout * 1000)
+.build();
+
+if (!validateSSlCertificate) {
+final SSLContext sslcontext = SSLUtils.getSSLContext();
+sslcontext.init(null, new X509TrustManager[]{new 
TrustAllManager()}, new SecureRandom());
+final SSLConnectionSocketFactory factory = new 
SSLConnectionSocketFactory(sslcontext, NoopHostnameVerifier.INSTANCE);
+this.httpClient = HttpClientBuilder.create()
+.setDefaultCredentialsProvider(provider)
+.setDefaultRequestConfig(config)
+.setSSLSocketFactory(factory)
+  

[GitHub] rhtyd commented on a change in pull request #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

rhtyd commented on a change in pull request #2293: CLOUDSTACK-10047: DVSwitch 
fixes and improvements
URL: https://github.com/apache/cloudstack/pull/2293#discussion_r144257564
 
 

 ##
 File path: utils/src/main/java/com/cloud/utils/UriUtils.java
 ##
 @@ -391,4 +391,57 @@ public static InputStream getInputStreamFromUrl(String 
url, String user, String
 return null;
 }
 }
+
+/**
+ * Expands a given vlan URI to a list of vlan IDs
+ * @param vlanAuthority the URI part without the vlan:// scheme
+ * @return returns list of vlan integer ids
+ */
+public static List expandVlanUri(final String vlanAuthority) {
+final List expandedVlans = new ArrayList<>();
+if (vlanAuthority == null || vlanAuthority.isEmpty()) {
+return expandedVlans;
+}
+for (final String vlanPart: vlanAuthority.split(",")) {
+if (vlanPart == null || vlanPart.isEmpty()) {
+continue;
+}
+final String[] range = vlanPart.split("-");
+if (range.length == 2) {
+Integer start = NumbersUtil.parseInt(range[0], -1);
+Integer end = NumbersUtil.parseInt(range[1], -1);
+if (start <= end && end > -1 && start > -1) {
+while (start <= end) {
+expandedVlans.add(start++);
+}
+}
+} else {
+final Integer value = NumbersUtil.parseInt(range[0], -1);
+if (value > -1) {
+expandedVlans.add(value);
+}
+}
+}
+return expandedVlans;
+}
+
+/**
+ * Checks if given vlan URI authorities overlap
+ * @param vlanRange1
+ * @param vlanRange2
+ * @return true if they overlap
+ */
+public static boolean checkVlanUriOverlap(final String vlanRange1, final 
String vlanRange2) {
+final List vlans1 = expandVlanUri(vlanRange1);
+final List vlans2 = expandVlanUri(vlanRange2);
+if (vlans1 == null || vlans2 == null) {
 
 Review comment:
   Yes, we need this check.
 

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 #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

rafaelweingartner commented on a change in pull request #2293: 
CLOUDSTACK-10047: DVSwitch fixes and improvements
URL: https://github.com/apache/cloudstack/pull/2293#discussion_r144258991
 
 

 ##
 File path: utils/src/main/java/com/cloud/utils/UriUtils.java
 ##
 @@ -391,4 +391,57 @@ public static InputStream getInputStreamFromUrl(String 
url, String user, String
 return null;
 }
 }
+
+/**
+ * Expands a given vlan URI to a list of vlan IDs
+ * @param vlanAuthority the URI part without the vlan:// scheme
+ * @return returns list of vlan integer ids
+ */
+public static List expandVlanUri(final String vlanAuthority) {
+final List expandedVlans = new ArrayList<>();
+if (vlanAuthority == null || vlanAuthority.isEmpty()) {
+return expandedVlans;
+}
+for (final String vlanPart: vlanAuthority.split(",")) {
+if (vlanPart == null || vlanPart.isEmpty()) {
+continue;
+}
+final String[] range = vlanPart.split("-");
+if (range.length == 2) {
+Integer start = NumbersUtil.parseInt(range[0], -1);
+Integer end = NumbersUtil.parseInt(range[1], -1);
+if (start <= end && end > -1 && start > -1) {
+while (start <= end) {
+expandedVlans.add(start++);
+}
+}
+} else {
+final Integer value = NumbersUtil.parseInt(range[0], -1);
+if (value > -1) {
+expandedVlans.add(value);
+}
+}
+}
+return expandedVlans;
+}
+
+/**
+ * Checks if given vlan URI authorities overlap
+ * @param vlanRange1
+ * @param vlanRange2
+ * @return true if they overlap
+ */
+public static boolean checkVlanUriOverlap(final String vlanRange1, final 
String vlanRange2) {
+final List vlans1 = expandVlanUri(vlanRange1);
+final List vlans2 = expandVlanUri(vlanRange2);
+if (vlans1 == null || vlans2 == null) {
 
 Review comment:
   Do you mind explaining why if the method `expandVlanUri` returns an empty 
list in the worst case?
 

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 #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

rafaelweingartner commented on a change in pull request #2293: 
CLOUDSTACK-10047: DVSwitch fixes and improvements
URL: https://github.com/apache/cloudstack/pull/2293#discussion_r144262001
 
 

 ##
 File path: utils/src/main/java/com/cloud/utils/UriUtils.java
 ##
 @@ -391,4 +391,57 @@ public static InputStream getInputStreamFromUrl(String 
url, String user, String
 return null;
 }
 }
+
+/**
+ * Expands a given vlan URI to a list of vlan IDs
+ * @param vlanAuthority the URI part without the vlan:// scheme
+ * @return returns list of vlan integer ids
+ */
+public static List expandVlanUri(final String vlanAuthority) {
+final List expandedVlans = new ArrayList<>();
+if (vlanAuthority == null || vlanAuthority.isEmpty()) {
+return expandedVlans;
+}
+for (final String vlanPart: vlanAuthority.split(",")) {
+if (vlanPart == null || vlanPart.isEmpty()) {
+continue;
+}
+final String[] range = vlanPart.split("-");
+if (range.length == 2) {
+Integer start = NumbersUtil.parseInt(range[0], -1);
+Integer end = NumbersUtil.parseInt(range[1], -1);
+if (start <= end && end > -1 && start > -1) {
+while (start <= end) {
+expandedVlans.add(start++);
+}
+}
+} else {
+final Integer value = NumbersUtil.parseInt(range[0], -1);
+if (value > -1) {
+expandedVlans.add(value);
+}
+}
+}
+return expandedVlans;
+}
+
+/**
+ * Checks if given vlan URI authorities overlap
+ * @param vlanRange1
+ * @param vlanRange2
+ * @return true if they overlap
+ */
+public static boolean checkVlanUriOverlap(final String vlanRange1, final 
String vlanRange2) {
+final List vlans1 = expandVlanUri(vlanRange1);
+final List vlans2 = expandVlanUri(vlanRange2);
+if (vlans1 == null || vlans2 == null) {
 
 Review comment:
   Ah that as well ;)
   
   I am sorry to bother, but if I do not understand something I keep asking 
until I can move along.
 

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 #2250: CLOUDSTACK-10057: listNetworkOfferings now returns the correct number of offerings.

[git]

blueorangutan commented on issue #2250: CLOUDSTACK-10057: listNetworkOfferings 
now returns the correct number  of offerings.
URL: https://github.com/apache/cloudstack/pull/2250#issuecomment-336105047
 
 
   @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 a change in pull request #2281: CLOUDSTACK-10102: New network type (L2)

[git]

rhtyd commented on a change in pull request #2281: CLOUDSTACK-10102: New 
network type (L2)
URL: https://github.com/apache/cloudstack/pull/2281#discussion_r144247885
 
 

 ##
 File path: server/src/com/cloud/network/IpAddressManagerImpl.java
 ##
 @@ -1088,6 +1087,7 @@ public void releasePodIp(Long id) throws 
CloudRuntimeException {
 }
 }
 
+
 
 Review comment:
   Remove extra newline?
 

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 #2281: CLOUDSTACK-10102: New network type (L2)

[git]

rhtyd commented on a change in pull request #2281: CLOUDSTACK-10102: New 
network type (L2)
URL: https://github.com/apache/cloudstack/pull/2281#discussion_r144248053
 
 

 ##
 File path: ui/scripts/configuration.js
 ##
 @@ -2403,13 +2403,20 @@
 //*** VPC checkbox ***
 var $useVpc = 
args.$form.find('.form-item[rel=\"useVpc\"]');
 var $useVpcCb = 
$useVpc.find("input[type=checkbox]");
+var $supportedServices = 
args.$form.find('.form-item[rel=\"supportedServices\"]');
 if ($guestTypeField.val() == 'Shared') 
{ //Shared network offering
 $useVpc.hide();
+$supportedServices.css('display', 
'inline-block');
 if ($useVpcCb.is(':checked')) { 
//if useVpc is checked,
 
$useVpcCb.removeAttr("checked"); //remove "checked" attribute in useVpc
 }
-} else { //Isolated network offering
+} else if ($guestTypeField.val() == 
'Isolated') { //Isolated network offering
 $useVpc.css('display', 
'inline-block');
+$supportedServices.css('display', 
'inline-block');
+}
+else if ($guestTypeField.val() == 
'L2') {
 
 Review comment:
   Put `} else if ...` on same line as line 2413.
 

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 #2281: CLOUDSTACK-10102: New network type (L2)

[git]

rhtyd commented on a change in pull request #2281: CLOUDSTACK-10102: New 
network type (L2)
URL: https://github.com/apache/cloudstack/pull/2281#discussion_r144248294
 
 

 ##
 File path: ui/scripts/configuration.js
 ##
 @@ -3315,6 +3328,13 @@
 delete inputData.isPersistent; //if 
Persistent checkbox is unchecked, do not pass isPersistent parameter to API 
call since we need to keep API call's size as small as possible (p.s. 
isPersistent is defaulted as false at server-side)
 }
 }
 
 Review comment:
   Put `} else if..` on same line.
 

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 #2281: CLOUDSTACK-10102: New network type (L2)

[git]

rhtyd commented on a change in pull request #2281: CLOUDSTACK-10102: New 
network type (L2)
URL: https://github.com/apache/cloudstack/pull/2281#discussion_r144247839
 
 

 ##
 File path: server/src/com/cloud/network/IpAddressManagerImpl.java
 ##
 @@ -815,7 +814,7 @@ public IPAddressVO doInTransaction(TransactionStatus 
status) throws Insufficient
 }
 addr.setState(assign ? IpAddress.State.Allocated : 
IpAddress.State.Allocating);
 
-if (vlanUse != VlanType.DirectAttached) {
+if (assign && vlanUse != VlanType.DirectAttached) {
 
 Review comment:
   Could this cause a backward compatibility issue?
 

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 #2284: CLOUDSTACK-10103: Cloudian Connector for CloudStack

[git]

rafaelweingartner commented on a change in pull request #2284: 
CLOUDSTACK-10103: Cloudian Connector for CloudStack
URL: https://github.com/apache/cloudstack/pull/2284#discussion_r144251252
 
 

 ##
 File path: 
plugins/integrations/cloudian/src/org/apache/cloudstack/cloudian/CloudianConnector.java
 ##
 @@ -0,0 +1,82 @@
+// 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.cloudian;
+
+import org.apache.cloudstack.framework.config.ConfigKey;
+
+import com.cloud.utils.component.PluggableService;
+
+public interface CloudianConnector extends PluggableService {
+
+ConfigKey CloudianConnectorEnabled = new ConfigKey<>("Advanced", 
Boolean.class, "cloudian.connector.enabled", "false",
+"If set to true, this enables the Cloudian Connector for 
CloudStack.", true);
+
+ConfigKey CloudianAdminHost = new ConfigKey<>("Advanced", 
String.class, "cloudian.admin.host", "s3-admin.cloudian.com",
+"The hostname of the Cloudian Admin server.", true);
+
+ConfigKey CloudianAdminPort = new ConfigKey<>("Advanced", 
Integer.class, "cloudian.admin.port", "19443",
+"The port of the Cloudian Admin server.", true);
+
+ConfigKey CloudianAdminProtocol = new ConfigKey<>("Advanced", 
String.class, "cloudian.admin.protocol", "https",
+"The protocol of the Cloudian Admin server.", true);
+
+ConfigKey CloudianValidateSSLSecurity = new 
ConfigKey<>("Advanced", Boolean.class, "cloudian.validate.ssl", "true",
+"When set to true, this will validate the SSL certificate when 
connecting to https/ssl enabled admin host.", true);
+
+ConfigKey CloudianAdminUser = new ConfigKey<>("Advanced", 
String.class, "cloudian.admin.user", "sysadmin",
+"The system admin user for accessing the Cloudian Admin server.", 
true);
+
+ConfigKey CloudianAdminPassword = new ConfigKey<>("Advanced", 
String.class, "cloudian.admin.password", "public",
+"The system admin password for the Cloudian Admin server.", true);
+
+ConfigKey CloudianAdminApiRequestTimeout = new 
ConfigKey<>("Advanced", Integer.class, "cloudian.api.request.timeout", "5",
+"The admin API request timeout in seconds.", true);
+
+ConfigKey CloudianCmcAdminUser = new ConfigKey<>("Advanced", 
String.class, "cloudian.cmc.admin.user", "admin",
+"The admin user name for accessing the Cloudian Management 
Console.", true);
+
+ConfigKey CloudianCmcHost = new ConfigKey<>("Advanced", 
String.class, "cloudian.cmc.host", "cmc.cloudian.com",
+"The hostname of the Cloudian Management Console.", true);
+
+ConfigKey CloudianCmcPort = new ConfigKey<>("Advanced", 
String.class, "cloudian.cmc.port", "8443",
+"The port of the Cloudian Management Console.", true);
+
+ConfigKey CloudianCmcProtocol = new ConfigKey<>("Advanced", 
String.class, "cloudian.cmc.protocol", "https",
+"The protocol of the Cloudian Management Console.", true);
+
+ConfigKey CloudianSsoKey = new ConfigKey<>("Advanced", 
String.class, "cloudian.sso.key", "ss0sh5r3dk3y",
+"The shared single sign-on key as configured in Cloudian CMC.", 
true);
+
+/**
+ * Returns the base Cloudian Management Console URL
+ * @return returns the url string
+ */
+String getCmcUrl();
+
+/**
+ * Checks if the Cloudian Connector is enabled
+ * @return returns true is connector is enabled
+ */
+boolean isEnabled();
+
+/**
+ * Generates single-sign on URL for logged in user
+ * @return returns the SSO URL string
+ */
+String generateSsoUrl();
 
 Review comment:
   Ah, I did not see if there was an interface, my bad...
 

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 #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

rhtyd commented on a change in pull request #2293: CLOUDSTACK-10047: DVSwitch 
fixes and improvements
URL: https://github.com/apache/cloudstack/pull/2293#discussion_r144258800
 
 

 ##
 File path: server/src/com/cloud/hypervisor/HypervisorGuruBase.java
 ##
 @@ -138,7 +143,24 @@ protected VirtualMachineTO 
toVirtualMachineTO(VirtualMachineProfile vmProfile) {
 if(vm.getType() == VirtualMachine.Type.NetScalerVm) {
 nicProfile.setBroadcastType(BroadcastDomainType.Native);
 }
-nics[i++] = toNicTO(nicProfile);
+NicTO nicTo = toNicTO(nicProfile);
+final NetworkVO network = 
_networkDao.findByUuid(nicTo.getNetworkUuid());
+if (network != null) {
+final Map details = 
networkOfferingDetailsDao.getNtwkOffDetails(network.getNetworkOfferingId());
+if (details != null) {
 
 Review comment:
   The block converts `NicProfile` to `NicTO`, the idea of the changes is to 
override certain security settings on dvswitch's portgroups if they already 
don't have the settings, using global settings. I'll explore further 
refactorings if I get time.
 

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 #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

rhtyd commented on a change in pull request #2293: CLOUDSTACK-10047: DVSwitch 
fixes and improvements
URL: https://github.com/apache/cloudstack/pull/2293#discussion_r144258800
 
 

 ##
 File path: server/src/com/cloud/hypervisor/HypervisorGuruBase.java
 ##
 @@ -138,7 +143,24 @@ protected VirtualMachineTO 
toVirtualMachineTO(VirtualMachineProfile vmProfile) {
 if(vm.getType() == VirtualMachine.Type.NetScalerVm) {
 nicProfile.setBroadcastType(BroadcastDomainType.Native);
 }
-nics[i++] = toNicTO(nicProfile);
+NicTO nicTo = toNicTO(nicProfile);
+final NetworkVO network = 
_networkDao.findByUuid(nicTo.getNetworkUuid());
+if (network != null) {
+final Map details = 
networkOfferingDetailsDao.getNtwkOffDetails(network.getNetworkOfferingId());
+if (details != null) {
 
 Review comment:
   The block converts `NicProfile` to `NicTO`, the idea of the changes is to 
override certain security settings on dvswitch's portgroups if they already 
don't have the settings, using global settings. 
 

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 #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

rafaelweingartner commented on a change in pull request #2293: 
CLOUDSTACK-10047: DVSwitch fixes and improvements
URL: https://github.com/apache/cloudstack/pull/2293#discussion_r144260215
 
 

 ##
 File path: utils/src/main/java/com/cloud/utils/UriUtils.java
 ##
 @@ -391,4 +391,57 @@ public static InputStream getInputStreamFromUrl(String 
url, String user, String
 return null;
 }
 }
+
+/**
+ * Expands a given vlan URI to a list of vlan IDs
+ * @param vlanAuthority the URI part without the vlan:// scheme
+ * @return returns list of vlan integer ids
+ */
+public static List expandVlanUri(final String vlanAuthority) {
+final List expandedVlans = new ArrayList<>();
+if (vlanAuthority == null || vlanAuthority.isEmpty()) {
+return expandedVlans;
+}
+for (final String vlanPart: vlanAuthority.split(",")) {
+if (vlanPart == null || vlanPart.isEmpty()) {
+continue;
+}
+final String[] range = vlanPart.split("-");
+if (range.length == 2) {
+Integer start = NumbersUtil.parseInt(range[0], -1);
+Integer end = NumbersUtil.parseInt(range[1], -1);
+if (start <= end && end > -1 && start > -1) {
+while (start <= end) {
+expandedVlans.add(start++);
+}
+}
+} else {
+final Integer value = NumbersUtil.parseInt(range[0], -1);
+if (value > -1) {
+expandedVlans.add(value);
+}
+}
+}
+return expandedVlans;
+}
+
+/**
+ * Checks if given vlan URI authorities overlap
+ * @param vlanRange1
+ * @param vlanRange2
+ * @return true if they overlap
+ */
+public static boolean checkVlanUriOverlap(final String vlanRange1, final 
String vlanRange2) {
+final List vlans1 = expandVlanUri(vlanRange1);
+final List vlans2 = expandVlanUri(vlanRange2);
+if (vlans1 == null || vlans2 == null) {
 
 Review comment:
   I understand that, but I think we should not over code or engineer. If we 
worry about the method `expandVlanUri` returning null, we can do something else 
to catch this.
   
   What about a test case for `expandVlanUri ` that fails if it returns null? 
Then, we can remove this check. On thing is to be defensive when a `null` case 
can happen, the other is to code expecting someone to make a mistake in the 
future (for that it is better to write unit test cases).
 

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 #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

rhtyd commented on a change in pull request #2293: CLOUDSTACK-10047: DVSwitch 
fixes and improvements
URL: https://github.com/apache/cloudstack/pull/2293#discussion_r144261072
 
 

 ##
 File path: utils/src/test/java/com/cloud/utils/UriUtilsTest.java
 ##
 @@ -57,4 +60,45 @@ public void getUpdateUri() {
 //XXX: Interesting cases not covered:
 // * port is ignored and left out from the return value
 }
+
+@Test
+public void expandVlanEmpty() {
+List vlans = UriUtils.expandVlanUri("");
+Assert.assertTrue(vlans.size() == 0);
+}
+
+@Test
+public void expandVlanSingleValue() {
+List vlans = UriUtils.expandVlanUri("10");
+Assert.assertTrue(vlans.size() == 1);
+Assert.assertEquals(vlans, Collections.singletonList(10));
+}
+
+@Test
+public void expandVlanValidRange() {
+List vlans = UriUtils.expandVlanUri("10-12,14,17,40-43");
+Assert.assertEquals(vlans, Arrays.asList(10,11,12,14,17,40,41,42,43));
+}
+
+@Test
+public void expandVlanInvalidRange() {
+List vlans = UriUtils.expandVlanUri("10-,12-14,-4,5-2");
+Assert.assertEquals(vlans, Arrays.asList(10,12,13,14));
+}
+
+@Test
+public void testVlanUriOverlap() {
+Assert.assertTrue(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
"10"));
+Assert.assertTrue(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
"32,33-44,30-31"));
+Assert.assertTrue(UriUtils.checkVlanUriOverlap("10-30", "25-35"));
+}
+
+@Test
+public void testVlanUriNoOverlap() {
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
null));
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
""));
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
"32"));
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("10,22,111", "12"));
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("100-200", 
"30-40,50,201-250"));
 
 Review comment:
   @rafaelweingartner unit tests for respective methods in questions are in 
this file, please see all the above lines.
 

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 #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

rhtyd commented on a change in pull request #2293: CLOUDSTACK-10047: DVSwitch 
fixes and improvements
URL: https://github.com/apache/cloudstack/pull/2293#discussion_r144261109
 
 

 ##
 File path: utils/src/main/java/com/cloud/utils/UriUtils.java
 ##
 @@ -391,4 +391,57 @@ public static InputStream getInputStreamFromUrl(String 
url, String user, String
 return null;
 }
 }
+
+/**
+ * Expands a given vlan URI to a list of vlan IDs
+ * @param vlanAuthority the URI part without the vlan:// scheme
+ * @return returns list of vlan integer ids
+ */
+public static List expandVlanUri(final String vlanAuthority) {
+final List expandedVlans = new ArrayList<>();
+if (vlanAuthority == null || vlanAuthority.isEmpty()) {
+return expandedVlans;
+}
+for (final String vlanPart: vlanAuthority.split(",")) {
+if (vlanPart == null || vlanPart.isEmpty()) {
+continue;
+}
+final String[] range = vlanPart.split("-");
+if (range.length == 2) {
+Integer start = NumbersUtil.parseInt(range[0], -1);
+Integer end = NumbersUtil.parseInt(range[1], -1);
+if (start <= end && end > -1 && start > -1) {
+while (start <= end) {
+expandedVlans.add(start++);
+}
+}
+} else {
+final Integer value = NumbersUtil.parseInt(range[0], -1);
+if (value > -1) {
+expandedVlans.add(value);
+}
+}
+}
+return expandedVlans;
+}
+
+/**
+ * Checks if given vlan URI authorities overlap
+ * @param vlanRange1
+ * @param vlanRange2
+ * @return true if they overlap
+ */
+public static boolean checkVlanUriOverlap(final String vlanRange1, final 
String vlanRange2) {
+final List vlans1 = expandVlanUri(vlanRange1);
+final List vlans2 = expandVlanUri(vlanRange2);
+if (vlans1 == null || vlans2 == null) {
 
 Review comment:
   I think we're over-discussing, not over-engineering :) I'll ping you on 
respective unit 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] rafaelweingartner commented on a change in pull request #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

rafaelweingartner commented on a change in pull request #2293: 
CLOUDSTACK-10047: DVSwitch fixes and improvements
URL: https://github.com/apache/cloudstack/pull/2293#discussion_r144262446
 
 

 ##
 File path: utils/src/test/java/com/cloud/utils/UriUtilsTest.java
 ##
 @@ -57,4 +60,45 @@ public void getUpdateUri() {
 //XXX: Interesting cases not covered:
 // * port is ignored and left out from the return value
 }
+
+@Test
+public void expandVlanEmpty() {
+List vlans = UriUtils.expandVlanUri("");
+Assert.assertTrue(vlans.size() == 0);
+}
+
+@Test
+public void expandVlanSingleValue() {
+List vlans = UriUtils.expandVlanUri("10");
+Assert.assertTrue(vlans.size() == 1);
+Assert.assertEquals(vlans, Collections.singletonList(10));
+}
+
+@Test
+public void expandVlanValidRange() {
+List vlans = UriUtils.expandVlanUri("10-12,14,17,40-43");
+Assert.assertEquals(vlans, Arrays.asList(10,11,12,14,17,40,41,42,43));
+}
+
+@Test
+public void expandVlanInvalidRange() {
+List vlans = UriUtils.expandVlanUri("10-,12-14,-4,5-2");
+Assert.assertEquals(vlans, Arrays.asList(10,12,13,14));
+}
+
+@Test
+public void testVlanUriOverlap() {
+Assert.assertTrue(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
"10"));
+Assert.assertTrue(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
"32,33-44,30-31"));
+Assert.assertTrue(UriUtils.checkVlanUriOverlap("10-30", "25-35"));
+}
+
+@Test
+public void testVlanUriNoOverlap() {
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
null));
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
""));
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
"32"));
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("10,22,111", "12"));
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("100-200", 
"30-40,50,201-250"));
 
 Review comment:
   I had already seen these tests, I did not say anything because in my option 
they are properly written ;)
   
   So, about that `null` case, in my option it is already covered, if for some 
reason someone alters the method to return null, your test cases will catch it 
(this is great!). That is why I was saying you do not need those null checks.
 

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 #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

rafaelweingartner commented on a change in pull request #2293: 
CLOUDSTACK-10047: DVSwitch fixes and improvements
URL: https://github.com/apache/cloudstack/pull/2293#discussion_r144263495
 
 

 ##
 File path: utils/src/test/java/com/cloud/utils/UriUtilsTest.java
 ##
 @@ -57,4 +60,45 @@ public void getUpdateUri() {
 //XXX: Interesting cases not covered:
 // * port is ignored and left out from the return value
 }
+
+@Test
+public void expandVlanEmpty() {
+List vlans = UriUtils.expandVlanUri("");
+Assert.assertTrue(vlans.size() == 0);
+}
+
+@Test
+public void expandVlanSingleValue() {
+List vlans = UriUtils.expandVlanUri("10");
+Assert.assertTrue(vlans.size() == 1);
+Assert.assertEquals(vlans, Collections.singletonList(10));
+}
+
+@Test
+public void expandVlanValidRange() {
+List vlans = UriUtils.expandVlanUri("10-12,14,17,40-43");
+Assert.assertEquals(vlans, Arrays.asList(10,11,12,14,17,40,41,42,43));
+}
+
+@Test
+public void expandVlanInvalidRange() {
+List vlans = UriUtils.expandVlanUri("10-,12-14,-4,5-2");
+Assert.assertEquals(vlans, Arrays.asList(10,12,13,14));
+}
+
+@Test
+public void testVlanUriOverlap() {
+Assert.assertTrue(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
"10"));
+Assert.assertTrue(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
"32,33-44,30-31"));
+Assert.assertTrue(UriUtils.checkVlanUriOverlap("10-30", "25-35"));
+}
+
+@Test
+public void testVlanUriNoOverlap() {
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
null));
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
""));
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
"32"));
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("10,22,111", "12"));
+Assert.assertFalse(UriUtils.checkVlanUriOverlap("100-200", 
"30-40,50,201-250"));
 
 Review comment:
   Ahaha, sometimes we do, but we should not. 
   
   I totally understand when we do, I also exaggerate sometimes, that is why I 
find it is great a review process to get another set of eyes to look at the 
problem and the code.
 

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 #2250: CLOUDSTACK-10057: listNetworkOfferings now returns the correct number of offerings.

[git]

blueorangutan commented on issue #2250: CLOUDSTACK-10057: listNetworkOfferings 
now returns the correct number  of offerings.
URL: https://github.com/apache/cloudstack/pull/2250#issuecomment-336113848
 
 
   Packaging result: ?centos6 ?centos7 ?debian. JID-1154
 

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 #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM and XENSERVER

[git]

blueorangutan commented on issue #2204: [CLOUDSTACK-10025] Adding Support for 
NoVNC Console for KVM and XENSERVER
URL: https://github.com/apache/cloudstack/pull/2204#issuecomment-336121981
 
 
   @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 #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM and XENSERVER

[git]

rhtyd commented on issue #2204: [CLOUDSTACK-10025] Adding Support for NoVNC 
Console for KVM and XENSERVER
URL: https://github.com/apache/cloudstack/pull/2204#issuecomment-336121806
 
 
   @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

[cloudstack] branch master updated: CLOUDSTACK-10060: ListUsage API always displays the Virtual size as '0' for Usage type=9 (snapshot) (#2257)

[bhaisaab]

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

bhaisaab 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 189b0e4  CLOUDSTACK-10060: ListUsage API always displays the Virtual 
size as '0' for Usage type=9 (snapshot) (#2257)
189b0e4 is described below

commit 189b0e4487c0caa0204b1dbb3cb5882ad2e7726e
Author: PranaliM 
AuthorDate: Thu Oct 12 18:17:18 2017 +0530

CLOUDSTACK-10060: ListUsage API always displays the Virtual size as '0' for 
Usage type=9 (snapshot) (#2257)

Bug Description:
In the listUsage API, the Virtual Size of Snapshot is always displayed as 
'0'.

Root Cause:
In case of snapshots, the usage is accounted for depending on the value of 
the global parameter, 'usage.snapshot.virtualsize.select'. If set to 'true', 
the usage calculation is done based on the Virtual Size, and if set to false, 
it is done based on the Physical size. In the Usage API, this value, (i.e. 
virtual or physical) is displayed as 'Size' field correctly, but the field 
'VirtualSize' is always displayed as 0. This is misleading.

Expected Output:
Since this is a Usage API, ideally only that size should be displayed which 
is used for billing, depending on the value of 
'usage.snapshot.virtualsize.select'.
There is another API - the ListSnapshot API that displays both, the 
physical as well as Virtual size and can be used to know both the sizes.

Fix Implemented:
Skipped showing the 'VirtualSize' Field for type=snapshot
---
 usage/src/com/cloud/usage/parser/StorageUsageParser.java | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/usage/src/com/cloud/usage/parser/StorageUsageParser.java 
b/usage/src/com/cloud/usage/parser/StorageUsageParser.java
index fccd00c..03aa97b 100644
--- a/usage/src/com/cloud/usage/parser/StorageUsageParser.java
+++ b/usage/src/com/cloud/usage/parser/StorageUsageParser.java
@@ -181,8 +181,11 @@ public class StorageUsageParser {
 usageDesc += "Snapshot ";
 break;
 }
-// Create the usage record
-usageDesc += "Id:" + storageId + " Size:" + size + "VirtualSize:" + 
virtualSize;
+//Create the usage record
+usageDesc += "Id:" + storageId + " Size:" + size;
+if (type != StorageTypes.SNAPSHOT) {
+usageDesc += " VirtualSize:" + virtualSize;
+}
 
 //ToDo: get zone id
 UsageVO usageRecord =

-- 
To stop receiving notification emails like this one, please contact
['"commits@cloudstack.apache.org" '].

[GitHub] rhtyd closed pull request #2257: CLOUDSTACK-10060:ListUsage API always displays the Virtual size as '0' for Usage type=9 (snapshot)

[git]

rhtyd closed pull request #2257: CLOUDSTACK-10060:ListUsage API always displays 
the Virtual size as '0' for Usage type=9 (snapshot)
URL: https://github.com/apache/cloudstack/pull/2257
 
 
   
 

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 #2267: CLOUDSTACK-10077: allow to have different VPN customer gateway configs for same gateway IP

[git]

rhtyd commented on issue #2267: CLOUDSTACK-10077: allow to have different VPN 
customer gateway configs for same gateway IP
URL: https://github.com/apache/cloudstack/pull/2267#issuecomment-336123383
 
 
   @resmo is that a regression of sorts, we holding this PR?
 

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 #2242: CLOUDSTACK-9958:Include tags of resources in listUsageRecords API

[git]

rhtyd commented on issue #2242: CLOUDSTACK-9958:Include tags of resources in 
listUsageRecords API
URL: https://github.com/apache/cloudstack/pull/2242#issuecomment-336125126
 
 
   @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 #2242: CLOUDSTACK-9958:Include tags of resources in listUsageRecords API

[git]

blueorangutan commented on issue #2242: CLOUDSTACK-9958:Include tags of 
resources in listUsageRecords API
URL: https://github.com/apache/cloudstack/pull/2242#issuecomment-336125290
 
 
   @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] nitin-maharana commented on issue #2294: Adding allocated IOPS to storage pool response

[git]

nitin-maharana commented on issue #2294: Adding allocated IOPS to storage pool 
response
URL: https://github.com/apache/cloudstack/pull/2294#issuecomment-336030471
 
 
   Code LGTM.
 

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 #2289: [4.11/master] Smoketest Health Check

[git]

blueorangutan commented on issue #2289: [4.11/master] Smoketest Health Check
URL: https://github.com/apache/cloudstack/pull/2289#issuecomment-336031058
 
 
   @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 #2289: [4.11/master] Smoketest Health Check

[git]

rhtyd commented on issue #2289: [4.11/master] Smoketest Health Check
URL: https://github.com/apache/cloudstack/pull/2289#issuecomment-336031031
 
 
   @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 #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM and XENSERVER

[git]

blueorangutan commented on issue #2204: [CLOUDSTACK-10025] Adding Support for 
NoVNC Console for KVM and XENSERVER
URL: https://github.com/apache/cloudstack/pull/2204#issuecomment-336133578
 
 
   Packaging result: ?centos6 ?centos7 ?debian. JID-1155
 

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 #2242: CLOUDSTACK-9958:Include tags of resources in listUsageRecords API

[git]

blueorangutan commented on issue #2242: CLOUDSTACK-9958:Include tags of 
resources in listUsageRecords API
URL: https://github.com/apache/cloudstack/pull/2242#issuecomment-336136345
 
 
   Packaging result: ?centos6 ?centos7 ?debian. JID-1156
 

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 #1558: CLOUDSTACK-10072: Remove unused code from "com.cloud.vm.UserVmManagerImpl"

[git]

blueorangutan commented on issue #1558: CLOUDSTACK-10072: Remove unused code 
from "com.cloud.vm.UserVmManagerImpl"
URL: https://github.com/apache/cloudstack/pull/1558#issuecomment-336137123
 
 
   Packaging result: ?centos6 ?centos7 ?debian. JID-1157
 

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] resmo commented on issue #2267: CLOUDSTACK-10077: allow to have different VPN customer gateway configs for same gateway IP

[git]

resmo commented on issue #2267: CLOUDSTACK-10077: allow to have different VPN 
customer gateway configs for same gateway IP
URL: https://github.com/apache/cloudstack/pull/2267#issuecomment-336138819
 
 
   not sure yet, we are investigating the root cause
 

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] syed commented on issue #2231: [CLOUDSTACK-10039] Adding IOPS/GB offering

[git]

syed commented on issue #2231: [CLOUDSTACK-10039] Adding IOPS/GB offering
URL: https://github.com/apache/cloudstack/pull/2231#issuecomment-336149555
 
 
   darn it! I'll resolve it now
 

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 issue #2291: Fix validation for parameter "vm.password.length"

[git]

nitin-maharana commented on issue #2291: Fix validation for parameter 
"vm.password.length"
URL: https://github.com/apache/cloudstack/pull/2291#issuecomment-336158765
 
 
   Thanks, @rafaelweingartner for the fix. Code LGTM. Can you please squash 
both the commits.
 

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 #2291: Fix validation for parameter "vm.password.length"

[git]

rafaelweingartner commented on issue #2291: Fix validation for parameter 
"vm.password.length"
URL: https://github.com/apache/cloudstack/pull/2291#issuecomment-336160965
 
 
   @nitin-maharana sure!
   I will do that in a sec
 

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] syed commented on issue #2231: [CLOUDSTACK-10039] Adding IOPS/GB offering

[git]

syed commented on issue #2231: [CLOUDSTACK-10039] Adding IOPS/GB offering
URL: https://github.com/apache/cloudstack/pull/2231#issuecomment-33613
 
 
   @rhtyd 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 #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

blueorangutan commented on issue #2293: CLOUDSTACK-10047: DVSwitch fixes and 
improvements
URL: https://github.com/apache/cloudstack/pull/2293#issuecomment-336200645
 
 
   Trillian test result (tid-1582)
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 26649 seconds
   Marvin logs: 
https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2293-t1582-kvm-centos7.zip
   Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Test completed. 60 look OK, 2 have error(s)
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_vpc_remote_access_vpn | `Failure` | 60.78 | test_vpc_vpn.py
   test_04_rvpc_privategw_static_routes | `Failure` | 237.96 | 
test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 117.19 | 
test_privategw_acl.py
   test_02_vpc_privategw_static_routes | `Failure` | 182.87 | 
test_privategw_acl.py
   test_01_vpc_privategw_acl | `Failure` | 56.63 | test_privategw_acl.py
   test_change_service_offering_for_vm_with_snapshots | Skipped | 0.00 | 
test_vm_snapshots.py
   test_09_copy_delete_template | Skipped | 0.01 | test_templates.py
   test_06_copy_template | Skipped | 0.00 | test_templates.py
   test_static_role_account_acls | Skipped | 0.01 | test_staticroles.py
   test_11_ss_nfs_version_on_ssvm | Skipped | 0.02 | test_ssvm.py
   test_01_scale_vm | Skipped | 0.00 | test_scale_vm.py
   test_01_primary_storage_iscsi | Skipped | 0.06 | test_primary_storage.py
   test_vm_nic_adapter_vmxnet3 | Skipped | 0.00 | test_nic_adapter_type.py
   test_nested_virtualization_vmware | Skipped | 0.00 | 
test_nested_virtualization.py
   test_06_copy_iso | Skipped | 0.00 | test_iso.py
   test_list_ha_for_host_valid | Skipped | 0.02 | test_hostha_simulator.py
   test_list_ha_for_host_invalid | Skipped | 0.02 | test_hostha_simulator.py
   test_list_ha_for_host | Skipped | 0.02 | test_hostha_simulator.py
   test_hostha_enable_feature_without_setting_provider | Skipped | 0.02 | 
test_hostha_simulator.py
   test_hostha_enable_feature_valid | Skipped | 0.02 | test_hostha_simulator.py
   test_hostha_disable_feature_valid | Skipped | 0.02 | test_hostha_simulator.py
   test_hostha_configure_invalid_provider | Skipped | 0.02 | 
test_hostha_simulator.py
   test_hostha_configure_default_driver | Skipped | 0.02 | 
test_hostha_simulator.py
   test_ha_verify_fsm_recovering | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_verify_fsm_fenced | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_verify_fsm_degraded | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_verify_fsm_available | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_multiple_mgmt_server_ownership | Skipped | 0.02 | 
test_hostha_simulator.py
   test_ha_list_providers | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_enable_feature_invalid | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_disable_feature_invalid | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_configure_enabledisable_across_clusterzones | Skipped | 0.02 | 
test_hostha_simulator.py
   test_configure_ha_provider_valid | Skipped | 0.02 | test_hostha_simulator.py
   test_configure_ha_provider_invalid | Skipped | 0.02 | 
test_hostha_simulator.py
   test_deploy_vgpu_enabled_vm | Skipped | 0.02 | test_deploy_vgpu_enabled_vm.py
   test_3d_gpu_support | Skipped | 0.03 | test_deploy_vgpu_enabled_vm.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] nvazquez opened a new pull request #2295: CLOUDSTACK-10109: Enable dedication of public IPs to SSVM and CPVM

[git]

nvazquez opened a new pull request #2295: CLOUDSTACK-10109: Enable dedication 
of public IPs to SSVM and CPVM
URL: https://github.com/apache/cloudstack/pull/2295
 
 
   JIRA Ticket: https://issues.apache.org/jira/browse/CLOUDSTACK-10109
   
   ### Description
   This feature allow admins to dedicate a range of public IP addresses to the 
SSVM and CPVM, such that they can be subject to specific external firewall 
rules. The option to dedicate a public IP range to the System VMs (SSVM & CPVM) 
is added to the `createVlanIpRange` API method and the UI.
   
   ### Solution
   Global setting 'system.vm.public.ip.reservation.mode.strictness' is added to 
determine if the use of the system VM reservation is strict (when true) or 
preferred (false), false by default.
   When a range has been dedicated to System VMs, CloudStack should apply IPs 
from that range to
   the public interfaces of the CPVM and the SSVM depending on global setting's 
value:
   - If the global setting is set to false: then CloudStack will use any unused 
and unreserved public IP
   addresses for system VMs only when the pool of reserved IPs has been 
exhausted
- If the global setting is set to true: then CloudStack will fail to deploy 
the system VM when the pool
   of reserved IPs has been exhausted, citing the lack of available IPs.
 

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] nvazquez commented on issue #2295: CLOUDSTACK-10109: Enable dedication of public IPs to SSVM and CPVM

[git]

nvazquez commented on issue #2295: CLOUDSTACK-10109: Enable dedication of 
public IPs to SSVM and CPVM
URL: https://github.com/apache/cloudstack/pull/2295#issuecomment-336210130
 
 
   @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 #2295: CLOUDSTACK-10109: Enable dedication of public IPs to SSVM and CPVM

[git]

blueorangutan commented on issue #2295: CLOUDSTACK-10109: Enable dedication of 
public IPs to SSVM and CPVM
URL: https://github.com/apache/cloudstack/pull/2295#issuecomment-336210180
 
 
   @nvazquez 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 #2295: CLOUDSTACK-10109: Enable dedication of public IPs to SSVM and CPVM

[git]

blueorangutan commented on issue #2295: CLOUDSTACK-10109: Enable dedication of 
public IPs to SSVM and CPVM
URL: https://github.com/apache/cloudstack/pull/2295#issuecomment-336217850
 
 
   Packaging result: ?centos6 ?centos7 ?debian. JID-1158
 

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] nvazquez commented on issue #2295: CLOUDSTACK-10109: Enable dedication of public IPs to SSVM and CPVM

[git]

nvazquez commented on issue #2295: CLOUDSTACK-10109: Enable dedication of 
public IPs to SSVM and CPVM
URL: https://github.com/apache/cloudstack/pull/2295#issuecomment-336218792
 
 
   @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] blueorangutan commented on issue #2295: CLOUDSTACK-10109: Enable dedication of public IPs to SSVM and CPVM

[git]

blueorangutan commented on issue #2295: CLOUDSTACK-10109: Enable dedication of 
public IPs to SSVM and CPVM
URL: https://github.com/apache/cloudstack/pull/2295#issuecomment-336219050
 
 
   @nvazquez 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] borisstoyanov commented on issue #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM and XENSERVER

[git]

borisstoyanov commented on issue #2204: [CLOUDSTACK-10025] Adding Support for 
NoVNC Console for KVM and XENSERVER
URL: https://github.com/apache/cloudstack/pull/2204#issuecomment-336221412
 
 
   @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] blueorangutan commented on issue #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM and XENSERVER

[git]

blueorangutan commented on issue #2204: [CLOUDSTACK-10025] Adding Support for 
NoVNC Console for KVM and XENSERVER
URL: https://github.com/apache/cloudstack/pull/2204#issuecomment-336221714
 
 
   @borisstoyanov 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] fmaximus commented on a change in pull request #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

fmaximus commented on a change in pull request #2293: CLOUDSTACK-10047: 
DVSwitch fixes and improvements
URL: https://github.com/apache/cloudstack/pull/2293#discussion_r144365634
 
 

 ##
 File path: server/src/com/cloud/hypervisor/HypervisorGuruBase.java
 ##
 @@ -138,7 +143,24 @@ protected VirtualMachineTO 
toVirtualMachineTO(VirtualMachineProfile vmProfile) {
 if(vm.getType() == VirtualMachine.Type.NetScalerVm) {
 nicProfile.setBroadcastType(BroadcastDomainType.Native);
 }
-nics[i++] = toNicTO(nicProfile);
+NicTO nicTo = toNicTO(nicProfile);
+final NetworkVO network = 
_networkDao.findByUuid(nicTo.getNetworkUuid());
+if (network != null) {
+final Map details = 
networkOfferingDetailsDao.getNtwkOffDetails(network.getNetworkOfferingId());
+if (details != null) {
+if 
(!details.containsKey(NetworkOffering.Detail.PromiscuousMode)) {
 
 Review comment:
   You might also use putIfAbsent here instead, which was added in Java 8
 

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] fmaximus commented on a change in pull request #2293: CLOUDSTACK-10047: DVSwitch fixes and improvements

[git]

fmaximus commented on a change in pull request #2293: CLOUDSTACK-10047: 
DVSwitch fixes and improvements
URL: https://github.com/apache/cloudstack/pull/2293#discussion_r144369183
 
 

 ##
 File path: utils/src/main/java/com/cloud/utils/UriUtils.java
 ##
 @@ -391,4 +391,57 @@ public static InputStream getInputStreamFromUrl(String 
url, String user, String
 return null;
 }
 }
+
+/**
+ * Expands a given vlan URI to a list of vlan IDs
+ * @param vlanAuthority the URI part without the vlan:// scheme
+ * @return returns list of vlan integer ids
+ */
+public static List expandVlanUri(final String vlanAuthority) {
+final List expandedVlans = new ArrayList<>();
+if (vlanAuthority == null || vlanAuthority.isEmpty()) {
+return expandedVlans;
+}
+for (final String vlanPart: vlanAuthority.split(",")) {
+if (vlanPart == null || vlanPart.isEmpty()) {
+continue;
+}
+final String[] range = vlanPart.split("-");
+if (range.length == 2) {
+Integer start = NumbersUtil.parseInt(range[0], -1);
+Integer end = NumbersUtil.parseInt(range[1], -1);
+if (start <= end && end > -1 && start > -1) {
+while (start <= end) {
+expandedVlans.add(start++);
+}
+}
+} else {
+final Integer value = NumbersUtil.parseInt(range[0], -1);
+if (value > -1) {
+expandedVlans.add(value);
+}
+}
+}
+return expandedVlans;
+}
+
+/**
+ * Checks if given vlan URI authorities overlap
+ * @param vlanRange1
+ * @param vlanRange2
+ * @return true if they overlap
+ */
+public static boolean checkVlanUriOverlap(final String vlanRange1, final 
String vlanRange2) {
+final List vlans1 = expandVlanUri(vlanRange1);
+final List vlans2 = expandVlanUri(vlanRange2);
+if (vlans1 == null || vlans2 == null) {
 
 Review comment:
   You could also use !Collections.disjoint(vlans1, vlans2)
 

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 commented on issue #2294: Adding allocated IOPS to storage pool response

[git]

mike-tutkowski commented on issue #2294: Adding allocated IOPS to storage pool 
response
URL: https://github.com/apache/cloudstack/pull/2294#issuecomment-336227035
 
 
   I guess one comment I have is that technically we are supposed to have a 
JIRA ticket for each PR. Perhaps you could open an enhancement ticket and 
reference it here, @syed - thanks!
 

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] syed commented on a change in pull request #2294: Adding allocated IOPS to storage pool response

[git]

syed commented on a change in pull request #2294: Adding allocated IOPS to 
storage pool response
URL: https://github.com/apache/cloudstack/pull/2294#discussion_r144377728
 
 

 ##
 File path: api/src/org/apache/cloudstack/api/response/StoragePoolResponse.java
 ##
 @@ -288,6 +292,10 @@ public void setCapacityIops(Long capacityIops) {
 this.capacityIops = capacityIops;
 }
 
+public void setAllocatedIops(Long allocatedIops) {
 
 Review comment:
   Hi @nitin-maharana. Thanks for the review. The getter is not needed as there 
is no one reading the vlaue.
 

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] syed commented on issue #2294: Adding allocated IOPS to storage pool response

[git]

syed commented on issue #2294: Adding allocated IOPS to storage pool response
URL: https://github.com/apache/cloudstack/pull/2294#issuecomment-336230473
 
 
   @mike-tutkowski I had opened the ticket in jira a while back. I've edited 
the PR to add the JIRA ticket
 

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 commented on issue #2294: [CLOUDSTACK-10039] Adding allocated IOPS to storage pool response

[git]

mike-tutkowski commented on issue #2294: [CLOUDSTACK-10039] Adding allocated 
IOPS to storage pool response
URL: https://github.com/apache/cloudstack/pull/2294#issuecomment-336232016
 
 
   Thanks, @syed!
 

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] nvazquez commented on issue #2295: CLOUDSTACK-10109: Enable dedication of public IPs to SSVM and CPVM

[git]

nvazquez commented on issue #2295: CLOUDSTACK-10109: Enable dedication of 
public IPs to SSVM and CPVM
URL: https://github.com/apache/cloudstack/pull/2295#issuecomment-336240972
 
 
   @borisstoyanov @rhtyd @DaanHoogland 
   
   Guys, one issue while porting this feature to 4.11 which was not present on 
4.6. In this commit: 
https://github.com/apache/cloudstack/commit/37301ed4540450c29be4f975d58b38dbeec6c296#diff-6adc687ad4a13d4b956fdc638299401b
 `domain_vlan_map` is introduced. When adding a new range on domain, a new 
entry on `domain_vlan_map` is created. As 
`IpAddressManagerImpl#fetchNewPublicIp` lists non dedicated zone wide vlans, 
but always returns range corresponding to vlan id = 1 (as there's no entry for 
it on domain_vlan_map) - check `ZoneWideNonDedicatedVlanSearch` search builder 
on `VlanDaoImpl`.
   What do you think will be the best solution for this case?
 

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 #2048: CLOUDSTACK-9880: Expansion of Management IP Range.

[git]

blueorangutan commented on issue #2048: CLOUDSTACK-9880: Expansion of 
Management IP Range.
URL: https://github.com/apache/cloudstack/pull/2048#issuecomment-336250591
 
 
   Trillian test result (tid-1584)
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 30001 seconds
   Marvin logs: 
https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2048-t1584-kvm-centos7.zip
   Intermitten failure detected: 
/marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
   Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermitten failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Test completed. 60 look OK, 2 have error(s)
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_vpc_remote_access_vpn | `Failure` | 50.72 | test_vpc_vpn.py
   test_04_rvpc_privategw_static_routes | `Failure` | 334.31 | 
test_privategw_acl.py
   test_change_service_offering_for_vm_with_snapshots | Skipped | 0.00 | 
test_vm_snapshots.py
   test_09_copy_delete_template | Skipped | 0.01 | test_templates.py
   test_06_copy_template | Skipped | 0.00 | test_templates.py
   test_static_role_account_acls | Skipped | 0.01 | test_staticroles.py
   test_11_ss_nfs_version_on_ssvm | Skipped | 0.02 | test_ssvm.py
   test_01_scale_vm | Skipped | 0.00 | test_scale_vm.py
   test_01_primary_storage_iscsi | Skipped | 0.06 | test_primary_storage.py
   test_vm_nic_adapter_vmxnet3 | Skipped | 0.00 | test_nic_adapter_type.py
   test_nested_virtualization_vmware | Skipped | 0.00 | 
test_nested_virtualization.py
   test_06_copy_iso | Skipped | 0.00 | test_iso.py
   test_list_ha_for_host_valid | Skipped | 0.02 | test_hostha_simulator.py
   test_list_ha_for_host_invalid | Skipped | 0.02 | test_hostha_simulator.py
   test_list_ha_for_host | Skipped | 0.02 | test_hostha_simulator.py
   test_hostha_enable_feature_without_setting_provider | Skipped | 0.02 | 
test_hostha_simulator.py
   test_hostha_enable_feature_valid | Skipped | 0.02 | test_hostha_simulator.py
   test_hostha_disable_feature_valid | Skipped | 0.02 | test_hostha_simulator.py
   test_hostha_configure_invalid_provider | Skipped | 0.02 | 
test_hostha_simulator.py
   test_hostha_configure_default_driver | Skipped | 0.02 | 
test_hostha_simulator.py
   test_ha_verify_fsm_recovering | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_verify_fsm_fenced | Skipped | 0.03 | test_hostha_simulator.py
   test_ha_verify_fsm_degraded | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_verify_fsm_available | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_multiple_mgmt_server_ownership | Skipped | 0.02 | 
test_hostha_simulator.py
   test_ha_list_providers | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_enable_feature_invalid | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_disable_feature_invalid | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_configure_enabledisable_across_clusterzones | Skipped | 0.02 | 
test_hostha_simulator.py
   test_configure_ha_provider_valid | Skipped | 0.02 | test_hostha_simulator.py
   test_configure_ha_provider_invalid | Skipped | 0.02 | 
test_hostha_simulator.py
   test_deploy_vgpu_enabled_vm | Skipped | 0.02 | test_deploy_vgpu_enabled_vm.py
   test_3d_gpu_support | Skipped | 0.03 | test_deploy_vgpu_enabled_vm.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 #2181: CLOUDSTACK-9957 Annotations

[git]

blueorangutan commented on issue #2181: CLOUDSTACK-9957 Annotations
URL: https://github.com/apache/cloudstack/pull/2181#issuecomment-336258357
 
 
   Trillian test result (tid-1581)
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 40655 seconds
   Marvin logs: 
https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2181-t1581-kvm-centos7.zip
   Intermitten failure detected: 
/marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
   Intermitten failure detected: /marvin/tests/smoke/test_host_annotations.py
   Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermitten failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermitten failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermitten failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Test completed. 57 look OK, 6 have error(s)
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_vpc_remote_access_vpn | `Failure` | 65.93 | test_vpc_vpn.py
   test_04_rvpc_privategw_static_routes | `Failure` | 507.95 | 
test_privategw_acl.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | 
test_snapshots.py
   ContextSuite context=TestRedundantIsolateNetworks>:setup | `Error` | 1842.94 
| test_routers_network_ops.py
   test_05_add_annotation_for_invvalid_entityType | `Error` | 0.08 | 
test_host_annotations.py
   ContextSuite context=TestDeployVirtioSCSIVM>:teardown | `Error` | 45.58 | 
test_deploy_virtio_scsi_vm.py
   test_change_service_offering_for_vm_with_snapshots | Skipped | 0.00 | 
test_vm_snapshots.py
   test_09_copy_delete_template | Skipped | 0.01 | test_templates.py
   test_06_copy_template | Skipped | 0.00 | test_templates.py
   test_static_role_account_acls | Skipped | 0.02 | test_staticroles.py
   test_11_ss_nfs_version_on_ssvm | Skipped | 0.02 | test_ssvm.py
   test_01_scale_vm | Skipped | 0.00 | test_scale_vm.py
   test_01_primary_storage_iscsi | Skipped | 0.08 | test_primary_storage.py
   test_vm_nic_adapter_vmxnet3 | Skipped | 0.00 | test_nic_adapter_type.py
   test_nested_virtualization_vmware | Skipped | 0.00 | 
test_nested_virtualization.py
   test_06_copy_iso | Skipped | 0.00 | test_iso.py
   test_list_ha_for_host_valid | Skipped | 0.02 | test_hostha_simulator.py
   test_list_ha_for_host_invalid | Skipped | 0.02 | test_hostha_simulator.py
   test_list_ha_for_host | Skipped | 0.02 | test_hostha_simulator.py
   test_hostha_enable_feature_without_setting_provider | Skipped | 0.02 | 
test_hostha_simulator.py
   test_hostha_enable_feature_valid | Skipped | 0.02 | test_hostha_simulator.py
   test_hostha_disable_feature_valid | Skipped | 0.02 | test_hostha_simulator.py
   test_hostha_configure_invalid_provider | Skipped | 0.03 | 
test_hostha_simulator.py
   test_hostha_configure_default_driver | Skipped | 0.02 | 
test_hostha_simulator.py
   test_ha_verify_fsm_recovering | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_verify_fsm_fenced | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_verify_fsm_degraded | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_verify_fsm_available | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_multiple_mgmt_server_ownership | Skipped | 0.02 | 
test_hostha_simulator.py
   test_ha_list_providers | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_enable_feature_invalid | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_disable_feature_invalid | Skipped | 0.03 | test_hostha_simulator.py
   test_ha_configure_enabledisable_across_clusterzones | Skipped | 0.02 | 
test_hostha_simulator.py
   test_configure_ha_provider_valid | Skipped | 0.02 | test_hostha_simulator.py
   test_configure_ha_provider_invalid | Skipped | 0.02 | 
test_hostha_simulator.py
   test_deploy_vgpu_enabled_vm | Skipped | 0.03 | test_deploy_vgpu_enabled_vm.py
   test_3d_gpu_support | Skipped | 0.03 | test_deploy_vgpu_enabled_vm.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