Alissa Bonas has uploaded a new change for review. Change subject: core: update storage connection without a domain ......................................................................
core: update storage connection without a domain Allow updating a storage connection regardless whether there are storage domains using it or not. Add relevant unitests. Till now it was allowed to update a connection only if there were storage domains using it. As part of the agenda to allow users to manage the storage connections independently, removing this restriction is necessary, since connections can be added and edited before a storage domain started to use it. Change-Id: I4b72f48d6ae47e3d6d2a50ce0b51693d22840161 Signed-off-by: Alissa Bonas <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java 2 files changed, 69 insertions(+), 39 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/52/15952/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java index 57fc3bd..240bc72 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java @@ -30,7 +30,6 @@ import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.common.vdscommands.VDSReturnValue; import org.ovirt.engine.core.compat.Guid; - import org.ovirt.engine.core.dao.StorageDomainDynamicDAO; import org.ovirt.engine.core.dao.StoragePoolIsoMapDAO; import org.ovirt.engine.core.dao.StorageServerConnectionDAO; @@ -67,7 +66,7 @@ } if (checkIsConnectionFieldEmpty(newConnectionDetails)) { - return false; + return false; } Guid vdsmId = getParameters().getVdsId(); @@ -100,26 +99,22 @@ if (domains == null) { domains = getStorageDomainsByConnId(newConnectionDetails.getid()); } - if (domains.isEmpty()) { - return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST); + if(doDomainsUseConnection()) { + if (domains.size() == 1) { + setStorageDomain(domains.get(0)); + } + else { + String domainNames = createDomainNamesList(domains); + addCanDoActionMessage(String.format("$domainNames %1$s", domainNames)); + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_BELONGS_TO_SEVERAL_STORAGE_DOMAINS); + } + // Check that the storage domain is in proper state to be edited + if (!isConnectionEditable(getStorageDomain())) { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_UNSUPPORTED_ACTION_FOR_STORAGE); + } } - else if (domains.size() == 1) { - setStorageDomain(domains.get(0)); - } - else { - String domainNames = createDomainNamesList(domains); - addCanDoActionMessage(String.format("$domainNames %1$s", domainNames)); - return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_BELONGS_TO_SEVERAL_STORAGE_DOMAINS); - } - - // Check that the storage domain is in proper state to be edited - if (!isConnectionEditable(getStorageDomain())) { - return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_UNSUPPORTED_ACTION_FOR_STORAGE); - } - return super.canDoAction(); } - protected String createDomainNamesList(List<StorageDomain> domains) { // Build domain names list to display in the error @@ -142,9 +137,11 @@ @Override protected void executeCommand() { + boolean isDomainUpdateRequired = doDomainsUseConnection(); StoragePoolIsoMap map = getStoragePoolIsoMap(); - - changeStorageDomainStatusInTransaction(map, StorageDomainStatus.Locked); + if (isDomainUpdateRequired) { + changeStorageDomainStatusInTransaction(map, StorageDomainStatus.Locked); + } // connect to the new path boolean hasConnectStorageSucceeded = connectToStorage(); VDSReturnValue returnValueUpdatedStorageDomain = null; @@ -156,24 +153,39 @@ getReturnValue().setFault(f); return; } - // update info such as free space - because we switched to a different server - returnValueUpdatedStorageDomain = getStatsForDomain(); - if (returnValueUpdatedStorageDomain.getSucceeded()) { - final StorageDomain updatedStorageDomain = (StorageDomain) returnValueUpdatedStorageDomain.getReturnValue(); - executeInNewTransaction(new TransactionMethod<Void>() { - @Override - public Void runInTransaction() { - getStorageConnDao().update(getParameters().getStorageServerConnection()); - getStorageDomainDynamicDao().update(updatedStorageDomain.getStorageDynamicData()); - return null; - } - }); + if (isDomainUpdateRequired) { + // update info such as free space - because we switched to a different server + returnValueUpdatedStorageDomain = getStatsForDomain(); + if (returnValueUpdatedStorageDomain.getSucceeded()) { + final StorageDomain updatedStorageDomain = + (StorageDomain) returnValueUpdatedStorageDomain.getReturnValue(); + executeInNewTransaction(new TransactionMethod<Void>() { + @Override + public Void runInTransaction() { + getStorageDomainDynamicDao().update(updatedStorageDomain.getStorageDynamicData()); + return null; + } + }); - setSucceeded(true); + } + else{ + restoreStateAfterUpdate(map,false); + return; + } } + getStorageConnDao().update(getParameters().getStorageServerConnection()); + restoreStateAfterUpdate(map,true); + } + + protected void restoreStateAfterUpdate(StoragePoolIsoMap map, boolean setSucceeded) { updateStatus(map, StorageDomainStatus.Maintenance); disconnectFromStorage(); + setSucceeded(setSucceeded); + } + + protected boolean doDomainsUseConnection() { + return domains != null && !domains.isEmpty(); } protected StoragePoolIsoMap getStoragePoolIsoMap() { @@ -254,8 +266,6 @@ protected StoragePoolIsoMapDAO getStoragePoolIsoMapDao() { return getDbFacade().getStoragePoolIsoMapDao(); } - - @Override protected Map<String, Pair<String, String>> getExclusiveLocks() { diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java index ebe3d8f..fcc22c3 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java @@ -295,8 +295,7 @@ List<StorageDomain> domains = new ArrayList<StorageDomain>(); when(storageConnDao.get(newNFSConnection.getid())).thenReturn(oldNFSConnection); doReturn(domains).when(command).getStorageDomainsByConnId(newNFSConnection.getid()); - CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, - VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST); + CanDoActionTestUtils.runAndAssertCanDoActionSuccess(command); } @Test @@ -334,7 +333,7 @@ } @Test - public void succeedUpdateNFSCommand() { + public void succeedUpdateNFSCommandWithDomain() { StorageServerConnections newNFSConnection = createNFSConnection( "multipass.my.domain.tlv.company.com:/export/allstorage/data2", StorageType.NFS, @@ -361,6 +360,26 @@ } @Test + public void succeedUpdateNFSCommandNoDomain() { + StorageServerConnections newNFSConnection = createNFSConnection( + "multipass.my.domain.tlv.company.com:/export/allstorage/data2", + StorageType.NFS, + NfsVersion.V4, + 300, + 0); + VDSReturnValue returnValueConnectSuccess = new VDSReturnValue(); + StoragePoolIsoMap map = new StoragePoolIsoMap(); + doReturn(map).when(command).getStoragePoolIsoMap(); + doReturn(false).when(command).doDomainsUseConnection(); + returnValueConnectSuccess.setSucceeded(true); + doReturn(true).when(command).connectToStorage(); + doNothing().when(storageConnDao).update(newNFSConnection); + doNothing().when(command).disconnectFromStorage(); + command.executeCommand(); + CommandAssertUtils.checkSucceeded(command, true); + } + + @Test public void failUpdateStats() { StorageServerConnections newNFSConnection = createNFSConnection( "multipass.my.domain.tlv.company.com:/export/allstorage/data2", @@ -370,6 +389,7 @@ 0); VDSReturnValue returnValueUpdate = new VDSReturnValue(); returnValueUpdate.setSucceeded(false); + doReturn(true).when(command).doDomainsUseConnection(); StoragePoolIsoMap map = new StoragePoolIsoMap(); doReturn(map).when(command).getStoragePoolIsoMap(); doReturn(returnValueUpdate).when(command).getStatsForDomain(); -- To view, visit http://gerrit.ovirt.org/15952 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4b72f48d6ae47e3d6d2a50ce0b51693d22840161 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alissa Bonas <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
