Re: Introduction

[ilya]

Welcome Rashmi!

On 4/7/16 9:58 PM, Rashmi Dixit wrote:
> Hello!
> 
> I am Rashmi Dixit and have recently joined the CloudPlatform team in 
> Accelerite. I have worked on a hybrid cloud management solution supporting 
> hypervisors such as KVM, Xen, VMware, HyperV and public clouds such as EC2. 
> My areas of interest are User Interface, networking.
> 
> I am really looking forward to contributing on CloudStack.
> 
> See you around!
> Rashmi
> 
> Rashmi Dixit
> Principal Product Engineer | CloudPlatform | www.accelerite.com
> 
> 
> 
> 
> DISCLAIMER
> ==
> This e-mail may contain privileged and confidential information which is the 
> property of Accelerite, a Persistent Systems business. It is intended only 
> for the use of the individual or entity to which it is addressed. If you are 
> not the intended recipient, you are not authorized to read, retain, copy, 
> print, distribute or use this message. If you have received this 
> communication in error, please notify the sender and delete all copies of 
> this message. Accelerite, a Persistent Systems business does not accept any 
> liability for virus infected mails.
> 

[GitHub] cloudstack pull request: CLOUDSTACK-9323: Fix cancel host maintena...

[abhinandanprateek]

Github user abhinandanprateek commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1454#discussion_r59155288
  
--- Diff: test/integration/component/test_host_maintenance.py ---
@@ -0,0 +1,325 @@
+# 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.
+""" BVT tests for Hosts Maintenance
+"""
+
+# Import Local Modules
+from marvin.codes import FAILED
+from marvin.cloudstackTestCase import *
+from marvin.cloudstackAPI import *
+from marvin.lib.utils import *
+from marvin.lib.base import *
+from marvin.lib.common import *
+from nose.plugins.attrib import attr
+
+from time import sleep
+
+_multiprocess_shared_ = False
+
+
+class TestHostMaintenance(cloudstackTestCase):
+
+def setUp(self):
+self.logger = logging.getLogger('TestHM')
+self.stream_handler = logging.StreamHandler()
+self.logger.setLevel(logging.DEBUG)
+self.logger.addHandler(self.stream_handler)
+self.apiclient = self.testClient.getApiClient()
+self.hypervisor = self.testClient.getHypervisorInfo()
+self.dbclient = self.testClient.getDbConnection()
+self.services = self.testClient.getParsedTestDataConfig()
+self.zone = get_zone(self.apiclient, 
self.testClient.getZoneForTests())
+self.pod = get_pod(self.apiclient, self.zone.id)
+self.cleanup = []
+self.services = {
+"service_offering": {
+"name": "Ultra Tiny Instance",
+"displaytext": "Ultra Tiny Instance",
+"cpunumber": 1,
+"cpuspeed": 100,
+"memory": 128,
+},
+"vm": {
+"username": "root",
+"password": "password",
+"ssh_port": 22,
+# Hypervisor type should be same as
+# hypervisor type of cluster
+"privateport": 22,
+"publicport": 22,
+"protocol": 'TCP',
+},
+"natrule": {
+"privateport": 22,
+"publicport": 22,
+"startport": 22,
+"endport": 22,
+"protocol": "TCP",
+"cidrlist": '0.0.0.0/0',
+},
+ "ostype": 'CentOS 5.3 (64-bit)',
+ "sleep": 60,
+ "timeout": 10,
+ }
+
+
+def tearDown(self):
+try:
+# Clean up, terminate the created templates
+cleanup_resources(self.apiclient, self.cleanup)
+
+except Exception as e:
+raise Exception("Warning: Exception during cleanup : %s" % e)
+
+return
+
+def createVMs(self, hostId, number):
+
+self.template = get_template(
+self.apiclient,
+self.zone.id,
+self.services["ostype"]
+)
+
+if self.template == FAILED:
+assert False, "get_template() failed to return template with 
description %s" % self.services["ostype"]
+
+self.logger.debug("Using template %s " % self.template.id)
+
+self.service_offering = ServiceOffering.create(
+self.apiclient,
+self.services["service_offering"]
+)
+self.logger.debug("Using service offering %s " % 
self.service_offering.id)
+
 

[GitHub] cloudstack pull request: CLOUDSTACK-6928: fix issue disk I/O throt...

[ustcweizhou]

Github user ustcweizhou commented on the pull request:

https://github.com/apache/cloudstack/pull/1410#issuecomment-208159074
  
@alexandrelimassantana volumeTO has setting of volume I/O throttling. it is 
different with volTO.
please have a look at storageMgr.setVolumeObjectTOThrottling.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9323: Fix cancel host maintena...

[abhinandanprateek]

Github user abhinandanprateek commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1454#discussion_r59152218
  
--- Diff: tools/marvin/marvin/lib/utils.py ---
@@ -520,4 +520,22 @@ def verifyRouterState(apiclient, routerid, 
allowedstates):
 if routers[0].state.lower() not in allowedstates:
 return [FAIL, "state of the router should be in %s but is %s" %
 (allowedstates, routers[0].state)]
-return [PASS, None]
\ No newline at end of file
+return [PASS, None]
+
+
+
+def wait_until(retry_interval=2, no_of_times=2, callback=None, 
*callback_args):
+""" Utility method to try out the callback method at most no_of_times 
with a interval of retry_interval,
+Will return immediately if callback returns True. The callback method 
should be written to return a list of values first being a boolean """
+
+if callback is None:
+return INVALID_INPUT
+success = False
+for i in range(0,no_of_times):
+time.sleep(retry_interval)
+success = callback(*callback_args)
--- End diff --

Although a list of return values is expected first being boolean value for 
success. But yes this will make it even more readable.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9323: Fix cancel host maintena...

[abhinandanprateek]

Github user abhinandanprateek commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1454#discussion_r59152198
  
--- Diff: tools/marvin/marvin/lib/utils.py ---
@@ -520,4 +520,22 @@ def verifyRouterState(apiclient, routerid, 
allowedstates):
 if routers[0].state.lower() not in allowedstates:
 return [FAIL, "state of the router should be in %s but is %s" %
 (allowedstates, routers[0].state)]
-return [PASS, None]
\ No newline at end of file
+return [PASS, None]
+
+
+
+def wait_until(retry_interval=2, no_of_times=2, callback=None, 
*callback_args):
+""" Utility method to try out the callback method at most no_of_times 
with a interval of retry_interval,
+Will return immediately if callback returns True. The callback method 
should be written to return a list of values first being a boolean """
+
+if callback is None:
+return INVALID_INPUT
--- End diff --

