nvazquez commented on a change in pull request #5114:
URL: https://github.com/apache/cloudstack/pull/5114#discussion_r686466300
##########
File path: server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
##########
@@ -2083,8 +2084,15 @@
if (clusterId != null) {
sc.setParameters("clusterId", clusterId);
}
- // Don't return DomR and ConsoleProxy volumes
- sc.setParameters("type", VirtualMachine.Type.ConsoleProxy,
VirtualMachine.Type.SecondaryStorageVm, VirtualMachine.Type.DomainRouter);
+
+ sc.setParameters("type", VirtualMachine.Type.User);
+
+ // Display all volumes for ROOT admin
+ if (forSystemVms && caller.getType() == Account.ACCOUNT_TYPE_ADMIN) {
Review comment:
At this point `forSystemVms` may produce a `NullPointerException`
##########
File path:
api/src/main/java/org/apache/cloudstack/api/command/user/volume/ListVolumesCmd.java
##########
@@ -136,6 +140,10 @@ public String getStorageId() {
return storageId;
}
+ public Boolean getForsystemvm() {
+ return forsystemvm;
Review comment:
To prevent the NPE I would propose to change this for:
`BooleanUtils.isTrue(forsystemvm)`
##########
File path:
api/src/main/java/org/apache/cloudstack/api/command/user/volume/ListVolumesCmd.java
##########
@@ -92,6 +92,10 @@
@Parameter(name = ApiConstants.STATE, type = CommandType.STRING,
description = "state of the volume. Possible values are: Ready, Allocated,
Destroy, Expunging, Expunged.")
private String state;
+ @Parameter(name = ApiConstants.FOR_SYSTEM_VMS, type = CommandType.BOOLEAN,
description = "list volumes of systemvms and routervm", since = "4.16",
authorized = {
+ RoleType.Admin})
+ private Boolean forsystemvm;
Review comment:
Minor: can you rename this field and its getter to camelcase?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
