[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #7061: Rollback of changes with errors during the VM assign
weizhouapache commented on code in PR #7061:
URL: https://github.com/apache/cloudstack/pull/7061#discussion_r1073196190
##
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##
@@ -7052,525 +7051,766 @@ 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;
+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()));
}
-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 vmId = cmd.getVmId();
+final UserVmVO vm = _vmDao.findById(vmId);
+validateIfVmExistsAndIsNotRunning(vm, vmId);
-if (newAccount.getState() == Account.State.DISABLED) {
-throw new InvalidParameterValueException("The new account owner "
+ cmd.getAccountName() + " is disabled.");
-}
+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);
+validateOldAndNewAccounts(oldAccount, newAccount, oldAccountId,
newAccountName, domainId);
+
+checkCallerAccessToAccounts(caller, oldAccount, newAccount);
-if (cmd.getProjectId() != null && cmd.getDomainId() == null) {
+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 volumes = _volsDao.findByInstance(vmId);
+validateIfVolumesHaveNoSnapshots(volumes);
+
+final ServiceOfferingVO offering =
[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #7061: Rollback of changes with errors during the VM assign
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 volumes = _volsDao.findByInstance(vmId);
+validateIfVolumesHaveNoSnapshots(volumes);
+
+final ServiceOfferingVO offering =
serviceOfferingDao.findByIdIncludingRemoved(vm.getId(),
vm.getServiceOfferingId());
