apache/cloudstack
1
0
Fork 0
mirror of https://github.com/apache/cloudstack.git synced 2025-11-03 04:12:31 +01:00
Code Issues Packages Projects Releases Wiki Activity

bug 9192: multiple improvementes to listVms command.

Browse Source 
1) No longer do multiple searches involving "domain" table; only one join with domain is being done.
2) Do join with domain table only when command is executed by domainAdmin
3) Added index for "path" field in "domain" table
4) No longer do joins with account table as account_id is already present in vm_instance table.
This commit is contained in:
alena 2011-03-29 16:42:25 -07:00
parent bdb42c306c
commit 2af46789cb
7 changed files with 84 additions and 127 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
api/src/com/cloud/api/commands/ListVMsCmd.java
View File

@ -47,7 +47,7 @@ public class ListVMsCmd extends BaseListCmd {
@Parameter(name=ApiConstants.DOMAIN_ID, type=CommandType.LONG, description="the domain ID. If used with the account parameter, lists virtual machines for the specified account in this domain.")
private Long domainId;
@Parameter(name=ApiConstants.IS_RECURSIVE, type=CommandType.BOOLEAN, description="defaults to false, but if true, lists all vms from the parent specified by the domain id till leaves.")
@Parameter(name=ApiConstants.IS_RECURSIVE, type=CommandType.BOOLEAN, description="Must be used with domainId parameter. Defaults to false, but if true, lists all vms from the parent specified by the domain id till leaves.")
private Boolean recursive;
@Parameter(name=ApiConstants.GROUP_ID, type=CommandType.LONG, description="the group ID")

6
server/src/com/cloud/acl/DomainChecker.java
View File

@ -53,7 +53,11 @@ public class DomainChecker extends AdapterBase implements SecurityChecker {
throw new PermissionDeniedException(account + " is disabled.");
}
if (!_domainDao.isChildDomain(account.getDomainId(), domain.getId())) {
if (account.getType() == Account.ACCOUNT_TYPE_NORMAL) {
if (account.getDomainId() != domain.getId()) {
throw new PermissionDeniedException(account + " does not have permission to operate within " + domain);
}
} else if (!_domainDao.isChildDomain(account.getDomainId(), domain.getId())) {
throw new PermissionDeniedException(account + " does not have permission to operate within " + domain);
}

2
server/src/com/cloud/api/ApiDBUtils.java
View File

@ -299,7 +299,7 @@ public class ApiDBUtils {
/////////////////////////////////////////////////////////////
public static Account findAccountById(Long accountId) {
return _accountDao.findById(accountId);
return _accountDao.findByIdIncludingRemoved(accountId);
}
public static Account findAccountByIdIncludingRemoved(Long accountId) {

9
server/src/com/cloud/api/ApiResponseHelper.java
View File

@ -995,17 +995,10 @@ public class ApiResponseHelper implements ResponseGenerator {
public UserVmResponse createUserVmResponse(UserVm userVm) {
UserVmResponse userVmResponse = new UserVmResponse();
Account acct = ApiDBUtils.findAccountById(Long.valueOf(userVm.getAccountId()));
// FIXME - this check should be done in searchForUserVm method in
// ManagementServerImpl;
// otherwise the number of vms returned is not going to match pageSize
// request parameter
if ((acct != null) && (acct.getRemoved() == null)) {
if (acct != null) {
userVmResponse.setAccountName(acct.getAccountName());
userVmResponse.setDomainId(acct.getDomainId());
userVmResponse.setDomainName(ApiDBUtils.findDomainById(acct.getDomainId()).getName());
} else {
return null; // the account has been deleted, skip this VM in the
// response
}
userVmResponse.setId(userVm.getId());

184
server/src/com/cloud/vm/UserVmManagerImpl.java
View File

@ -2621,58 +2621,57 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
@Override
public List<UserVmVO> searchForUserVMs(ListVMsCmd cmd) throws InvalidParameterValueException, PermissionDeniedException {
Account account = UserContext.current().getCaller();
Account caller = UserContext.current().getCaller();
Long domainId = cmd.getDomainId();
String accountName = cmd.getAccountName();
Long accountId = null;
Boolean isRecursive = cmd.isRecursive();
String hypervisor = cmd.getHypervisor();
List<DomainVO> domainsToSearchForVms = new ArrayList<DomainVO>();
boolean isAdmin = false;
Long accountId = null;
String path = null;
if ((account == null) || isAdmin(account.getType())) {
isAdmin = true;
if (domainId != null) {
if ((account != null) && !_domainDao.isChildDomain(account.getDomainId(), domainId)) {
throw new PermissionDeniedException("Invalid domain id (" + domainId + ") given, unable to list virtual machines.");
}
if (accountName != null) {
account = _accountDao.findActiveAccount(accountName, domainId);
if (account == null) {
throw new InvalidParameterValueException("Unable to find account " + accountName + " in domain " + domainId);
}
accountId = account.getId();
}
}
if (account.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) {
DomainVO domain = _domainDao.findById(account.getDomainId());
if (domain != null) {
path = domain.getPath();
}
}
} else {
accountId = account.getId();
}
if(isRecursive == null) {
isRecursive = false;
}
if(isRecursive && domainId != null) {
DomainVO parentDomain = _domainDao.findById(domainId);
if(parentDomain.getName().equals("ROOT")) {
domainsToSearchForVms.addAll(_domainDao.listAll());
return recursivelySearchForVms(cmd, path, isAdmin, domainsToSearchForVms, accountId);
}else {
domainsToSearchForVms.add(parentDomain);
domainsToSearchForVms.addAll(_domainDao.findAllChildren(parentDomain.getPath(), parentDomain.getId()));
return recursivelySearchForVms(cmd, path, isAdmin, domainsToSearchForVms, accountId);
}
} else if(isRecursive && domainId == null){
if (isRecursive != null && isRecursive && domainId == null){
throw new InvalidParameterValueException("Please enter a parent domain id for listing vms recursively");
}
if (domainId != null) {
//Verify if user is authorized to see instances belonging to the domain
DomainVO domain = _domainDao.findById(domainId);
if (domain == null) {
throw new InvalidParameterValueException("Domain id=" + domainId + " doesn't exist");
}
_accountMgr.checkAccess(caller, domain);
}
boolean isAdmin = false;
if (_accountMgr.isAdmin(caller.getType())) {
isAdmin = true;
if (accountName != null && domainId != null) {
caller = _accountDao.findActiveAccount(accountName, domainId);
if (caller == null) {
throw new InvalidParameterValueException("Unable to find account " + accountName + " in domain " + domainId);
}
accountId = caller.getId();
}
if (caller.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) {
if (isRecursive == null) {
DomainVO domain = _domainDao.findById(caller.getDomainId());
path = domain.getPath();
}
}
} else {
accountId = caller.getId();
}
if (isRecursive != null && isRecursive && isAdmin) {
if (isRecursive) {
DomainVO domain = _domainDao.findById(domainId);
path = domain.getPath();
domainId = null;
}
}
Criteria c = new Criteria("id", Boolean.TRUE, cmd.getStartIndex(), cmd.getPageSizeVal());
c.addCriteria(Criteria.KEYWORD, cmd.getKeyword());
c.addCriteria(Criteria.ID, cmd.getId());
@ -2683,19 +2682,22 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
c.addCriteria(Criteria.FOR_VIRTUAL_NETWORK, cmd.getForVirtualNetwork());
c.addCriteria(Criteria.NETWORKID, cmd.getNetworkId());
if (domainId != null) {
c.addCriteria(Criteria.DOMAINID, domainId);
}
if (path != null) {
c.addCriteria(Criteria.PATH, path);
}
if (HypervisorType.getType(hypervisor) != HypervisorType.None){
c.addCriteria(Criteria.HYPERVISOR, hypervisor);
}else if (hypervisor != null){
} else if (hypervisor != null){
throw new InvalidParameterValueException("Invalid HypervisorType " + hypervisor);
}
// ignore these search requests if it's not an admin
if (isAdmin == true) {
c.addCriteria(Criteria.DOMAINID, domainId);
if (isAdmin) {
c.addCriteria(Criteria.PODID, cmd.getPodId());
c.addCriteria(Criteria.HOSTID, cmd.getHostId());
c.addCriteria(Criteria.STORAGE_ID, cmd.getStorageId());
@ -2709,60 +2711,14 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
return searchForUserVMs(c);
}
private List<UserVmVO> recursivelySearchForVms(ListVMsCmd cmd, String path, boolean isAdmin, List<DomainVO> domainToSearchWithin, Long accountId) {
List<UserVmVO> result = new ArrayList<UserVmVO>();
String hypervisor = cmd.getHypervisor();
for(DomainVO domain : domainToSearchWithin) {
Criteria c = new Criteria("id", Boolean.TRUE, cmd.getStartIndex(), cmd.getPageSizeVal());
c.addCriteria(Criteria.KEYWORD, cmd.getKeyword());
c.addCriteria(Criteria.ID, cmd.getId());
c.addCriteria(Criteria.NAME, cmd.getInstanceName());
c.addCriteria(Criteria.STATE, cmd.getState());
c.addCriteria(Criteria.DATACENTERID, cmd.getZoneId());
c.addCriteria(Criteria.GROUPID, cmd.getGroupId());
c.addCriteria(Criteria.FOR_VIRTUAL_NETWORK, cmd.getForVirtualNetwork());
c.addCriteria(Criteria.NETWORKID, cmd.getNetworkId());
if (path != null) {
c.addCriteria(Criteria.PATH, path);
}
// ignore these search requests if it's not an admin
if (isAdmin == true) {
c.addCriteria(Criteria.DOMAINID, domain.getId());
c.addCriteria(Criteria.PODID, cmd.getPodId());
c.addCriteria(Criteria.HOSTID, cmd.getHostId());
}
if (HypervisorType.getType(hypervisor) != HypervisorType.None){
c.addCriteria(Criteria.HYPERVISOR, hypervisor);
}else if (hypervisor != null){
throw new InvalidParameterValueException("Invalid HypervisorType " + hypervisor);
}
if (accountId != null) {
c.addCriteria(Criteria.ACCOUNTID, new Object[] {accountId});
}
c.addCriteria(Criteria.ISADMIN, isAdmin);
result.addAll(searchForUserVMs(c));
}
return result;
}
@Override
public List<UserVmVO> searchForUserVMs(Criteria c) {
Filter searchFilter = new Filter(UserVmVO.class, c.getOrderBy(), c.getAscending(), c.getOffset(), c.getLimit());
SearchBuilder<UserVmVO> sb = _vmDao.createSearchBuilder();
// some criteria matter for generating the join condition
Object[] accountIds = (Object[]) c.getCriteria(Criteria.ACCOUNTID);
Object domainId = c.getCriteria(Criteria.DOMAINID);
// get the rest of the criteria
Object id = c.getCriteria(Criteria.ID);
Object name = c.getCriteria(Criteria.NAME);
Object state = c.getCriteria(Criteria.STATE);
@ -2793,21 +2749,14 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
sb.and("hypervisorType", sb.entity().getHypervisorType(), SearchCriteria.Op.EQ);
sb.and("hostIdEQ", sb.entity().getHostId(), SearchCriteria.Op.EQ);
sb.and("hostIdIN", sb.entity().getHostId(), SearchCriteria.Op.IN);
sb.and("domainId", sb.entity().getDomainId(), SearchCriteria.Op.EQ);
if (domainId != null || path != null) {
// if accountId isn't specified, we can do a domain match for the admin case
if (path != null) {
SearchBuilder<DomainVO> domainSearch = _domainDao.createSearchBuilder();
domainSearch.and("id", domainSearch.entity().getId(), SearchCriteria.Op.EQ);
domainSearch.and("path", domainSearch.entity().getPath(), SearchCriteria.Op.LIKE);
sb.join("domainSearch", domainSearch, sb.entity().getDomainId(), domainSearch.entity().getId(), JoinBuilder.JoinType.INNER);
}
if (storageId != null) {
SearchBuilder<VolumeVO> volumeSearch = _volsDao.createSearchBuilder();
volumeSearch.and("poolId", volumeSearch.entity().getPoolId(), SearchCriteria.Op.EQ);
sb.join("volumeSearch", volumeSearch, sb.entity().getId(), volumeSearch.entity().getInstanceId(), JoinBuilder.JoinType.INNER);
}
if (groupId != null && (Long)groupId == -1) {
SearchBuilder<InstanceGroupVMMapVO> vmSearch = _groupVMMapDao.createSearchBuilder();
vmSearch.and("instanceId", vmSearch.entity().getInstanceId(), SearchCriteria.Op.EQ);
@ -2829,13 +2778,14 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
sb.join("nicSearch", nicSearch, sb.entity().getId(), nicSearch.entity().getInstanceId(), JoinBuilder.JoinType.INNER);
}
SearchBuilder<AccountVO> accountRemoved = _accountDao.createSearchBuilder();
accountRemoved.and("accountremoved", accountRemoved.entity().getRemoved(), SearchCriteria.Op.NULL);
sb.join("accountRemoved", accountRemoved, sb.entity().getAccountId(), accountRemoved.entity().getId(), JoinBuilder.JoinType.INNER);
if (storageId != null) {
SearchBuilder<VolumeVO> volumeSearch = _volsDao.createSearchBuilder();
volumeSearch.and("poolId", volumeSearch.entity().getPoolId(), SearchCriteria.Op.EQ);
sb.join("volumeSearch", volumeSearch, sb.entity().getId(), volumeSearch.entity().getInstanceId(), JoinBuilder.JoinType.INNER);
}
// populate the search criteria with the values passed in
SearchCriteria<UserVmVO> sc = sb.create();
SearchCriteria<UserVmVO> sc = sb.create();
if (groupId != null && (Long)groupId == -1){
sc.setJoinParameters("vmSearch", "instanceId", (Object)null);
} else if (groupId != null ) {
@ -2855,6 +2805,7 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
if (id != null) {
sc.setParameters("id", id);
}
if (accountIds != null) {
if (accountIds.length == 1) {
if (accountIds[0] != null) {
@ -2863,18 +2814,16 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
} else {
sc.setParameters("accountIdIN", accountIds);
}
} else if (domainId != null) {
sc.setJoinParameters("domainSearch", "id", domainId);
}
if (domainId != null) {
sc.setParameters("domainId", domainId);
}
if (path != null) {
sc.setJoinParameters("domainSearch", "path", path + "%");
}
if (storageId != null) {
sc.setJoinParameters("volumeSearch", "poolId", storageId);
}
if (networkId != null) {
sc.setJoinParameters("nicSearch", "networkId", networkId);
}
@ -2882,6 +2831,7 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
if (name != null) {
sc.setParameters("name", "%" + name + "%");
}
if (state != null) {
if (notState != null && (Boolean) notState == true) {
sc.setParameters("stateNEQ", state);
@ -2893,6 +2843,8 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
if (hypervisor != null){
sc.setParameters("hypervisorType", hypervisor);
}
//Don't show Destroyed and Expunging vms to the end user
if ((isAdmin != null) && ((Boolean) isAdmin != true)) {
sc.setParameters("stateNIN", "Destroyed", "Expunging");
}
@ -2900,7 +2852,7 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
if (zone != null) {
sc.setParameters("dataCenterId", zone);
if(state == null) {
if (state == null) {
sc.setParameters("stateNEQ", "Destroyed");
}
}
@ -2929,6 +2881,10 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
}
}
}
if (storageId != null) {
sc.setJoinParameters("volumeSearch", "poolId", storageId);
}
return _vmDao.search(sc, searchFilter);
}

3
setup/db/create-schema.sql
View File

@ -967,7 +967,8 @@ CREATE TABLE `cloud`.`domain` (
`removed` datetime COMMENT 'date removed',
`state` char(32) NOT NULL default 'Active' COMMENT 'state of the domain',
PRIMARY KEY (`id`),
UNIQUE (parent, name, removed)
UNIQUE (parent, name, removed),
INDEX `i_domain__path`(`path`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
CREATE TABLE `cloud`.`account` (

5
setup/db/schema-222to224.sql
View File

@ -57,4 +57,7 @@ ALTER TABLE `cloud`.`cluster` ADD INDEX `i_cluster__allocation_state`(`allocatio
ALTER TABLE `cloud`.`host_pod_ref` ADD COLUMN `allocation_state` varchar(32) NOT NULL DEFAULT 'Enabled';
ALTER TABLE `cloud`.`host_pod_ref` ADD INDEX `i_host_pod_ref__allocation_state`(`allocation_state`);
ALTER TABLE `cloud`.`host` ADD COLUMN `allocation_state` varchar(32) NOT NULL DEFAULT 'Enabled';
ALTER TABLE `cloud`.`host` ADD INDEX `i_host__allocation_state`(`allocation_state`);
ALTER TABLE `cloud`.`host` ADD INDEX `i_host__allocation_state`(`allocation_state`);
ALTER TABLE `cloud`.`domain` ADD INDEX `i_domain__path`(`path`);