[jira] [Commented] (CLOUDSTACK-10352) XenServer: Support online storage migration from non-managed to managed storage

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10352?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16451288#comment-16451288
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10352:
-

mike-tutkowski commented on issue #2502: [CLOUDSTACK-10352] XenServer: Support 
online migration of a virtual disk from non-managed to managed storage
URL: https://github.com/apache/cloudstack/pull/2502#issuecomment-384094428
 
 
   @syed Can you take a look at this PR? @borisroman has approved it. Also, 
Travis and Jenkins are green on it. This PR seems like something that may be of 
use to you guys at some point. 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


> XenServer: Support online storage migration from non-managed to managed 
> storage
> ---
>
> Key: CLOUDSTACK-10352
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10352
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: Management Server, XenServer
> Environment: XenServer
>Reporter: Mike Tutkowski
>Assignee: Mike Tutkowski
>Priority: Major
> Fix For: 4.12.0.0
>
>
> Allow a user to online migrate a volume from non-managed storage to managed 
> storage.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10352) XenServer: Support online storage migration from non-managed to managed storage

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10352?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16450438#comment-16450438
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10352:
-

mike-tutkowski commented on issue #2502: [CLOUDSTACK-10352] XenServer: Support 
online migration of a virtual disk from non-managed to managed storage
URL: https://github.com/apache/cloudstack/pull/2502#issuecomment-384058338
 
 
   I have added a test suite for this feature.


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


> XenServer: Support online storage migration from non-managed to managed 
> storage
> ---
>
> Key: CLOUDSTACK-10352
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10352
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: Management Server, XenServer
> Environment: XenServer
>Reporter: Mike Tutkowski
>Assignee: Mike Tutkowski
>Priority: Major
> Fix For: 4.12.0.0
>
>
> Allow a user to online migrate a volume from non-managed storage to managed 
> storage.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (CLOUDSTACK-10372) Move entire network(s) to different acocunts/projects

[JnSchl (JIRA)]

JnSchl created CLOUDSTACK-10372:
---

 Summary: Move entire network(s) to different acocunts/projects
 Key: CLOUDSTACK-10372
 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10372
 Project: CloudStack
  Issue Type: New Feature
  Security Level: Public (Anyone can view this level - this is the default.)
Reporter: JnSchl


It would be great to move an entire network (maybe networks) to different 
acocunts/projects. All VMs must be moved as well. At the moment several 
relations must be removed before it's possible to move a VM or migrate all VMs 
within a network. Also the public ips must be recreated. All of this is not 
very fast and leads to a downtime.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (CLOUDSTACK-10371) Internal Names of VMs if a move was performed

[JnSchl (JIRA)]

JnSchl created CLOUDSTACK-10371:
---

 Summary: Internal Names of VMs if a move was performed
 Key: CLOUDSTACK-10371
 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10371
 Project: CloudStack
  Issue Type: New Feature
  Security Level: Public (Anyone can view this level - this is the default.)
Reporter: JnSchl


The "Internal Names" of VMs follow a specific schema.

If a VM is moved to another account or project the internal name does not 
change and the schema breaks.

There is some potential to get confused if you try to find/list/debug moved VMs 
in the UI and on the hypervisor level too.

Maybe the best way is to have the ability to change the naming schema to a 
neutral one in the global settings. Changing the internal name during a move 
might an alternative way. Goal should be not break the naming schema if some 
VMs are moved during their lifecycle.

In addition, the second number displays the exact amount of deployed VMs, maybe 
a neutral suffix might be better.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (CLOUDSTACK-10370) Easy way to control/limit bandwidth for public traffic / internet connection

[JnSchl (JIRA)]

JnSchl created CLOUDSTACK-10370:
---

 Summary: Easy way to control/limit bandwidth for public traffic / 
internet connection
 Key: CLOUDSTACK-10370
 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10370
 Project: CloudStack
  Issue Type: New Feature
  Security Level: Public (Anyone can view this level - this is the default.)
  Components: Virtual Router
Reporter: JnSchl


As far as we now it is currently only possible to assign a new network offering 
to change the max bandwidth.

It would be great to have an easy way to define and update the bandwidth for 
up- and downloads.

In addition, it is important to disallow the right to update the bandwidth for 
accounts via the roles section.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (CLOUDSTACK-10369) Change the role of existing accounts or assign individual roles to account users

[JnSchl (JIRA)]

JnSchl created CLOUDSTACK-10369:
---

 Summary: Change the role of existing accounts or assign individual 
roles to account users
 Key: CLOUDSTACK-10369
 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10369
 Project: CloudStack
  Issue Type: New Feature
  Security Level: Public (Anyone can view this level - this is the default.)
Reporter: JnSchl


Currently it's not possible to change the role of existing accounts. Beeing 
able to change the role of accounts is however important during the lifecycle 
of customers.

An other way might be to set individual Roles to specific Account Users.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (CLOUDSTACK-10368) Define bandwidth for customer gateways

[JnSchl (JIRA)]

JnSchl created CLOUDSTACK-10368:
---

 Summary: Define bandwidth for customer gateways
 Key: CLOUDSTACK-10368
 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10368
 Project: CloudStack
  Issue Type: New Feature
  Security Level: Public (Anyone can view this level - this is the default.)
  Components: VPC
Reporter: JnSchl


The possibility to set / limit the internet bandwidth for customer gateways 
would be a great feature.

Reason for this request is on the one hand the possibility to control the 
bandwidth for a specific S2S VPN and on the other hand, to be able to charge 
the customers according the bandwidth booked for each customer gateway.

It should also be possible to assign accounts/users the right to update the 
bandwidth (maybe Roles).



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (CLOUDSTACK-10367) Define the "scope” for new VPCs and Customer Gateways

[JnSchl (JIRA)]

JnSchl created CLOUDSTACK-10367:
---

 Summary: Define the "scope” for new VPCs and Customer Gateways
 Key: CLOUDSTACK-10367
 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10367
 Project: CloudStack
  Issue Type: New Feature
  Security Level: Public (Anyone can view this level - this is the default.)
  Components: VPC
Reporter: JnSchl


It should be possible to set the "scope" for a new VPC and for new customer 
gateways as Root or Domain Admin. Currently we have to login to a specific 
account to add a new VPC and new customer gateways. Projects are not directly 
affected.

Currently it is possible to define the scope for guest networks only.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (CLOUDSTACK-10366) Ability to limit the number of customer gateways for VPCs

[JnSchl (JIRA)]

JnSchl created CLOUDSTACK-10366:
---

 Summary: Ability to limit the number of customer gateways for VPCs
 Key: CLOUDSTACK-10366
 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10366
 Project: CloudStack
  Issue Type: New Feature
  Security Level: Public (Anyone can view this level - this is the default.)
  Components: VPC
Reporter: JnSchl


It would be great to have the ability to limit the number of customer gateways 
for VPC and its S2S VPN functionality.

Scope should be domains, accounts and projects.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10356) Fix Some Potential NPE

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10356?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449873#comment-16449873
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10356:
-

blueorangutan commented on issue #2573: [CLOUDSTACK-10356] Fix NPE in 
Cloudstack found with NPEDetector 
URL: https://github.com/apache/cloudstack/pull/2573#issuecomment-383935669
 
 
   Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1968


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


> Fix Some Potential NPE 
> ---
>
> Key: CLOUDSTACK-10356
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10356
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Affects Versions: 4.12.0.0
>Reporter: lujie
>Priority: Major
> Attachments: CLOUDSTACK-10356_1.patch
>
>
> We have developed a static analysis tool 
> [NPEDetector|https://github.com/lujiefsi/NPEDetector] to find some potential 
> NPE. Our analysis shows that some callees may return null in corner case(e.g. 
> node crash , IO exception), some of their callers have  _!=null_ check but 
> some do not have. In this issue we post a patch which can add  !=null  based 
> on existed !=null  check. For example:
> Callee GlobalLoadBalancingRulesServiceImpl#lookupGslbServiceProvider:
> {code:java}
> protected GslbServiceProvider lookupGslbServiceProvider() {
> return _gslbProviders.size() == 0 ? null : _gslbProviders.get(0);// may 
> return null;
> }
> {code}
> Caller GlobalLoadBalancingRulesServiceImpl#checkGslbServiceEnabledInZone have 
> _!=null_:
> {code:java}
> private boolean checkGslbServiceEnabledInZone(long zoneId, long 
> physicalNetworkId) {
>GslbServiceProvider gslbProvider = lookupGslbServiceProvider();
>if (gslbProvider == null) {
>   throw new CloudRuntimeException("No GSLB provider is available");
>}
>return gslbProvider.isServiceEnabledInZone(zoneId, physicalNetworkId);
> }
> {code}
> but another 
> GlobalLoadBalancingRulesServiceImpl#applyGlobalLoadBalancerRuleConfig does 
> not have !=null check:
> {code:java}
> GslbServiceProvider gslbProvider = lookupGslbServiceProvider();
> siteLb.setGslbProviderPublicIp(gslbProvider.getZoneGslbProviderPublicIp(dataCenterId,physicalNetworkId));
> .{code}
> So we will add below code in non-(!=null) caller 
> GlobalLoadBalancingRulesServiceImpl#applyGlobalLoadBalancerRuleConfig
> {code:java}
> if (gslbProvider == null) {
> throw new CloudRuntimeException("No GSLB provider is available");
> }
> {code}
> But due to we are not very  familiar with CLOUDSTACK, hope some expert can 
> review it.
> Thanks



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10326) Prevent hosts fall into Maintenance when there are running VMs on it

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10326?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449869#comment-16449869
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10326:
-

blueorangutan commented on issue #2493: CLOUDSTACK-10326: Prevent hosts fall 
into Maintenance when there are running VMs on it
URL: https://github.com/apache/cloudstack/pull/2493#issuecomment-383935102
 
 
   Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1967


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


> Prevent hosts fall into Maintenance when there are running VMs on it
> 
>
> Key: CLOUDSTACK-10326
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10326
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Affects Versions: 4.11.0.0
>Reporter: Nicolas Vazquez
>Assignee: Nicolas Vazquez
>Priority: Major
> Fix For: 4.11.1.0
>
> Attachments: CLOUDSTACK-10326-Debug.png, 
> CLOUDSTACK-10326-InitialState.png, CLOUDSTACK-10326-Migrating.png, 
> CLOUDSTACK-10326-MigrationFailed.png
>
>
> This issue was discovered, fixed and tested on KVM, but applies for every 
> hypervisor.
> h2. Background
> When enabling maintenance mode in a host, host state is put into 
> 'PrepareForMaintenance' and running VMs are migrated into another host. After 
> every VM is migrated, host goes to 'Maintenance' state.
> Checks are performed on ResourceManagerImpl.checkAndMaintan() method:
>  * List VMs with host_id = HOST_ID
>  * List VMs with last_host_id = HOST_ID and state=Migrating
> When both queries are empty, then the host can be put into Maintenance.
> When a VM is being migrated to DEST_HOST, its host_id column is set to 
> DEST_HOST, last_host_id = ORIGIN_HOST and state = Migrating. If then 
> migration fails, host_id = last_host_id = ORIGIN_HOST 
> h2. Issue
> This sequence:
>  * Enable maintenance mode on ORIGIN_HOST
>  * VMs start being migrated to a host, say DEST_HOST
>  * checkAndMaintain() starts:
>  ** First check passes (no VM with host_id = ORIGIN_HOST_ID as those are 
> being migrated)
>  ** Before the second check, one or more migrations fail
>  ** Second check passes, however there are VMs running on the host as 
> migrations have failed.
>  * Host goes into Maintenance state.
> Screenshots attached, query executed on each case:
> select id, name, instance_name, state, host_id, last_host_id from vm_instance;



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-8415) [VMware] SSVM shutdown during snapshot operation results in disks to be left behind

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8415?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449852#comment-16449852
 ] 

ASF GitHub Bot commented on CLOUDSTACK-8415:


rhtyd commented on issue #2090: CLOUDSTACK-8415 [VMware] SSVM shutdown during 
snapshot operation results in disks to be left behind
URL: https://github.com/apache/cloudstack/pull/2090#issuecomment-383932004
 
 
   ping @sureshanaparti can you fix the 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


> [VMware] SSVM shutdown during snapshot operation results in disks to be left 
> behind 
> 
>
> Key: CLOUDSTACK-8415
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8415
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Likitha Shetty
>Assignee: Suresh Kumar Anaparti
>Priority: Major
> Fix For: Future
>
>
> Partial disks are residue of a failed snapshot operation caused by SSVM 
> reboot/shutdown. The disks do not get cleaned up on secondary storage and 
> need to be cleaned up manually to release storage.
> +Steps to reproduce+
> 1. Initiate a volume snapshot operation.
> 2. Destroy SSVM while the operation is in progress.
> 3. Check the snapshot folder in secondary storage - Files including disks are 
> present in the folder and are never cleaned up. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-9906) Create snapshot after volume resize for the volume with existing snapshot(s)

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9906?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449850#comment-16449850
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9906:


rhtyd commented on issue #2079: CLOUDSTACK-9906 Create snapshot after volume 
resize for the volume with existing snapshot(s)
URL: https://github.com/apache/cloudstack/pull/2079#issuecomment-383931662
 
 
   Ping @sureshanaparti can you fix the conflicts, and use either 4.11 or 4.12? 
Perhaps, reopen the PR? 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


> Create snapshot after volume resize for the volume with existing snapshot(s)
> 
>
> Key: CLOUDSTACK-9906
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9906
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: Snapshot, Volumes
> Environment: XenServer
>Reporter: Suresh Kumar Anaparti
>Assignee: Suresh Kumar Anaparti
>Priority: Major
>
> Snapshot of a volume fails if the volume has an existing snapshot(s) taken 
> before resizing of the volume. This behavior needs to be improved so that 
> taking snapshot would be successful when there is an existing snapshot(s) 
> before volume resizing.
> REPRO STEPS 
> =
> - Create a volume with 10 GB and attach to any VM.
> - Take snapshot  of the volume.
> - Change volume size from 10GB to 15 GB.
> - Take snapshot of the volume after resizing it.
> - Create a volume from the snapshot taken after resizing volume.
> - Attach the volume (created above) to any VM and check the size.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-9781) ACS records ID in events tables instead of UUID.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449847#comment-16449847
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9781:


rhtyd commented on issue #1940: CLOUDSTACK-9781:ACS records ID in events tables 
instead of UUID.
URL: https://github.com/apache/cloudstack/pull/1940#issuecomment-383931051
 
 
   Ping @syed, if you don't reply we'll have to consider other's review. Let's 
discuss? 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


