Michael Kublin has uploaded a new change for review.
Change subject: engine: Clean up of exceptions
......................................................................
engine: Clean up of exceptions
The following patch is removing unneeded exceptions, unneeded catch of
exceptions,
unneeded code inside of catch block.
Also contains some small clean ups of IrsBrokerCommand
Change-Id: I27352290878620e922b1ba263172267ec9001aa0
Signed-off-by: Michael Kublin <mkub...@redhat.com>
---
M
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/GetStoragePoolInfoVDSCommand.java
D
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IRSNetworkException.java
M
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
M
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/GetVGListVDSCommand.java
M
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HSMGetStorageDomainInfoVDSCommand.java
5 files changed, 78 insertions(+), 178 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/67/9267/1
diff --git
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/GetStoragePoolInfoVDSCommand.java
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/GetStoragePoolInfoVDSCommand.java
index 85b19f5..90a05ba 100644
---
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/GetStoragePoolInfoVDSCommand.java
+++
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/GetStoragePoolInfoVDSCommand.java
@@ -9,8 +9,6 @@
import
org.ovirt.engine.core.common.vdscommands.GetStoragePoolInfoVDSCommandParameters;
import org.ovirt.engine.core.compat.Guid;
import org.ovirt.engine.core.compat.KeyValuePairCompat;
-import org.ovirt.engine.core.utils.log.Log;
-import org.ovirt.engine.core.utils.log.LogFactory;
import org.ovirt.engine.core.utils.log.Logged;
import org.ovirt.engine.core.utils.log.Logged.LogLevel;
import
org.ovirt.engine.core.vdsbroker.vdsbroker.GetStorageDomainStatsVDSCommand;
@@ -31,7 +29,7 @@
protected void ExecuteIrsBrokerCommand() {
_result =
getIrsProxy().getStoragePoolInfo(getParameters().getStoragePoolId().toString());
ProceedProxyReturnValue();
- storage_pool sp = ParseStoragePoolInfoResult(_result.mStoragePoolInfo);
+ storage_pool sp =
VdsBrokerObjectsBuilder.buildStoragePool(_result.mStoragePoolInfo);
Guid masterId = Guid.Empty;
if (_result.mStoragePoolInfo.containsKey("master_uuid")) {
masterId = new
Guid(_result.mStoragePoolInfo.getItem("master_uuid").toString());
@@ -65,25 +63,8 @@
return domainsList;
}
- private storage_pool ParseStoragePoolInfoResult(XmlRpcStruct xmlRpcStruct)
{
- try {
-
- return VdsBrokerObjectsBuilder.buildStoragePool(xmlRpcStruct);
-
- } catch (RuntimeException ex) {
- log.errorFormat(
- "irsBroker::BuildStorageDynamicFromXmlRpcStruct::Failed
building Storage dynamic, xmlRpcStruct = {0}",
- xmlRpcStruct.toString());
- IRSErrorException outEx = new IRSErrorException(ex);
- log.error(outEx);
- throw outEx;
- }
- }
-
@Override
protected StatusForXmlRpc getReturnStatus() {
return _result.mStatus;
}
-
- private static Log log =
LogFactory.getLog(GetStoragePoolInfoVDSCommand.class);
}
diff --git
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IRSNetworkException.java
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IRSNetworkException.java
deleted file mode 100644
index 9750d9e..0000000
---
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IRSNetworkException.java
+++ /dev/null
@@ -1,17 +0,0 @@
-package org.ovirt.engine.core.vdsbroker.irsbroker;
-
-public class IRSNetworkException extends IRSGenericException implements
java.io.Serializable {
- // protected IRSNetworkException(SerializationInfo info, StreamingContext
- // context)
- // {
- // super(info, context);
- // }
- public IRSNetworkException(RuntimeException baseException) {
- super("IRSNetworkException: ", baseException);
- }
-
- public IRSNetworkException(String errorStr) {
- super("IRSNetworkException: " + errorStr);
-
- }
-}
diff --git
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
index eab357be..a42b4b9 100644
---
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
+++
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
@@ -309,17 +309,10 @@
&& domainInDb.getstatus() !=
StorageDomainStatus.Locked
&& !domainsInVds.contains(domainInDb.getId()))
{
// domain not attached to pool anymore
-
TransactionSupport.executeInScope(TransactionScopeOption.Suppress,
- new TransactionMethod<Object>() {
- @Override
- public Object runInTransaction() {
- DbFacade.getInstance()
- .getStoragePoolIsoMapDao()
- .remove(new
StoragePoolIsoMapId(domainInDb.getId(),
- _storagePoolId));
- return null;
- }
- });
+ DbFacade.getInstance()
+ .getStoragePoolIsoMapDao()
+ .remove(new
StoragePoolIsoMapId(domainInDb.getId(),
+ _storagePoolId));
}
}
}
@@ -989,7 +982,7 @@
if
(returnValue.mStoragePoolInfo.contains(IrsProperties.isoPrefix)) {
mIsoPrefix =
returnValue.mStoragePoolInfo.getItem(IrsProperties.isoPrefix).toString();
}
- } catch (java.lang.Exception ex) {
+ } catch (Exception ex) {
log.errorFormat("IrsBroker::IsoPrefix Failed to get
IRS statistics.");
}
}
@@ -1455,7 +1448,7 @@
synchronized (_lockObject) {
if (_vdssInProblem.containsKey(vdsId)) {
- for (Guid domainId : new
HashSet<Guid>(_vdssInProblem.get(vdsId))) {
+ for (Guid domainId : _vdssInProblem.get(vdsId)) {
DomainRecoveredFromProblem(domainId, vdsId, vdsName);
}
}
@@ -1607,20 +1600,11 @@
} else {
isStartReconstruct = true;
}
- } catch (IRSNetworkException ex) {
- getVDSReturnValue().setExceptionString(ex.toString());
- getVDSReturnValue().setExceptionObject(ex);
- getVDSReturnValue().setVdsError(ex.getVdsError());
- throw new IRSGenericException("Storage Domain server not
found", ex);
} catch (IRSUnicodeArgumentException ex) {
- getVDSReturnValue().setExceptionString(ex.toString());
- getVDSReturnValue().setExceptionObject(ex);
- getVDSReturnValue().setVdsError(ex.getVdsError());
throw new IRSGenericException("UNICODE characters are not
supported.", ex);
} catch (IRSStoragePoolStatusException ex) {
- getVDSReturnValue().setExceptionString(ex.toString());
- getVDSReturnValue().setExceptionObject(ex);
- getVDSReturnValue().setVdsError(ex.getVdsError());
+ throw ex;
+ } catch (IrsOperationFailedNoFailoverException ex) {
throw ex;
} catch (IRSNonOperationalException ex) {
getVDSReturnValue().setExceptionString(ex.toString());
@@ -1631,12 +1615,6 @@
getCurrentIrsProxyData().setCurrentVdsId(Guid.Empty);
}
failover();
- } catch (IrsOperationFailedNoFailoverException ex) {
- getVDSReturnValue().setExceptionString(ex.toString());
- getVDSReturnValue().setExceptionObject(ex);
- getVDSReturnValue().setVdsError(ex.getVdsError());
- logException(ex);
- getVDSReturnValue().setSucceeded(false);
} catch (IRSErrorException ex) {
getVDSReturnValue().setExceptionString(ex.toString());
getVDSReturnValue().setExceptionObject(ex);
@@ -1650,7 +1628,7 @@
getVDSReturnValue().setExceptionString(ex.toString());
getVDSReturnValue().setExceptionObject(ex);
if (ex instanceof VDSExceptionBase) {
-
getVDSReturnValue().setVdsError(((VDSExceptionBase)ex).getVdsError());
+ getVDSReturnValue().setVdsError(((VDSExceptionBase)
ex).getVdsError());
}
if (ExceptionUtils.getRootCause(ex) != null &&
ExceptionUtils.getRootCause(ex) instanceof
SocketException) {
@@ -1662,9 +1640,7 @@
// realize what to do in each case:
failover();
} finally {
- if
(_irsProxyData.containsKey(getParameters().getStoragePoolId())) {
- getCurrentIrsProxyData().getTriedVdssList().clear();
- }
+ getCurrentIrsProxyData().getTriedVdssList().clear();
}
}
if (isStartReconstruct) {
@@ -1715,14 +1691,6 @@
*/
private void logException(Throwable ex) {
log.errorFormat("IrsBroker::Failed::{0} due to: {1}",
getCommandName(), ExceptionUtils.getMessage(ex));
- }
-
- protected static Guid[] convertStringGuidArray(String[] idArray) {
- Guid[] guidArray = new Guid[idArray.length];
- for (int idx = 0; idx < idArray.length; idx++) {
- guidArray[idx] = new Guid(idArray[idx]);
- }
- return guidArray;
}
public static Long AssignLongValue(XmlRpcStruct input, String name) {
diff --git
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/GetVGListVDSCommand.java
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/GetVGListVDSCommand.java
index 7a9862d..237bf2c 100644
---
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/GetVGListVDSCommand.java
+++
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/GetVGListVDSCommand.java
@@ -5,9 +5,6 @@
import org.ovirt.engine.core.common.utils.EnumUtils;
import org.ovirt.engine.core.common.vdscommands.VdsIdVDSCommandParametersBase;
import org.ovirt.engine.core.compat.Guid;
-import org.ovirt.engine.core.utils.log.Log;
-import org.ovirt.engine.core.utils.log.LogFactory;
-import org.ovirt.engine.core.vdsbroker.irsbroker.IRSErrorException;
import org.ovirt.engine.core.vdsbroker.irsbroker.IrsBrokerCommand;
import org.ovirt.engine.core.vdsbroker.xmlrpc.XmlRpcStruct;
@@ -33,38 +30,30 @@
protected java.util.ArrayList<storage_domains> ParseVGList(XmlRpcStruct[]
vgList) {
java.util.ArrayList<storage_domains> result = new
java.util.ArrayList<storage_domains>(vgList.length);
for (XmlRpcStruct vg : vgList) {
- try {
- storage_domains sDomain = new storage_domains();
- if (vg.contains("name")) {
- try {
- sDomain.setId(new Guid(vg.getItem("name").toString()));
- } catch (java.lang.Exception e) {
- sDomain.setstorage_name(vg.getItem("name").toString());
- }
+ storage_domains sDomain = new storage_domains();
+ if (vg.contains("name")) {
+ try {
+ sDomain.setId(new Guid(vg.getItem("name").toString()));
+ } catch (java.lang.Exception e) {
+ sDomain.setstorage_name(vg.getItem("name").toString());
}
- sDomain.setstorage(vg.getItem("vgUUID").toString());
- Long size = IrsBrokerCommand.AssignLongValue(vg, "vgfree");
- if (size != null) {
- sDomain.setavailable_disk_size((int) (size /
IrsBrokerCommand.BYTES_TO_GB));
- }
- size = IrsBrokerCommand.AssignLongValue(vg, "vgsize");
- if (size != null && sDomain.getavailable_disk_size() != null) {
- sDomain.setused_disk_size((int) (size /
IrsBrokerCommand.BYTES_TO_GB)
- - sDomain.getavailable_disk_size());
- }
- if (vg.containsKey("vgtype")) {
-
sDomain.setstorage_type(EnumUtils.valueOf(StorageType.class,
vg.getItem("vgtype").toString(), true));
- } else {
- sDomain.setstorage_type(StorageType.ALL);
- }
- result.add(sDomain);
- } catch (RuntimeException ex) {
- log.errorFormat("irsBroker::ParseVGList::Failed building
Storage domain, xmlRpcStruct = {0}",
- vg.toString());
- IRSErrorException outEx = new IRSErrorException(ex);
- log.error(outEx);
- throw outEx;
}
+ sDomain.setstorage(vg.getItem("vgUUID").toString());
+ Long size = IrsBrokerCommand.AssignLongValue(vg, "vgfree");
+ if (size != null) {
+ sDomain.setavailable_disk_size((int) (size /
IrsBrokerCommand.BYTES_TO_GB));
+ }
+ size = IrsBrokerCommand.AssignLongValue(vg, "vgsize");
+ if (size != null && sDomain.getavailable_disk_size() != null) {
+ sDomain.setused_disk_size((int) (size /
IrsBrokerCommand.BYTES_TO_GB)
+ - sDomain.getavailable_disk_size());
+ }
+ if (vg.containsKey("vgtype")) {
+ sDomain.setstorage_type(EnumUtils.valueOf(StorageType.class,
vg.getItem("vgtype").toString(), true));
+ } else {
+ sDomain.setstorage_type(StorageType.ALL);
+ }
+ result.add(sDomain);
}
return result;
}
@@ -73,6 +62,4 @@
protected Object getReturnValueFromBroker() {
return _result;
}
-
- private static Log log = LogFactory.getLog(GetVGListVDSCommand.class);
}
diff --git
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HSMGetStorageDomainInfoVDSCommand.java
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HSMGetStorageDomainInfoVDSCommand.java
index 307bc04..454fc2c 100644
---
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HSMGetStorageDomainInfoVDSCommand.java
+++
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HSMGetStorageDomainInfoVDSCommand.java
@@ -12,9 +12,6 @@
import org.ovirt.engine.core.common.utils.Pair;
import
org.ovirt.engine.core.common.vdscommands.HSMGetStorageDomainInfoVDSCommandParameters;
import org.ovirt.engine.core.dal.dbbroker.DbFacade;
-import org.ovirt.engine.core.utils.log.Log;
-import org.ovirt.engine.core.utils.log.LogFactory;
-import org.ovirt.engine.core.vdsbroker.irsbroker.IRSErrorException;
import org.ovirt.engine.core.vdsbroker.xmlrpc.XmlRpcStruct;
public class HSMGetStorageDomainInfoVDSCommand<P extends
HSMGetStorageDomainInfoVDSCommandParameters>
@@ -35,66 +32,52 @@
}
private static Pair<storage_domain_static, SANState>
BuildStorageStaticFromXmlRpcStruct(XmlRpcStruct xmlRpcStruct) {
- try {
- Pair<storage_domain_static, SANState> returnValue = new
Pair<storage_domain_static, SANState>();
- storage_domain_static sdStatic = new storage_domain_static();
- if (xmlRpcStruct.contains("name")) {
-
sdStatic.setstorage_name(xmlRpcStruct.getItem("name").toString());
- }
- if (xmlRpcStruct.contains("type")) {
- sdStatic.setstorage_type(EnumUtils.valueOf(StorageType.class,
xmlRpcStruct.getItem("type").toString(),
- true));
- }
- if (xmlRpcStruct.contains("class")) {
- String domainType = xmlRpcStruct.getItem("class").toString();
- if ("backup".equalsIgnoreCase(domainType)) {
-
sdStatic.setstorage_domain_type(StorageDomainType.ImportExport);
- } else {
-
sdStatic.setstorage_domain_type(EnumUtils.valueOf(StorageDomainType.class,
domainType, true));
- }
- }
- if (xmlRpcStruct.contains("version")) {
- sdStatic.setStorageFormat(
-
StorageFormatType.forValue(xmlRpcStruct.getItem("version").toString()));
- }
- if (sdStatic.getstorage_type() != StorageType.UNKNOWN) {
- if (sdStatic.getstorage_type() == StorageType.NFS &&
xmlRpcStruct.contains("remotePath")) {
- String path =
xmlRpcStruct.getItem("remotePath").toString();
- List<storage_server_connections> connections =
DbFacade.getInstance()
-
.getStorageServerConnectionDao().getAllForStorage(path);
- if (connections.isEmpty()) {
- sdStatic.setConnection(new
storage_server_connections());
- sdStatic.getConnection().setconnection(path);
-
sdStatic.getConnection().setstorage_type(StorageType.NFS);
- } else {
- sdStatic.setstorage(connections.get(0).getid());
- sdStatic.setConnection(connections.get(0));
- }
- } else if (sdStatic.getstorage_type() != StorageType.NFS &&
(xmlRpcStruct.contains("vguuid"))) {
-
sdStatic.setstorage(xmlRpcStruct.getItem("vguuid").toString());
- }
- }
- if (xmlRpcStruct.contains("state")) {
- returnValue.setSecond(EnumUtils.valueOf(SANState.class,
xmlRpcStruct.getItem("state")
- .toString()
- .toUpperCase(),
- false));
- }
- returnValue.setFirst(sdStatic);
- return returnValue;
- } catch (RuntimeException ex) {
- log.errorFormat(
- "vdsBroker::BuildStorageStaticFromXmlRpcStruct::Failed
building Storage static, xmlRpcStruct = {0}",
- xmlRpcStruct.toString());
- IRSErrorException outEx = new IRSErrorException(ex);
- log.error(outEx);
- if (log.isDebugEnabled()) {
-
log.debug("vdsBroker::BuildStorageStaticFromXmlRpcStruct::Failed building
Storage static stack:"
- + ex.getStackTrace(),
- ex);
- }
- throw outEx;
+ Pair<storage_domain_static, SANState> returnValue = new
Pair<storage_domain_static, SANState>();
+ storage_domain_static sdStatic = new storage_domain_static();
+ if (xmlRpcStruct.contains("name")) {
+ sdStatic.setstorage_name(xmlRpcStruct.getItem("name").toString());
}
+ if (xmlRpcStruct.contains("type")) {
+ sdStatic.setstorage_type(EnumUtils.valueOf(StorageType.class,
xmlRpcStruct.getItem("type").toString(),
+ true));
+ }
+ if (xmlRpcStruct.contains("class")) {
+ String domainType = xmlRpcStruct.getItem("class").toString();
+ if ("backup".equalsIgnoreCase(domainType)) {
+
sdStatic.setstorage_domain_type(StorageDomainType.ImportExport);
+ } else {
+
sdStatic.setstorage_domain_type(EnumUtils.valueOf(StorageDomainType.class,
domainType, true));
+ }
+ }
+ if (xmlRpcStruct.contains("version")) {
+ sdStatic.setStorageFormat(
+
StorageFormatType.forValue(xmlRpcStruct.getItem("version").toString()));
+ }
+ if (sdStatic.getstorage_type() != StorageType.UNKNOWN) {
+ if (sdStatic.getstorage_type() == StorageType.NFS &&
xmlRpcStruct.contains("remotePath")) {
+ String path = xmlRpcStruct.getItem("remotePath").toString();
+ List<storage_server_connections> connections =
DbFacade.getInstance()
+
.getStorageServerConnectionDao().getAllForStorage(path);
+ if (connections.isEmpty()) {
+ sdStatic.setConnection(new storage_server_connections());
+ sdStatic.getConnection().setconnection(path);
+ sdStatic.getConnection().setstorage_type(StorageType.NFS);
+ } else {
+ sdStatic.setstorage(connections.get(0).getid());
+ sdStatic.setConnection(connections.get(0));
+ }
+ } else if (sdStatic.getstorage_type() != StorageType.NFS &&
(xmlRpcStruct.contains("vguuid"))) {
+ sdStatic.setstorage(xmlRpcStruct.getItem("vguuid").toString());
+ }
+ }
+ if (xmlRpcStruct.contains("state")) {
+ returnValue.setSecond(EnumUtils.valueOf(SANState.class,
xmlRpcStruct.getItem("state")
+ .toString()
+ .toUpperCase(),
+ false));
+ }
+ returnValue.setFirst(sdStatic);
+ return returnValue;
}
@Override
@@ -106,6 +89,4 @@
protected Object getReturnValueFromBroker() {
return _result;
}
-
- private static final Log log =
LogFactory.getLog(HSMGetStorageDomainInfoVDSCommand.class);
}
--
To view, visit http://gerrit.ovirt.org/9267
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I27352290878620e922b1ba263172267ec9001aa0
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Michael Kublin <mkub...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
