Sergey Gotliv has uploaded a new change for review. Change subject: [wip]engine: Remove getImageDomainsList API ......................................................................
[wip]engine: Remove getImageDomainsList API We decided to refactor ImportVmCommand to make flow more simple and reduce the number of calls to VDSM. The first step is removing getImagesDomainsList API which takes storagePoolId and imageId as an input and returns all storage domains from the pool containing that image. What we are really need is to check the image existence in the specific storage domain therefore more appropriate API to use is getImagesList which returns all image ids contained by the given storage domain. Change-Id: I33069871db34e8b129c4d80098523654cd2a0629 Signed-off-by: Sergey Gotliv <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyTemplateCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommandTestAbstract.java D backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/GetImageDomainsListVDSCommandParameters.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/VDSCommandType.java D backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/GetImageDomainsListVDSCommand.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IIrsServer.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsServerWrapper.java 8 files changed, 48 insertions(+), 148 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/51/15951/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyTemplateCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyTemplateCommand.java index 01f2aeb..921046e 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyTemplateCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyTemplateCommand.java @@ -31,7 +31,7 @@ import org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface; import org.ovirt.engine.core.common.errors.VdcBllErrors; import org.ovirt.engine.core.common.errors.VdcBllMessages; -import org.ovirt.engine.core.common.vdscommands.GetImageDomainsListVDSCommandParameters; +import org.ovirt.engine.core.common.vdscommands.GetImagesListVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.common.vdscommands.VDSReturnValue; import org.ovirt.engine.core.compat.Guid; @@ -204,21 +204,30 @@ } protected boolean checkIfDisksExist(Iterable<DiskImage> disksList) { + Map<Guid, List<Guid>> imagesPerStorageDomain = new HashMap<>(); + List<Guid> images = new ArrayList<>(); + for (DiskImage disk : disksList) { - VDSReturnValue runVdsCommand = getBackend() - .getResourceManager() - .RunVdsCommand( - VDSCommandType.GetImageDomainsList, - new GetImageDomainsListVDSCommandParameters(getStoragePool().getId() - .getValue(), disk.getId())); - if (runVdsCommand.getSucceeded()) { - ArrayList<Guid> domains = (ArrayList<Guid>) runVdsCommand.getReturnValue(); - if (domains.contains(imageToDestinationDomainMap.get(disk.getId()))) { - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_ALREADY_CONTAINS_DISK); + Guid targetStorageDomainId = imageToDestinationDomainMap.get(disk.getId()); + if (imagesPerStorageDomain.containsKey(targetStorageDomainId)) { + images = imagesPerStorageDomain.get(targetStorageDomainId); + } else { + VDSReturnValue runVdsCommand = getBackend() + .getResourceManager() + .RunVdsCommand( + VDSCommandType.GetImagesList, + new GetImagesListVDSCommandParameters(targetStorageDomainId)); + if (runVdsCommand.getSucceeded()) { + images = (List<Guid>) runVdsCommand.getReturnValue(); + imagesPerStorageDomain.put(disk.getId(), images); + } else if (runVdsCommand.getVdsError().getCode() == VdcBllErrors.GetStorageDomainListError) { + addCanDoActionMessage(VdcBllMessages.ERROR_GET_STORAGE_DOMAIN_LIST); return false; } - } else if (runVdsCommand.getVdsError().getCode() == VdcBllErrors.GetStorageDomainListError) { - addCanDoActionMessage(VdcBllMessages.ERROR_GET_STORAGE_DOMAIN_LIST); + } + + if (images.contains(disk.getId())) { + addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_ALREADY_CONTAINS_DISK); return false; } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmCommandTest.java index 95c35e9..1dbd2da 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmCommandTest.java @@ -1,24 +1,5 @@ package org.ovirt.engine.core.bll; -import static junit.framework.Assert.assertFalse; -import static junit.framework.Assert.assertTrue; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyInt; -import static org.mockito.Matchers.anyListOf; -import static org.mockito.Matchers.anyString; -import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.when; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -46,9 +27,6 @@ import org.ovirt.engine.core.common.config.ConfigValues; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.common.interfaces.VDSBrokerFrontend; -import org.ovirt.engine.core.common.vdscommands.VDSCommandType; -import org.ovirt.engine.core.common.vdscommands.VDSParametersBase; -import org.ovirt.engine.core.common.vdscommands.VDSReturnValue; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.compat.NGuid; import org.ovirt.engine.core.compat.Version; @@ -59,6 +37,24 @@ import org.ovirt.engine.core.dao.VmDAO; import org.ovirt.engine.core.dao.VmTemplateDAO; import org.ovirt.engine.core.utils.MockConfigRule; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static junit.framework.Assert.assertFalse; +import static junit.framework.Assert.assertTrue; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.anyListOf; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) @SuppressWarnings("serial") @@ -106,7 +102,6 @@ mockStorageDomainDAOGetForStoragePool(); mockVmTemplateDAOReturnVmTemplate(); mockDiskImageDAOGetSnapshotById(); - mockGetImageDomainsListVdsCommand(); mockVerifyAddVM(cmd); mockConfig(); mockConfigSizeDefaults(); @@ -312,7 +307,6 @@ mockDiskImageDAOGetSnapshotById(); mockStorageDomainDAOGetForStoragePool(domainSizeGB); mockStorageDomainDAOGet(domainSizeGB); - mockGetImageDomainsListVdsCommand(); mockConfig(); mockConfigSizeRequirements(sizeRequired); VM vm = createVm(); @@ -414,15 +408,6 @@ img.setId(Guid.NewGuid()); img.setStorageIds(new ArrayList<Guid>(Arrays.asList(STORAGE_DOMAIN_ID))); return img; - } - - private void mockGetImageDomainsListVdsCommand() { - ArrayList<Guid> guids = new ArrayList<Guid>(1); - guids.add(Guid.NewGuid()); - VDSReturnValue returnValue = new VDSReturnValue(); - returnValue.setReturnValue(guids); - when(vdsBrokerFrontend.RunVdsCommand(eq(VDSCommandType.GetImageDomainsList), - Matchers.<VDSParametersBase> any(VDSParametersBase.class))).thenReturn(returnValue); } protected StorageDomain createStorageDomain(int availableSpace) { diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommandTestAbstract.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommandTestAbstract.java index d6285a9..f9435f2 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommandTestAbstract.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommandTestAbstract.java @@ -131,7 +131,8 @@ } private void mockVds() { - mockGetImageDomainsListVdsCommand(100, 100); + mockGetStorageDomainList(100, 100); + mockGetImagesList(); } private void mockGlobalParameters() { @@ -142,22 +143,17 @@ storagePool = mockStoragePool(); } - protected void mockGetImageDomainsListVdsCommand(int availableDiskSizeFirstDomain, - int availableDiskSizeSecondDomain) { - mockGetStorageDomainList(availableDiskSizeFirstDomain, availableDiskSizeSecondDomain); - - // Mock VDS return value. + private void mockGetImagesList() { VDSReturnValue returnValue = new VDSReturnValue(); - returnValue.setReturnValue(mockStorageGuidList(storageDomainsList)); - when(vdsBrokerFrontend.RunVdsCommand(eq(VDSCommandType.GetImageDomainsList), + returnValue.setReturnValue(new ArrayList<Guid>()); + when(vdsBrokerFrontend.RunVdsCommand(eq(VDSCommandType.GetImagesList), Matchers.<VDSParametersBase> any(VDSParametersBase.class))).thenReturn(returnValue); } - protected void mockGetStorageDomainList - (int availableDiskSizeFirstDomain, int availableDiskSizeSecondDomain) { + + private void mockGetStorageDomainList(int availableDiskSizeFirstDomain, int availableDiskSizeSecondDomain) { // Mock Dao - storageDomainsList = - getStorageDomainList(availableDiskSizeFirstDomain, availableDiskSizeSecondDomain); + storageDomainsList = getStorageDomainList(availableDiskSizeFirstDomain, availableDiskSizeSecondDomain); mockDiskImageDAO(); mockStorageDomainDAO(storageDomainsList); } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/GetImageDomainsListVDSCommandParameters.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/GetImageDomainsListVDSCommandParameters.java deleted file mode 100644 index faf06b8..0000000 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/GetImageDomainsListVDSCommandParameters.java +++ /dev/null @@ -1,28 +0,0 @@ -package org.ovirt.engine.core.common.vdscommands; - -import org.ovirt.engine.core.compat.Guid; - -public class GetImageDomainsListVDSCommandParameters extends IrsBaseVDSCommandParameters { - public GetImageDomainsListVDSCommandParameters(Guid storagePoolId, Guid imageGroupId) { - super(storagePoolId); - setImageGroupId(imageGroupId); - } - - private Guid privateImageGroupId = new Guid(); - - public Guid getImageGroupId() { - return privateImageGroupId; - } - - private void setImageGroupId(Guid value) { - privateImageGroupId = value; - } - - public GetImageDomainsListVDSCommandParameters() { - } - - @Override - public String toString() { - return String.format("%s, imageGroupId = %s", super.toString(), getImageGroupId()); - } -} diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/VDSCommandType.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/VDSCommandType.java index d130d3b..5e5ecb1 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/VDSCommandType.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/VDSCommandType.java @@ -85,7 +85,6 @@ SyncImageGroupData("org.ovirt.engine.core.vdsbroker.irsbroker"), VmReplicateDiskStart("org.ovirt.engine.core.vdsbroker.vdsbroker"), VmReplicateDiskFinish("org.ovirt.engine.core.vdsbroker.vdsbroker"), - GetImageDomainsList("org.ovirt.engine.core.vdsbroker.irsbroker"), GetImagesList("org.ovirt.engine.core.vdsbroker.irsbroker"), GetVolumesList("org.ovirt.engine.core.vdsbroker.irsbroker"), CreateVG("org.ovirt.engine.core.vdsbroker.vdsbroker"), diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/GetImageDomainsListVDSCommand.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/GetImageDomainsListVDSCommand.java deleted file mode 100644 index c765df2..0000000 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/GetImageDomainsListVDSCommand.java +++ /dev/null @@ -1,52 +0,0 @@ -package org.ovirt.engine.core.vdsbroker.irsbroker; - -import org.ovirt.engine.core.common.errors.VDSError; -import org.ovirt.engine.core.common.errors.VdcBllErrors; -import org.ovirt.engine.core.common.vdscommands.GetImageDomainsListVDSCommandParameters; -import org.ovirt.engine.core.compat.Guid; -import org.ovirt.engine.core.vdsbroker.vdsbroker.StatusForXmlRpc; - -public class GetImageDomainsListVDSCommand<P extends GetImageDomainsListVDSCommandParameters> - extends IrsBrokerCommand<P> { - private StorageDomainGuidListReturnForXmlRpc _result; - - public GetImageDomainsListVDSCommand(P parameters) { - super(parameters); - } - - @Override - protected void ExecuteIrsBrokerCommand() { - _result = getIrsProxy().getImageDomainsList(getParameters().getStoragePoolId().toString(), - getParameters().getImageGroupId().toString()); - ProceedProxyReturnValue(); - java.util.ArrayList<Guid> tempRetValue = new java.util.ArrayList<Guid>(_result.mStorageDomainGuidList.length); - for (int i = 0; i < _result.mStorageDomainGuidList.length; i++) { - tempRetValue.add(new Guid(_result.mStorageDomainGuidList[i])); - } - setReturnValue(tempRetValue); - } - - @Override - protected StatusForXmlRpc getReturnStatus() { - return _result.mStatus; - } - - @Override - protected Object getReturnValueFromBroker() { - return _result; - } - - @Override - protected void ProceedProxyReturnValue() { - VdcBllErrors returnStatus = GetReturnValueFromStatus(getReturnStatus()); - switch (returnStatus) { - case GetStorageDomainListError: - getVDSReturnValue().setVdsError(new VDSError(returnStatus, getReturnStatus().mMessage)); - getVDSReturnValue().setSucceeded(false); - break; - default: - super.ProceedProxyReturnValue(); - break; - } - } -} diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IIrsServer.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IIrsServer.java index 048b131..2cb0193 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IIrsServer.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IIrsServer.java @@ -65,8 +65,6 @@ OneUuidReturnForXmlRpc syncImageData(String spUUID, String srcDomUUID, String imgGUID, String dstDomUUID, String syncType); - StorageDomainGuidListReturnForXmlRpc getImageDomainsList(String spUUID, String imgUUID); - StatusOnlyReturnForXmlRpc setMaxHosts(int maxHosts); StatusOnlyReturnForXmlRpc updateVM(String spUUID, Map[] vms); diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsServerWrapper.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsServerWrapper.java index 6bb35fb..11bfb4d 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsServerWrapper.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsServerWrapper.java @@ -213,13 +213,6 @@ } @Override - public StorageDomainGuidListReturnForXmlRpc getImageDomainsList(String spUUID, String imgUUID) { - Map<String, Object> xmlRpcReturnValue = irsServer.getImageDomainsList(spUUID, imgUUID); - StorageDomainGuidListReturnForXmlRpc wrapper = new StorageDomainGuidListReturnForXmlRpc(xmlRpcReturnValue); - return wrapper; - } - - @Override public StatusOnlyReturnForXmlRpc setMaxHosts(int maxHosts) { Map<String, Object> xmlRpcReturnValue = irsServer.setMaxHosts(maxHosts); StatusOnlyReturnForXmlRpc wrapper = new StatusOnlyReturnForXmlRpc(xmlRpcReturnValue); -- To view, visit http://gerrit.ovirt.org/15951 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I33069871db34e8b129c4d80098523654cd2a0629 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