> ACS records ID in events tables instead of UUID.
> 
>
> Key: CLOUDSTACK-9781
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9781
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Jayant Patil
>Priority: Major
>
> ISSUE
> =
> Wrong presentation of volume id in ACS events.
> While creating a snapshot, only volume ID is mentioned in the events. For 
> example, “Scheduled async job for creating snapshot for volume Id:270". On 
> looking into the notification, user is not able to identify the volume. So 
> modified event description with UUID.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10326) Prevent hosts fall into Maintenance when there are running VMs on it

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10326?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449827#comment-16449827
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10326:
-

nvazquez commented on issue #2493: CLOUDSTACK-10326: Prevent hosts fall into 
Maintenance when there are running VMs on it
URL: https://github.com/apache/cloudstack/pull/2493#issuecomment-383927219
 
 
   @rhtyd I removed it from the 4.11.1 milestone. Let me close it for the 
moment, I'll reopen when resuming the work on it


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


> Prevent hosts fall into Maintenance when there are running VMs on it
> 
>
> Key: CLOUDSTACK-10326
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10326
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Affects Versions: 4.11.0.0
>Reporter: Nicolas Vazquez
>Assignee: Nicolas Vazquez
>Priority: Major
> Fix For: 4.11.1.0
>
> Attachments: CLOUDSTACK-10326-Debug.png, 
> CLOUDSTACK-10326-InitialState.png, CLOUDSTACK-10326-Migrating.png, 
> CLOUDSTACK-10326-MigrationFailed.png
>
>
> This issue was discovered, fixed and tested on KVM, but applies for every 
> hypervisor.
> h2. Background
> When enabling maintenance mode in a host, host state is put into 
> 'PrepareForMaintenance' and running VMs are migrated into another host. After 
> every VM is migrated, host goes to 'Maintenance' state.
> Checks are performed on ResourceManagerImpl.checkAndMaintan() method:
>  * List VMs with host_id = HOST_ID
>  * List VMs with last_host_id = HOST_ID and state=Migrating
> When both queries are empty, then the host can be put into Maintenance.
> When a VM is being migrated to DEST_HOST, its host_id column is set to 
> DEST_HOST, last_host_id = ORIGIN_HOST and state = Migrating. If then 
> migration fails, host_id = last_host_id = ORIGIN_HOST 
> h2. Issue
> This sequence:
>  * Enable maintenance mode on ORIGIN_HOST
>  * VMs start being migrated to a host, say DEST_HOST
>  * checkAndMaintain() starts:
>  ** First check passes (no VM with host_id = ORIGIN_HOST_ID as those are 
> being migrated)
>  ** Before the second check, one or more migrations fail
>  ** Second check passes, however there are VMs running on the host as 
> migrations have failed.
>  * Host goes into Maintenance state.
> Screenshots attached, query executed on each case:
> select id, name, instance_name, state, host_id, last_host_id from vm_instance;



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10326) Prevent hosts fall into Maintenance when there are running VMs on it

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10326?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449828#comment-16449828
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10326:
-

nvazquez closed pull request #2493: CLOUDSTACK-10326: Prevent hosts fall into 
Maintenance when there are running VMs on it
URL: https://github.com/apache/cloudstack/pull/2493
 
 
   

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

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

diff --git a/engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java 
b/engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java
index 69efea42df9..6fda4a15c32 100755
--- a/engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java
+++ b/engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java
@@ -150,4 +150,6 @@
 VMInstanceVO findVMByHostNameInZone(String hostName, long zoneId);
 
 boolean isPowerStateUpToDate(long instanceId);
+
+List listNonMigratingVmsByHostEqualsLastHost(long hostId);
 }
diff --git a/engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java 
b/engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java
index 6e97d1275a6..1565f53233b 100755
--- a/engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java
+++ b/engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java
@@ -93,6 +93,7 @@
 protected GenericSearchBuilder 
DistinctHostNameSearch;
 protected SearchBuilder HostAndStateSearch;
 protected SearchBuilder StartingWithNoHostSearch;
+protected SearchBuilder NotMigratingSearch;
 
 @Inject
 ResourceTagDao _tagsDao;
@@ -280,6 +281,11 @@ protected void init() {
 DistinctHostNameSearch.join("nicSearch", nicSearch, 
DistinctHostNameSearch.entity().getId(), nicSearch.entity().getInstanceId(), 
JoinBuilder.JoinType.INNER);
 DistinctHostNameSearch.done();
 
+NotMigratingSearch = createSearchBuilder();
+NotMigratingSearch.and("host", 
NotMigratingSearch.entity().getHostId(), Op.EQ);
+NotMigratingSearch.and("lastHost", 
NotMigratingSearch.entity().getLastHostId(), Op.EQ);
+NotMigratingSearch.and("state", 
NotMigratingSearch.entity().getState(), Op.NEQ);
+NotMigratingSearch.done();
 }
 
 @Override
@@ -304,6 +310,15 @@ protected void init() {
 return listBy(sc);
 }
 
