stephankruggg commented on code in PR #7061:
URL: https://github.com/apache/cloudstack/pull/7061#discussion_r1097906811
##########
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<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.");
+ /**
+ * Validates if the provided VM does not have any existing Port
Forwarding, Load Balancer, Static Nat, and One to One Nat rules.
+ * If any rules exist, throws a {@link InvalidParameterValueException}.
+ * @param vm the VM to be checked for the rules.
+ * @param vmId the ID of the VM to be checked.
+ * @throws InvalidParameterValueException
+ */
+ protected void validateIfVmHasNoRules(UserVmVO vm, Long vmId) throws
InvalidParameterValueException {
+ s_logger.trace(String.format("Validating if VM [%s] has no Port
Forwarding, Static Nat, Load Balancing or One to One Nat rules.", vm));
+
+ List<PortForwardingRuleVO> portForwardingRules =
_portForwardingDao.listByVm(vmId);
+ if (CollectionUtils.isNotEmpty(portForwardingRules)) {
+ throw new InvalidParameterValueException(String.format("Remove any
Port Forwarding rules for VM [%s] before assigning it to another user.", vm));
}
- List<FirewallRuleVO> snrules =
_rulesDao.listStaticNatByVmId(vm.getId());
- if (snrules != null && snrules.size() > 0) {
- throw new InvalidParameterValueException("Remove the StaticNat
rules for this VM before assigning to another user.");
+
+ List<FirewallRuleVO> staticNatRules =
_rulesDao.listStaticNatByVmId(vmId);
+ if (CollectionUtils.isNotEmpty(staticNatRules)) {
+ throw new InvalidParameterValueException(String.format("Remove the
StaticNat rules for VM [%s] before assigning it to another user.", vm));
}
- List<LoadBalancerVMMapVO> maps =
_loadBalancerVMMapDao.listByInstanceId(vm.getId());
- if (maps != null && maps.size() > 0) {
- throw new InvalidParameterValueException("Remove the load
balancing rules for this VM before assigning to another user.");
+
+ List<LoadBalancerVMMapVO> loadBalancerVmMaps =
_loadBalancerVMMapDao.listByInstanceId(vmId);
+ if (CollectionUtils.isNotEmpty(loadBalancerVmMaps)) {
+ throw new InvalidParameterValueException(String.format("Remove the
Load Balancing rules for VM [%s] before assigning it to another user.", vm));
}
- // check for one on one nat
- List<IPAddressVO> ips =
_ipAddressDao.findAllByAssociatedVmId(cmd.getVmId());
+
+ List<IPAddressVO> ips = _ipAddressDao.findAllByAssociatedVmId(vmId);
for (IPAddressVO ip : ips) {
if (ip.isOneToOneNat()) {
- throw new InvalidParameterValueException("Remove the one to
one nat rule for this VM for ip " + ip.toString());
+ throw new InvalidParameterValueException(String.format("Remove
the One to One Nat rule for VM [%s] for IP [%s].", vm, ip));
}
}
+ }
- final List<VolumeVO> volumes = _volsDao.findByInstance(cmd.getVmId());
-
+ protected void validateIfVolumesHaveNoSnapshots(List<VolumeVO> volumes)
throws InvalidParameterValueException {
+ s_logger.trace("Verifying if there are any snapshots for any of the VM
volumes.");
for (VolumeVO volume : volumes) {
- List<SnapshotVO> snapshots =
_snapshotDao.listByStatusNotIn(volume.getId(),
Snapshot.State.Destroyed,Snapshot.State.Error);
- if (snapshots != null && snapshots.size() > 0) {
- throw new InvalidParameterValueException(
- "Snapshots exists for volume: "+ volume.getName()+ ",
Detach volume or remove snapshots for volume before assigning VM to another
user.");
+ s_logger.trace(String.format("Verifying snapshots for volume
[%s].", volume));
+ List<SnapshotVO> snapshots =
_snapshotDao.listByStatusNotIn(volume.getId(), Snapshot.State.Destroyed,
Snapshot.State.Error);
+ if (CollectionUtils.isNotEmpty(snapshots)) {
+ throw new
InvalidParameterValueException(String.format("Snapshots exist for volume [%s].
Detach volume or remove snapshots for the volume before assigning VM to "
+ + "another user.", volume.getName()));
}
}
+ }
- DataCenterVO zone = _dcDao.findById(vm.getDataCenterId());
-
- // Get serviceOffering and Volumes for Virtual Machine
- final ServiceOfferingVO offering =
serviceOfferingDao.findByIdIncludingRemoved(vm.getId(),
vm.getServiceOfferingId());
-
- //Remove vm from instance group
- removeInstanceFromInstanceGroup(cmd.getVmId());
+ /**
+ * Verifies if the CPU, RAM and volume size do not exceed the account and
the primary storage limit.
+ * If any limit is exceeded, throws a {@link ResourceAllocationException}.
+ * @param account The account to check if CPU and RAM limit has been
exceeded.
+ * @param vm The VM which can exceed resource limits.
+ * @param vmId The ID for the VM which can exceed resource limits.
+ * @param offering The service offering which can exceed resource limits.
+ * @param volumes The volumes whose total size can exceed resource limits.
+ * @throws ResourceAllocationException
+ */
+ protected void verifyResourceLimitsForAccountAndStorage(Account account,
UserVmVO vm, Long vmId, ServiceOfferingVO offering, List<VolumeVO> volumes)
+ throws ResourceAllocationException {
- // VV 2: check if account/domain is with in resource limits to create
a new vm
- if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) {
- resourceLimitCheck(newAccount, vm.isDisplayVm(), new
Long(offering.getCpu()), new Long(offering.getRamSize()));
+ s_logger.trace(String.format("Verifying if CPU and RAM for VM [%s] do
not exceed account [%s] limit.", vm, account));
+ if (!isResourceCountRunningVmsOnlyEnabled()) {
+ resourceLimitCheck(account, vm.isDisplayVm(),
Long.valueOf(offering.getCpu()), Long.valueOf(offering.getRamSize()));
}
- // VV 3: check if volumes and primary storage space are with in
resource limits
- _resourceLimitMgr.checkResourceLimit(newAccount, ResourceType.volume,
_volsDao.findByInstance(cmd.getVmId()).size());
+ s_logger.trace(String.format("Verifying if volume size for VM [%s]
does not exceed account [%s] limit.", vm, account));
+ _resourceLimitMgr.checkResourceLimit(account, ResourceType.volume,
_volsDao.findByInstance(vmId).size());
Long totalVolumesSize = (long)0;
for (VolumeVO volume : volumes) {
totalVolumesSize += volume.getSize();
}
- _resourceLimitMgr.checkResourceLimit(newAccount,
ResourceType.primary_storage, totalVolumesSize);
+ s_logger.trace(String.format("Verifying if primary storage for VM [%s]
does not exceed account [%s] limit.", vm, account));
+ _resourceLimitMgr.checkResourceLimit(account,
ResourceType.primary_storage, totalVolumesSize);
+ }
+
+ protected void validateIfNewOwnerHasAccessToTemplate(UserVmVO vm, Account
newAccount, VirtualMachineTemplate template) {
+ s_logger.trace(String.format("Validating if new owner [%s] has access
to the template specified for VM [%s].", newAccount, vm));
- // VV 4: Check if new owner can use the vm template
- VirtualMachineTemplate template =
_templateDao.findByIdIncludingRemoved(vm.getTemplateId());
if (template == null) {
- throw new InvalidParameterValueException(String.format("Template
for VM: %s cannot be found", vm.getUuid()));
+ throw new InvalidParameterValueException(String.format("Template
for VM [%s] cannot be found.", vm.getUuid()));
}
+
if (!template.isPublicTemplate()) {
Account templateOwner =
_accountMgr.getAccount(template.getAccountId());
_accountMgr.checkAccess(newAccount, null, true, templateOwner);
}
+ }
- // VV 5: check the new account can create vm in the domain
- DomainVO domain = _domainDao.findById(cmd.getDomainId());
- _accountMgr.checkAccess(newAccount, domain);
+ /**
+ * Executes all ownership steps necessary to assign a VM to another user:
+ * generating a destroy VM event ({@link EventTypes}),
+ * decrementing the old user resource count ({@link
#resourceCountDecrement(long, Boolean, Long, Long)}),
+ * removing the VM from its instance group ({@link
#removeInstanceFromInstanceGroup(long)}),
+ * updating the VM owner to the new account ({@link
#updateVmOwner(Account, UserVmVO, Long, Long)}),
+ * updating the volumes to the new account ({@link
#updateVolumesOwner(List, Account, Account, Long)}),
+ * updating the network for the VM ({@link #updateVmNetwork(AssignVMCmd,
Account, UserVmVO, Account, VirtualMachineTemplate)}),
+ * incrementing the new user resource count ({@link
#resourceCountIncrement(long, Boolean, Long, Long)}),
+ * and generating a create VM event ({@link EventTypes}).
+ * @param cmd The assignVMCmd.
+ * @param caller The account calling the assignVMCmd.
+ * @param oldAccount The old account from whom the VM will be moved.
+ * @param newAccount The new account to whom the VM will move.
+ * @param vm The VM to be moved between accounts.
+ * @param offering The service offering which will be used to decrement
and increment resource counts.
+ * @param volumes The volumes of the VM which will be assigned to another
user.
+ * @param template The template of the VM which will be assigned to
another user.
+ * @param domainId The ID of the domain where the VM which will be
assigned to another user is.
+ */
+ protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account
caller, Account oldAccount, Account newAccount, UserVmVO vm, ServiceOfferingVO
offering,
+ List<VolumeVO> volumes,
VirtualMachineTemplate template, Long domainId) {
- Transaction.execute(new TransactionCallbackNoReturn() {
- @Override
- public void doInTransactionWithoutResult(TransactionStatus status)
{
- //generate destroy vm event for usage
- UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_DESTROY,
vm.getAccountId(), vm.getDataCenterId(),
- vm.getId(), vm.getHostName(),
vm.getServiceOfferingId(), vm.getTemplateId(),
- vm.getHypervisorType().toString(),
VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
- // update resource counts for old account
- resourceCountDecrement(oldAccount.getAccountId(),
vm.isDisplayVm(), new Long(offering.getCpu()), new Long(offering.getRamSize()));
-
- // OWNERSHIP STEP 1: update the vm owner
- vm.setAccountId(newAccount.getAccountId());
- vm.setDomainId(cmd.getDomainId());
- _vmDao.persist(vm);
+ s_logger.trace(String.format("Generating destroy event for VM [%s].",
vm));
+ UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_DESTROY,
vm.getAccountId(), vm.getDataCenterId(), vm.getId(), vm.getHostName(),
vm.getServiceOfferingId(),
+ vm.getTemplateId(), vm.getHypervisorType().toString(),
VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
- // OS 2: update volume
- for (VolumeVO volume : volumes) {
-
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_DELETE,
volume.getAccountId(), volume.getDataCenterId(), volume.getId(),
volume.getName(),
- Volume.class.getName(), volume.getUuid(),
volume.isDisplayVolume());
-
_resourceLimitMgr.decrementResourceCount(oldAccount.getAccountId(),
ResourceType.volume);
-
_resourceLimitMgr.decrementResourceCount(oldAccount.getAccountId(),
ResourceType.primary_storage, new Long(volume.getSize()));
- volume.setAccountId(newAccount.getAccountId());
- volume.setDomainId(newAccount.getDomainId());
- _volsDao.persist(volume);
-
_resourceLimitMgr.incrementResourceCount(newAccount.getAccountId(),
ResourceType.volume);
-
_resourceLimitMgr.incrementResourceCount(newAccount.getAccountId(),
ResourceType.primary_storage, new Long(volume.getSize()));
-
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_CREATE,
volume.getAccountId(), volume.getDataCenterId(), volume.getId(),
volume.getName(),
- volume.getDiskOfferingId(),
volume.getTemplateId(), volume.getSize(), Volume.class.getName(),
- volume.getUuid(), volume.isDisplayVolume());
- }
-
- //update resource count of new account
- if (!
VirtualMachineManager.ResourceCountRunningVMsonly.value()) {
- resourceCountIncrement(newAccount.getAccountId(),
vm.isDisplayVm(), new Long(offering.getCpu()), new Long(offering.getRamSize()));
- }
+ s_logger.trace(String.format("Decrementing old account [%s] resource
count.", oldAccount));
+ resourceCountDecrement(oldAccount.getAccountId(), vm.isDisplayVm(),
Long.valueOf(offering.getCpu()), Long.valueOf(offering.getRamSize()));
- //generate usage events to account for this change
- UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE,
vm.getAccountId(), vm.getDataCenterId(), vm.getId(),
- vm.getHostName(), vm.getServiceOfferingId(),
vm.getTemplateId(), vm.getHypervisorType().toString(),
- VirtualMachine.class.getName(), vm.getUuid(),
vm.isDisplayVm());
- }
- });
+ s_logger.trace(String.format("Removing VM [%s] from its instance
group.", vm));
+ removeInstanceFromInstanceGroup(vm.getId());
+
+ Long newAccountId = newAccount.getAccountId();
+ updateVmOwner(newAccount, vm, domainId, newAccountId);
+
+ updateVolumesOwner(volumes, oldAccount, newAccount, newAccountId);
+
+ try {
+ updateVmNetwork(cmd, caller, vm, newAccount, template);
+ } catch (InsufficientCapacityException | ResourceAllocationException
e) {
Review Comment:
`moveVmToUser` already publishes an event in case an error occurs.
--
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]
