DaanHoogland commented on code in PR #6938: URL: https://github.com/apache/cloudstack/pull/6938#discussion_r1037899327
########## api/src/main/java/org/apache/cloudstack/api/command/user/volume/AssignVolumeCmd.java: ########## @@ -0,0 +1,119 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.command.user.volume; + +import com.cloud.exception.ResourceAllocationException; +import com.cloud.user.Account; +import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.user.UserCmd; +import org.apache.cloudstack.api.response.AccountResponse; +import org.apache.cloudstack.api.response.ProjectResponse; +import org.apache.cloudstack.api.response.VolumeResponse; +import org.apache.log4j.Logger; + +import com.cloud.storage.Volume; + +import java.util.Map; + +@APICommand(name = AssignVolumeCmd.CMD_NAME, responseObject = VolumeResponse.class, description = "Changes ownership of a Volume from one account to another.", entityType = { + Volume.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) +public class AssignVolumeCmd extends BaseCmd implements UserCmd { + public static final Logger LOGGER = Logger.getLogger(AssignVolumeCmd.class.getName()); + public static final String CMD_NAME = "assignVolume"; + + ///////////////////////////////////////////////////// + //////////////// API parameters ///////////////////// + ///////////////////////////////////////////////////// + + @Parameter(name = ApiConstants.VOLUME_ID, type = CommandType.UUID, entityType = VolumeResponse.class, required = true, description = "The ID of the volume to be reassigned.") + private Long volumeId; + + @Parameter(name = ApiConstants.ACCOUNT_ID, type = CommandType.UUID, entityType = AccountResponse.class, + description = "The ID of the account to which the volume will be assigned. Mutually exclusive with parameter 'projectid'.") + private Long accountId; + + @Parameter(name = ApiConstants.PROJECT_ID, type = CommandType.UUID, entityType = ProjectResponse.class, + description = "The ID of the project to which the volume will be assigned. Mutually exclusive with 'accountid'.") + private Long projectid; + + ///////////////////////////////////////////////////// + /////////////////// Accessors /////////////////////// + ///////////////////////////////////////////////////// + + public Long getVolumeId() { + return volumeId; + } + + public Long getAccountId() { + return accountId; + } + + public Long getProjectid() { + return projectid; + } + + ///////////////////////////////////////////////////// + /////////////// API Implementation/////////////////// + ///////////////////////////////////////////////////// + + @Override + public void execute() { + try { + Volume result = _volumeService.assignVolumeToAccount(this); + if (result == null) { + Map<String,String> fullParams = getFullUrlParams(); + if (accountId != null) { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to move volume [%s] to account [%s].", fullParams.get("volumeid"), + fullParams.get("accountid"))); Review Comment: maybe get "volumeid", "accountid" and "projectid" from `ApiContants`? ########## api/src/main/java/org/apache/cloudstack/api/command/user/volume/AssignVolumeCmd.java: ########## @@ -0,0 +1,119 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.command.user.volume; + +import com.cloud.exception.ResourceAllocationException; +import com.cloud.user.Account; +import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.user.UserCmd; +import org.apache.cloudstack.api.response.AccountResponse; +import org.apache.cloudstack.api.response.ProjectResponse; +import org.apache.cloudstack.api.response.VolumeResponse; +import org.apache.log4j.Logger; + +import com.cloud.storage.Volume; + +import java.util.Map; + +@APICommand(name = AssignVolumeCmd.CMD_NAME, responseObject = VolumeResponse.class, description = "Changes ownership of a Volume from one account to another.", entityType = { + Volume.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) +public class AssignVolumeCmd extends BaseCmd implements UserCmd { + public static final Logger LOGGER = Logger.getLogger(AssignVolumeCmd.class.getName()); + public static final String CMD_NAME = "assignVolume"; + + ///////////////////////////////////////////////////// + //////////////// API parameters ///////////////////// + ///////////////////////////////////////////////////// + + @Parameter(name = ApiConstants.VOLUME_ID, type = CommandType.UUID, entityType = VolumeResponse.class, required = true, description = "The ID of the volume to be reassigned.") + private Long volumeId; + + @Parameter(name = ApiConstants.ACCOUNT_ID, type = CommandType.UUID, entityType = AccountResponse.class, + description = "The ID of the account to which the volume will be assigned. Mutually exclusive with parameter 'projectid'.") + private Long accountId; + + @Parameter(name = ApiConstants.PROJECT_ID, type = CommandType.UUID, entityType = ProjectResponse.class, + description = "The ID of the project to which the volume will be assigned. Mutually exclusive with 'accountid'.") + private Long projectid; + + ///////////////////////////////////////////////////// + /////////////////// Accessors /////////////////////// + ///////////////////////////////////////////////////// + + public Long getVolumeId() { + return volumeId; + } + + public Long getAccountId() { + return accountId; + } + + public Long getProjectid() { + return projectid; + } + + ///////////////////////////////////////////////////// + /////////////// API Implementation/////////////////// + ///////////////////////////////////////////////////// + + @Override + public void execute() { + try { + Volume result = _volumeService.assignVolumeToAccount(this); + if (result == null) { + Map<String,String> fullParams = getFullUrlParams(); + if (accountId != null) { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to move volume [%s] to account [%s].", fullParams.get("volumeid"), + fullParams.get("accountid"))); + } + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to move volume [%s] to project [%s].", fullParams.get("volumeid"), + fullParams.get("projectid"))); + } + + VolumeResponse response = _responseGenerator.createVolumeResponse(getResponseView(), result); + response.setResponseName(getCommandName()); + setResponseObject(response); + + } catch (CloudRuntimeException | ResourceAllocationException e) { + String msg = String.format("Assign volume command for volume [%s] failed due to [%s].", getFullUrlParams().get("volumeid"), e.getMessage()); + LOGGER.error(msg, e); + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, msg); + } + } + + @Override + public String getCommandName() { + return CMD_NAME + RESPONSE_SUFFIX; Review Comment: ```suggestion return CMD_NAME.toLower() + RESPONSE_SUFFIX; ``` ########## engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java: ########## @@ -806,7 +811,7 @@ protected Void createVolumeFromBaseImageCallBack(AsyncCallbackDispatcher<VolumeS // hack for Vmware: host is down, previously download template to the host needs to be re-downloaded, so we need to reset // template_spool_ref entry here to NOT_DOWNLOADED and Allocated state Answer ans = result.getAnswer(); - if (ans != null && ans instanceof CopyCmdAnswer && ans.getDetails().contains(REQUEST_TEMPLATE_RELOAD)) { + if (ans != null && ans instanceof CopyCmdAnswer && ans.getDetails().contains(StorageProcessor.REQUEST_TEMPLATE_RELOAD)) { Review Comment: ```suggestion if (ans instanceof CopyCmdAnswer && ans.getDetails().contains(StorageProcessor.REQUEST_TEMPLATE_RELOAD)) { ``` ########## api/src/main/java/org/apache/cloudstack/api/command/user/volume/AssignVolumeCmd.java: ########## @@ -0,0 +1,119 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.command.user.volume; + +import com.cloud.exception.ResourceAllocationException; +import com.cloud.user.Account; +import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.user.UserCmd; +import org.apache.cloudstack.api.response.AccountResponse; +import org.apache.cloudstack.api.response.ProjectResponse; +import org.apache.cloudstack.api.response.VolumeResponse; +import org.apache.log4j.Logger; + +import com.cloud.storage.Volume; + +import java.util.Map; + +@APICommand(name = AssignVolumeCmd.CMD_NAME, responseObject = VolumeResponse.class, description = "Changes ownership of a Volume from one account to another.", entityType = { + Volume.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) Review Comment: ```suggestion @APICommand(name = AssignVolumeCmd.CMD_NAME, responseObject = VolumeResponse.class, description = "Changes ownership of a Volume from one account to another.", entityType = { Volume.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.18.0.0") ``` alternatively "4.19.0.0" ########## engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java: ########## @@ -330,9 +330,15 @@ protected EndPoint findEndpointForImageStorage(DataStore store) { if (storeScope.getScopeType() == ScopeType.ZONE) { dcId = storeScope.getScopeId(); } - // find ssvm that can be used to download data to store. For zone-wide - // image store, use SSVM for that zone. For region-wide store, - // we can arbitrarily pick one ssvm to do that task + + return findSsvm(dcId); + } + + /** + * Finds an SSVM that can be used to execute a command. + * For zone-wide image store, use SSVM for that zone. For region-wide store, we can arbitrarily pick one SSVM to do the task. + * */ + public EndPoint findSsvm(long dcId) { Review Comment: :+1: -- 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]
