Tal Nisan has uploaded a new change for review. Change subject: core: Log error correctly when connecting domain for removal ......................................................................
core: Log error correctly when connecting domain for removal When connecting to a storage domain as a part of it's removal (or format in case of ISO domains) if the connection failed the error is not logged correctly and instead we get a generic error, this was fixed and instead the generic error we will get the real cause Change-Id: Idd7435336acbec41edbdd70568f1dfdce664f875 Bug-Url: https://bugzilla.redhat.com/1003266 Signed-off-by: Tal Nisan <tni...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/BaseFsStorageHelper.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/FCPStorageHelper.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/IStorageHelper.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveStorageDomainCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHelperBase.java 6 files changed, 53 insertions(+), 24 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/42/33442/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/BaseFsStorageHelper.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/BaseFsStorageHelper.java index c2e9943..bd72855 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/BaseFsStorageHelper.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/BaseFsStorageHelper.java @@ -8,30 +8,35 @@ import org.ovirt.engine.core.bll.Backend; import org.ovirt.engine.core.common.action.StorageServerConnectionParametersBase; import org.ovirt.engine.core.common.action.VdcActionType; +import org.ovirt.engine.core.common.action.VdcReturnValueBase; import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.StorageDomainStatic; import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageServerConnections; +import org.ovirt.engine.core.common.errors.VdcFault; +import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.DbFacade; public abstract class BaseFsStorageHelper extends StorageHelperBase { @Override - protected boolean runConnectionStorageToDomain(StorageDomain storageDomain, Guid vdsId, int type) { - boolean returnValue = false; + protected Pair<Boolean, VdcFault> runConnectionStorageToDomain(StorageDomain storageDomain, Guid vdsId, int type) { + Pair<Boolean, VdcFault> result; StorageServerConnections connection = DbFacade.getInstance().getStorageServerConnectionDao().get( storageDomain.getStorage()); if (connection != null) { - returnValue = Backend + VdcReturnValueBase returnValue = Backend .getInstance() .runInternalAction(VdcActionType.forValue(type), - new StorageServerConnectionParametersBase(connection, vdsId)).getSucceeded(); + new StorageServerConnectionParametersBase(connection, vdsId)); + result = new Pair<>(returnValue.getSucceeded(), returnValue.getFault()); } else { + result = new Pair<>(false, null); log.warn("Did not connect host: " + vdsId + " to storage domain: " + storageDomain.getStorageName() + " because connection for connectionId:" + storageDomain.getStorage() + " is null."); } - return returnValue; + return result; } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/FCPStorageHelper.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/FCPStorageHelper.java index 36b1df1..aa1d492 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/FCPStorageHelper.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/FCPStorageHelper.java @@ -5,13 +5,15 @@ import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageDomainStatic; +import org.ovirt.engine.core.common.errors.VdcFault; +import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.compat.Guid; public class FCPStorageHelper extends StorageHelperBase { @Override - protected boolean runConnectionStorageToDomain(StorageDomain storageDomain, Guid vdsId, int type) { - return true; + protected Pair<Boolean, VdcFault> runConnectionStorageToDomain(StorageDomain storageDomain, Guid vdsId, int type) { + return new Pair<>(true, null); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java index b527679..1bc6462 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java @@ -15,6 +15,8 @@ import org.ovirt.engine.core.common.businessentities.StorageServerConnections; import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface; +import org.ovirt.engine.core.common.errors.VdcFault; +import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.common.vdscommands.StorageServerConnectionManagementVDSParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.common.vdscommands.VDSReturnValue; @@ -27,18 +29,19 @@ public class ISCSIStorageHelper extends StorageHelperBase { @Override - protected boolean runConnectionStorageToDomain(StorageDomain storageDomain, Guid vdsId, int type) { + protected Pair<Boolean, VdcFault> runConnectionStorageToDomain(StorageDomain storageDomain, Guid vdsId, int type) { return runConnectionStorageToDomain(storageDomain, vdsId, type, null, Guid.Empty); } @SuppressWarnings("unchecked") @Override - protected boolean runConnectionStorageToDomain(StorageDomain storageDomain, + protected Pair<Boolean, VdcFault> runConnectionStorageToDomain(StorageDomain storageDomain, Guid vdsId, int type, LUNs lun, Guid storagePoolId) { boolean isSuccess = true; + VDSReturnValue returnValue = null; List<StorageServerConnections> list = (lun == null) ? DbFacade.getInstance() .getStorageServerConnectionDao().getAllForVolumeGroup(storageDomain.getStorage()) @@ -55,7 +58,7 @@ if (storageDomain != null && storageDomain.getStoragePoolId() != null) { poolId = storageDomain.getStoragePoolId(); } - VDSReturnValue returnValue = Backend + returnValue = Backend .getInstance() .getResourceManager() .RunVdsCommand( @@ -67,7 +70,12 @@ isSuccess = isConnectSucceeded((Map<String, String>) returnValue.getReturnValue(), list); } } - return isSuccess; + VdcFault vdcFault = null; + if (!isSuccess && returnValue != null && returnValue.getVdsError() != null) { + vdcFault = new VdcFault(); + vdcFault.setError(returnValue.getVdsError().getCode()); + } + return new Pair<>(isSuccess, vdcFault); } public static List<StorageServerConnections> updateIfaces(List<StorageServerConnections> conns, Guid vdsId) { @@ -224,12 +232,12 @@ @Override public boolean connectStorageToDomainByVdsId(StorageDomain storageDomain, Guid vdsId) { - return runConnectionStorageToDomain(storageDomain, vdsId, VDSCommandType.ConnectStorageServer.getValue()); + return runConnectionStorageToDomain(storageDomain, vdsId, VDSCommandType.ConnectStorageServer.getValue()).getFirst(); } @Override public boolean disconnectStorageFromDomainByVdsId(StorageDomain storageDomain, Guid vdsId) { - return runConnectionStorageToDomain(storageDomain, vdsId, VDSCommandType.DisconnectStorageServer.getValue()); + return runConnectionStorageToDomain(storageDomain, vdsId, VDSCommandType.DisconnectStorageServer.getValue()).getFirst(); } @Override @@ -238,13 +246,13 @@ vdsId, VDSCommandType.ConnectStorageServer.getValue(), lun, - storagePoolId); + storagePoolId).getFirst(); } @Override public boolean disconnectStorageFromLunByVdsId(StorageDomain storageDomain, Guid vdsId, LUNs lun) { return runConnectionStorageToDomain(storageDomain, vdsId, VDSCommandType.DisconnectStorageServer.getValue(), - lun, Guid.Empty); + lun, Guid.Empty).getFirst(); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/IStorageHelper.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/IStorageHelper.java index b646972..b6ccd90 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/IStorageHelper.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/IStorageHelper.java @@ -7,12 +7,16 @@ import org.ovirt.engine.core.common.businessentities.StorageDomainStatic; import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageServerConnections; +import org.ovirt.engine.core.common.errors.VdcFault; +import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.compat.Guid; public interface IStorageHelper { boolean connectStorageToDomainByVdsId(StorageDomain storageDomain, Guid vdsId); + Pair<Boolean, VdcFault> connectStorageToDomainByVdsIdDetails(StorageDomain storageDomain, Guid vdsId); + boolean disconnectStorageFromDomainByVdsId(StorageDomain storageDomain, Guid vdsId); boolean connectStorageToLunByVdsId(StorageDomain storageDomain, Guid vdsId, LUNs lun, Guid storagePoolId); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveStorageDomainCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveStorageDomainCommand.java index 6f60af9..7b79c16 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveStorageDomainCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveStorageDomainCommand.java @@ -20,6 +20,7 @@ import org.ovirt.engine.core.common.errors.VdcBLLException; import org.ovirt.engine.core.common.errors.VdcBllErrors; import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.common.errors.VdcFault; import org.ovirt.engine.core.common.locks.LockingGroup; import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.common.vdscommands.FormatStorageDomainVDSCommandParameters; @@ -56,7 +57,9 @@ } if (!isISO(dom) && !isExport(dom) || format) { - if (!connectStorage()) { + Pair<Boolean, VdcFault> connectResult = connectStorage(); + if (!connectResult.getFirst()) { + getReturnValue().setFault(connectResult.getSecond()); return; } @@ -145,8 +148,8 @@ return true; } - private boolean connectStorage() { - return getStorageHelper(getStorageDomain()).connectStorageToDomainByVdsId(getStorageDomain(), + private Pair<Boolean, VdcFault> connectStorage() { + return getStorageHelper(getStorageDomain()).connectStorageToDomainByVdsIdDetails(getStorageDomain(), getVds().getId()); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHelperBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHelperBase.java index bfcfb23..228db38 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHelperBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHelperBase.java @@ -15,6 +15,8 @@ import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.errors.VdcBllErrors; +import org.ovirt.engine.core.common.errors.VdcFault; +import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector; @@ -28,25 +30,30 @@ protected final Log log = LogFactory.getLog(getClass()); - protected abstract boolean runConnectionStorageToDomain(StorageDomain storageDomain, Guid vdsId, int type); + protected abstract Pair<Boolean, VdcFault> runConnectionStorageToDomain(StorageDomain storageDomain, Guid vdsId, int type); - protected boolean runConnectionStorageToDomain(StorageDomain storageDomain, + protected Pair<Boolean, VdcFault> runConnectionStorageToDomain(StorageDomain storageDomain, Guid vdsId, int type, LUNs lun, Guid storagePoolId) { - return true; + return new Pair<>(true, null); } @Override public boolean connectStorageToDomainByVdsId(StorageDomain storageDomain, Guid vdsId) { + return connectStorageToDomainByVdsIdDetails(storageDomain, vdsId).getFirst(); + } + + @Override + public Pair<Boolean, VdcFault> connectStorageToDomainByVdsIdDetails(StorageDomain storageDomain, Guid vdsId) { return runConnectionStorageToDomain(storageDomain, vdsId, VdcActionType.ConnectStorageToVds.getValue()); } @Override public boolean disconnectStorageFromDomainByVdsId(StorageDomain storageDomain, Guid vdsId) { return runConnectionStorageToDomain(storageDomain, vdsId, - VdcActionType.DisconnectStorageServerConnection.getValue()); + VdcActionType.DisconnectStorageServerConnection.getValue()).getFirst(); } @Override @@ -55,13 +62,13 @@ vdsId, VdcActionType.ConnectStorageToVds.getValue(), lun, - storagePoolId); + storagePoolId).getFirst(); } @Override public boolean disconnectStorageFromLunByVdsId(StorageDomain storageDomain, Guid vdsId, LUNs lun) { return runConnectionStorageToDomain(storageDomain, vdsId, - VdcActionType.DisconnectStorageServerConnection.getValue(), lun, Guid.Empty); + VdcActionType.DisconnectStorageServerConnection.getValue(), lun, Guid.Empty).getFirst(); } @Override -- To view, visit http://gerrit.ovirt.org/33442 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Idd7435336acbec41edbdd70568f1dfdce664f875 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tal Nisan <tni...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