Due to following two reason I preferred returning an error code instead of 
an exception.
1. Returning a pre-defined error code makes it  usage more flexible as 
raising an exception or continue will be defined by the user of the method.
2. "INVALID_INPUT" is a Marvin error code used by other utility methods to 
signal bad inputs and this maintains that pattern.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r59151301
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/vpc/VPCOSPFConfigCmd.java ---
@@ -0,0 +1,73 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.cloudstack.api.command.admin.vpc;
+
+import java.util.Map;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.VPCOSPFConfigResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.log4j.Logger;
+
+import com.cloud.network.vpc.VpcProvisioningService;
+import com.cloud.user.Account;
+
+@APICommand(name = "vpcOSPFConfig", description = "Return zone level ospf 
configuration", responseObject = VPCOSPFConfigResponse.class, since = "4.9.0", 
requestHasSensitiveInfo = true, responseHasSensitiveInfo = false)
+public class VPCOSPFConfigCmd extends BaseCmd {
+public static final Logger s_logger = 
Logger.getLogger(VPCOSPFConfigCmd.class);
+private static final String s_name = "vpcospfconfigresponse";
--- End diff --

This constant is only used in the ``getCommandName`` method.  Why not 
in-line and simplify the code?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r59151239
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/vpc/VPCOSPFConfigCmd.java ---
@@ -0,0 +1,73 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.cloudstack.api.command.admin.vpc;
+
+import java.util.Map;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.VPCOSPFConfigResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.log4j.Logger;
+
+import com.cloud.network.vpc.VpcProvisioningService;
+import com.cloud.user.Account;
+
+@APICommand(name = "vpcOSPFConfig", description = "Return zone level ospf 
configuration", responseObject = VPCOSPFConfigResponse.class, since = "4.9.0", 
requestHasSensitiveInfo = true, responseHasSensitiveInfo = false)
+public class VPCOSPFConfigCmd extends BaseCmd {
+public static final Logger s_logger = 
Logger.getLogger(VPCOSPFConfigCmd.class);
+private static final String s_name = "vpcospfconfigresponse";
+
+/
+ API parameters /
+/
+
+@Parameter(name = ApiConstants.ZONE_ID, type = CommandType.UUID, 
entityType = ZoneResponse.class, required = true, description = "the ID of the 
Zone")
+private Long zoneid;
+
+@Inject
+public VpcProvisioningService _vpcProvSvc;
+
+public Long getId() {
+return zoneid;
+}
+
+@Override
+public void execute() {
--- End diff --

Add a check that ``zoneid`` is not null and greater than zero.  Also, add a 
unit test case to verify the behavior of the ``execute`` method.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r59151152
  
--- Diff: api/src/com/cloud/network/vpc/OSPFZoneConfig.java ---
@@ -0,0 +1,332 @@
+// 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 com.cloud.network.vpc;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.commons.lang.ArrayUtils;
+import org.apache.commons.lang.StringUtils;
+
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.utils.net.NetUtils;
+
+public class OSPFZoneConfig {
+
+private long zoneId;
+private Protocol protocol;
+private String ospfArea;
+private Short helloInterval;
+private Short deadInterval;
+private Short retransmitInterval;
+private Short transitDelay;
+private Authentication authentication;
+private String password;
+private String[] superCIDRList;
+private Boolean enabled;
+
+public static final String s_protocol = "protocol";
+public static final String s_area = "area";
+public static final String s_helloInterval = "hellointerval";
+public static final String s_deadInterval = "deadinterval";
+public static final String s_retransmitInterval = "retransmitinterval";
+public static final String s_transitDelay = "transitdelay";
+public static final String s_authentication = "authentication";
+public static final String s_superCIDR = "supercidr";
+public static final String s_password = "password";
+public static final String s_enabled = "enabled";
+
+public Protocol getProtocol() {
+return protocol;
+}
+
+public void setProtocol(Protocol protocol) {
+this.protocol = protocol;
+}
+
+public String getOspfArea() {
+return ospfArea;
+}
+
+public void setOspfArea(String ospfArea) {
+this.ospfArea = ospfArea;
+}
+
+public Short getHelloInterval() {
+return helloInterval;
+}
+
+public void setHelloInterval(Short helloInterval) {
+this.helloInterval = helloInterval;
+}
+
+public Short getDeadInterval() {
+return deadInterval;
+}
+
+public void setDeadInterval(Short deadInterval) {
+this.deadInterval = deadInterval;
+}
+
+public Short getRetransmitInterval() {
+return retransmitInterval;
+}
+
+public void setRetransmitInterval(Short retransmitInterval) {
+this.retransmitInterval = retransmitInterval;
+}
+
+public Short getTransitDelay() {
+return transitDelay;
+}
+
+public void setTransitDelay(Short transitDelay) {
+this.transitDelay = transitDelay;
+}
+
+public Authentication getAuthentication() {
+return authentication;
+}
+
+public void setAuthentication(Authentication authentication) {
+this.authentication = authentication;
+}
+
+public String getPassword() {
+return password;
+}
+
+public void setPassword(String password) {
+this.password = password;
+}
+
+public String[] getSuperCIDR() {
+return superCIDRList;
+}
+
+public void setSuperCIDR(String[] superCIDR) {
+this.superCIDRList = superCIDR;
+}
+
+public Boolean getEnabled() {
+return enabled;
+}
+
+public void setEnabled(Boolean enabled) {
+this.enabled = enabled;
+}
+
+public enum Params {
+PROTOCOL, AREA, HELLO_INTERVAL, DEAD_INTERVAL, 
RETRANSMIT_INTERVAL, TRANSIT_DELAY, AUTHENTICATION, SUPER_CIDR, PASSWORD, 
ENABLED
+}
+
+public enum Protocol {
+OSPF, BGP
+}
+

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r59150902
  
--- Diff: api/src/com/cloud/network/vpc/OSPFZoneConfig.java ---
@@ -0,0 +1,332 @@
+// 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 com.cloud.network.vpc;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.commons.lang.ArrayUtils;
+import org.apache.commons.lang.StringUtils;
+
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.utils.net.NetUtils;
+
+public class OSPFZoneConfig {
+
+private long zoneId;
+private Protocol protocol;
+private String ospfArea;
+private Short helloInterval;
+private Short deadInterval;
+private Short retransmitInterval;
+private Short transitDelay;
+private Authentication authentication;
+private String password;
+private String[] superCIDRList;
+private Boolean enabled;
+
+public static final String s_protocol = "protocol";
+public static final String s_area = "area";
+public static final String s_helloInterval = "hellointerval";
+public static final String s_deadInterval = "deadinterval";
+public static final String s_retransmitInterval = "retransmitinterval";
+public static final String s_transitDelay = "transitdelay";
+public static final String s_authentication = "authentication";
+public static final String s_superCIDR = "supercidr";
+public static final String s_password = "password";
+public static final String s_enabled = "enabled";
+
+public Protocol getProtocol() {
+return protocol;
+}
+
+public void setProtocol(Protocol protocol) {
+this.protocol = protocol;
+}
+
+public String getOspfArea() {
+return ospfArea;
+}
+
+public void setOspfArea(String ospfArea) {
+this.ospfArea = ospfArea;
+}
+
+public Short getHelloInterval() {
+return helloInterval;
+}
+
+public void setHelloInterval(Short helloInterval) {
+this.helloInterval = helloInterval;
+}
+
+public Short getDeadInterval() {
+return deadInterval;
+}
+
+public void setDeadInterval(Short deadInterval) {
+this.deadInterval = deadInterval;
+}
+
+public Short getRetransmitInterval() {
+return retransmitInterval;
+}
+
+public void setRetransmitInterval(Short retransmitInterval) {
+this.retransmitInterval = retransmitInterval;
+}
+
+public Short getTransitDelay() {
+return transitDelay;
+}
+
+public void setTransitDelay(Short transitDelay) {
+this.transitDelay = transitDelay;
+}
+
+public Authentication getAuthentication() {
+return authentication;
+}
+
+public void setAuthentication(Authentication authentication) {
+this.authentication = authentication;
+}
+
+public String getPassword() {
+return password;
+}
+
+public void setPassword(String password) {
+this.password = password;
+}
+
+public String[] getSuperCIDR() {
+return superCIDRList;
+}
+
+public void setSuperCIDR(String[] superCIDR) {
+this.superCIDRList = superCIDR;
+}
+
+public Boolean getEnabled() {
+return enabled;
+}
+
+public void setEnabled(Boolean enabled) {
+this.enabled = enabled;
+}
+
+public enum Params {
+PROTOCOL, AREA, HELLO_INTERVAL, DEAD_INTERVAL, 
RETRANSMIT_INTERVAL, TRANSIT_DELAY, AUTHENTICATION, SUPER_CIDR, PASSWORD, 
ENABLED
+}
+
+public enum Protocol {
+OSPF, BGP
+}
+

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r59150749
  
--- Diff: api/src/com/cloud/network/vpc/OSPFZoneConfig.java ---
@@ -0,0 +1,332 @@
+// 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 com.cloud.network.vpc;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.commons.lang.ArrayUtils;
+import org.apache.commons.lang.StringUtils;
+
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.utils.net.NetUtils;
+
+public class OSPFZoneConfig {
+
+private long zoneId;
+private Protocol protocol;
+private String ospfArea;
+private Short helloInterval;
+private Short deadInterval;
+private Short retransmitInterval;
+private Short transitDelay;
+private Authentication authentication;
+private String password;
+private String[] superCIDRList;
+private Boolean enabled;
+
+public static final String s_protocol = "protocol";
+public static final String s_area = "area";
+public static final String s_helloInterval = "hellointerval";
+public static final String s_deadInterval = "deadinterval";
+public static final String s_retransmitInterval = "retransmitinterval";
+public static final String s_transitDelay = "transitdelay";
+public static final String s_authentication = "authentication";
+public static final String s_superCIDR = "supercidr";
+public static final String s_password = "password";
+public static final String s_enabled = "enabled";
+
+public Protocol getProtocol() {
+return protocol;
+}
+
+public void setProtocol(Protocol protocol) {
+this.protocol = protocol;
+}
+
+public String getOspfArea() {
+return ospfArea;
+}
+
+public void setOspfArea(String ospfArea) {
+this.ospfArea = ospfArea;
+}
+
+public Short getHelloInterval() {
+return helloInterval;
+}
+
+public void setHelloInterval(Short helloInterval) {
+this.helloInterval = helloInterval;
+}
+
+public Short getDeadInterval() {
+return deadInterval;
+}
+
+public void setDeadInterval(Short deadInterval) {
+this.deadInterval = deadInterval;
+}
+
+public Short getRetransmitInterval() {
+return retransmitInterval;
+}
+
+public void setRetransmitInterval(Short retransmitInterval) {
+this.retransmitInterval = retransmitInterval;
+}
+
+public Short getTransitDelay() {
+return transitDelay;
+}
+
+public void setTransitDelay(Short transitDelay) {
+this.transitDelay = transitDelay;
+}
+
+public Authentication getAuthentication() {
+return authentication;
+}
+
+public void setAuthentication(Authentication authentication) {
+this.authentication = authentication;
+}
+
+public String getPassword() {
+return password;
+}
+
+public void setPassword(String password) {
+this.password = password;
+}
+
+public String[] getSuperCIDR() {
+return superCIDRList;
+}
+
+public void setSuperCIDR(String[] superCIDR) {
+this.superCIDRList = superCIDR;
+}
+
+public Boolean getEnabled() {
+return enabled;
+}
+
+public void setEnabled(Boolean enabled) {
+this.enabled = enabled;
+}
+
+public enum Params {
+PROTOCOL, AREA, HELLO_INTERVAL, DEAD_INTERVAL, 
RETRANSMIT_INTERVAL, TRANSIT_DELAY, AUTHENTICATION, SUPER_CIDR, PASSWORD, 
ENABLED
+}
+
+public enum Protocol {
+OSPF, BGP
+}
+

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r59150683
  
--- Diff: api/src/com/cloud/network/vpc/OSPFZoneConfig.java ---
@@ -0,0 +1,332 @@
+// 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 com.cloud.network.vpc;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.commons.lang.ArrayUtils;
+import org.apache.commons.lang.StringUtils;
+
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.utils.net.NetUtils;
+
+public class OSPFZoneConfig {
+
+private long zoneId;
+private Protocol protocol;
+private String ospfArea;
+private Short helloInterval;
+private Short deadInterval;
+private Short retransmitInterval;
+private Short transitDelay;
+private Authentication authentication;
+private String password;
+private String[] superCIDRList;
--- End diff --

Given that we are keeping an eye towards IPv6, it seems appropriate to 
create an immutable class to represent a CIDR with semantics that work for both 
IPv4 and IPv6 addresses.  This class would deprecate all CIDR related functions 
in ``NetUtils``.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r59150605
  
--- Diff: api/src/com/cloud/network/vpc/OSPFZoneConfig.java ---
@@ -0,0 +1,332 @@
+// 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 com.cloud.network.vpc;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.commons.lang.ArrayUtils;
+import org.apache.commons.lang.StringUtils;
+
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.utils.net.NetUtils;
+
+public class OSPFZoneConfig {
+
+private long zoneId;
+private Protocol protocol;
+private String ospfArea;
+private Short helloInterval;
+private Short deadInterval;
+private Short retransmitInterval;
+private Short transitDelay;
--- End diff --

@abhinandanprateek why did you chose to represent these time values as 
``Short`` rather than ``Duration``?  My issue with ``Short`` is there is no 
notion of units (e.g. seconds, milliseconds) etc.  Therefore, the conversion 
semantics are impossible to determine when just represented as a numeric value.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9340: General DB Optimization

[serg38]

Github user serg38 commented on the pull request:

https://github.com/apache/cloudstack/pull/1466#issuecomment-208121230
  
@rafaelweingartner 

 Here you change this table "ovs_tunnel_network", you dropped the primary 
key "id"; Will that remove only the index or the field too?
Also, this change on database structures should be reflected on the 
pseudo "JPA" mapping that we use right?
For instance, the class "com.cloud.network.ovs.dao.OvsTunnelNetworkVO" 
that represents the "ovs_tunnel_network" table, it is annotated as the "id" 
field being the id, and not the "ovs_tunnel_network"
>

We don't change any table structure but indexes so no changes in the 
mapping should  be required. For ovs_tunnel_network it was the only table in 
ACS DB that had primary key  not based on ID whereas ID was an unique index. In 
create-schema.sql
CREATE TABLE `cloud`.`ovs_tunnel_network`(
  `id` bigint unsigned NOT NULL UNIQUE AUTO_INCREMENT,
  `from` bigint unsigned COMMENT 'from host id',
  `to` bigint unsigned COMMENT 'to host id',
  `network_id` bigint unsigned COMMENT 'network identifier',
  `key` int unsigned COMMENT 'gre key',
  `port_name` varchar(32) COMMENT 'in port on open vswitch',
  `state` varchar(16) default 'FAILED' COMMENT 'result of tunnel 
creatation',
  PRIMARY KEY(`from`, `to`, `network_id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

What we are proposing is to swap them by making ID a primary key and 
creating a new unique index based on 3 columns `from`, `to` and `network_id`.

>
Here you remove duplicated primary keys.
You are removing indexes and not primary keys per se, right?
I see you removing two indexes, does that mean that the table has 
another one? A third one?
Because if you remove the ID field index, if we use a select by that 
field, it would cause a full table scan, right?

That's correct. We are removing indexes that are duplicate to the primary 
key. In some tables there  are 2 such indexes.

>
 did you execute some evaluation to see which fields were being most used 
to create an index for them? 
For instance, why did you create an index for the field called "type" 
of table "op_it_work"
>
Yes we did. Our prod DB has 1 mil+ VMs and volumes and 20K+ accounts. We 
started seeing degradation in simple list calls. We reviewed and analyzed all 
MySQL slow queries to see if any of them use full table scans. So far we found 
7 cases where extra index could help. Altogether these changes reduced the load 
on Mysql by 30% (it doesn't spend time any more constantly doing full table 
scans) and improved some API calls by up to 200%. 

>>
  This will remove from the projection the VMs already removed. Aren't they 
needed
>
We analyzed both the code and other DB tables. account_vmstats_view is only 
used in account_view and only in the join using these clauses
LEFT JOIN `account_vmstats_view` `runningvm` ON (((`account`.`id` = 
`runningvm`.`account_id`)
AND (`runningvm`.`state` = 'Running'
LEFT JOIN `account_vmstats_view` `stoppedvm` ON (((`account`.`id` = 
`stoppedvm`.`account_id`)  AND (`stoppedvm`.`state` = 'Stopped'
Since only Running and Stopped VMs used there is no need to bring Expunged 
one into the vmstats view. This changes alone improve listAccounts call 
retrieval from 11 sec to 3 ( x4 improvement).




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...

[mike-tutkowski]

Github user mike-tutkowski commented on the pull request:

https://github.com/apache/cloudstack/pull/1403#issuecomment-208120039
  
@jburwell Unfortunately Google's ImmutableMap doesn't support null for a 
value (which we use). I noticed this while running regression tests and googled 
to see if there is a way around it. Google suggested using an unmodifiableMap, 
but it doesn't provide the same benefits (we wouldn't be making a copying of 
the underlying data). It does allow you to provide back to callers a read-only 
implementation, though.

I could allows iterate over the passed-in list and only add to the 
ImmutableMap entries that don't have null for a value. Not super elegant, but 
it would work.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9340: General DB Optimization

[rafaelweingartner]

Github user rafaelweingartner commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1466#discussion_r59147080
  
--- Diff: setup/db/db/schema-481to490.sql ---
@@ -413,3 +413,89 @@ VIEW `user_vm_view` AS
 
 -- Add cluster.storage.operations.exclude property
 INSERT INTO `cloud`.`configuration` (`category`, `instance`, `component`, 
`name`, `description`, `default_value`, `updated`, `scope`, `is_dynamic`) 
VALUES ('Advanced', 'DEFAULT', 'CapacityManager', 
'cluster.storage.operations.exclude', 'Exclude cluster from storage 
operations', 'false', now(), 'Cluster', '1');
+
+-- Added in CLOUDSTACK-9340: General DB optimization, 4 cases:
+
+- 1) Incorrect PRIMARY key
+ALTER TABLE `cloud`.`ovs_tunnel_network` 
+DROP PRIMARY KEY,
+ADD PRIMARY KEY (`id`),
+DROP INDEX `id` ,
+ADD UNIQUE INDEX `i_to_from_network_id` (`to` ASC, `from` ASC, 
`network_id` ASC);
+
+- 2) Duplicate PRIMARY KEY
+ALTER TABLE `cloud`.`user_vm` DROP INDEX `id_2` ,DROP INDEX `id` ;
+ALTER TABLE `cloud`.`domain_router` DROP INDEX `id_2` ,DROP INDEX `id` ;
+ALTER TABLE `cloud`.`vm_instance` DROP INDEX `id_2` ,DROP INDEX `id` ;
+ALTER TABLE `cloud`.`account_vlan_map` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`account_vnet_map` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`baremetal_rct` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`cluster` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`conditions` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`counter` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`data_center` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`dc_storage_network_ip_range` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`dedicated_resources` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`host_pod_ref` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`iam_group` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`iam_policy` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`iam_policy_permission` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`image_store_details` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`instance_group` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`netapp_lun` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`netapp_pool` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`netapp_volume` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`network_acl_item_cidrs` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`network_offerings` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`nic_secondary_ips` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`nics` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`op_ha_work` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`op_host` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`op_host_transfer` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`op_networks` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`op_nwgrp_work` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`op_vm_ruleset_log` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`op_vpc_distributed_router_sequence_no` DROP INDEX 
`id` ;
+ALTER TABLE `cloud`.`pod_vlan_map` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`portable_ip_address` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`portable_ip_range` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`region` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`remote_access_vpn` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`snapshot_details` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`snapshots` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`storage_pool` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`storage_pool_details` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`storage_pool_work` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`user_ip_address` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`user_ipv6_address` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`user_statistics` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`version` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`vlan` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`vm_disk_statistics` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`vm_snapshot_details` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`vm_work_job` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`vpc_gateways` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`vpn_users` DROP INDEX `id` ;
+
+- 3) Missing indexes (Add indexes to avoid full table scans)
+ALTER TABLE `cloud`.`op_it_work` ADD INDEX `i_type_and_updated` (`type` 
ASC, `updated_at` ASC);
+ALTER TABLE `cloud`.`vm_root_disk_tags` ADD INDEX `i_vm_id` (`vm_id` ASC);
+ALTER TABLE `cloud`.`vm_compute_tags` ADD INDEX `i_vm_id` (`vm_id` ASC);
+ALTER TABLE `cloud`.`vm_network_map` ADD INDEX `i_vm_id` (`vm_id` ASC);
+ALTER TABLE `cloud`.`ssh_keypairs` ADD INDEX `i_public_key` (`public_key` 
ASC);
+ALTER TABLE `cloud`.`user_vm_details` ADD INDEX `i_name_vm_id` (`vm_id` 
ASC, `name` ASC);
+ALTER TABLE `cloud`.`instance_group` ADD INDEX `i_name` (`name` ASC);
+
+- 4) Some views query (Change view to improve account retrieval speed) 
--- End diff --

This will remove from the projection the VMs 

[GitHub] cloudstack pull request: CLOUDSTACK-9340: General DB Optimization

[rafaelweingartner]

Github user rafaelweingartner commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1466#discussion_r59146919
  
--- Diff: setup/db/db/schema-481to490.sql ---
@@ -413,3 +413,89 @@ VIEW `user_vm_view` AS
 
 -- Add cluster.storage.operations.exclude property
 INSERT INTO `cloud`.`configuration` (`category`, `instance`, `component`, 
`name`, `description`, `default_value`, `updated`, `scope`, `is_dynamic`) 
VALUES ('Advanced', 'DEFAULT', 'CapacityManager', 
'cluster.storage.operations.exclude', 'Exclude cluster from storage 
operations', 'false', now(), 'Cluster', '1');
+
+-- Added in CLOUDSTACK-9340: General DB optimization, 4 cases:
+
+- 1) Incorrect PRIMARY key
+ALTER TABLE `cloud`.`ovs_tunnel_network` 
+DROP PRIMARY KEY,
+ADD PRIMARY KEY (`id`),
+DROP INDEX `id` ,
+ADD UNIQUE INDEX `i_to_from_network_id` (`to` ASC, `from` ASC, 
`network_id` ASC);
+
+- 2) Duplicate PRIMARY KEY
+ALTER TABLE `cloud`.`user_vm` DROP INDEX `id_2` ,DROP INDEX `id` ;
+ALTER TABLE `cloud`.`domain_router` DROP INDEX `id_2` ,DROP INDEX `id` ;
+ALTER TABLE `cloud`.`vm_instance` DROP INDEX `id_2` ,DROP INDEX `id` ;
+ALTER TABLE `cloud`.`account_vlan_map` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`account_vnet_map` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`baremetal_rct` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`cluster` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`conditions` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`counter` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`data_center` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`dc_storage_network_ip_range` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`dedicated_resources` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`host_pod_ref` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`iam_group` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`iam_policy` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`iam_policy_permission` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`image_store_details` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`instance_group` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`netapp_lun` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`netapp_pool` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`netapp_volume` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`network_acl_item_cidrs` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`network_offerings` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`nic_secondary_ips` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`nics` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`op_ha_work` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`op_host` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`op_host_transfer` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`op_networks` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`op_nwgrp_work` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`op_vm_ruleset_log` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`op_vpc_distributed_router_sequence_no` DROP INDEX 
`id` ;
+ALTER TABLE `cloud`.`pod_vlan_map` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`portable_ip_address` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`portable_ip_range` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`region` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`remote_access_vpn` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`snapshot_details` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`snapshots` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`storage_pool` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`storage_pool_details` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`storage_pool_work` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`user_ip_address` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`user_ipv6_address` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`user_statistics` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`version` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`vlan` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`vm_disk_statistics` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`vm_snapshot_details` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`vm_work_job` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`vpc_gateways` DROP INDEX `id` ;
+ALTER TABLE `cloud`.`vpn_users` DROP INDEX `id` ;
+
+- 3) Missing indexes (Add indexes to avoid full table scans)
--- End diff --

did you execute some evaluation to see which fields were being most used to 
create an index for them?

For instance, why did you create an index for the field called "type" of 
table "op_it_work"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9340: General DB Optimization

[rafaelweingartner]

Github user rafaelweingartner commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1466#discussion_r59146860
  
--- Diff: setup/db/db/schema-481to490.sql ---
@@ -413,3 +413,89 @@ VIEW `user_vm_view` AS
 
 -- Add cluster.storage.operations.exclude property
 INSERT INTO `cloud`.`configuration` (`category`, `instance`, `component`, 
`name`, `description`, `default_value`, `updated`, `scope`, `is_dynamic`) 
VALUES ('Advanced', 'DEFAULT', 'CapacityManager', 
'cluster.storage.operations.exclude', 'Exclude cluster from storage 
operations', 'false', now(), 'Cluster', '1');
+
+-- Added in CLOUDSTACK-9340: General DB optimization, 4 cases:
+
+- 1) Incorrect PRIMARY key
+ALTER TABLE `cloud`.`ovs_tunnel_network` 
+DROP PRIMARY KEY,
+ADD PRIMARY KEY (`id`),
+DROP INDEX `id` ,
+ADD UNIQUE INDEX `i_to_from_network_id` (`to` ASC, `from` ASC, 
`network_id` ASC);
+
+- 2) Duplicate PRIMARY KEY
+ALTER TABLE `cloud`.`user_vm` DROP INDEX `id_2` ,DROP INDEX `id` ;
--- End diff --

Here you remove duplicated primary keys.
You are removing indexes and not primary keys per se, right?
I see you removing two indexes, does that mean that the table has another 
one? A third one?
Because if you remove the ID field index, if we use a select by that field, 
it would cause a full table scan, right?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9340: General DB Optimization

[rafaelweingartner]

Github user rafaelweingartner commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1466#discussion_r59146682
  
--- Diff: setup/db/db/schema-481to490.sql ---
@@ -413,3 +413,89 @@ VIEW `user_vm_view` AS
 
 -- Add cluster.storage.operations.exclude property
 INSERT INTO `cloud`.`configuration` (`category`, `instance`, `component`, 
`name`, `description`, `default_value`, `updated`, `scope`, `is_dynamic`) 
VALUES ('Advanced', 'DEFAULT', 'CapacityManager', 
'cluster.storage.operations.exclude', 'Exclude cluster from storage 
operations', 'false', now(), 'Cluster', '1');
+
+-- Added in CLOUDSTACK-9340: General DB optimization, 4 cases:
+
+- 1) Incorrect PRIMARY key
+ALTER TABLE `cloud`.`ovs_tunnel_network` 
--- End diff --

Here you change this table "ovs_tunnel_network", you dropped the primary 
key "id"; Will that remove only the index or the field too?

Also, this change on database structures should be reflected on the pseudo 
"JPA" mapping that we use right?

For instance, the class "com.cloud.network.ovs.dao.OvsTunnelNetworkVO" that 
represents the "ovs_tunnel_network" table, it is annotated as the "id" field 
being the id, and not the "ovs_tunnel_network"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Remove unused params from NetworkHelperIm...

[pedro-martins]

Github user pedro-martins commented on the pull request:

https://github.com/apache/cloudstack/pull/1484#issuecomment-208088632
  
@rafaelweingartner 
Done. the Jira is CLOUDSTACK-9343 if you can take a look.
Ty


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Remove unused params from NetworkHelperIm...

[pedro-martins]

Github user pedro-martins commented on the pull request:

https://github.com/apache/cloudstack/pull/1484#issuecomment-208087293
  
@rafaelweingartner 
I will open a Jira, the way that I found the problem was described in an 
answer to Daan Hoogland in the same PR.
Ty


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Remove unused params from NetworkHelperIm...

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1484#issuecomment-208086640
  
Where did you two analyzed that? How did you two find this problem?
How about opening a Jira ticket detailing the problem, so we can have a log 
of it.
Please, pick one of the PRs and close the other.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Remove unused params from NetworkHelperIm...

[pedro-martins]

Github user pedro-martins commented on the pull request:

https://github.com/apache/cloudstack/pull/1484#issuecomment-208086436
  
Hi @GabrielBrascher .
I think this params are removed in this PR 
https://github.com/apache/cloudstack/pull/1447
=)
Ty.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unnecessary code from getGuestOsT...

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1262#issuecomment-208076663
  
I agree with you.
We did not use this solution at first because of this:
```
protected CitrixResourceBase citrixResourceBase = new CitrixResourceBase() {
  @Override
protected String getPatchFilePath() {
return null;
}
};
```

Probably I was trying to be too perfectionist; I did not like that. 
Anyways, I hope we bump Mockito version soon, so I can fix that.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unnecessary code from getGuestOsT...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1262#issuecomment-208074579
  
@rafaelweingartner a lot less code (not in the diff but in the result) :+1:


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unnecessary code from getGuestOsT...

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1262#issuecomment-208071389
  
@DaanHoogland Done.
What do you think now?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unnecessary code from getGuestOsT...

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1262#issuecomment-208064343
  
Got your point.
@cristofolini is a little without time now, so I will do that.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unnecessary code from getGuestOsT...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1262#issuecomment-208064261
  
Well, I am not interested in elegant at this stage, just in reducing the 
code base if it make sense. The simplest solution is an named test resource 
class as child of CitrixResourceBase and test on that one.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unnecessary code from getGuestOsT...

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1262#issuecomment-208064117
  
Sure it does.

And yes we can create a spy from an abstract class. The problem is that 
with the current version of Mockito the spy method only works for Objects and 
not for classes; we would need to create an anonymous class. Using the mock 
method would no work.

I also thought about using an anonymous class, but that seemed not elegant 
to me. However, if you find it interesting, we could do that way (with an 
anonymous class). Then, we would not even need to work with mocks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unnecessary code from getGuestOsT...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1262#issuecomment-208063842
  
Well @rafaelweingartner you can make a test class for each of the child 
classes of the abstract class, as done now or you can make a 
CitrixResourceTest-class child of the abstract class and test with that one. 
You could even make it an anonymous class in the test. I am not sure but maybe 
you can even mock/spy an abstract class and have the testable method be called 
without any child. Does that explain?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: [DISCUSS] Request for comments : VPC Inline LoadBalancer (new plugin)

[Kris Sterckx]

Hi all


Thanks for reviewing the FS. Based on the received comments I clarified
further in the FS that the Vpc Inline LB appliance solution is based on the
Internal LB appliance solution, only now extended with secondary IP's and
static NAT to Public IP.

I also corrected the "management" nic to "control" nic. The text really
meant eth1, i.e the link-local nic on KVM.

Pls find the updated text :

https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61340894

Architecture and Design description

We will introduce a new CloudStack network plugin “VpcInlineLbVm” which is
based on the Internal LoadBalancer plugin and which just like the Internal
LB plugin is implementing load balancing based on at-run-time deployed
appliances based on the VR (Router VM) template (which defaults to the
System VM template), but the LB solution now extended with static NAT to
secondary IP's.

The VPC Inline LB appliance therefore is a regular System VM, exactly the
same as the Internal LB appliance today. Meaning it has 1 guest nic and 1
control (link-local / management) nic.

With the new proposed VpcInlineLbVm set as the Public LB provider of a VPC,
when a Public IP is acquired for this VPC and LB rules are configured on
this public IP, a VPC Inline LB appliance is deployed if not yet existing,
and an additional guest IP is allocated and set as secondary IP on the
appliance guest nic, upon which static NAT is configured from the Public IP
to the secondary guest IP.  (See below outline for the detailed algorithm.)

*In summary*, the VPC Inline LB appliance is reusing the Internal LB
appliance but its solution now extended with Static NAT from Public IP's to
secondary (load balanced) IP's at the LB appliance guest nic.


Hi Ilya,

Let me know pls whether that clarifies and brings new light to the
questions asked.

Can you pls indicate, given the suggested approach of reusing the appliance
mechanism already used for Internal LB, whether this addresses the concern
or, when it doesn't, pls further clarify the issue seen in this approach.

Thanks!


Hi Sanjeev, to your 1st question:

Will this LB appliance be placed between guest vm's and the Nuage VSP
provider(Nuage VSP and lb appliance will have one nic in guest network)?

> Please note that the LB appliance is a standard System VM, having 1 nic
in Guest network and 1 nic in Control. There is as such no relation between
this Appliance and the Nuage VSP.

In the case where Nuage VSP is the Connectivity provider, the appliance has
a guest nic in a Nuage VSP managed (VXLAN) network, like all guest VM's
would have. But that is dependent on the provider selection.

In the specific case of Nuage VSP, publicly load balanced traffic will
indeed flow as : (pls read-on to your 2nd question also) :
-> incoming traffic on Public IP  (Nuage VSP managed)
-> .. being Static NAT'ted to Secondary IP on Vpc Inline LB VM
 (NAT'ting is Nuage VSP managed)
-> .. being load balanced to real-server guest VM IP's  (Vpc Inline LB
VM appliance managed)
-> .. reaching the real-server guest VM IP

To your 2nd question:

Is there any specific reason for traffic filtering on lb appliance instead
of Nuage VSP ? If we configure firewall rules for LB services on the Nuage
VSP instead of the inline lb appliance (iptable rules  for lb traffic),
traffic can be filtered on the Nuage VSP before Natting?

> Please note that the generic Static NAT delegation is applicable : the
realization of the Static NAT rules being set up, depends on the Static NAT
provider in the VPC offering. In case Nuage VSP is the provider for Static
NAT (which it would be in the case of a Nuage SDN backed deployment), the
NAT’ting is effectively done by the Nuage VSP.  If anyone else is the
provider, than this provider is being delegated to.

Thanks!


Let me know whether this overall clarifies.

Pls don't hesitate to ask and further questions.


Best regards,


Kris Sterckx



On 25 March 2016 at 07:08, Sanjeev Neelarapu <
sanjeev.neelar...@accelerite.com> wrote:

> Hi Nick Livens,
>
> I have gone through the FS and following are my review comments:
>
> 1. Will this LB appliance be placed between guest vms and the Nuage VSP
> provider(Nuage VSP and lb appliance will have one nic in guest network)?
> 2. Is there any specific reason for traffic filtering on lb appliance
> instead of Nuage VPS ? If we configure firewall rules for LB services on
> the Nuage VSP instead of the inline lb appliance (iptable rules  for lb
> traffic), traffic can be filtered on the Nuage VSP before Natting?
>
> Best Regards,
> Sanjeev N
> Chief Product Engineer, Accelerite
> Off: +91 40 6722 9368 | EMail: sanjeev.neelar...@accelerite.com
>
>
> -Original Message-
> From: ilya [mailto:ilya.mailing.li...@gmail.com]
> Sent: Thursday, March 24, 2016 10:16 PM
> To: dev@cloudstack.apache.org
> Subject: Re: [DISCUSS] Request for comments : VPC Inline LoadBalancer (new
> plugin)
>
> Hi Nick,
>
> Being fan of SDN, I gave this proposal a 

[GitHub] cloudstack pull request: Removed unnecessary code from getGuestOsT...

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1262#issuecomment-208059482
  
@DaanHoogland, I did not understand what you meant with test-child.

We could do the test a little bit different if we had a newer version of 
Mockito using the spy method. But, with the one we are using, we did not find 
much ways to write tests for that class, given that it is an abstract class.

Could you elaborate a little bit more your idea?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix Sync of template.properties in Swift

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1331#issuecomment-208034027
  
@syed, you could delete the line 631, or use it as a java doc that explains 
the method.

I believe that there is, at least, one test you can do here. You could test 
if an exception happens while calling the “execute” method with the 
“deleteCommand” variable. If an exception happens it cannot be re-thrown by 
this method, and it has to be logged as a debug message.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Reimplement router.redundant.vrrp.interva...

[remibergsma]

GitHub user remibergsma opened a pull request:

https://github.com/apache/cloudstack/pull/1486

Reimplement router.redundant.vrrp.interval setting

Global setting `router.redundant.vrrp.interval` is not used any more and it 
is now set to a hardcoded 1. 

This results in a failover from master->backup when the backup doesn't hear 
from the master in ~3.6sec. This is a bit too tight, as we've seen failovers 
during live migrations. We could reproduce it in about half of the cases. 
Setting this to setting to 2 (tested it by hardcoding it in the systemvms) 
gives twice as much time and we didn't see issues any more. Instead of updating 
the hardcoded setting from 1 to 2, I reimplemented the global setting by 
sending it to the router with the cmd_line, as the non-VPC router also does.

Background:
Why is the maximum failover time in the example 3.6 seconds? This comes 
from the advertisement interval and the skew time. The default advertisement 
interval is 1 second (configurable in keepalived.conf). The skew time helps to 
keep everyone from trying to transition at once. It is a number between 0 and 
1, based on the formula (256 - priority) / 256

As defined in the RFC, the backup must receive an advertisement from the 
master every (3 * advert_int) + skew_time seconds. If it doesn't hear anything 
from the master, it takes over. With a backup router priority of 100 (as in the 
example), the failover will happen at most 3.6 seconds after the master goes 
down.

Source: http://www.hollenback.net/KeepalivedForNetworkReliability


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/remibergsma/cloudstack 
reimplement-vrrp-setting-47

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1486.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1486


commit c33358db848faf8c8891e00e0100a2627b177407
Author: Remi Bergsma 
Date:   2016-03-23T15:33:20Z

Have rVPCs use the router.redundant.vrrp.interval setting

It defaults to 1, which is hardcoded in the template:

./cosmic/cosmic-core/systemvm/patches/debian/config/opt/cloud/templates/keepalived.conf.templ

As non-VPC redundant routers use this setting, I think it makes sense to 
use it for rVPCs as well.

We also need a change to pickup the cmd_line parameter and use it in the 
Python code that configures the router.

commit 408478413ad0469265dfa0ce9101d6337f558ab2
Author: Remi Bergsma 
Date:   2016-03-23T15:56:54Z

Configure rVPC for router.redundant.vrrp.interval advert_int setting




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Set default networkDomain to empty instea...

[remibergsma]

GitHub user remibergsma opened a pull request:

https://github.com/apache/cloudstack/pull/1485

Set default networkDomain to empty instead of username

The 10th field of `createUserAccount` is `networkDomain` (See 
`AccountService.java`) and it is set to a var named `admin`, which is the user 
name.
So, the first user that is created in a domain that links to LDAP, creates 
the account within the domain, and sets the `networkDomain` field to the 
username. All next users are created in the same account.

Then we have the situation that in domain SBP we have a user `rbergsma` 
that logs in first, gets an account created and then (unless you override) all 
VMs started in the SBP domain will have network domain `rbergsma`. That is 
highly confusing and not what is should be.

The `linkDomainToLdap` api call has no `networkDomain` field, so I propose 
to make this field empty (set it to null). It's a sting and null / empty is 
allowed.

One can also specify the networkDomain when creating a VPC and also there 
it is allowed to be null.

When te networkDomain is needed (and is not set in the domain and not in 
the VPC) it is constructed by using `guest.domain.suffix` so there always is a 
networkDomain to be used.

It makes more sense to manually set it on a domain level, or specify it on 
the VPC and in the final case end up with something that is clearly generated 
(like cs342cloud.local) rather than the username of someone else.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/remibergsma/cloudstack fix-ldap-default-domain

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1485.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1485


commit 9e1859ee2bbe82ad742c30cd9ca9aa7393d34f36
Author: Remi Bergsma 
Date:   2016-04-10T17:50:32Z

Set default networkDomain to empty instead of username

The 10th field of createUserAccount is 'networkDomain' 
(AccountService.java) and it is set to a var named 'admin', which is the user 
name.
So, the first user that is created in a domain that links to LDAP, creates 
the account within the domain, and sets the 'networkDomain' field to the 
username. All next users are created in the same account.

Then we have the situation that in domain SBP we have a user 'rbergsma' 
that logs in first, gets an account created and then (unless you override) all 
VMs started in the SBP domain will have network domain 'rbergsma'. That is 
highly confusing and not what is should be.

linkDomainToLdap api call has no 'networkDomain' field, so I propose to 
make this field empty (set it to null). It's a sting and null / empty is 
allowed.

One can also specify the networkDomain when creating a VPC and also there 
it is allowed to be null.

When te networkDomain is needed (and is not set in the domain and not in 
the VPC) it is constructed by using guest.domain.suffix so there always is a 
netWork domain to be used.

It makes more sense to manually set it on a domain level, or specify it on 
the VPC and in the final case end up with something that is clearly generated 
(like cs342cloud.local) rather than the username of someone else.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9340: General DB Optimization

[nvazquez]

Github user nvazquez commented on the pull request:

https://github.com/apache/cloudstack/pull/1466#issuecomment-207994438
  
@bhaisaab Most of the indexes being dropped are duplicate. In 
create-schema.sql quite a lot of tables are created like this:
CREATE TABLE `cloud`.`version` (
`id` bigint unsigned NOT NULL UNIQUE AUTO_INCREMENT COMMENT 'id',
`version` char(40) NOT NULL UNIQUE COMMENT 'version',
`updated` datetime NOT NULL COMMENT 'Date this version table was updated',
`step` char(32) NOT NULL COMMENT 'Step in the upgrade to this version',
PRIMARY KEY (`id`),
INDEX `i_version__version`(`version`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

When column ID is marked as UNIQUE MySQL creates an unique index ID. Then 
below a primary key ID is defined. Primary key in its nature already guarantees 
uniqueness so it is safe to drop index ID leaving only primary key to remain.
There are more than 50 tables like this plus 3 more tables where unique 
index based on ID column was created twice. For example for domain_router table 
one duplicate index was created initially in create-schema.sql and another 
duplicate was generated in schema-442to450.sql when the following line was 
executed:
ALTER TABLE `cloud`.`domain_router` MODIFY `id` bigint unsigned 
AUTO_INCREMENT UNIQUE NOT NULL;


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GabrielBrascher]

Github user GabrielBrascher commented on the pull request:

https://github.com/apache/cloudstack/pull/1263#issuecomment-207989355
  
@DaanHoogland thanks for the tip.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1483#issuecomment-207988893
  
@alexandrelimassantana please make a pull request to the branch that the PR 
is created from, Remi can include you change in this one by pulling it to his 
branch.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unnecessary code from getGuestOsT...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1262#issuecomment-207987383
  
@rafaelweingartner how about creating a test-child instead of testing in 
all child-tests ;)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GabrielBrascher]

GitHub user GabrielBrascher reopened a pull request:

https://github.com/apache/cloudstack/pull/1263

Removed unused code from com.cloud.api.ApiServer

**Removed “\_” from variables names**: private variables with “\_” 
at the beginning is common in C++ but not in Java.
 
**Removed unused code from ApiServer:**
- com.cloud.api.ApiServer.getPluggableServices(): unused method;
- com.cloud.api.ApiServer.getApiAccessCheckers(): unused method;

**Methods and variables access level reviewed:**
- com.cloud.api.ApiServer.handleAsyncJobPublishEvent(String, String 
,Object): this method was private but the annotation @MessageHandler requests 
public methods, as can be seen in 
org.apache.cloudstack.framework.messagebus.MessageDispatcher.buildHandlerMethodCache(Class\),
 which searches methods with the @MessageHandler annotation and changes
it to be accessible (“setAccessible(true)”). Thus, there is no reason 
for handleAsyncJobPublishEvent be a private method and lead some other dev to 
wrong conclusions about the use of the method; 
- Global variables and methods called just by this class (ApiServer) were 
changed to private.
 
**Changed variables and methods from static to non-static (if possible):** 
as some variables/methods are used just by one object of this class, 
instantiated by Spring, they were changed to non-static.
 
With that, calls from com.cloud.api.ApiServlet.ApiServlet() that used 
static methods from ApiServer, were changed from ApiServer.\ 
to \_apiServer.\ that refers to the 
org.apache.cloudstack.api.ApiServerService interface. Thus, methods 
com.cloud.api.ApiServer.getJSONContentType() and 
com.cloud.api.ApiServer.isSecureSessionCookieEnabled() had to be added in the 
interface (org.apache.cloudstack.api.ApiServerService, interface implemented by 
class ApiServer).

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/rafaelweingartner/cloudstack 
lrg-cs-hackday-018

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1263.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1263


commit 9ba72a500a141131ad5085028cc1a60b88a3dd0c
Author: gabrascher 
Date:   2015-12-20T11:38:04Z

The goal of this PR is to review com.cloud.api.ApiServer class, with the
following actions:

Removed “_” in beginning of global variables names:
Variables was changed from “_” to “”, as
this convension (private veriables with “_”) is common in C++ but not in
Java.

Removed unused code from ApiServer:
- com.cloud.api.ApiServer.getPluggableServices():
Unused method.
- com.cloud.api.ApiServer.getApiAccessCheckers():
Unused method.

Methods and variables access level reviewed:
- com.cloud.api.ApiServer.handleAsyncJobPublishEvent(String, String,
Object):
This method was private but the annotation @MessageHandler requests
public methods, as can be seen in

org.apache.cloudstack.framework.messagebus.MessageDispatcher.buildHandlerMethodCache(Class),
which searches methods with the @MessageHandler annotation and changes
it to accessible (“setAccessible(true)”). Thus, there is no reason for
handleAsyncJobPublishEvent be a private method.

- Global variables and methods called just by this class (ApiServer)
were changed to private.

Changed variables and methods from static to non static (if possible):
As some variables/methods are used just by one object of this class
(instantiated by springer), they were changed to non static.

With that, calls from com.cloud.api.ApiServlet.ApiServlet() that used
static methods from ApiServer, was changed from
ApiServer. to _apiServer. that refers to
the org.apache.cloudstack.api.ApiServerService interface. Thus, methods
com.cloud.api.ApiServer.getJSONContentType() and
com.cloud.api.ApiServer.isSecureSessionCookieEnabled() had to be
included in the interface (org.apache.cloudstack.api.ApiServerService,
interface implemented by class ApiServer).

However, com.cloud.api.ApiServer.isEncodeApiResponse() was keept static,
as its call hierarchy would have to be changed (more than planed for
this PR).




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GabrielBrascher]

Github user GabrielBrascher closed the pull request at:

https://github.com/apache/cloudstack/pull/1263


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1263#issuecomment-207956609
  
@GabrielBrascher you could try to close the PR and reopen it. It matters if 
the queue at builds.apache.org has a full queue or not.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Remove classes with no references

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1453#issuecomment-207954572
  
I am having other problems (after the merge) in my test environment, not 
related to the PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---