From 6f680375f31a2aa491d25fdc411fa2ae1f8db465 Mon Sep 17 00:00:00 2001 From: abhishek Date: Tue, 17 Aug 2010 11:28:23 -0700 Subject: [PATCH] Refactoring some more vm commands, as well as refactoring the account and user validation into a method. Please use the same for future validation calls --- .../com/cloud/api/commands/UpdateVMCmd.java | 99 +++++----------- .../com/cloud/api/commands/UpgradeVMCmd.java | 4 - .../com/cloud/server/ManagementServer.java | 2 +- .../cloud/server/ManagementServerImpl.java | 24 ---- server/src/com/cloud/vm/UserVmManager.java | 3 + .../src/com/cloud/vm/UserVmManagerImpl.java | 109 +++++++++++++++--- 6 files changed, 123 insertions(+), 118 deletions(-) diff --git a/server/src/com/cloud/api/commands/UpdateVMCmd.java b/server/src/com/cloud/api/commands/UpdateVMCmd.java index 254e1df6b13..410c5829ea4 100644 --- a/server/src/com/cloud/api/commands/UpdateVMCmd.java +++ b/server/src/com/cloud/api/commands/UpdateVMCmd.java @@ -25,27 +25,21 @@ import java.util.Map; import org.apache.log4j.Logger; import com.cloud.api.BaseCmd; +import com.cloud.api.Implementation; import com.cloud.api.Parameter; import com.cloud.api.ServerApiException; +import com.cloud.api.BaseCmd.Manager; +import com.cloud.api.response.UpgradeVmResponse; import com.cloud.user.Account; import com.cloud.utils.Pair; import com.cloud.vm.UserVmVO; - + +@Implementation(method="updateVirtualMachine", manager=Manager.UserVmManager) public class UpdateVMCmd extends BaseCmd{ public static final Logger s_logger = Logger.getLogger(UpdateVMCmd.class.getName()); private static final String s_name = "updatevirtualmachineresponse"; private static final List> s_properties = new ArrayList>(); - static { - s_properties.add(new Pair(BaseCmd.Properties.DISPLAY_NAME, Boolean.FALSE)); - s_properties.add(new Pair(BaseCmd.Properties.GROUP, Boolean.FALSE)); - s_properties.add(new Pair(BaseCmd.Properties.HA_ENABLE, Boolean.FALSE)); - s_properties.add(new Pair(BaseCmd.Properties.ID, Boolean.TRUE)); - - s_properties.add(new Pair(BaseCmd.Properties.ACCOUNT_OBJ, Boolean.FALSE)); - s_properties.add(new Pair(BaseCmd.Properties.USER_ID, Boolean.FALSE)); - } - ///////////////////////////////////////////////////// //////////////// API parameters ///////////////////// ///////////////////////////////////////////////////// @@ -86,71 +80,30 @@ public class UpdateVMCmd extends BaseCmd{ /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// + private UserVmVO responseObject = null; + @Override public String getName() { return s_name; - } - @Override - public List> getProperties() { - return s_properties; - } - - @Override - public List> execute(Map params) { - Long userId = (Long)params.get(BaseCmd.Properties.USER_ID.getName()); - Account account = (Account)params.get(BaseCmd.Properties.ACCOUNT_OBJ.getName()); - Long vmId = (Long)params.get(BaseCmd.Properties.ID.getName()); - String group = (String)params.get(BaseCmd.Properties.GROUP.getName()); - String displayName = (String)params.get(BaseCmd.Properties.DISPLAY_NAME.getName()); - Boolean enable = (Boolean)params.get(BaseCmd.Properties.HA_ENABLE.getName()); - UserVmVO vmInstance = null; - - // default userId to SYSTEM user - if (userId == null) { - userId = Long.valueOf(1); - } - - // Verify input parameters - try { - vmInstance = getManagementServer().findUserVMInstanceById(vmId.longValue()); - } catch (Exception ex1) { - throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "unable to find virtual machine by id"); - } - - if (vmInstance == null) { - throw new ServerApiException(BaseCmd.PARAM_ERROR, "unable to find virtual machine with id " + vmId); - } - - if (account != null) { - if (!isAdmin(account.getType()) && (account.getId().longValue() != vmInstance.getAccountId())) { - throw new ServerApiException(BaseCmd.VM_INVALID_PARAM_ERROR, "unable to find a virtual machine with id " + vmId + " for this account"); - } else if (!getManagementServer().isChildDomain(account.getDomainId(), vmInstance.getDomainId())) { - throw new ServerApiException(BaseCmd.PARAM_ERROR, "Invalid virtual machine id (" + vmId + ") given, unable to update virtual machine."); - } + } + + public static String getResultObjectName() { + return "virtualmachine"; + } + + @Override + public String getResponse() + { + UpgradeVmResponse response = new UpgradeVmResponse(); + UserVmVO userVm = (UserVmVO)getResponseObject(); + + UserVmVO responseObject = (UserVmVO)getResponseObject(); + if (responseObject != null) + { + //just pass back success or failure from here } - - if (group == null) { - group = vmInstance.getGroup(); - } + + return null; + } - if (displayName == null) { - displayName = vmInstance.getDisplayName(); - } - - if (enable == null) { - enable = vmInstance.isHaEnabled(); - } - - long accountId = vmInstance.getAccountId(); - - try { - getManagementServer().updateVirtualMachine(vmId, displayName, group, enable, userId, accountId); - } catch (Exception ex) { - throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to update virtual machine" + vmId + ": internal error."); - } - - List> returnValues = new ArrayList>(); - returnValues.add(new Pair(BaseCmd.Properties.SUCCESS.getName(), Boolean.TRUE)); - return returnValues; - } } diff --git a/server/src/com/cloud/api/commands/UpgradeVMCmd.java b/server/src/com/cloud/api/commands/UpgradeVMCmd.java index 72c6d553daa..3e5133df639 100644 --- a/server/src/com/cloud/api/commands/UpgradeVMCmd.java +++ b/server/src/com/cloud/api/commands/UpgradeVMCmd.java @@ -18,8 +18,6 @@ package com.cloud.api.commands; -import java.util.Date; - import org.apache.log4j.Logger; import com.cloud.api.BaseCmd; @@ -27,8 +25,6 @@ import com.cloud.api.Implementation; import com.cloud.api.Parameter; import com.cloud.api.BaseCmd.Manager; import com.cloud.api.response.UpgradeVmResponse; -import com.cloud.domain.DomainVO; -import com.cloud.serializer.Param; import com.cloud.vm.UserVmVO; @Implementation(method="upgradeVirtualMachine", manager=Manager.UserVmManager) diff --git a/server/src/com/cloud/server/ManagementServer.java b/server/src/com/cloud/server/ManagementServer.java index 3157573c246..f13c55323fb 100644 --- a/server/src/com/cloud/server/ManagementServer.java +++ b/server/src/com/cloud/server/ManagementServer.java @@ -744,7 +744,7 @@ public interface ManagementServer { * @param userId - id of user performing the update on the virtual machine * @param accountId - id of the account that owns the virtual machine */ - void updateVirtualMachine(long vmId, String displayName, String group, boolean enable, Long userId, long accountId); +// void updateVirtualMachine(long vmId, String displayName, String group, boolean enable, Long userId, long accountId); /** * Updates a storage pool. diff --git a/server/src/com/cloud/server/ManagementServerImpl.java b/server/src/com/cloud/server/ManagementServerImpl.java index fe0f4880c4c..48cf549166b 100755 --- a/server/src/com/cloud/server/ManagementServerImpl.java +++ b/server/src/com/cloud/server/ManagementServerImpl.java @@ -2709,30 +2709,6 @@ public class ManagementServerImpl implements ManagementServer { return _asyncMgr.submitAsyncJob(job, true); } */ - - @Override - public void updateVirtualMachine(long vmId, String displayName, String group, boolean enable, Long userId, long accountId) { - UserVmVO vm = _userVmDao.findById(vmId); - if (vm == null) { - throw new CloudRuntimeException("Unable to find virual machine with id " + vmId); - } - - boolean haEnabled = vm.isHaEnabled(); - _userVmDao.updateVM(vmId, displayName, group, enable); - if (haEnabled != enable) { - String description = null; - String type = null; - if (enable) { - description = "Successfully enabled HA for virtual machine " + vm.getName(); - type = EventTypes.EVENT_VM_ENABLE_HA; - } else { - description = "Successfully disabled HA for virtual machine " + vm.getName(); - type = EventTypes.EVENT_VM_DISABLE_HA; - } - // create a event for the change in HA Enabled flag - EventUtils.saveEvent(userId, accountId, EventVO.LEVEL_INFO, type, description, null); - } - } @Override public StoragePoolVO updateStoragePool(long poolId, String tags) throws IllegalArgumentException { diff --git a/server/src/com/cloud/vm/UserVmManager.java b/server/src/com/cloud/vm/UserVmManager.java index de760347254..25de24fbb42 100644 --- a/server/src/com/cloud/vm/UserVmManager.java +++ b/server/src/com/cloud/vm/UserVmManager.java @@ -22,6 +22,7 @@ import java.util.List; import com.cloud.agent.api.VmStatsEntry; import com.cloud.api.ServerApiException; +import com.cloud.api.commands.UpdateVMCmd; import com.cloud.api.commands.UpgradeVMCmd; import com.cloud.async.executor.DestroyVMExecutor; import com.cloud.async.executor.OperationResponse; @@ -222,4 +223,6 @@ public interface UserVmManager extends Manager, VirtualMachineManager */ void releaseGuestIpAddress(UserVmVO userVm); + void updateVirtualMachine(UpdateVMCmd cmd); + } diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java index c1cdd76a3ce..1c707b63689 100644 --- a/server/src/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/com/cloud/vm/UserVmManagerImpl.java @@ -66,6 +66,7 @@ import com.cloud.agent.api.storage.CreatePrivateTemplateCommand; import com.cloud.alert.AlertManager; import com.cloud.api.BaseCmd; import com.cloud.api.ServerApiException; +import com.cloud.api.commands.UpdateVMCmd; import com.cloud.api.commands.UpgradeVMCmd; import com.cloud.async.AsyncJobExecutor; import com.cloud.async.AsyncJobManager; @@ -1110,22 +1111,7 @@ public class UserVmManagerImpl implements UserVmManager { throw new ServerApiException(BaseCmd.VM_INVALID_PARAM_ERROR, "unable to find a virtual machine with id " + virtualMachineId); } - if (account != null) - { - if (!isAdmin(account.getType()) && (account.getId().longValue() != vmInstance.getAccountId())) - { - throw new ServerApiException(BaseCmd.VM_INVALID_PARAM_ERROR, "unable to find a virtual machine with id " + virtualMachineId + " for this account"); - } - else if (_domainDao.isChildDomain(account.getDomainId(),vmInstance.getDomainId())) - { - throw new ServerApiException(BaseCmd.PARAM_ERROR, "Invalid virtual machine id (" + virtualMachineId + ") given, unable to upgrade virtual machine."); - } - } - - // If command is executed via 8096 port, set userId to the id of System account (1) - if (userId == null) { - userId = Long.valueOf(User.UID_SYSTEM); - } + userId = accountAndUserValidation(virtualMachineId, account, userId,vmInstance); // Check that the specified service offering ID is valid ServiceOfferingVO newServiceOffering = _offeringDao.findById(serviceOfferingId); @@ -1185,6 +1171,27 @@ public class UserVmManagerImpl implements UserVmManager { } + private Long accountAndUserValidation(Long virtualMachineId,Account account, Long userId, UserVmVO vmInstance) throws ServerApiException + { + if (account != null) + { + if (!isAdmin(account.getType()) && (account.getId().longValue() != vmInstance.getAccountId())) + { + throw new ServerApiException(BaseCmd.VM_INVALID_PARAM_ERROR, "unable to find a virtual machine with id " + virtualMachineId + " for this account"); + } + else if (_domainDao.isChildDomain(account.getDomainId(),vmInstance.getDomainId())) + { + throw new ServerApiException(BaseCmd.PARAM_ERROR, "Invalid virtual machine id (" + virtualMachineId + ") given, unable to upgrade virtual machine."); + } + } + + // If command is executed via 8096 port, set userId to the id of System account (1) + if (userId == null) { + userId = Long.valueOf(User.UID_SYSTEM); + } + return userId; + } + @Override public HashMap getVirtualMachineStatistics(long hostId, String hostName, List vmIds) throws InternalErrorException { HashMap vmStatsById = new HashMap(); @@ -3044,4 +3051,74 @@ public class UserVmManagerImpl implements UserVmManager { (accountType == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) || (accountType == Account.ACCOUNT_TYPE_READ_ONLY_ADMIN)); } + + @Override + public void updateVirtualMachine(UpdateVMCmd cmd) + { + + String displayName = cmd.getDisplayName(); + String group = cmd.getGroup(); + Boolean ha = cmd.getHaEnable(); + Long id = cmd.getId(); + Account account = (Account)UserContext.current().getAccountObject(); + Long userId = UserContext.current().getUserId(); + + //Input validation + UserVmVO vmInstance = null; + + // Verify input parameters + try + { + vmInstance = _userVmDao.findById(id.longValue()); + } + catch (Exception ex1) + { + throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "unable to find virtual machine by id"); + } + + if (vmInstance == null) { + throw new ServerApiException(BaseCmd.PARAM_ERROR, "unable to find virtual machine with id " + id); + } + + userId = accountAndUserValidation(id, account, userId,vmInstance); + + if (group == null) { + group = vmInstance.getGroup(); + } + + if (displayName == null) { + displayName = vmInstance.getDisplayName(); + } + + if (ha == null) { + ha = vmInstance.isHaEnabled(); + } + + long accountId = vmInstance.getAccountId(); + + + UserVmVO vm = _userVmDao.findById(id); + if (vm == null) { + throw new CloudRuntimeException("Unable to find virual machine with id " + id); + } + + boolean haEnabled = vm.isHaEnabled(); + _userVmDao.updateVM(id, displayName, group, ha); + if (haEnabled != ha) { + String description = null; + String type = null; + if (ha) + { + description = "Successfully enabled HA for virtual machine " + vm.getName(); + type = EventTypes.EVENT_VM_ENABLE_HA; + } + else + { + description = "Successfully disabled HA for virtual machine " + vm.getName(); + type = EventTypes.EVENT_VM_DISABLE_HA; + } + // create a event for the change in HA Enabled flag + EventUtils.saveEvent(userId, accountId, EventVO.LEVEL_INFO, type, description, null); + } + } }