You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ra...@apache.org on 2015/06/03 08:16:34 UTC
git commit: updated refs/heads/master to b31b842
Repository: cloudstack
Updated Branches:
refs/heads/master bec44bffb -> b31b8425d
CLOUDSTACK-8525: NPE while updating the state of the volume after deletion
The volume is already deleted (may be by the cleanup thread) and hence
the NPE. Added a not null check for the volumevo and returning false
from the state transition
This closes #321
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/b31b8425
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/b31b8425
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/b31b8425
Branch: refs/heads/master
Commit: b31b8425df9b442a572aec7a2d462b80881fea0f
Parents: bec44bf
Author: Rajani Karuturi <ra...@gmail.com>
Authored: Thu May 28 11:41:33 2015 +0530
Committer: Rajani Karuturi <ra...@gmail.com>
Committed: Wed Jun 3 11:45:02 2015 +0530
----------------------------------------------------------------------
.../cloudstack/storage/volume/VolumeObject.java | 6 +-
.../storage/volume/VolumeObjectTest.java | 77 ++++++++++++++++++++
2 files changed, 81 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/b31b8425/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java
----------------------------------------------------------------------
diff --git a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java
index 1f574d5..e851870 100644
--- a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java
+++ b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java
@@ -178,8 +178,10 @@ public class VolumeObject implements VolumeInfo {
boolean result = false;
try {
volumeVO = volumeDao.findById(volumeVO.getId());
- result = _volStateMachine.transitTo(volumeVO, event, null, volumeDao);
- volumeVO = volumeDao.findById(volumeVO.getId());
+ if(volumeVO != null) {
+ result = _volStateMachine.transitTo(volumeVO, event, null, volumeDao);
+ volumeVO = volumeDao.findById(volumeVO.getId());
+ }
} catch (NoTransitionException e) {
String errorMessage = "Failed to transit volume: " + getVolumeId() + ", due to: " + e.toString();
s_logger.debug(errorMessage);
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/b31b8425/engine/storage/volume/test/org/apache/cloudstack/storage/volume/VolumeObjectTest.java
----------------------------------------------------------------------
diff --git a/engine/storage/volume/test/org/apache/cloudstack/storage/volume/VolumeObjectTest.java b/engine/storage/volume/test/org/apache/cloudstack/storage/volume/VolumeObjectTest.java
new file mode 100644
index 0000000..8118013
--- /dev/null
+++ b/engine/storage/volume/test/org/apache/cloudstack/storage/volume/VolumeObjectTest.java
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.cloudstack.storage.volume;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.storage.datastore.ObjectInDataStoreManager;
+import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import com.cloud.storage.Storage;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.DiskOfferingDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.vm.dao.VMInstanceDao;
+
+@RunWith(MockitoJUnitRunner.class)
+public class VolumeObjectTest {
+
+ @Mock
+ VolumeDao volumeDao;
+
+ @Mock
+ VolumeDataStoreDao volumeStoreDao;
+
+ @Mock
+ ObjectInDataStoreManager objectInStoreMgr;
+
+ @Mock
+ VMInstanceDao vmInstanceDao;
+
+ @Mock
+ DiskOfferingDao diskOfferingDao;
+
+ @InjectMocks
+ VolumeObject volumeObject;
+
+ @Before
+ public void setUp() throws Exception {
+ volumeObject.configure(Mockito.mock(DataStore.class), new VolumeVO("name", 1l, 1l, 1l, 1l, 1l, "folder", "path", Storage.ProvisioningType.THIN, 1l, Volume.Type.DATADISK));
+ }
+
+ /**
+ * Tests the following scenario:
+ * If the volume gets deleted by another thread (cleanup) and the cleanup is attempted again, the volume isnt found in DB and hence NPE occurs
+ * during transition
+ */
+ @Test
+ public void testStateTransit() {
+ boolean result = volumeObject.stateTransit(Volume.Event.OperationFailed);
+ Assert.assertFalse("since the volume doesnt exist in the db, the operation should fail but, should not throw any exception", result);
+ }
+}
\ No newline at end of file