+@Override
+public List listNonMigratingVmsByHostEqualsLastHost(long 
hostId) {
+SearchCriteria sc = NotMigratingSearch.create();
+sc.setParameters("host", hostId);
+sc.setParameters("lastHost", hostId);
+sc.setParameters("state", State.Migrating);
+return listBy(sc);
+}
+
 @Override
 public List listByZoneId(long zoneId) {
 SearchCriteria sc = AllFieldsSearch.create();
diff --git a/server/src/com/cloud/resource/ResourceManagerImpl.java 
b/server/src/com/cloud/resource/ResourceManagerImpl.java
index 2966d41d8bf..25bfc4c9c67 100755
--- a/server/src/com/cloud/resource/ResourceManagerImpl.java
+++ b/server/src/com/cloud/resource/ResourceManagerImpl.java
@@ -1296,10 +1296,17 @@ public boolean checkAndMaintain(final long hostId) {
 if (host.getType() != Host.Type.Storage) {
 final List vos = _vmDao.listByHostId(hostId);
 final List vosMigrating = 
_vmDao.listVmsMigratingFromHost(hostId);
+final List failedMigratedVms = 
_vmDao.listNonMigratingVmsByHostEqualsLastHost(hostId);
 if (vos.isEmpty() && vosMigrating.isEmpty()) {
-resourceStateTransitTo(host, 
ResourceState.Event.InternalEnterMaintenance, _nodeId);
-hostInMaintenance = true;
-
ActionEventUtils.onCompletedActionEvent(CallContext.current().getCallingUserId(),
 CallContext.current().getCallingAccountId(), EventVO.LEVEL_INFO, 
EventTypes.EVENT_MAINTENANCE_PREPARE, "completed maintenance for host " + 
hostId, 0);
+if (!failedMigratedVms.isEmpty()) {
+s_logger.debug("Unable to migrate " + 
failedMigratedVms.size() + " VM(s) from host " + host.getUuid());
+resourceStateTransitTo(host, 
ResourceState.Event.UnableToMigrate, _nodeId);
+} else {
+s_logger.debug("Host " + host.getUuid() + " entering 
in Maintenance");
+resourceStateTransitTo(host, 
ResourceState.Event.InternalEnterMaintenance, _nodeId);
+hostInMaintenance = true;
+
ActionEventUtils.onCompletedActionEvent(CallContext.current().getCallingUserId(),
 CallContext.current().getCallingAccountId(), EventVO.LEVEL_INFO, 
EventTypes.EVENT_MAINTENANCE_PREPARE, "completed maintenance for host " + 
hostId, 0);
+}
 

[jira] [Commented] (CLOUDSTACK-10327) SSO fails with error "Session Expired", except for root admin

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10327?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449813#comment-16449813
 ] 

ASF subversion and git services commented on CLOUDSTACK-10327:
--

Commit 9a13227a78acb14d9fd4f9ad5246ce67396cef7a in cloudstack's branch 
refs/heads/master from [~olemasle]
[ https://gitbox.apache.org/repos/asf?p=cloudstack.git;h=9a13227 ]

CLOUDSTACK-10327: Do not invalidate the session when an API command is not 
available (#2498)

CloudStack SSO (using security.singlesignon.key) does not work anymore with 
CloudStack 4.11, since commit 9988c26, which introduced a regression due to a 
refactoring: every API request that is not "validated" generates the same error 
(401 - Unauthorized) and invalidates the session.

However, CloudStack UI executes a call to listConfigurations in method 
bypassLoginCheck. A non-admin user does not have the permissions to execute 
this request, which causes an error 401:

{"listconfigurationsresponse":{"uuidList":[],"errorcode":401,"errortext":"unable
 to verify user credentials and/or request signature"}}
The session (already created by SSO) is then invalidated and the user cannot 
access to CloudStack UI (error "Session Expired").

Before 9988c26 (up to CloudStack 4.10), an error 432 was returned (and ignored):

{"errorresponse":{"uuidList":[],"errorcode":432,"cserrorcode":,"errortext":"The
 user is not allowed to request the API command or the API command does not 
exist"}}
Even if the call to listConfigurations was removed, another call to listIdps 
also lead to an error 401 for user accounts if the SAML plugin is not enabled.

This pull request aims to fix the SSO issue, by restoring errors 432 (instead 
of 401 + invalidate session) for commands not available. However, if an API 
command is explicitly denied using ACLs or if the session key is incorrect, it 
still generates an error 401 and invalidates the session.

> SSO fails with error "Session Expired", except for root admin
> -
>
> Key: CLOUDSTACK-10327
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10327
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: API
>Affects Versions: 4.11.0.0
>Reporter: Olivier Lemasle
>Assignee: Olivier Lemasle
>Priority: Critical
>
> CloudStack SSO (using {{security.singlesignon.key}}) does not work anymore 
> with CloudStack 4.11, since commit 
> [9988c26|https://github.com/apache/cloudstack/commit/9988c269b259b84c0b8436bad17f88dbc1d706e7#diff-16f2bfa56c6e8760760dd2b27b47d5b4]
> This commit introduced a new feature (the ability to limit admin API calls to 
> a network CIDR), but also a regression due to a refactoring: every API 
> request that is not "validated" generates the same error (401 - Unauthorized) 
> and *invalidates the session*.
> However, during an SSO login, CloudStack executes (since ACS 4.7), a [call to 
> "listConfigurations"|https://github.com/apache/cloudstack/blob/8a3943b7632eddf3856a19e7d9a3fee82dd325be/ui/scripts/cloudStack.js#L172],
>  an API command reserved for root admins. When the user is not a root admin, 
> he does not have the privileges for this command.
> With CloudStack up to 4.10, an error 432 was returned (and ignored):
> {noformat}
> {"errorresponse":{"uuidList":[],"errorcode":432,"cserrorcode":,"errortext":"The
>  user is not allowed to request the API command or the API command does not 
> exist"}}
> {noformat}
> With CloudStack 4.11, the error 432 is replaced by an error 401 and the 
> session is invalidated. Then the next API calls lead to an error "Session 
> Expired" and the user cannot log in.
> {noformat}
> {"listconfigurationsresponse":{"uuidList":[],"errorcode":401,"errortext":"unable
>  to verify user credentials and/or request signature"}}
> {noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10356) Fix Some Potential NPE

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10356?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449811#comment-16449811
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10356:
-

blueorangutan commented on issue #2573: [CLOUDSTACK-10356] Fix NPE in 
Cloudstack found with NPEDetector 
URL: https://github.com/apache/cloudstack/pull/2573#issuecomment-383922721
 
 
   @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


> Fix Some Potential NPE 
> ---
>
> Key: CLOUDSTACK-10356
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10356
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Affects Versions: 4.12.0.0
>Reporter: lujie
>Priority: Major
> Attachments: CLOUDSTACK-10356_1.patch
>
>
> We have developed a static analysis tool 
> [NPEDetector|https://github.com/lujiefsi/NPEDetector] to find some potential 
> NPE. Our analysis shows that some callees may return null in corner case(e.g. 
> node crash , IO exception), some of their callers have  _!=null_ check but 
> some do not have. In this issue we post a patch which can add  !=null  based 
> on existed !=null  check. For example:
> Callee GlobalLoadBalancingRulesServiceImpl#lookupGslbServiceProvider:
> {code:java}
> protected GslbServiceProvider lookupGslbServiceProvider() {
> return _gslbProviders.size() == 0 ? null : _gslbProviders.get(0);// may 
> return null;
> }
> {code}
> Caller GlobalLoadBalancingRulesServiceImpl#checkGslbServiceEnabledInZone have 
> _!=null_:
> {code:java}
> private boolean checkGslbServiceEnabledInZone(long zoneId, long 
> physicalNetworkId) {
>GslbServiceProvider gslbProvider = lookupGslbServiceProvider();
>if (gslbProvider == null) {
>   throw new CloudRuntimeException("No GSLB provider is available");
>}
>return gslbProvider.isServiceEnabledInZone(zoneId, physicalNetworkId);
> }
> {code}
> but another 
> GlobalLoadBalancingRulesServiceImpl#applyGlobalLoadBalancerRuleConfig does 
> not have !=null check:
> {code:java}
> GslbServiceProvider gslbProvider = lookupGslbServiceProvider();
> siteLb.setGslbProviderPublicIp(gslbProvider.getZoneGslbProviderPublicIp(dataCenterId,physicalNetworkId));
> .{code}
> So we will add below code in non-(!=null) caller 
> GlobalLoadBalancingRulesServiceImpl#applyGlobalLoadBalancerRuleConfig
> {code:java}
> if (gslbProvider == null) {
> throw new CloudRuntimeException("No GSLB provider is available");
> }
> {code}
> But due to we are not very  familiar with CLOUDSTACK, hope some expert can 
> review it.
> Thanks



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10356) Fix Some Potential NPE

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10356?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449810#comment-16449810
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10356:
-

rhtyd commented on issue #2573: [CLOUDSTACK-10356] Fix NPE in Cloudstack found 
with NPEDetector 
URL: https://github.com/apache/cloudstack/pull/2573#issuecomment-383922582
 
 
   @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


> Fix Some Potential NPE 
> ---
>
> Key: CLOUDSTACK-10356
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10356
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Affects Versions: 4.12.0.0
>Reporter: lujie
>Priority: Major
> Attachments: CLOUDSTACK-10356_1.patch
>
>
> We have developed a static analysis tool 
> [NPEDetector|https://github.com/lujiefsi/NPEDetector] to find some potential 
> NPE. Our analysis shows that some callees may return null in corner case(e.g. 
> node crash , IO exception), some of their callers have  _!=null_ check but 
> some do not have. In this issue we post a patch which can add  !=null  based 
> on existed !=null  check. For example:
> Callee GlobalLoadBalancingRulesServiceImpl#lookupGslbServiceProvider:
> {code:java}
> protected GslbServiceProvider lookupGslbServiceProvider() {
> return _gslbProviders.size() == 0 ? null : _gslbProviders.get(0);// may 
> return null;
> }
> {code}
> Caller GlobalLoadBalancingRulesServiceImpl#checkGslbServiceEnabledInZone have 
> _!=null_:
> {code:java}
> private boolean checkGslbServiceEnabledInZone(long zoneId, long 
> physicalNetworkId) {
>GslbServiceProvider gslbProvider = lookupGslbServiceProvider();
>if (gslbProvider == null) {
>   throw new CloudRuntimeException("No GSLB provider is available");
>}
>return gslbProvider.isServiceEnabledInZone(zoneId, physicalNetworkId);
> }
> {code}
> but another 
> GlobalLoadBalancingRulesServiceImpl#applyGlobalLoadBalancerRuleConfig does 
> not have !=null check:
> {code:java}
> GslbServiceProvider gslbProvider = lookupGslbServiceProvider();
> siteLb.setGslbProviderPublicIp(gslbProvider.getZoneGslbProviderPublicIp(dataCenterId,physicalNetworkId));
> .{code}
> So we will add below code in non-(!=null) caller 
> GlobalLoadBalancingRulesServiceImpl#applyGlobalLoadBalancerRuleConfig
> {code:java}
> if (gslbProvider == null) {
> throw new CloudRuntimeException("No GSLB provider is available");
> }
> {code}
> But due to we are not very  familiar with CLOUDSTACK, hope some expert can 
> review it.
> Thanks



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10326) Prevent hosts fall into Maintenance when there are running VMs on it

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10326?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449805#comment-16449805
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10326:
-

blueorangutan commented on issue #2493: CLOUDSTACK-10326: Prevent hosts fall 
into Maintenance when there are running VMs on it
URL: https://github.com/apache/cloudstack/pull/2493#issuecomment-383922138
 
 
   @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


> Prevent hosts fall into Maintenance when there are running VMs on it
> 
>
> Key: CLOUDSTACK-10326
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10326
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Affects Versions: 4.11.0.0
>Reporter: Nicolas Vazquez
>Assignee: Nicolas Vazquez
>Priority: Major
> Fix For: 4.11.1.0
>
> Attachments: CLOUDSTACK-10326-Debug.png, 
> CLOUDSTACK-10326-InitialState.png, CLOUDSTACK-10326-Migrating.png, 
> CLOUDSTACK-10326-MigrationFailed.png
>
>
> This issue was discovered, fixed and tested on KVM, but applies for every 
> hypervisor.
> h2. Background
> When enabling maintenance mode in a host, host state is put into 
> 'PrepareForMaintenance' and running VMs are migrated into another host. After 
> every VM is migrated, host goes to 'Maintenance' state.
> Checks are performed on ResourceManagerImpl.checkAndMaintan() method:
>  * List VMs with host_id = HOST_ID
>  * List VMs with last_host_id = HOST_ID and state=Migrating
> When both queries are empty, then the host can be put into Maintenance.
> When a VM is being migrated to DEST_HOST, its host_id column is set to 
> DEST_HOST, last_host_id = ORIGIN_HOST and state = Migrating. If then 
> migration fails, host_id = last_host_id = ORIGIN_HOST 
> h2. Issue
> This sequence:
>  * Enable maintenance mode on ORIGIN_HOST
>  * VMs start being migrated to a host, say DEST_HOST
>  * checkAndMaintain() starts:
>  ** First check passes (no VM with host_id = ORIGIN_HOST_ID as those are 
> being migrated)
>  ** Before the second check, one or more migrations fail
>  ** Second check passes, however there are VMs running on the host as 
> migrations have failed.
>  * Host goes into Maintenance state.
> Screenshots attached, query executed on each case:
> select id, name, instance_name, state, host_id, last_host_id from vm_instance;



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10199) Support requesting a specific IPv4 address in Basic Networking during Instance creation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449803#comment-16449803
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10199:
-

GabrielBrascher commented on a change in pull request #2595: CLOUDSTACK-10199: 
Support requesting a specific IPv4 address
URL: https://github.com/apache/cloudstack/pull/2595#discussion_r183720103
 
 

 ##
 File path: engine/schema/src/main/java/com/cloud/dc/dao/VlanDaoImpl.java
 ##
 @@ -81,6 +81,22 @@ public VlanVO findByZoneAndVlanId(long zoneId, String 
vlanId) {
 sc.setParameters("vlanId", vlanId);
 return findOneBy(sc);
 }
+
+/**
+ * Returns a vlan by the network id.
+ */
+@Override
+public VlanVO findByNetworkId(long networkId) {
+List vlanVoList = listVlansByNetworkId(networkId);
+VlanVO vlanVo = null;
+for (VlanVO vlan : vlanVoList) {
+if (vlan.getRemoved() == null) {
 
 Review comment:
   Well noticed, 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


> Support requesting a specific IPv4 address in Basic Networking during 
> Instance creation
> ---
>
> Key: CLOUDSTACK-10199
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10199
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: API
> Environment: CloudStack 4.10
>Reporter: Wido den Hollander
>Priority: Major
>  Labels: basic-networking
>
> DirectPodBasedNetworkGuru does not support requesting a custom IP-Address 
> while creating a new NIC/Instance.
> {quote}
> Error 530: Does not support custom ip allocation at this time: 
> NicProfile[0-0-null-null-null
> {
>   "cserrorcode": 4250,
>   "errorcode": 530,
>   "errortext": "Does not support custom ip allocation at this time: 
> NicProfile[0-0-null-null-null",
>   "uuidList": []
> }
> {quote}
> Some use-cases prefer the ability to request the IPv4 address which the 
> Instance will get.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10326) Prevent hosts fall into Maintenance when there are running VMs on it

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10326?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449804#comment-16449804
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10326:
-

rhtyd commented on issue #2493: CLOUDSTACK-10326: Prevent hosts fall into 
Maintenance when there are running VMs on it
URL: https://github.com/apache/cloudstack/pull/2493#issuecomment-383921944
 
 
   Comments on this PR status and review - @DaanHoogland @nvazquez 
@borisstoyanov and others?
   @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


> Prevent hosts fall into Maintenance when there are running VMs on it
> 
>
> Key: CLOUDSTACK-10326
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10326
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Affects Versions: 4.11.0.0
>Reporter: Nicolas Vazquez
>Assignee: Nicolas Vazquez
>Priority: Major
> Fix For: 4.11.1.0
>
> Attachments: CLOUDSTACK-10326-Debug.png, 
> CLOUDSTACK-10326-InitialState.png, CLOUDSTACK-10326-Migrating.png, 
> CLOUDSTACK-10326-MigrationFailed.png
>
>
> This issue was discovered, fixed and tested on KVM, but applies for every 
> hypervisor.
> h2. Background
> When enabling maintenance mode in a host, host state is put into 
> 'PrepareForMaintenance' and running VMs are migrated into another host. After 
> every VM is migrated, host goes to 'Maintenance' state.
> Checks are performed on ResourceManagerImpl.checkAndMaintan() method:
>  * List VMs with host_id = HOST_ID
>  * List VMs with last_host_id = HOST_ID and state=Migrating
> When both queries are empty, then the host can be put into Maintenance.
> When a VM is being migrated to DEST_HOST, its host_id column is set to 
> DEST_HOST, last_host_id = ORIGIN_HOST and state = Migrating. If then 
> migration fails, host_id = last_host_id = ORIGIN_HOST 
> h2. Issue
> This sequence:
>  * Enable maintenance mode on ORIGIN_HOST
>  * VMs start being migrated to a host, say DEST_HOST
>  * checkAndMaintain() starts:
>  ** First check passes (no VM with host_id = ORIGIN_HOST_ID as those are 
> being migrated)
>  ** Before the second check, one or more migrations fail
>  ** Second check passes, however there are VMs running on the host as 
> migrations have failed.
>  * Host goes into Maintenance state.
> Screenshots attached, query executed on each case:
> select id, name, instance_name, state, host_id, last_host_id from vm_instance;



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10327) SSO fails with error "Session Expired", except for root admin

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10327?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449801#comment-16449801
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10327:
-

rhtyd commented on issue #2498: CLOUDSTACK-10327: Do not invalidate the session 
when API command not found
URL: https://github.com/apache/cloudstack/pull/2498#issuecomment-383921632
 
 
   Merged based on test results and code review.


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


> SSO fails with error "Session Expired", except for root admin
> -
>
> Key: CLOUDSTACK-10327
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10327
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: API
>Affects Versions: 4.11.0.0
>Reporter: Olivier Lemasle
>Assignee: Olivier Lemasle
>Priority: Critical
>
> CloudStack SSO (using {{security.singlesignon.key}}) does not work anymore 
> with CloudStack 4.11, since commit 
> [9988c26|https://github.com/apache/cloudstack/commit/9988c269b259b84c0b8436bad17f88dbc1d706e7#diff-16f2bfa56c6e8760760dd2b27b47d5b4]
> This commit introduced a new feature (the ability to limit admin API calls to 
> a network CIDR), but also a regression due to a refactoring: every API 
> request that is not "validated" generates the same error (401 - Unauthorized) 
> and *invalidates the session*.
> However, during an SSO login, CloudStack executes (since ACS 4.7), a [call to 
> "listConfigurations"|https://github.com/apache/cloudstack/blob/8a3943b7632eddf3856a19e7d9a3fee82dd325be/ui/scripts/cloudStack.js#L172],
>  an API command reserved for root admins. When the user is not a root admin, 
> he does not have the privileges for this command.
> With CloudStack up to 4.10, an error 432 was returned (and ignored):
> {noformat}
> {"errorresponse":{"uuidList":[],"errorcode":432,"cserrorcode":,"errortext":"The
>  user is not allowed to request the API command or the API command does not 
> exist"}}
> {noformat}
> With CloudStack 4.11, the error 432 is replaced by an error 401 and the 
> session is invalidated. Then the next API calls lead to an error "Session 
> Expired" and the user cannot log in.
> {noformat}
> {"listconfigurationsresponse":{"uuidList":[],"errorcode":401,"errortext":"unable
>  to verify user credentials and/or request signature"}}
> {noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10327) SSO fails with error "Session Expired", except for root admin

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10327?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449802#comment-16449802
 ] 

ASF subversion and git services commented on CLOUDSTACK-10327:
--

Commit 9a13227a78acb14d9fd4f9ad5246ce67396cef7a in cloudstack's branch 
refs/heads/4.11 from [~olemasle]
[ https://gitbox.apache.org/repos/asf?p=cloudstack.git;h=9a13227 ]

CLOUDSTACK-10327: Do not invalidate the session when an API command is not 
available (#2498)

CloudStack SSO (using security.singlesignon.key) does not work anymore with 
CloudStack 4.11, since commit 9988c26, which introduced a regression due to a 
refactoring: every API request that is not "validated" generates the same error 
(401 - Unauthorized) and invalidates the session.

However, CloudStack UI executes a call to listConfigurations in method 
bypassLoginCheck. A non-admin user does not have the permissions to execute 
this request, which causes an error 401:

{"listconfigurationsresponse":{"uuidList":[],"errorcode":401,"errortext":"unable
 to verify user credentials and/or request signature"}}
The session (already created by SSO) is then invalidated and the user cannot 
access to CloudStack UI (error "Session Expired").

Before 9988c26 (up to CloudStack 4.10), an error 432 was returned (and ignored):

{"errorresponse":{"uuidList":[],"errorcode":432,"cserrorcode":,"errortext":"The
 user is not allowed to request the API command or the API command does not 
exist"}}
Even if the call to listConfigurations was removed, another call to listIdps 
also lead to an error 401 for user accounts if the SAML plugin is not enabled.

This pull request aims to fix the SSO issue, by restoring errors 432 (instead 
of 401 + invalidate session) for commands not available. However, if an API 
command is explicitly denied using ACLs or if the session key is incorrect, it 
still generates an error 401 and invalidates the session.

> SSO fails with error "Session Expired", except for root admin
> -
>
> Key: CLOUDSTACK-10327
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10327
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: API
>Affects Versions: 4.11.0.0
>Reporter: Olivier Lemasle
>Assignee: Olivier Lemasle
>Priority: Critical
>
> CloudStack SSO (using {{security.singlesignon.key}}) does not work anymore 
> with CloudStack 4.11, since commit 
> [9988c26|https://github.com/apache/cloudstack/commit/9988c269b259b84c0b8436bad17f88dbc1d706e7#diff-16f2bfa56c6e8760760dd2b27b47d5b4]
> This commit introduced a new feature (the ability to limit admin API calls to 
> a network CIDR), but also a regression due to a refactoring: every API 
> request that is not "validated" generates the same error (401 - Unauthorized) 
> and *invalidates the session*.
> However, during an SSO login, CloudStack executes (since ACS 4.7), a [call to 
> "listConfigurations"|https://github.com/apache/cloudstack/blob/8a3943b7632eddf3856a19e7d9a3fee82dd325be/ui/scripts/cloudStack.js#L172],
>  an API command reserved for root admins. When the user is not a root admin, 
> he does not have the privileges for this command.
> With CloudStack up to 4.10, an error 432 was returned (and ignored):
> {noformat}
> {"errorresponse":{"uuidList":[],"errorcode":432,"cserrorcode":,"errortext":"The
>  user is not allowed to request the API command or the API command does not 
> exist"}}
> {noformat}
> With CloudStack 4.11, the error 432 is replaced by an error 401 and the 
> session is invalidated. Then the next API calls lead to an error "Session 
> Expired" and the user cannot log in.
> {noformat}
> {"listconfigurationsresponse":{"uuidList":[],"errorcode":401,"errortext":"unable
>  to verify user credentials and/or request signature"}}
> {noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10327) SSO fails with error "Session Expired", except for root admin

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10327?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449800#comment-16449800
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10327:
-

rhtyd closed pull request #2498: CLOUDSTACK-10327: Do not invalidate the 
session when API command not found
URL: https://github.com/apache/cloudstack/pull/2498
 
 
   

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

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

diff --git 
a/api/src/main/java/com/cloud/exception/UnavailableCommandException.java 
b/api/src/main/java/com/cloud/exception/UnavailableCommandException.java
new file mode 100644
index 000..d5b7faa8f36
--- /dev/null
+++ b/api/src/main/java/com/cloud/exception/UnavailableCommandException.java
@@ -0,0 +1,36 @@
+// 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.exception;
+
+import com.cloud.utils.SerialVersionUID;
+
+public class UnavailableCommandException extends PermissionDeniedException {
+
+private static final long serialVersionUID = 
SerialVersionUID.UnavailableCommandException;
+
+protected UnavailableCommandException() {
+super();
+}
+
+public UnavailableCommandException(String msg) {
+super(msg);
+}
+
+public UnavailableCommandException(String msg, Throwable cause) {
+super(msg, cause);
+}
+}
diff --git 
a/plugins/acl/dynamic-role-based/src/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java
 
b/plugins/acl/dynamic-role-based/src/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java
index 3dfdb01b2e5..d10c191151f 100644
--- 
a/plugins/acl/dynamic-role-based/src/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java
+++ 
b/plugins/acl/dynamic-role-based/src/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java
@@ -25,6 +25,7 @@
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.exception.UnavailableCommandException;
 import org.apache.cloudstack.api.APICommand;
 
 import com.cloud.exception.PermissionDeniedException;
@@ -53,8 +54,7 @@ protected DynamicRoleBasedAPIAccessChecker() {
 }
 
 private void denyApiAccess(final String commandName) throws 
PermissionDeniedException {
-throw new PermissionDeniedException("The API does not exist or is 
blacklisted for the account's role. " +
-"The account with is not allowed to request the api: " + 
commandName);
+throw new PermissionDeniedException("The API " + commandName + " is 
blacklisted for the account's role.");
 }
 
 public boolean isDisabled() {
@@ -99,8 +99,7 @@ public boolean checkAccess(User user, String commandName) 
throws PermissionDenie
 }
 
 // Default deny all
-denyApiAccess(commandName);
-return false;
+throw new UnavailableCommandException("The API " + commandName + " 
does not exist or is not available for this account.");
 }
 
 public void addApiToRoleBasedAnnotationsMap(final RoleType roleType, final 
String commandName) {
diff --git 
a/plugins/acl/static-role-based/src/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java
 
b/plugins/acl/static-role-based/src/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java
index f3dc3a3b8d7..4a33a0876c3 100644
--- 
a/plugins/acl/static-role-based/src/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java
+++ 
b/plugins/acl/static-role-based/src/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java
@@ -25,6 +25,7 @@
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.exception.UnavailableCommandException;
 import org.apache.log4j.Logger;
 
 import org.apache.cloudstack.api.APICommand;
@@ -45,6 +46,7 @@
 protected static final Logger LOGGER = 
Logger.getLogger(StaticRoleBasedAPIAccessChecker.class);
 
 private Set commandPropertyFiles = new HashSet();
+private Set commandNames = new HashSet();
 private Set 

[jira] [Commented] (CLOUDSTACK-10365) Inconsistent boolean-related method names

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10365?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449729#comment-16449729
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10365:
-

BruceKuiLiu closed pull request #2602: CLOUDSTACK-10365: Change the "getXXX" 
boolean-related method names to…
URL: https://github.com/apache/cloudstack/pull/2602
 
 
   

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

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

diff --git a/api/src/main/java/com/cloud/agent/api/to/S3TO.java 
b/api/src/main/java/com/cloud/agent/api/to/S3TO.java
index ec6bc02cca0..233238cf793 100644
--- a/api/src/main/java/com/cloud/agent/api/to/S3TO.java
+++ b/api/src/main/java/com/cloud/agent/api/to/S3TO.java
@@ -201,7 +201,7 @@ public DataStoreRole getRole() {
 return DataStoreRole.Image;
 }
 
-public boolean getEnableRRS() {
+public boolean isEnableRRS() {
 return enableRRS;
 }
 
diff --git a/api/src/main/java/com/cloud/offering/NetworkOffering.java 
b/api/src/main/java/com/cloud/offering/NetworkOffering.java
index 0c8378908e6..e822e39d803 100644
--- a/api/src/main/java/com/cloud/offering/NetworkOffering.java
+++ b/api/src/main/java/com/cloud/offering/NetworkOffering.java
@@ -122,7 +122,7 @@
 
 boolean getIsPersistent();
 
-boolean getInternalLb();
+boolean isInternalLb();
 
 boolean getPublicLb();
 
diff --git a/api/src/main/java/com/cloud/offering/ServiceOffering.java 
b/api/src/main/java/com/cloud/offering/ServiceOffering.java
index 196d2b4eb47..822168dd4c4 100644
--- a/api/src/main/java/com/cloud/offering/ServiceOffering.java
+++ b/api/src/main/java/com/cloud/offering/ServiceOffering.java
@@ -76,7 +76,7 @@
 /**
  * @return Does this service plan offer HA?
  */
-boolean getOfferHA();
+boolean isOfferHA();
 
 /**
  * @return Does this service plan offer VM to use CPU resources beyond the 
service offering limits?
@@ -86,7 +86,7 @@
 /**
  * @return Does this service plan support Volatile VM that is, discard 
VM's root disk and create a new one on reboot?
  */
-boolean getVolatileVm();
+boolean isVolatileVm();
 
 /**
  * @return the rate in megabits per sec to which a VM's network interface 
is throttled to
diff --git a/api/src/main/java/com/cloud/storage/GuestOS.java 
b/api/src/main/java/com/cloud/storage/GuestOS.java
index 371260bec64..33c08722ad3 100644
--- a/api/src/main/java/com/cloud/storage/GuestOS.java
+++ b/api/src/main/java/com/cloud/storage/GuestOS.java
@@ -33,5 +33,5 @@
 
 Date getRemoved();
 
-boolean getIsUserDefined();
+boolean isUserDefined();
 }
diff --git a/api/src/main/java/com/cloud/template/VirtualMachineTemplate.java 
b/api/src/main/java/com/cloud/template/VirtualMachineTemplate.java
index 564f3b987be..ad2f6363170 100644
--- a/api/src/main/java/com/cloud/template/VirtualMachineTemplate.java
+++ b/api/src/main/java/com/cloud/template/VirtualMachineTemplate.java
@@ -101,9 +101,9 @@
 
 String getDisplayText();
 
-boolean getEnablePassword();
+boolean isEnablePassword();
 
-boolean getEnableSshKey();
+boolean isEnableSshKey();
 
 boolean isCrossZones();
 
diff --git 
a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java
 
b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java
index 0bde79bbd7b..3daea8d2075 100644
--- 
a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java
+++ 
b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java
@@ -210,7 +210,7 @@ public String getDeploymentPlanner() {
 return deploymentPlanner;
 }
 
-public boolean getCustomized() {
+public boolean isCustomized() {
 return (cpuNumber == null || memory == null || cpuSpeed == null);
 }
 
diff --git 
a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java
 
b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java
index 8eea632a72e..7b9ffa03289 100644
--- 
a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java
+++ 
b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java
@@ -107,7 +107,7 @@ public Long getSize() {
 return size;
 }
 
-public boolean getShrinkOk() {
+public boolean isShrinkOk() {
 return shrinkOk;
 }
 
diff --git 
a/core/src/main/java/com/cloud/storage/template/S3TemplateDownloader.java 
b/core/src/main/java/com/cloud/storage/template/S3TemplateDownloader.java
index d584cdffb10..0506dab22c0 100644
--- 

[jira] [Commented] (CLOUDSTACK-10362) Inconsistent method names

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10362?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449730#comment-16449730
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10362:
-

BruceKuiLiu closed pull request #2600: CLOUDSTACK-10362: Change the "getXXX" 
method names to "isXXX".
URL: https://github.com/apache/cloudstack/pull/2600
 
 
   

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

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

diff --git 
a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java
 
b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java
index 0bde79bbd7b..3448698cd37 100644
--- 
a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java
+++ 
b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java
@@ -166,15 +166,15 @@ public String getServiceOfferingName() {
 return serviceOfferingName;
 }
 
-public Boolean getOfferHa() {
+public Boolean isOfferHa() {
 return offerHa == null ? Boolean.FALSE : offerHa;
 }
 
-public Boolean GetLimitCpuUse() {
+public Boolean isLimitCpuUse() {
 return limitCpuUse == null ? Boolean.FALSE : limitCpuUse;
 }
 
-public Boolean getVolatileVm() {
+public Boolean isVolatileVm() {
 return isVolatile == null ? Boolean.FALSE : isVolatile;
 }
 
diff --git 
a/api/src/main/java/org/apache/cloudstack/api/command/admin/vlan/ListVlanIpRangesCmd.java
 
b/api/src/main/java/org/apache/cloudstack/api/command/admin/vlan/ListVlanIpRangesCmd.java
index d4f2d5ad611..d5edd301a66 100644
--- 
a/api/src/main/java/org/apache/cloudstack/api/command/admin/vlan/ListVlanIpRangesCmd.java
+++ 
b/api/src/main/java/org/apache/cloudstack/api/command/admin/vlan/ListVlanIpRangesCmd.java
@@ -119,7 +119,7 @@ public Long getNetworkId() {
 return networkId;
 }
 
-public Boolean getForVirtualNetwork() {
+public Boolean isForVirtualNetwork() {
 return forVirtualNetwork;
 }
 
diff --git 
a/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/UpdateVPCCmdByAdmin.java
 
b/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/UpdateVPCCmdByAdmin.java
index 8606c32a71e..d7761def204 100644
--- 
a/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/UpdateVPCCmdByAdmin.java
+++ 
b/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/UpdateVPCCmdByAdmin.java
@@ -34,7 +34,7 @@
 
 @Override
 public void execute(){
-Vpc result = _vpcService.updateVpc(getId(), getVpcName(), 
getDisplayText(), getCustomId(), getDisplayVpc());
+Vpc result = _vpcService.updateVpc(getId(), getVpcName(), 
getDisplayText(), getCustomId(), isDisplayVpc());
 if (result != null) {
 VpcResponse response = 
_responseGenerator.createVpcResponse(ResponseView.Full, result);
 response.setResponseName(getCommandName());
diff --git 
a/api/src/main/java/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java
 
b/api/src/main/java/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java
index 0a0e7a12076..d590081104a 100644
--- 
a/api/src/main/java/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java
+++ 
b/api/src/main/java/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java
@@ -133,11 +133,11 @@ public Long getAssociatedNetworkId() {
 return associatedNetworkId;
 }
 
-public Boolean getIsSourceNat() {
+public Boolean isSourceNat() {
 return isSourceNat;
 }
 
-public Boolean getIsStaticNat() {
+public Boolean isStaticNat() {
 return isStaticNat;
 }
 
diff --git 
a/api/src/main/java/org/apache/cloudstack/api/command/user/network/ListNetworksCmd.java
 
b/api/src/main/java/org/apache/cloudstack/api/command/user/network/ListNetworksCmd.java
index 041d6417044..a61c59734c6 100644
--- 
a/api/src/main/java/org/apache/cloudstack/api/command/user/network/ListNetworksCmd.java
+++ 
b/api/src/main/java/org/apache/cloudstack/api/command/user/network/ListNetworksCmd.java
@@ -123,11 +123,11 @@ public Long getPhysicalNetworkId() {
 return supportedServices;
 }
 
-public Boolean getRestartRequired() {
+public Boolean isRestartRequired() {
 return restartRequired;
 }
 
-public Boolean getSpecifyIpRanges() {
+public Boolean isSpecifyIpRanges() {
 return specifyIpRanges;
 }
 
diff --git 
a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java 
b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java
index 8161fb2564b..7fc54884099 

[jira] [Commented] (CLOUDSTACK-10364) Inconsiste "setXXX" method names.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449732#comment-16449732
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10364:
-

BruceKuiLiu closed pull request #2601: CLOUDSTACK-10364: Change the "setXXX" 
method names to "getXXX".
URL: https://github.com/apache/cloudstack/pull/2601
 
 
   

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

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

diff --git 
a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
 
b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
index 11b22c494f4..0951ca4e7ff 100644
--- 
a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
+++ 
b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
@@ -298,11 +298,11 @@ public void detach(LibvirtVMDef.InterfaceDef iface) {
 Script.runSimpleBashScript("test -d /sys/class/net/" + 
iface.getBrName() + "/brif/" + iface.getDevName() + " && brctl delif " + 
iface.getBrName() + " " + iface.getDevName());
 }
 
-private String setVnetBrName(String pifName, String vnetId) {
+private String getVnetBrName(String pifName, String vnetId) {
 return "br" + pifName + "-" + vnetId;
 }
 
-private String setVxnetBrName(String pifName, String vnetId) {
+private String getVxnetBrName(String pifName, String vnetId) {
 return "brvx-" + vnetId;
 }
 
@@ -317,9 +317,9 @@ private String createVnetBr(String vNetId, String pifKey, 
String protocol) throw
 }
 String brName = "";
 if (protocol.equals(Networks.BroadcastDomainType.Vxlan.scheme())) {
-brName = setVxnetBrName(nic, vNetId);
+brName = getVxnetBrName(nic, vNetId);
 } else {
-brName = setVnetBrName(nic, vNetId);
+brName = getVnetBrName(nic, vNetId);
 }
 createVnet(vNetId, nic, brName, protocol);
 return brName;


 


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


> Inconsiste "setXXX" method names.
> -
>
> Key: CLOUDSTACK-10364
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10364
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: KuiLIU
>Priority: Major
>
> The following two methods are named as "setXXX",
> actually, they are not setter but getter.
> Thus, they should be renamed as "getXXX".
> {code:java}
>  private String setVnetBrName(String pifName, String vnetId) {
> return "br" + pifName + "-" + vnetId;
> }
> private String setVxnetBrName(String pifName, String vnetId) {
> return "brvx-" + vnetId;
> }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10363) Inconsistent "getXXX" and "listXXX" method names.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10363?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449715#comment-16449715
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10363:
-

nitin-maharana commented on issue #2599: CLOUDSTACK-10363: Change the "getXXX" 
and "listXXX" method names to "…
URL: https://github.com/apache/cloudstack/pull/2599#issuecomment-383906750
 
 
   I think it would be good to merge them into one. Let's wait for others view.


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


> Inconsistent "getXXX" and "listXXX" method names.
> -
>
> Key: CLOUDSTACK-10363
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10363
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: KuiLIU
>Priority: Major
>
> The following methods are not direct getter or list.
> They try to find the target objects with the related arguments.
> So that, renaming them as "findXXX" should be more intuitive.
> {code:java}
> //cloudstack/plugins/network-elements/ovs/src/main/java/com/cloud/network/ovs/dao/OvsTunnelInterfaceDaoImpl.java
> @Override
> public OvsTunnelInterfaceVO getByHostAndLabel(long hostId, String label) {
> SearchCriteria sc = hostAndLabelSearch.create();
> sc.setParameters("host_id", hostId);
> sc.setParameters("label", label);
> return findOneBy(sc);
> }
> //cloudstack/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/PremiumSecondaryStorageManagerImpl.java
> private List listActiveCommands(long dcId, Date 
> cutTime) {
> SearchCriteria sc = activeCommandSearch.create();
> sc.setParameters("created", cutTime);
> sc.setJoinParameters("hostSearch", "dc", dcId);
> sc.setJoinParameters("hostSearch", "status", Status.Up);
> return _cmdExecLogDao.search(sc, null);
> }
> //cloudstack/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java
> private AsyncJobDispatcher getWakeupDispatcher(AsyncJob job) {
> if (_jobDispatchers != null) {
> List joinRecords = 
> _joinMapDao.listJoinRecords(job.getId());
> if (joinRecords.size() > 0) {
> AsyncJobJoinMapVO joinRecord = joinRecords.get(0);
> for (AsyncJobDispatcher dispatcher : _jobDispatchers) {
> if 
> (dispatcher.getName().equals(joinRecord.getWakeupDispatcher()))
> return dispatcher;
> }
> } else {
> s_logger.warn("job-" + job.getId() + " is scheduled for 
> wakeup run, but there is no joining info anymore");
> }
> }
> return null;
> }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10363) Inconsistent "getXXX" and "listXXX" method names.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10363?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449706#comment-16449706
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10363:
-

nitin-maharana commented on issue #2599: CLOUDSTACK-10363: Change the "getXXX" 
and "listXXX" method names to "…
URL: https://github.com/apache/cloudstack/pull/2599#issuecomment-383903358
 
 
   Hi @BruceKuiLiu, Thanks for the change, now the method names are more 
meaningful. Would be nice if you club all the similar PRs to one. (#2599, 
#2600, #2601, #2602)


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


> Inconsistent "getXXX" and "listXXX" method names.
> -
>
> Key: CLOUDSTACK-10363
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10363
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: KuiLIU
>Priority: Major
>
> The following methods are not direct getter or list.
> They try to find the target objects with the related arguments.
> So that, renaming them as "findXXX" should be more intuitive.
> {code:java}
> //cloudstack/plugins/network-elements/ovs/src/main/java/com/cloud/network/ovs/dao/OvsTunnelInterfaceDaoImpl.java
> @Override
> public OvsTunnelInterfaceVO getByHostAndLabel(long hostId, String label) {
> SearchCriteria sc = hostAndLabelSearch.create();
> sc.setParameters("host_id", hostId);
> sc.setParameters("label", label);
> return findOneBy(sc);
> }
> //cloudstack/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/PremiumSecondaryStorageManagerImpl.java
> private List listActiveCommands(long dcId, Date 
> cutTime) {
> SearchCriteria sc = activeCommandSearch.create();
> sc.setParameters("created", cutTime);
> sc.setJoinParameters("hostSearch", "dc", dcId);
> sc.setJoinParameters("hostSearch", "status", Status.Up);
> return _cmdExecLogDao.search(sc, null);
> }
> //cloudstack/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java
> private AsyncJobDispatcher getWakeupDispatcher(AsyncJob job) {
> if (_jobDispatchers != null) {
> List joinRecords = 
> _joinMapDao.listJoinRecords(job.getId());
> if (joinRecords.size() > 0) {
> AsyncJobJoinMapVO joinRecord = joinRecords.get(0);
> for (AsyncJobDispatcher dispatcher : _jobDispatchers) {
> if 
> (dispatcher.getName().equals(joinRecord.getWakeupDispatcher()))
> return dispatcher;
> }
> } else {
> s_logger.warn("job-" + job.getId() + " is scheduled for 
> wakeup run, but there is no joining info anymore");
> }
> }
> return null;
> }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10363) Inconsistent "getXXX" and "listXXX" method names.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10363?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449704#comment-16449704
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10363:
-

BruceKuiLiu commented on issue #2599: CLOUDSTACK-10363: Change the "getXXX" and 
"listXXX" method names to "…
URL: https://github.com/apache/cloudstack/pull/2599#issuecomment-383904258
 
 
   Hi @nitin-maharana ,
   I think that #2600, #2061, #2602 can be classified into three different 
changes.
   If it is necessary, I will merge them into one commit.


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


> Inconsistent "getXXX" and "listXXX" method names.
> -
>
> Key: CLOUDSTACK-10363
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10363
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: KuiLIU
>Priority: Major
>
> The following methods are not direct getter or list.
> They try to find the target objects with the related arguments.
> So that, renaming them as "findXXX" should be more intuitive.
> {code:java}
> //cloudstack/plugins/network-elements/ovs/src/main/java/com/cloud/network/ovs/dao/OvsTunnelInterfaceDaoImpl.java
> @Override
> public OvsTunnelInterfaceVO getByHostAndLabel(long hostId, String label) {
> SearchCriteria sc = hostAndLabelSearch.create();
> sc.setParameters("host_id", hostId);
> sc.setParameters("label", label);
> return findOneBy(sc);
> }
> //cloudstack/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/PremiumSecondaryStorageManagerImpl.java
> private List listActiveCommands(long dcId, Date 
> cutTime) {
> SearchCriteria sc = activeCommandSearch.create();
> sc.setParameters("created", cutTime);
> sc.setJoinParameters("hostSearch", "dc", dcId);
> sc.setJoinParameters("hostSearch", "status", Status.Up);
> return _cmdExecLogDao.search(sc, null);
> }
> //cloudstack/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java
> private AsyncJobDispatcher getWakeupDispatcher(AsyncJob job) {
> if (_jobDispatchers != null) {
> List joinRecords = 
> _joinMapDao.listJoinRecords(job.getId());
> if (joinRecords.size() > 0) {
> AsyncJobJoinMapVO joinRecord = joinRecords.get(0);
> for (AsyncJobDispatcher dispatcher : _jobDispatchers) {
> if 
> (dispatcher.getName().equals(joinRecord.getWakeupDispatcher()))
> return dispatcher;
> }
> } else {
> s_logger.warn("job-" + job.getId() + " is scheduled for 
> wakeup run, but there is no joining info anymore");
> }
> }
> return null;
> }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10363) Inconsistent "getXXX" and "listXXX" method names.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10363?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449699#comment-16449699
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10363:
-

nitin-maharana commented on issue #2599: CLOUDSTACK-10363: Change the "getXXX" 
and "listXXX" method names to "…
URL: https://github.com/apache/cloudstack/pull/2599#issuecomment-383903358
 
 
   Hi @BruceKuiLiu, Thanks for the change, now the method names are more 
meaningful. Would be nice if you club all the similar PRs to one. (#2600, 
#2601, #2602)


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


> Inconsistent "getXXX" and "listXXX" method names.
> -
>
> Key: CLOUDSTACK-10363
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10363
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: KuiLIU
>Priority: Major
>
> The following methods are not direct getter or list.
> They try to find the target objects with the related arguments.
> So that, renaming them as "findXXX" should be more intuitive.
> {code:java}
> //cloudstack/plugins/network-elements/ovs/src/main/java/com/cloud/network/ovs/dao/OvsTunnelInterfaceDaoImpl.java
> @Override
> public OvsTunnelInterfaceVO getByHostAndLabel(long hostId, String label) {
> SearchCriteria sc = hostAndLabelSearch.create();
> sc.setParameters("host_id", hostId);
> sc.setParameters("label", label);
> return findOneBy(sc);
> }
> //cloudstack/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/PremiumSecondaryStorageManagerImpl.java
> private List listActiveCommands(long dcId, Date 
> cutTime) {
> SearchCriteria sc = activeCommandSearch.create();
> sc.setParameters("created", cutTime);
> sc.setJoinParameters("hostSearch", "dc", dcId);
> sc.setJoinParameters("hostSearch", "status", Status.Up);
> return _cmdExecLogDao.search(sc, null);
> }
> //cloudstack/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java
> private AsyncJobDispatcher getWakeupDispatcher(AsyncJob job) {
> if (_jobDispatchers != null) {
> List joinRecords = 
> _joinMapDao.listJoinRecords(job.getId());
> if (joinRecords.size() > 0) {
> AsyncJobJoinMapVO joinRecord = joinRecords.get(0);
> for (AsyncJobDispatcher dispatcher : _jobDispatchers) {
> if 
> (dispatcher.getName().equals(joinRecord.getWakeupDispatcher()))
> return dispatcher;
> }
> } else {
> s_logger.warn("job-" + job.getId() + " is scheduled for 
> wakeup run, but there is no joining info anymore");
> }
> }
> return null;
> }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10360) Inconsistent method name

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10360?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449688#comment-16449688
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10360:
-

BruceKuiLiu commented on a change in pull request #2598: CLOUDSTACK-10360: 
Change the method name.
URL: https://github.com/apache/cloudstack/pull/2598#discussion_r183698884
 
 

 ##
 File path: 
framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java
 ##
 @@ -166,7 +166,7 @@ public ConfigurationDao global() {
 return _configDao;
 }
 
-public ScopedConfigStorage scoped(ConfigKey config) {
+public ScopedConfigStorage findStorage(ConfigKey config) {
 
 Review comment:
   Thanks, "findScopedConfigStorage" is more clear.


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


> Inconsistent method name
> 
>
> Key: CLOUDSTACK-10360
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10360
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: KuiLIU
>Priority: Major
>
> The following method is named as "scoped" that seems to whether the variable 
> config is scoped in _scopedStorages or not.
> Actually, the method tries to find a storage of which scope equals to the 
> scope of config.
> So that, the method name "findStorage" should be more clear than "scoped".
> {code:java}
> public ScopedConfigStorage scoped(ConfigKey config) {
> for (ScopedConfigStorage storage : _scopedStorages) {
> if (storage.getScope() == config.scope()) {
> return storage;
> }
> }
> throw new CloudRuntimeException("Unable to find config storage for 
> this scope: " + config.scope() + " for " + config.key());
> }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10199) Support requesting a specific IPv4 address in Basic Networking during Instance creation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449597#comment-16449597
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10199:
-

rafaelweingartner commented on a change in pull request #2595: 
CLOUDSTACK-10199: Support requesting a specific IPv4 address
URL: https://github.com/apache/cloudstack/pull/2595#discussion_r183678534
 
 

 ##
 File path: 
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
 ##
 @@ -883,6 +885,73 @@ public void saveExtraDhcpOptions(final String 
networkUuid, final Long nicId, fin
 return new Pair(vmNic, Integer.valueOf(deviceId));
 }
 
+/**
+ * If the requested IPv4 address from the NicProfile was configured then 
it configures the IPv4 address, Netmask and Gateway to deploy the VM with the 
requested IP. 
+ */
+protected void configureNicProfileBasedOnRequestedIp(NicProfile 
requestedNicProfile, NicProfile nicProfile, Network network) {
+String requestedIpv4Address = requestedNicProfile.getRequestedIPv4();
+if (requestedIpv4Address == null) {
 
 Review comment:
   You can extract lines 893-902 to a specific method that validates 
`requestedIpv4Address `


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


> Support requesting a specific IPv4 address in Basic Networking during 
> Instance creation
> ---
>
> Key: CLOUDSTACK-10199
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10199
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: API
> Environment: CloudStack 4.10
>Reporter: Wido den Hollander
>Priority: Major
>  Labels: basic-networking
>
> DirectPodBasedNetworkGuru does not support requesting a custom IP-Address 
> while creating a new NIC/Instance.
> {quote}
> Error 530: Does not support custom ip allocation at this time: 
> NicProfile[0-0-null-null-null
> {
>   "cserrorcode": 4250,
>   "errorcode": 530,
>   "errortext": "Does not support custom ip allocation at this time: 
> NicProfile[0-0-null-null-null",
>   "uuidList": []
> }
> {quote}
> Some use-cases prefer the ability to request the IPv4 address which the 
> Instance will get.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10199) Support requesting a specific IPv4 address in Basic Networking during Instance creation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449600#comment-16449600
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10199:
-

rafaelweingartner commented on a change in pull request #2595: 
CLOUDSTACK-10199: Support requesting a specific IPv4 address
URL: https://github.com/apache/cloudstack/pull/2595#discussion_r183678861
 
 

 ##
 File path: 
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
 ##
 @@ -883,6 +885,73 @@ public void saveExtraDhcpOptions(final String 
networkUuid, final Long nicId, fin
 return new Pair(vmNic, Integer.valueOf(deviceId));
 }
 
+/**
+ * If the requested IPv4 address from the NicProfile was configured then 
it configures the IPv4 address, Netmask and Gateway to deploy the VM with the 
requested IP. 
+ */
+protected void configureNicProfileBasedOnRequestedIp(NicProfile 
requestedNicProfile, NicProfile nicProfile, Network network) {
+String requestedIpv4Address = requestedNicProfile.getRequestedIPv4();
+if (requestedIpv4Address == null) {
+s_logger.debug(String.format("The requested [IPv4 address=%s] is 
null", requestedIpv4Address));
+return;
+}
+if 
(org.apache.commons.lang3.StringUtils.isBlank(requestedIpv4Address)) {
+throw new InvalidParameterValueException(String.format("The 
requested [IPv4 address='%s'] is empty or blank", requestedIpv4Address));
+}
+if (!NetUtils.isValidIp4(requestedIpv4Address)) {
+throw new InvalidParameterValueException(String.format("The 
requested [IPv4 address='%s'] is not a valid IP address", 
requestedIpv4Address));
+}
+
+VlanVO vlanVo = _vlanDao.findByNetworkId(network.getId());
+if (vlanVo == null) {
+throw new CloudRuntimeException(String.format("Trying to configure 
a Nic with the requested [IPv4='%s'] but cannot find a Vlan for the [network 
id='%s']",
+requestedIpv4Address, network.getId()));
+}
+
+acquireLockAndCheckIfIpv4IsFree(network, requestedIpv4Address);
 
 Review comment:
   Should not you validate the `vlanVo` before trying to acquire the IP?
   I mean, you are executing the validation on `vlanVo` afterwards.


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


> Support requesting a specific IPv4 address in Basic Networking during 
> Instance creation
> ---
>
> Key: CLOUDSTACK-10199
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10199
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: API
> Environment: CloudStack 4.10
>Reporter: Wido den Hollander
>Priority: Major
>  Labels: basic-networking
>
> DirectPodBasedNetworkGuru does not support requesting a custom IP-Address 
> while creating a new NIC/Instance.
> {quote}
> Error 530: Does not support custom ip allocation at this time: 
> NicProfile[0-0-null-null-null
> {
>   "cserrorcode": 4250,
>   "errorcode": 530,
>   "errortext": "Does not support custom ip allocation at this time: 
> NicProfile[0-0-null-null-null",
>   "uuidList": []
> }
> {quote}
> Some use-cases prefer the ability to request the IPv4 address which the 
> Instance will get.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10199) Support requesting a specific IPv4 address in Basic Networking during Instance creation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449598#comment-16449598
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10199:
-

rafaelweingartner commented on a change in pull request #2595: 
CLOUDSTACK-10199: Support requesting a specific IPv4 address
URL: https://github.com/apache/cloudstack/pull/2595#discussion_r183680786
 
 

 ##
 File path: engine/schema/src/main/java/com/cloud/dc/dao/VlanDaoImpl.java
 ##
 @@ -81,6 +81,22 @@ public VlanVO findByZoneAndVlanId(long zoneId, String 
vlanId) {
 sc.setParameters("vlanId", vlanId);
 return findOneBy(sc);
 }
+
+/**
+ * Returns a vlan by the network id.
+ */
+@Override
+public VlanVO findByNetworkId(long networkId) {
+List vlanVoList = listVlansByNetworkId(networkId);
+VlanVO vlanVo = null;
+for (VlanVO vlan : vlanVoList) {
+if (vlan.getRemoved() == null) {
 
 Review comment:
   I have the same doubt. It looks like a logic that should be somewhere else. 
If the network can use multiple `Vlan`, then should not you look for the 
correct one (the one that is in the same CIDR) as the requested IPv4?


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


> Support requesting a specific IPv4 address in Basic Networking during 
> Instance creation
> ---
>
> Key: CLOUDSTACK-10199
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10199
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: API
> Environment: CloudStack 4.10
>Reporter: Wido den Hollander
>Priority: Major
>  Labels: basic-networking
>
> DirectPodBasedNetworkGuru does not support requesting a custom IP-Address 
> while creating a new NIC/Instance.
> {quote}
> Error 530: Does not support custom ip allocation at this time: 
> NicProfile[0-0-null-null-null
> {
>   "cserrorcode": 4250,
>   "errorcode": 530,
>   "errortext": "Does not support custom ip allocation at this time: 
> NicProfile[0-0-null-null-null",
>   "uuidList": []
> }
> {quote}
> Some use-cases prefer the ability to request the IPv4 address which the 
> Instance will get.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10199) Support requesting a specific IPv4 address in Basic Networking during Instance creation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449599#comment-16449599
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10199:
-

rafaelweingartner commented on a change in pull request #2595: 
CLOUDSTACK-10199: Support requesting a specific IPv4 address
URL: https://github.com/apache/cloudstack/pull/2595#discussion_r183534529
 
 

 ##
 File path: 
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
 ##
 @@ -883,6 +885,73 @@ public void saveExtraDhcpOptions(final String 
networkUuid, final Long nicId, fin
 return new Pair(vmNic, Integer.valueOf(deviceId));
 }
 
+/**
+ * If the requested IPv4 address from the NicProfile was configured then 
it configures the IPv4 address, Netmask and Gateway to deploy the VM with the 
requested IP. 
+ */
+protected void configureNicProfileBasedOnRequestedIp(NicProfile 
requestedNicProfile, NicProfile nicProfile, Network network) {
+String requestedIpv4Address = requestedNicProfile.getRequestedIPv4();
+if (requestedIpv4Address == null) {
+s_logger.debug(String.format("The requested [IPv4 address=%s] is 
null", requestedIpv4Address));
 
 Review comment:
   If `requestedIpv4Address ` is null, then it is null.  I do not think that we 
need to log it then. I mean, the value of `requestedIpv4Address `


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


> Support requesting a specific IPv4 address in Basic Networking during 
> Instance creation
> ---
>
> Key: CLOUDSTACK-10199
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10199
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: API
> Environment: CloudStack 4.10
>Reporter: Wido den Hollander
>Priority: Major
>  Labels: basic-networking
>
> DirectPodBasedNetworkGuru does not support requesting a custom IP-Address 
> while creating a new NIC/Instance.
> {quote}
> Error 530: Does not support custom ip allocation at this time: 
> NicProfile[0-0-null-null-null
> {
>   "cserrorcode": 4250,
>   "errorcode": 530,
>   "errortext": "Does not support custom ip allocation at this time: 
> NicProfile[0-0-null-null-null",
>   "uuidList": []
> }
> {quote}
> Some use-cases prefer the ability to request the IPv4 address which the 
> Instance will get.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10365) Inconsistent boolean-related method names

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10365?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449581#comment-16449581
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10365:
-

BruceKuiLiu opened a new pull request #2602: CLOUDSTACK-10365: Change the 
"getXXX" boolean-related method names to…
URL: https://github.com/apache/cloudstack/pull/2602
 
 
   … "isXXX".
   
   These boolean-return methods are named as "getXXX".
   Other boolean-return methods are named as "isXXX".
   Considering there methods will return boolean values, it should be more 
clear and consistent to rename them as "isXXX".
   


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


> Inconsistent boolean-related method names
> -
>
> Key: CLOUDSTACK-10365
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10365
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: KuiLIU
>Priority: Major
>
> These boolean-return methods are named as "getXXX".
> Other boolean-return methods are named as "isXXX".
> Considering there methods will return boolean values, it should be more clear 
> to rename them as "isXXX".
> {code:java}
>  public boolean getEnableRRS() {
>  return enableRRS;
>  }
> public boolean getEnableRRS() {
>  return enableRRS;
>  }
>  public boolean getShrinkOk() {
>  return shrinkOk;
>  }
>  
>  public boolean getSourceNat() {
>  return sourceNat;
>  }
>  
>  public boolean getInternalLb() {
>  return internalLb;
>  }
>  
>  public boolean getOfferHA() {
>  return offerHA;
>  }
>  
>   public boolean getVolatileVm() {
>  return volatileVm;
>  }
>  
>  public boolean getIsUserDefined() {
>  return isUserDefined;
>  }
>  
>   public boolean getEnablePassword() {
>  return enablePassword;
>  }
>  
>  public boolean getEnableSshKey() {
>  return enableSshKey;
>  }
>  
>  public boolean getUuidTranslation() {
>  return _doUuidTranslation;
>  }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (CLOUDSTACK-10365) Inconsistent boolean-related method names

[KuiLIU (JIRA)]

KuiLIU created CLOUDSTACK-10365:
---

 Summary: Inconsistent boolean-related method names
 Key: CLOUDSTACK-10365
 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10365
 Project: CloudStack
  Issue Type: Improvement
  Security Level: Public (Anyone can view this level - this is the default.)
Reporter: KuiLIU


These boolean-return methods are named as "getXXX".
Other boolean-return methods are named as "isXXX".
Considering there methods will return boolean values, it should be more clear 
to rename them as "isXXX".

{code:java}
 public boolean getEnableRRS() {
 return enableRRS;
 }

public boolean getEnableRRS() {
 return enableRRS;
 }

 public boolean getShrinkOk() {
 return shrinkOk;
 }
 
 public boolean getSourceNat() {
 return sourceNat;
 }
 
 public boolean getInternalLb() {
 return internalLb;
 }
 
 public boolean getOfferHA() {
 return offerHA;
 }
 
  public boolean getVolatileVm() {
 return volatileVm;
 }
 
 public boolean getIsUserDefined() {
 return isUserDefined;
 }
 
  public boolean getEnablePassword() {
 return enablePassword;
 }
 
 public boolean getEnableSshKey() {
 return enableSshKey;
 }
 
 public boolean getUuidTranslation() {
 return _doUuidTranslation;
 }
{code}




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10360) Inconsistent method name

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10360?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449578#comment-16449578
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10360:
-

rafaelweingartner commented on a change in pull request #2598: 
CLOUDSTACK-10360: Change the method name.
URL: https://github.com/apache/cloudstack/pull/2598#discussion_r183675863
 
 

 ##
 File path: 
framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java
 ##
 @@ -166,7 +166,7 @@ public ConfigurationDao global() {
 return _configDao;
 }
 
-public ScopedConfigStorage scoped(ConfigKey config) {
+public ScopedConfigStorage findStorage(ConfigKey config) {
 
 Review comment:
   I would use something like `findScopedConfigStorage`. This `findStorage` can 
be confused with find storage pool.


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


> Inconsistent method name
> 
>
> Key: CLOUDSTACK-10360
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10360
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: KuiLIU
>Priority: Major
>
> The following method is named as "scoped" that seems to whether the variable 
> config is scoped in _scopedStorages or not.
> Actually, the method tries to find a storage of which scope equals to the 
> scope of config.
> So that, the method name "findStorage" should be more clear than "scoped".
> {code:java}
> public ScopedConfigStorage scoped(ConfigKey config) {
> for (ScopedConfigStorage storage : _scopedStorages) {
> if (storage.getScope() == config.scope()) {
> return storage;
> }
> }
> throw new CloudRuntimeException("Unable to find config storage for 
> this scope: " + config.scope() + " for " + config.key());
> }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10364) Inconsiste "setXXX" method names.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449540#comment-16449540
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10364:
-

BruceKuiLiu opened a new pull request #2601: CLOUDSTACK-10364: Change the 
"setXXX" method names to "getXXX".
URL: https://github.com/apache/cloudstack/pull/2601
 
 
   The two methods are named as "setXXX",
   actually, they are not setter but getter.
   Thus, they should be renamed as "getXXX".
   


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


> Inconsiste "setXXX" method names.
> -
>
> Key: CLOUDSTACK-10364
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10364
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: KuiLIU
>Priority: Major
>
> The following two methods are named as "setXXX",
> actually, they are not setter but getter.
> Thus, they should be renamed as "getXXX".
> {code:java}
>  private String setVnetBrName(String pifName, String vnetId) {
> return "br" + pifName + "-" + vnetId;
> }
> private String setVxnetBrName(String pifName, String vnetId) {
> return "brvx-" + vnetId;
> }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (CLOUDSTACK-10364) Inconsiste "setXXX" method names.

[KuiLIU (JIRA)]

KuiLIU created CLOUDSTACK-10364:
---

 Summary: Inconsiste "setXXX" method names.
 Key: CLOUDSTACK-10364
 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10364
 Project: CloudStack
  Issue Type: Improvement
  Security Level: Public (Anyone can view this level - this is the default.)
Reporter: KuiLIU


The following two methods are named as "setXXX",
actually, they are not setter but getter.
Thus, they should be renamed as "getXXX".
{code:java}
 private String setVnetBrName(String pifName, String vnetId) {
return "br" + pifName + "-" + vnetId;
}

private String setVxnetBrName(String pifName, String vnetId) {
return "brvx-" + vnetId;
}
{code}




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10362) Inconsistent method names

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10362?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449536#comment-16449536
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10362:
-

BruceKuiLiu opened a new pull request #2600: CLOUDSTACK-10362: Change the 
"getXXX" method names to "isXXX".
URL: https://github.com/apache/cloudstack/pull/2600
 
 
   These Boolean-return methods are named "getXXX",
   but other Boolean-return methods are named "isXXX", such as the following 
two methods.
   They will return boolean values, rename them as "isXXX" should be more clear 
than "getXXX".
   
   
api/src/main/java/org/apache/cloudstack/api/command/admin/vlan/ListVlanIpRangesCmd.java
  ```
 public Boolean getForVirtualNetwork() {
return forVirtualNetwork;
}
   ```
   
api/src/main/java/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java
   ```
   public Boolean isForVirtualNetwork() {
   return forVirtualNetwork;
   }
   ```


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


> Inconsistent method names
> -
>
> Key: CLOUDSTACK-10362
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10362
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: KuiLIU
>Priority: Major
>
> Some Boolean-return methods are named "getXXX",
> but other Boolean-return methods are named "isXXX", such as the following two 
> methods.
> They will return boolean values, rename them as "isXXX" should be more clear 
> than "getXXX".
> api/src/main/java/org/apache/cloudstack/api/command/admin/vlan/ListVlanIpRangesCmd.java
> {code:java}
>  public Boolean getForVirtualNetwork() {
>  return forVirtualNetwork;
>  }
> {code}
> api/src/main/java/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java
> {code:java}
> public Boolean isForVirtualNetwork() {
> return forVirtualNetwork;
> }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10363) Inconsistent "getXXX" and "listXXX" method names.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10363?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449533#comment-16449533
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10363:
-

BruceKuiLiu opened a new pull request #2599: CLOUDSTACK-10363: Change the 
"getXXX" and "listXXX" method names to "…
URL: https://github.com/apache/cloudstack/pull/2599
 
 
   …findXXX".
   
   These three methods are not direct getter or list.
   They try to find the target objects with the related arguments.
   So that, renaming them as "findXXX" should be more intuitive.
   


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


> Inconsistent "getXXX" and "listXXX" method names.
> -
>
> Key: CLOUDSTACK-10363
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10363
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: KuiLIU
>Priority: Major
>
> The following methods are not direct getter or list.
> They try to find the target objects with the related arguments.
> So that, renaming them as "findXXX" should be more intuitive.
> {code:java}
> //cloudstack/plugins/network-elements/ovs/src/main/java/com/cloud/network/ovs/dao/OvsTunnelInterfaceDaoImpl.java
> @Override
> public OvsTunnelInterfaceVO getByHostAndLabel(long hostId, String label) {
> SearchCriteria sc = hostAndLabelSearch.create();
> sc.setParameters("host_id", hostId);
> sc.setParameters("label", label);
> return findOneBy(sc);
> }
> //cloudstack/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/PremiumSecondaryStorageManagerImpl.java
> private List listActiveCommands(long dcId, Date 
> cutTime) {
> SearchCriteria sc = activeCommandSearch.create();
> sc.setParameters("created", cutTime);
> sc.setJoinParameters("hostSearch", "dc", dcId);
> sc.setJoinParameters("hostSearch", "status", Status.Up);
> return _cmdExecLogDao.search(sc, null);
> }
> //cloudstack/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java
> private AsyncJobDispatcher getWakeupDispatcher(AsyncJob job) {
> if (_jobDispatchers != null) {
> List joinRecords = 
> _joinMapDao.listJoinRecords(job.getId());
> if (joinRecords.size() > 0) {
> AsyncJobJoinMapVO joinRecord = joinRecords.get(0);
> for (AsyncJobDispatcher dispatcher : _jobDispatchers) {
> if 
> (dispatcher.getName().equals(joinRecord.getWakeupDispatcher()))
> return dispatcher;
> }
> } else {
> s_logger.warn("job-" + job.getId() + " is scheduled for 
> wakeup run, but there is no joining info anymore");
> }
> }
> return null;
> }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (CLOUDSTACK-10363) Inconsistent "getXXX" and "listXXX" method names.

[KuiLIU (JIRA)]

KuiLIU created CLOUDSTACK-10363:
---

 Summary: Inconsistent "getXXX" and "listXXX" method names.
 Key: CLOUDSTACK-10363
 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10363
 Project: CloudStack
  Issue Type: Improvement
  Security Level: Public (Anyone can view this level - this is the default.)
Reporter: KuiLIU


The following methods are not direct getter or list.
They try to find the target objects with the related arguments.
So that, renaming them as "findXXX" should be more intuitive.
{code:java}
//cloudstack/plugins/network-elements/ovs/src/main/java/com/cloud/network/ovs/dao/OvsTunnelInterfaceDaoImpl.java
@Override
public OvsTunnelInterfaceVO getByHostAndLabel(long hostId, String label) {
SearchCriteria sc = hostAndLabelSearch.create();
sc.setParameters("host_id", hostId);
sc.setParameters("label", label);
return findOneBy(sc);
}

//cloudstack/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/PremiumSecondaryStorageManagerImpl.java
private List listActiveCommands(long dcId, Date cutTime) {
SearchCriteria sc = activeCommandSearch.create();

sc.setParameters("created", cutTime);
sc.setJoinParameters("hostSearch", "dc", dcId);
sc.setJoinParameters("hostSearch", "status", Status.Up);

return _cmdExecLogDao.search(sc, null);
}

//cloudstack/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java
private AsyncJobDispatcher getWakeupDispatcher(AsyncJob job) {
if (_jobDispatchers != null) {
List joinRecords = 
_joinMapDao.listJoinRecords(job.getId());
if (joinRecords.size() > 0) {
AsyncJobJoinMapVO joinRecord = joinRecords.get(0);
for (AsyncJobDispatcher dispatcher : _jobDispatchers) {
if 
(dispatcher.getName().equals(joinRecord.getWakeupDispatcher()))
return dispatcher;
}
} else {
s_logger.warn("job-" + job.getId() + " is scheduled for wakeup 
run, but there is no joining info anymore");
}
}
return null;
}
{code}




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (CLOUDSTACK-10362) Inconsistent method names

[KuiLIU (JIRA)]

KuiLIU created CLOUDSTACK-10362:
---

 Summary: Inconsistent method names
 Key: CLOUDSTACK-10362
 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10362
 Project: CloudStack
  Issue Type: Improvement
  Security Level: Public (Anyone can view this level - this is the default.)
Reporter: KuiLIU


Some Boolean-return methods are named "getXXX",
but other Boolean-return methods are named "isXXX", such as the following two 
methods.
They will return boolean values, rename them as "isXXX" should be more clear 
than "getXXX".

api/src/main/java/org/apache/cloudstack/api/command/admin/vlan/ListVlanIpRangesCmd.java
{code:java}
 public Boolean getForVirtualNetwork() {
 return forVirtualNetwork;
 }
{code}
api/src/main/java/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java
{code:java}
public Boolean isForVirtualNetwork() {
return forVirtualNetwork;
}
{code}




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (CLOUDSTACK-10361) Inconsistent method name "getVlanAccount"

[KuiLIU (JIRA)]

KuiLIU created CLOUDSTACK-10361:
---

 Summary: Inconsistent method name "getVlanAccount"
 Key: CLOUDSTACK-10361
 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10361
 Project: CloudStack
  Issue Type: Improvement
  Security Level: Public (Anyone can view this level - this is the default.)
Reporter: KuiLIU


The following method is named "getVlanAccount", but the method is not a simple 
getter.
It tries to find an account with the provided vlanId.
Then, the method name "findVlanAccount" should be more intuitive than the 
"getVlanAccount".
{code:java}
@Override
public Account getVlanAccount(final long vlanId) {
final Vlan vlan = _vlanDao.findById(vlanId);

// if vlan is Virtual Account specific, get vlan information from the
// accountVlanMap; otherwise get account information
// from the network
if (vlan.getVlanType() == VlanType.VirtualNetwork) {
final List maps = 
_accountVlanMapDao.listAccountVlanMapsByVlan(vlanId);
if (maps != null && !maps.isEmpty()) {
return _accountMgr.getAccount(maps.get(0).getAccountId());
}
}

return null;
}
{code}




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10360) Inconsistent method name

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10360?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449455#comment-16449455
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10360:
-

BruceKuiLiu opened a new pull request #2598: CLOUDSTACK-10360: Change the 
method name.
URL: https://github.com/apache/cloudstack/pull/2598
 
 
   The method is named as "scoped" that seems to whether the variable config is 
scoped in _scopedStorages or not.
   Actually, the method tries to find a storage of which scope equals to the 
scope of config.
   So that, the method name "findStorage" should be more clear than "scoped".
   


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


> Inconsistent method name
> 
>
> Key: CLOUDSTACK-10360
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10360
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: KuiLIU
>Priority: Major
>
> The following method is named as "scoped" that seems to whether the variable 
> config is scoped in _scopedStorages or not.
> Actually, the method tries to find a storage of which scope equals to the 
> scope of config.
> So that, the method name "findStorage" should be more clear than "scoped".
> {code:java}
> public ScopedConfigStorage scoped(ConfigKey config) {
> for (ScopedConfigStorage storage : _scopedStorages) {
> if (storage.getScope() == config.scope()) {
> return storage;
> }
> }
> throw new CloudRuntimeException("Unable to find config storage for 
> this scope: " + config.scope() + " for " + config.key());
> }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (CLOUDSTACK-10360) Inconsistent method name

[KuiLIU (JIRA)]

KuiLIU created CLOUDSTACK-10360:
---

 Summary: Inconsistent method name
 Key: CLOUDSTACK-10360
 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10360
 Project: CloudStack
  Issue Type: Improvement
  Security Level: Public (Anyone can view this level - this is the default.)
Reporter: KuiLIU


The following method is named as "scoped" that seems to whether the variable 
config is scoped in _scopedStorages or not.
Actually, the method tries to find a storage of which scope equals to the scope 
of config.
So that, the method name "findStorage" should be more clear than "scoped".
{code:java}
public ScopedConfigStorage scoped(ConfigKey config) {
for (ScopedConfigStorage storage : _scopedStorages) {
if (storage.getScope() == config.scope()) {
return storage;
}
}

throw new CloudRuntimeException("Unable to find config storage for this 
scope: " + config.scope() + " for " + config.key());
}
{code}




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Resolved] (CLOUDSTACK-10359) Inconsistent method names

[KuiLIU (JIRA)]

 [ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10359?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

KuiLIU resolved CLOUDSTACK-10359.
-
Resolution: Fixed

> Inconsistent method names
> -
>
> Key: CLOUDSTACK-10359
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10359
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: KuiLIU
>Priority: Major
>
> The following two methods are named "getXXX".
> The two method are checking the status of variables.
> "getCustomized" is not as intuitive as "isCustomized".
> "getIsSystem" is not as intuitive as "isSystem" as well.
> {code:java}
> public boolean getCustomized() {
> return (cpuNumber == null || memory == null || cpuSpeed == null);
> }
>public Boolean getIsSystem() {
> return isSystem == null ? false : isSystem;
> }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10199) Support requesting a specific IPv4 address in Basic Networking during Instance creation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449379#comment-16449379
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10199:
-

marcaurele commented on a change in pull request #2595: CLOUDSTACK-10199: 
Support requesting a specific IPv4 address
URL: https://github.com/apache/cloudstack/pull/2595#discussion_r183617503
 
 

 ##
 File path: 
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
 ##
 @@ -883,6 +885,73 @@ public void saveExtraDhcpOptions(final String 
networkUuid, final Long nicId, fin
 return new Pair(vmNic, Integer.valueOf(deviceId));
 }
 
+/**
+ * If the requested IPv4 address from the NicProfile was configured then 
it configures the IPv4 address, Netmask and Gateway to deploy the VM with the 
requested IP. 
+ */
+protected void configureNicProfileBasedOnRequestedIp(NicProfile 
requestedNicProfile, NicProfile nicProfile, Network network) {
+String requestedIpv4Address = requestedNicProfile.getRequestedIPv4();
+if (requestedIpv4Address == null) {
+s_logger.debug(String.format("The requested [IPv4 address=%s] is 
null", requestedIpv4Address));
 
 Review comment:
   Could be simplified since `requestedIpv4Address` is always null to avoid 
parameter.


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


> Support requesting a specific IPv4 address in Basic Networking during 
> Instance creation
> ---
>
> Key: CLOUDSTACK-10199
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10199
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: API
> Environment: CloudStack 4.10
>Reporter: Wido den Hollander
>Priority: Major
>  Labels: basic-networking
>
> DirectPodBasedNetworkGuru does not support requesting a custom IP-Address 
> while creating a new NIC/Instance.
> {quote}
> Error 530: Does not support custom ip allocation at this time: 
> NicProfile[0-0-null-null-null
> {
>   "cserrorcode": 4250,
>   "errorcode": 530,
>   "errortext": "Does not support custom ip allocation at this time: 
> NicProfile[0-0-null-null-null",
>   "uuidList": []
> }
> {quote}
> Some use-cases prefer the ability to request the IPv4 address which the 
> Instance will get.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (CLOUDSTACK-10199) Support requesting a specific IPv4 address in Basic Networking during Instance creation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449378#comment-16449378
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10199:
-

marcaurele commented on a change in pull request #2595: CLOUDSTACK-10199: 
Support requesting a specific IPv4 address
URL: https://github.com/apache/cloudstack/pull/2595#discussion_r183617380
 
 

 ##
 File path: 
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
 ##
 @@ -883,6 +885,73 @@ public void saveExtraDhcpOptions(final String 
networkUuid, final Long nicId, fin
 return new Pair(vmNic, Integer.valueOf(deviceId));
 }
 
+/**
+ * If the requested IPv4 address from the NicProfile was configured then 
it configures the IPv4 address, Netmask and Gateway to deploy the VM with the 
requested IP. 
+ */
+protected void configureNicProfileBasedOnRequestedIp(NicProfile 
requestedNicProfile, NicProfile nicProfile, Network network) {
+String requestedIpv4Address = requestedNicProfile.getRequestedIPv4();
+if (requestedIpv4Address == null) {
+s_logger.debug(String.format("The requested [IPv4 address=%s] is 
null", requestedIpv4Address));
+return;
+}
+if 
(org.apache.commons.lang3.StringUtils.isBlank(requestedIpv4Address)) {
+throw new InvalidParameterValueException(String.format("The 
requested [IPv4 address='%s'] is empty or blank", requestedIpv4Address));
+}
+if (!NetUtils.isValidIp4(requestedIpv4Address)) {
+throw new InvalidParameterValueException(String.format("The 
requested [IPv4 address='%s'] is not a valid IP address", 
requestedIpv4Address));
+}
+
+VlanVO vlanVo = _vlanDao.findByNetworkId(network.getId());
+if (vlanVo == null) {
+throw new CloudRuntimeException(String.format("Trying to configure 
a Nic with the requested [IPv4='%s'] but cannot find a Vlan for the [network 
id='%s']",
+requestedIpv4Address, network.getId()));
+}
+
+acquireLockAndCheckIfIpv4IsFree(network, requestedIpv4Address);
+
+String ipv4Gateway = vlanVo.getVlanGateway();
+String ipv4Netmask = vlanVo.getVlanNetmask();
+
+if (org.apache.commons.lang3.StringUtils.isBlank(ipv4Gateway) || 
!NetUtils.isValidIp4(ipv4Gateway)) {
+throw new CloudRuntimeException(String.format("The 
[IPv4Gateway='%s'] from [VlanId='%s'] is not valid", ipv4Gateway, 
vlanVo.getId()));
+}
+if (org.apache.commons.lang3.StringUtils.isBlank(ipv4Netmask) || 
!NetUtils.isValidIp4Netmask(ipv4Netmask)) {
+throw new CloudRuntimeException(String.format("The 
[IPv4Netmask='%s'] from [VlanId='%s'] is not valid", ipv4Netmask, 
vlanVo.getId()));
+}
+
+nicProfile.setIPv4Address(requestedIpv4Address);
+nicProfile.setIPv4Gateway(ipv4Gateway);
+nicProfile.setIPv4Netmask(ipv4Netmask);
+
+if (nicProfile.getMacAddress() == null) {
+try {
+String macAddress = 
_networkModel.getNextAvailableMacAddressInNetwork(network.getId());
+nicProfile.setMacAddress(macAddress);
+} catch (InsufficientAddressCapacityException e) {
+e.printStackTrace();
 
 Review comment:
   Require a logger here and to throw an exception


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


> Support requesting a specific IPv4 address in Basic Networking during 
> Instance creation
> ---
>
> Key: CLOUDSTACK-10199
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10199
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: API
> Environment: CloudStack 4.10
>Reporter: Wido den Hollander
>Priority: Major
>  Labels: basic-networking
>
> DirectPodBasedNetworkGuru does not support requesting a custom IP-Address 
> while creating a new NIC/Instance.
> {quote}
> Error 530: Does not support custom ip allocation at this time: 
> NicProfile[0-0-null-null-null
> {
>   "cserrorcode": 4250,
>   "errorcode": 530,
>   "errortext": "Does not support custom ip allocation at this time: 
> NicProfile[0-0-null-null-null",
>   "uuidList": []
> }
> {quote}
> Some use-cases prefer the ability to request the IPv4 address which the 
> Instance will get.



--
This message was sent by Atlassian 

[jira] [Commented] (CLOUDSTACK-10199) Support requesting a specific IPv4 address in Basic Networking during Instance creation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449375#comment-16449375
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10199:
-

marcaurele commented on a change in pull request #2595: CLOUDSTACK-10199: 
Support requesting a specific IPv4 address
URL: https://github.com/apache/cloudstack/pull/2595#discussion_r183617175
 
 

 ##
 File path: 
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
 ##
 @@ -883,6 +885,73 @@ public void saveExtraDhcpOptions(final String 
networkUuid, final Long nicId, fin
 return new Pair(vmNic, Integer.valueOf(deviceId));
 }
 
+/**
+ * If the requested IPv4 address from the NicProfile was configured then 
it configures the IPv4 address, Netmask and Gateway to deploy the VM with the 
requested IP. 
+ */
+protected void configureNicProfileBasedOnRequestedIp(NicProfile 
requestedNicProfile, NicProfile nicProfile, Network network) {
+String requestedIpv4Address = requestedNicProfile.getRequestedIPv4();
+if (requestedIpv4Address == null) {
+s_logger.debug(String.format("The requested [IPv4 address=%s] is 
null", requestedIpv4Address));
+return;
+}
+if 
(org.apache.commons.lang3.StringUtils.isBlank(requestedIpv4Address)) {
+throw new InvalidParameterValueException(String.format("The 
requested [IPv4 address='%s'] is empty or blank", requestedIpv4Address));
+}
+if (!NetUtils.isValidIp4(requestedIpv4Address)) {
+throw new InvalidParameterValueException(String.format("The 
requested [IPv4 address='%s'] is not a valid IP address", 
requestedIpv4Address));
+}
+
+VlanVO vlanVo = _vlanDao.findByNetworkId(network.getId());
+if (vlanVo == null) {
+throw new CloudRuntimeException(String.format("Trying to configure 
a Nic with the requested [IPv4='%s'] but cannot find a Vlan for the [network 
id='%s']",
+requestedIpv4Address, network.getId()));
+}
+
+acquireLockAndCheckIfIpv4IsFree(network, requestedIpv4Address);
+
+String ipv4Gateway = vlanVo.getVlanGateway();
+String ipv4Netmask = vlanVo.getVlanNetmask();
+
+if (org.apache.commons.lang3.StringUtils.isBlank(ipv4Gateway) || 
!NetUtils.isValidIp4(ipv4Gateway)) {
+throw new CloudRuntimeException(String.format("The 
[IPv4Gateway='%s'] from [VlanId='%s'] is not valid", ipv4Gateway, 
vlanVo.getId()));
+}
+if (org.apache.commons.lang3.StringUtils.isBlank(ipv4Netmask) || 
!NetUtils.isValidIp4Netmask(ipv4Netmask)) {
+throw new CloudRuntimeException(String.format("The 
[IPv4Netmask='%s'] from [VlanId='%s'] is not valid", ipv4Netmask, 
vlanVo.getId()));
+}
+
+nicProfile.setIPv4Address(requestedIpv4Address);
+nicProfile.setIPv4Gateway(ipv4Gateway);
+nicProfile.setIPv4Netmask(ipv4Netmask);
+
+if (nicProfile.getMacAddress() == null) {
+try {
+String macAddress = 
_networkModel.getNextAvailableMacAddressInNetwork(network.getId());
+nicProfile.setMacAddress(macAddress);
+} catch (InsufficientAddressCapacityException e) {
+e.printStackTrace();
+}
+}
+}
+
+/**
+ *  Acquires lock of in table "user_ip_address" and checks if the 
requested IPv4 address is Free.
+ */
+protected void acquireLockAndCheckIfIpv4IsFree(Network network, String 
requestedIpv4Address) {
+IPAddressVO ipVO = 
_ipAddressDao.findByIpAndSourceNetworkId(network.getId(), requestedIpv4Address);
+if (ipVO == null) {
+throw new CloudRuntimeException(String.format("Cannot find 
IPAddressVO for guest [IPv4 address='%s'] and [network id=%s]", 
requestedIpv4Address, network.getId()));
+}
+IPAddressVO lockedIpVO = 
_ipAddressDao.acquireInLockTable(ipVO.getId());
+if (lockedIpVO.getState() != IPAddressVO.State.Free) {
+String ipState = lockedIpVO.getState().toString();
 
 Review comment:
   You don't need to have a variable holding the state, you can use 
`lockedIpVo.getState()` directly in the `String.format()` function, its value 
won't change after releasing the lock on the VO.


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


> Support requesting a specific IPv4 address in Basic Networking during 
> Instance creation
> ---
>
> Key: CLOUDSTACK-10199
>  

[jira] [Commented] (CLOUDSTACK-10199) Support requesting a specific IPv4 address in Basic Networking during Instance creation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449371#comment-16449371
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10199:
-

marcaurele commented on a change in pull request #2595: CLOUDSTACK-10199: 
Support requesting a specific IPv4 address
URL: https://github.com/apache/cloudstack/pull/2595#discussion_r183616840
 
 

 ##
 File path: 
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
 ##
 @@ -883,6 +885,73 @@ public void saveExtraDhcpOptions(final String 
networkUuid, final Long nicId, fin
 return new Pair(vmNic, Integer.valueOf(deviceId));
 }
 
+/**
+ * If the requested IPv4 address from the NicProfile was configured then 
it configures the IPv4 address, Netmask and Gateway to deploy the VM with the 
requested IP. 
+ */
+protected void configureNicProfileBasedOnRequestedIp(NicProfile 
requestedNicProfile, NicProfile nicProfile, Network network) {
+String requestedIpv4Address = requestedNicProfile.getRequestedIPv4();
+if (requestedIpv4Address == null) {
+s_logger.debug(String.format("The requested [IPv4 address=%s] is 
null", requestedIpv4Address));
+return;
+}
+if 
(org.apache.commons.lang3.StringUtils.isBlank(requestedIpv4Address)) {
+throw new InvalidParameterValueException(String.format("The 
requested [IPv4 address='%s'] is empty or blank", requestedIpv4Address));
+}
+if (!NetUtils.isValidIp4(requestedIpv4Address)) {
+throw new InvalidParameterValueException(String.format("The 
requested [IPv4 address='%s'] is not a valid IP address", 
requestedIpv4Address));
+}
+
+VlanVO vlanVo = _vlanDao.findByNetworkId(network.getId());
+if (vlanVo == null) {
+throw new CloudRuntimeException(String.format("Trying to configure 
a Nic with the requested [IPv4='%s'] but cannot find a Vlan for the [network 
id='%s']",
+requestedIpv4Address, network.getId()));
+}
+
+acquireLockAndCheckIfIpv4IsFree(network, requestedIpv4Address);
+
+String ipv4Gateway = vlanVo.getVlanGateway();
+String ipv4Netmask = vlanVo.getVlanNetmask();
+
+if (org.apache.commons.lang3.StringUtils.isBlank(ipv4Gateway) || 
!NetUtils.isValidIp4(ipv4Gateway)) {
+throw new CloudRuntimeException(String.format("The 
[IPv4Gateway='%s'] from [VlanId='%s'] is not valid", ipv4Gateway, 
vlanVo.getId()));
+}
+if (org.apache.commons.lang3.StringUtils.isBlank(ipv4Netmask) || 
!NetUtils.isValidIp4Netmask(ipv4Netmask)) {
+throw new CloudRuntimeException(String.format("The 
[IPv4Netmask='%s'] from [VlanId='%s'] is not valid", ipv4Netmask, 
vlanVo.getId()));
+}
+
+nicProfile.setIPv4Address(requestedIpv4Address);
+nicProfile.setIPv4Gateway(ipv4Gateway);
+nicProfile.setIPv4Netmask(ipv4Netmask);
+
+if (nicProfile.getMacAddress() == null) {
+try {
+String macAddress = 
_networkModel.getNextAvailableMacAddressInNetwork(network.getId());
+nicProfile.setMacAddress(macAddress);
+} catch (InsufficientAddressCapacityException e) {
+e.printStackTrace();
+}
+}
+}
+
+/**
+ *  Acquires lock of in table "user_ip_address" and checks if the 
requested IPv4 address is Free.
+ */
+protected void acquireLockAndCheckIfIpv4IsFree(Network network, String 
requestedIpv4Address) {
+IPAddressVO ipVO = 
_ipAddressDao.findByIpAndSourceNetworkId(network.getId(), requestedIpv4Address);
+if (ipVO == null) {
+throw new CloudRuntimeException(String.format("Cannot find 
IPAddressVO for guest [IPv4 address='%s'] and [network id=%s]", 
requestedIpv4Address, network.getId()));
+}
+IPAddressVO lockedIpVO = 
_ipAddressDao.acquireInLockTable(ipVO.getId());
 
 Review comment:
   `lockedIpVO` can be `null` 
(https://github.com/apache/cloudstack/blob/master/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java#L1049).
 Moreover, it would be better IMO to put the lock/release operation with a 
`try/finally` block to ensure to release the lock all the time (when it could 
get it).


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


> Support requesting a specific IPv4 address in Basic Networking during 
> Instance creation
> ---
>
> Key: CLOUDSTACK-10199
> 

[jira] [Commented] (CLOUDSTACK-10199) Support requesting a specific IPv4 address in Basic Networking during Instance creation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449362#comment-16449362
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10199:
-

marcaurele commented on a change in pull request #2595: CLOUDSTACK-10199: 
Support requesting a specific IPv4 address
URL: https://github.com/apache/cloudstack/pull/2595#discussion_r183615564
 
 

 ##
 File path: 
engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java
 ##
 @@ -159,4 +177,256 @@ public void testDontRemoveDhcpServiceWhenNotProvided() {
 verify(testOrchastrator._ntwkSrvcDao, 
never()).getProviderForServiceInNetwork(network.getId(), Service.Dhcp);
 verify(testOrchastrator._networksDao, 
times(1)).findById(nic.getNetworkId());
 }
+
+@Test
+public void testConfigureNicProfileBasedOnRequestedIpTestMacNull() {
+Network network = mock(Network.class);
+NicProfile requestedNicProfile = new NicProfile();
+NicProfile nicProfile = Mockito.spy(new NicProfile());
+
+configureTestConfigureNicProfileBasedOnRequestedIpTests(nicProfile, 
0l, false, IPAddressVO.State.Free, "192.168.100.1", "255.255.255.0", 
"00-88-14-4D-4C-FB",
+requestedNicProfile, null, "192.168.100.150");
+
+
testOrchastrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, 
nicProfile, network);
+
+verifyAndAssert("192.168.100.150", "192.168.100.1", "255.255.255.0", 
nicProfile, 1, 1);
+}
+
+@Test
+public void 
testConfigureNicProfileBasedOnRequestedIpTestNicProfileMacNotNull() {
+Network network = mock(Network.class);
+NicProfile requestedNicProfile = new NicProfile();
+NicProfile nicProfile = Mockito.spy(new NicProfile());
+
+configureTestConfigureNicProfileBasedOnRequestedIpTests(nicProfile, 
0l, false, IPAddressVO.State.Free, "192.168.100.1", "255.255.255.0", 
"00-88-14-4D-4C-FB",
+requestedNicProfile, "00-88-14-4D-4C-FB", "192.168.100.150");
+
+
testOrchastrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, 
nicProfile, network);
+
+verifyAndAssert("192.168.100.150", "192.168.100.1", "255.255.255.0", 
nicProfile, 1, 0);
+}
+
+@Test
+public void testConfigureNicProfileBasedOnRequestedIpTestRequestedIpNull() 
{
+testConfigureNicProfileBasedOnRequestedIpTestRequestedIp(null, "", 
false);
+}
+
+@Test
+public void 
testConfigureNicProfileBasedOnRequestedIpTestRequestedIpIsBlank() {
+testConfigureNicProfileBasedOnRequestedIpTestRequestedIp("", "The 
requested [IPv4 address=''] is empty or blank", true);
+}
+
+@Test
+public void 
testConfigureNicProfileBasedOnRequestedIpTestRequestedIpIsNotValid() {
+testConfigureNicProfileBasedOnRequestedIpTestRequestedIp("123", "The 
requested [IPv4 address='123'] is not a valid IP address", true);
+}
+
+private void 
testConfigureNicProfileBasedOnRequestedIpTestRequestedIp(String 
requestedIpv4Address, String exceptionAssert, boolean expectException) {
+Network network = mock(Network.class);
+NicProfile requestedNicProfile = new NicProfile();
+NicProfile nicProfile = Mockito.spy(new NicProfile());
+
+configureTestConfigureNicProfileBasedOnRequestedIpTests(nicProfile, 
0l, false, IPAddressVO.State.Free, "192.168.100.1", "255.255.255.0", 
"00-88-14-4D-4C-FB",
+requestedNicProfile, null, requestedIpv4Address);
+try {
+
testOrchastrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, 
nicProfile, network);
+} catch (InvalidParameterValueException e) {
+assertEquals(exceptionAssert, e.getMessage());
+hasException = true;
+}
+assertEquals(expectException, hasException);
+
+verifyAndAssert(null, null, null, nicProfile, 0, 0);
+}
+
+@Test
+public void testConfigureNicProfileBasedOnRequestedIpTestGatewayIsBlank() {
+testConfigureNicProfileBasedOnRequestedIpTestGateway("");
+}
+
+@Test
+public void 
testConfigureNicProfileBasedOnRequestedIpTestGatewayIsNotValid() {
+testConfigureNicProfileBasedOnRequestedIpTestGateway("123");
+}
+
+@Test
+public void testConfigureNicProfileBasedOnRequestedIpTestGatewayIsNull() {
+testConfigureNicProfileBasedOnRequestedIpTestGateway(null);
+}
+
+private void testConfigureNicProfileBasedOnRequestedIpTestGateway(String 
ipv4Gateway) {
+Network network = mock(Network.class);
+NicProfile requestedNicProfile = new NicProfile();
+NicProfile nicProfile = Mockito.spy(new NicProfile());
+
+configureTestConfigureNicProfileBasedOnRequestedIpTests(nicProfile, 
0l, false, IPAddressVO.State.Free, ipv4Gateway, "255.255.255.0", 
"00-88-14-4D-4C-FB",
+requestedNicProfile, 

[jira] [Commented] (CLOUDSTACK-10199) Support requesting a specific IPv4 address in Basic Networking during Instance creation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449358#comment-16449358
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10199:
-

marcaurele commented on a change in pull request #2595: CLOUDSTACK-10199: 
Support requesting a specific IPv4 address
URL: https://github.com/apache/cloudstack/pull/2595#discussion_r183615227
 
 

 ##
 File path: engine/schema/src/main/java/com/cloud/dc/dao/VlanDaoImpl.java
 ##
 @@ -81,6 +81,22 @@ public VlanVO findByZoneAndVlanId(long zoneId, String 
vlanId) {
 sc.setParameters("vlanId", vlanId);
 return findOneBy(sc);
 }
+
+/**
+ * Returns a vlan by the network id.
+ */
+@Override
+public VlanVO findByNetworkId(long networkId) {
+List vlanVoList = listVlansByNetworkId(networkId);
+VlanVO vlanVo = null;
+for (VlanVO vlan : vlanVoList) {
+if (vlan.getRemoved() == null) {
 
 Review comment:
   `listVlansByNetworkId` won't return removed entry since it's using the 
normal DAO `listBy()`.
   Another question: why picking the first one when the result could return 
many?


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


> Support requesting a specific IPv4 address in Basic Networking during 
> Instance creation
> ---
>
> Key: CLOUDSTACK-10199
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10199
> Project: CloudStack
>  Issue Type: Improvement
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: API
> Environment: CloudStack 4.10
>Reporter: Wido den Hollander
>Priority: Major
>  Labels: basic-networking
>
> DirectPodBasedNetworkGuru does not support requesting a custom IP-Address 
> while creating a new NIC/Instance.
> {quote}
> Error 530: Does not support custom ip allocation at this time: 
> NicProfile[0-0-null-null-null
> {
>   "cserrorcode": 4250,
>   "errorcode": 530,
>   "errortext": "Does not support custom ip allocation at this time: 
> NicProfile[0-0-null-null-null",
>   "uuidList": []
> }
> {quote}
> Some use-cases prefer the ability to request the IPv4 address which the 
> Instance will get.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)