From c689d4a69699f8fab9a30b87f4d4ad04d25bcb6c Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 30 Mar 2017 16:35:37 +0200 Subject: [PATCH] CE-113 trace logging and rethrow instead of nesting CloudRuntimeException --- .../storage/snapshot/SnapshotServiceImpl.java | 3 +++ .../cloudstack/storage/volume/VolumeServiceImpl.java | 8 +++++++- .../cloud/storage/snapshot/SnapshotManagerImpl.java | 11 ++++++++++- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java index d13df9288a5..6b5fe364a2a 100644 --- a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java +++ b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java @@ -282,6 +282,9 @@ public class SnapshotServiceImpl implements SnapshotService { try { SnapshotResult res = future.get(); if (res.isFailed()) { + // TODO add cleanup actions in this case + // TODO think of whether a runtime exception is really what we want] + throw new CloudRuntimeException(res.getResult()); } SnapshotInfo destSnapshot = res.getSnapshot(); diff --git a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java index 2e72286b222..ba66d1ebb81 100644 --- a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java +++ b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java @@ -1993,8 +1993,14 @@ public class VolumeServiceImpl implements VolumeService { SnapshotInfo snapshot = null; try { snapshot = snapshotMgr.takeSnapshot(volume); + } catch (CloudRuntimeException cre) { + s_logger.error("Take snapshot: " + volume.getId() + " failed", cre); + // TODO deal with cleaning the mess + throw cre; } catch (Exception e) { - s_logger.debug("Take snapshot: " + volume.getId() + " failed", e); + if(s_logger.isDebugEnabled()) { + s_logger.debug("unknown exception while taking snapshot for volume " + volume.getId() + " was caught", e); + } throw new CloudRuntimeException("Failed to take snapshot", e); } diff --git a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java index 92937ba35c6..f3ea88bdcbe 100755 --- a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -1114,8 +1114,17 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement } catch (Exception e) { s_logger.debug("post process snapshot failed", e); } + } catch (CloudRuntimeException cre) { + if(s_logger.isDebugEnabled()) { + s_logger.debug("Failed to create snapshot" + cre.getLocalizedMessage()); + } + _resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.snapshot); + _resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.secondary_storage, new Long(volume.getSize())); + throw cre; } catch (Exception e) { - s_logger.debug("Failed to create snapshot", e); + if(s_logger.isDebugEnabled()) { + s_logger.debug("Failed to create snapshot", e); + } _resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.snapshot); _resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.secondary_storage, new Long(volume.getSize())); throw new CloudRuntimeException("Failed to create snapshot", e);