weizhouapache commented on code in PR #7061:
URL: https://github.com/apache/cloudstack/pull/7061#discussion_r1062529433
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -7052,525 +7051,779 @@ public VirtualMachine
migrateVirtualMachineWithVolume(Long vmId, Host destinatio
@DB
@Override
@ActionEvent(eventType = EventTypes.EVENT_VM_MOVE, eventDescription =
"move VM to another user", async = false)
- public UserVm moveVMToUser(final AssignVMCmd cmd) throws
ResourceAllocationException, ConcurrentOperationException,
ResourceUnavailableException, InsufficientCapacityException {
- // VERIFICATIONS and VALIDATIONS
-
- // VV 1: verify the two users
+ public UserVm moveVmToUser(final AssignVMCmd cmd) throws
ResourceAllocationException, InsufficientCapacityException {
Account caller = CallContext.current().getCallingAccount();
- if (!_accountMgr.isRootAdmin(caller.getId())
- && !_accountMgr.isDomainAdmin(caller.getId())) { // only
- // root
- // admin
- // can
- // assign
- // VMs
- throw new InvalidParameterValueException("Only domain admins are
allowed to assign VMs and not " + caller.getType());
- }
-
- // get and check the valid VM
- final UserVmVO vm = _vmDao.findById(cmd.getVmId());
- if (vm == null) {
- throw new InvalidParameterValueException("There is no vm by that
id " + cmd.getVmId());
- } else if (vm.getState() == State.Running) { // VV 3: check if vm is
- // running
- if (s_logger.isDebugEnabled()) {
- s_logger.debug("VM is Running, unable to move the vm " + vm);
- }
- InvalidParameterValueException ex = new
InvalidParameterValueException("VM is Running, unable to move the vm with
specified vmId");
- ex.addProxyObject(vm.getUuid(), "vmId");
- throw ex;
- }
-
- final Account oldAccount =
_accountService.getActiveAccountById(vm.getAccountId());
- if (oldAccount == null) {
- throw new InvalidParameterValueException("Invalid account for VM "
+ vm.getAccountId() + " in domain.");
- }
- final Account newAccount = _accountMgr.finalizeOwner(caller,
cmd.getAccountName(), cmd.getDomainId(), cmd.getProjectId());
- if (newAccount == null) {
- throw new InvalidParameterValueException("Invalid accountid=" +
cmd.getAccountName() + " in domain " + cmd.getDomainId());
+ Long callerId = caller.getId();
+ s_logger.trace(String.format("Verifying if caller [%s] is root or
domain admin.", caller));
+ if (!_accountMgr.isRootAdmin(callerId) &&
!_accountMgr.isDomainAdmin(callerId)) {
+ throw new InvalidParameterValueException(String.format("Only root
or domain admins are allowed to assign VMs. Caller [%s] is of type [%s].",
caller, caller.getType()));
}
- if (newAccount.getState() == Account.State.DISABLED) {
- throw new InvalidParameterValueException("The new account owner "
+ cmd.getAccountName() + " is disabled.");
- }
+ Long vmId = cmd.getVmId();
+ final UserVmVO vm = _vmDao.findById(vmId);
+ validateIfVmExistsAndIsNotRunning(vm, vmId);
- if (cmd.getProjectId() != null && cmd.getDomainId() == null) {
+ Long domainId = cmd.getDomainId();
+ Long projectId = cmd.getProjectId();
+ Long oldAccountId = vm.getAccountId();
+ String newAccountName = cmd.getAccountName();
+ final Account oldAccount =
_accountService.getActiveAccountById(oldAccountId);
+ final Account newAccount = _accountMgr.finalizeOwner(caller,
newAccountName, domainId, projectId);
+ validateAccountsAndCallerAccessToThem(caller, oldAccount, newAccount,
oldAccountId, newAccountName, domainId);
+
+ s_logger.trace(String.format("Verifying if the provided domain ID [%s]
is valid.", domainId));
+ if (projectId != null && domainId == null) {
throw new InvalidParameterValueException("Please provide a valid
domain ID; cannot assign VM to a project if domain ID is NULL.");
}
- //check caller has access to both the old and new account
- _accountMgr.checkAccess(caller, null, true, oldAccount);
- _accountMgr.checkAccess(caller, null, true, newAccount);
+ validateIfVmHasNoRules(vm, vmId);
- // make sure the accounts are not same
- if (oldAccount.getAccountId() == newAccount.getAccountId()) {
- throw new InvalidParameterValueException("The new account is the
same as the old account. Account id =" + oldAccount.getAccountId());
+ final List<VolumeVO> volumes = _volsDao.findByInstance(vmId);
+ validateIfVolumesHaveNoSnapshots(volumes);
+
+ final ServiceOfferingVO offering =
serviceOfferingDao.findByIdIncludingRemoved(vm.getId(),
vm.getServiceOfferingId());
+ verifyResourceLimitsForAccountAndStorage(newAccount, vm, vmId,
offering, volumes);
+
+ VirtualMachineTemplate template =
_templateDao.findByIdIncludingRemoved(vm.getTemplateId());
+ validateIfNewOwnerHasAccessToTemplate(vm, newAccount, template);
+
+ DomainVO domain = _domainDao.findById(domainId);
+ s_logger.trace(String.format("Verifying if the new account [%s] has
access to the specified domain [%s].", newAccount, domain));
+ _accountMgr.checkAccess(newAccount, domain);
+
+ Transaction.execute(new TransactionCallbackNoReturn() {
+ @Override
+ public void doInTransactionWithoutResult(TransactionStatus status)
{
+ executeStepsToChangeOwnershipOfVm(cmd, caller, oldAccount,
newAccount, vm, offering, volumes, template, domainId);
+ }
+ });
+
+ s_logger.info(String.format("VM [%s] now belongs to account [%s].",
vm.getInstanceName(), newAccountName));
+ return vm;
+ }
+
+ protected void validateIfVmExistsAndIsNotRunning(UserVmVO vm, Long vmId) {
+ s_logger.trace(String.format("Validating if VM [%s] exists and is not
in state [%s].", vmId, State.Running));
+
+ if (vm == null) {
+ throw new InvalidParameterValueException(String.format("There is
no VM by ID [%s].", vmId));
+ } else if (vm.getState() == State.Running) {
+ String errMsg = String.format("Unable to move VM [%s] in [%s]
state.", vm, vm.getState());
+ s_logger.warn(errMsg);
+ throw new InvalidParameterValueException(errMsg);
}
+ }
- // don't allow to move the vm if there are existing PF/LB/Static Nat
- // rules, or vm is assigned to static Nat ip
- List<PortForwardingRuleVO> pfrules =
_portForwardingDao.listByVm(cmd.getVmId());
- if (pfrules != null && pfrules.size() > 0) {
- throw new InvalidParameterValueException("Remove the Port
forwarding rules for this VM before assigning to another user.");
+ /**
+ * Verifies if both old and new accounts are valid ({@link
#validateOldAndNewAccounts}) and if the caller has access to them ({@link
#checkCallerAccessToAccounts}).
+ * @param caller The caller to check for access.
+ * @param oldAccount The old account to be checked for validity.
+ * @param newAccount The new account to be checked for validity.
+ * @param oldAccountId The ID of the old account to be checked for
validity.
+ * @param newAccountName The name of the new account to be checked for
validity.
+ * @param domainId The ID of the domain where to validate the conditions.
+ */
+ protected void validateAccountsAndCallerAccessToThem(Account caller,
Account oldAccount, Account newAccount, Long oldAccountId, String
newAccountName, Long domainId) {
Review Comment:
is it necessary to create this new method ?
--
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]
