diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java index b8788fbef98..4d106f5c4f6 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java @@ -36,6 +36,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotResult; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService; import org.apache.cloudstack.engine.subsystem.api.storage.StorageCacheManager; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.async.AsyncCallFuture; import org.apache.cloudstack.framework.async.AsyncCallbackDispatcher; @@ -79,6 +80,8 @@ public class SnapshotServiceImpl implements SnapshotService { StorageCacheManager _cacheMgr; @Inject private SnapshotDetailsDao _snapshotDetailsDao; + @Inject + VolumeDataFactory volFactory; static private class CreateSnapshotContext extends AsyncRpcContext { final SnapshotInfo snapshot; @@ -428,11 +431,15 @@ public class SnapshotServiceImpl implements SnapshotService { @Override public boolean revertSnapshot(SnapshotInfo snapshot) { + PrimaryDataStore store = null; SnapshotInfo snapshotOnPrimaryStore = _snapshotFactory.getSnapshot(snapshot.getId(), DataStoreRole.Primary); if (snapshotOnPrimaryStore == null) { - throw new CloudRuntimeException("Cannot find an entry for snapshot " + snapshot.getId() + " on primary storage pools"); + s_logger.warn("Cannot find an entry for snapshot " + snapshot.getId() + " on primary storage pools, searching with volume's primary storage pool"); + VolumeInfo volumeInfo = volFactory.getVolume(snapshot.getVolumeId(), DataStoreRole.Primary); + store = (PrimaryDataStore)volumeInfo.getDataStore(); + } else { + store = (PrimaryDataStore)snapshotOnPrimaryStore.getDataStore(); } - PrimaryDataStore store = (PrimaryDataStore)snapshotOnPrimaryStore.getDataStore(); AsyncCallFuture future = new AsyncCallFuture(); RevertSnapshotContext context = new RevertSnapshotContext(null, snapshot, future); diff --git a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImplTest.java b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImplTest.java new file mode 100644 index 00000000000..ec5c35501bb --- /dev/null +++ b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImplTest.java @@ -0,0 +1,98 @@ +/* + * 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.snapshot; + +import com.cloud.storage.DataStoreRole; +import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreDriver; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotResult; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; +import org.apache.cloudstack.framework.async.AsyncCallFuture; +import org.apache.cloudstack.framework.async.AsyncCallbackDispatcher; +import org.apache.cloudstack.storage.command.CommandResult; +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.MockitoAnnotations; +import org.mockito.Spy; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.support.AnnotationConfigContextLoader; + +@RunWith(PowerMockRunner.class) +@PrepareForTest({SnapshotServiceImpl.class}) +@ContextConfiguration(loader = AnnotationConfigContextLoader.class) +public class SnapshotServiceImplTest { + + @Spy + @InjectMocks + private SnapshotServiceImpl snapshotService = new SnapshotServiceImpl(); + + @Mock + VolumeDataFactory volFactory; + + @Mock + SnapshotDataFactory _snapshotFactory; + + @Mock + AsyncCallFuture futureMock; + + @Mock + AsyncCallbackDispatcher caller; + + @Before + public void testSetUp() throws Exception { + MockitoAnnotations.initMocks(this); + } + + @Test + public void testRevertSnapshotWithNoPrimaryStorageEntry() throws Exception { + SnapshotInfo snapshot = Mockito.mock(SnapshotInfo.class); + VolumeInfo volumeInfo = Mockito.mock(VolumeInfo.class); + + Mockito.when(snapshot.getId()).thenReturn(1L); + Mockito.when(snapshot.getVolumeId()).thenReturn(1L); + Mockito.when(_snapshotFactory.getSnapshot(1L, DataStoreRole.Primary)).thenReturn(null); + Mockito.when(volFactory.getVolume(1L, DataStoreRole.Primary)).thenReturn(volumeInfo); + + PrimaryDataStore store = Mockito.mock(PrimaryDataStore.class); + Mockito.when(volumeInfo.getDataStore()).thenReturn(store); + + PrimaryDataStoreDriver driver = Mockito.mock(PrimaryDataStoreDriver.class); + Mockito.when(store.getDriver()).thenReturn(driver); + Mockito.doNothing().when(driver).revertSnapshot(snapshot, null, caller); + + SnapshotResult result = Mockito.mock(SnapshotResult.class); + PowerMockito.whenNew(AsyncCallFuture.class).withNoArguments().thenReturn(futureMock); + Mockito.when(futureMock.get()).thenReturn(result); + Mockito.when(result.isFailed()).thenReturn(false); + + Assert.assertEquals(true, snapshotService.revertSnapshot(snapshot)); + } + +} diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java index 92e737e528e..4071c1bcb45 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java @@ -178,11 +178,13 @@ public class LibvirtRevertSnapshotCommandWrapper extends CommandWrapper getSnapshot(SnapshotObjectTO snapshotOnPrimaryStorage, SnapshotObjectTO snapshotOnSecondaryStorage, - KVMStoragePool kvmStoragePoolPrimary, KVMStoragePool kvmStoragePoolSecondary){ - String snapshotPath = snapshotOnPrimaryStorage.getPath(); - - if (Files.exists(Paths.get(snapshotPath))) { - return new Pair<>(snapshotPath, snapshotOnPrimaryStorage); + KVMStoragePool kvmStoragePoolPrimary, KVMStoragePool kvmStoragePoolSecondary) { + String snapshotPath = null; + if (snapshotOnPrimaryStorage != null) { + snapshotPath = snapshotOnPrimaryStorage.getPath(); + if (Files.exists(Paths.get(snapshotPath))) { + return new Pair<>(snapshotPath, snapshotOnPrimaryStorage); + } } if (kvmStoragePoolSecondary == null) { @@ -190,8 +192,10 @@ public class LibvirtRevertSnapshotCommandWrapper extends CommandWrapper callback) { - RevertSnapshotCommand cmd = new RevertSnapshotCommand((SnapshotObjectTO)snapshot.getTO(), (SnapshotObjectTO)snapshotOnPrimaryStore.getTO()); + SnapshotObjectTO dataOnPrimaryStorage = null; + if (snapshotOnPrimaryStore != null) { + dataOnPrimaryStorage = (SnapshotObjectTO)snapshotOnPrimaryStore.getTO(); + } + RevertSnapshotCommand cmd = new RevertSnapshotCommand((SnapshotObjectTO)snapshot.getTO(), dataOnPrimaryStorage); CommandResult result = new CommandResult(); try { - EndPoint ep = epSelector.select(snapshotOnPrimaryStore); + EndPoint ep = null; + if (snapshotOnPrimaryStore != null) { + ep = epSelector.select(snapshotOnPrimaryStore); + } else { + VolumeInfo volumeInfo = volFactory.getVolume(snapshot.getVolumeId(), DataStoreRole.Primary); + ep = epSelector.select(volumeInfo); + } if ( ep == null ){ String errMsg = "No remote endpoint to send RevertSnapshotCommand, check if host or ssvm is down?"; s_logger.error(errMsg);