diff --git a/conf/springConfigXml/volumeSnapshot.xml b/conf/springConfigXml/volumeSnapshot.xml
index 09022186cb2..d26ca551061 100755
--- a/conf/springConfigXml/volumeSnapshot.xml
+++ b/conf/springConfigXml/volumeSnapshot.xml
@@ -26,6 +26,7 @@
+
diff --git a/header/src/main/java/org/zstack/header/storage/snapshot/VolumeSnapshotDBSyncExtensionPoint.java b/header/src/main/java/org/zstack/header/storage/snapshot/VolumeSnapshotDBSyncExtensionPoint.java
new file mode 100644
index 00000000000..9a86d0f29cf
--- /dev/null
+++ b/header/src/main/java/org/zstack/header/storage/snapshot/VolumeSnapshotDBSyncExtensionPoint.java
@@ -0,0 +1,9 @@
+package org.zstack.header.storage.snapshot;
+
+import org.zstack.header.volume.VolumeInventory;
+
+public interface VolumeSnapshotDBSyncExtensionPoint {
+ VolumeSnapshotInventory syncVolumeSnapshotDBAfterTakeSnapshot(VolumeInventory volume,
+ VolumeSnapshotInventory snapshot,
+ String volumeNewInstallPath);
+}
diff --git a/plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java b/plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
index ea36ca8d56b..d58fdaaf509 100755
--- a/plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
+++ b/plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
@@ -71,6 +71,7 @@
import org.zstack.header.rest.JsonAsyncRESTCallback;
import org.zstack.header.rest.RESTFacade;
import org.zstack.header.storage.primary.*;
+import org.zstack.header.storage.snapshot.*;
import org.zstack.header.tag.SystemTagInventory;
import org.zstack.header.vm.*;
import org.zstack.header.vm.devices.DeviceAddress;
@@ -2944,6 +2945,7 @@ public void success(TakeSnapshotResponse ret) {
" data corruption may happen", ret.getNewVolumeInstallPath()));
}
+ syncSnapshotMetadataAfterHypervisorSuccess(msg, ret);
extEmitter.afterTakeSnapshot((KVMHostInventory) getSelfInventory(), msg, cmd, ret);
reply.setNewVolumeInstallPath(ret.getNewVolumeInstallPath());
reply.setSnapshotInstallPath(ret.getSnapshotInstallPath());
@@ -2980,6 +2982,27 @@ public void handle(ErrorCode errCode, Map data) {
}).start();
}
+ private void syncSnapshotMetadataAfterHypervisorSuccess(TakeSnapshotOnHypervisorMsg msg, TakeSnapshotResponse ret) {
+ VolumeInventory volume = msg.getVolume();
+ if (volume == null || msg.getSnapshotName() == null) {
+ return;
+ }
+
+ VolumeSnapshotInventory snapshot = new VolumeSnapshotInventory();
+ snapshot.setUuid(msg.getSnapshotName());
+ snapshot.setVolumeUuid(volume.getUuid());
+ snapshot.setPrimaryStorageUuid(volume.getPrimaryStorageUuid());
+ snapshot.setPrimaryStorageInstallPath(ret.getSnapshotInstallPath());
+ snapshot.setType(VolumeSnapshotConstant.HYPERVISOR_SNAPSHOT_TYPE.toString());
+ snapshot.setStatus(VolumeSnapshotStatus.Ready.toString());
+ snapshot.setSize(ret.getSize());
+ snapshot.setFormat(volume.getFormat());
+
+ for (VolumeSnapshotDBSyncExtensionPoint ext : pluginRgty.getExtensionList(VolumeSnapshotDBSyncExtensionPoint.class)) {
+ ext.syncVolumeSnapshotDBAfterTakeSnapshot(volume, snapshot, ret.getNewVolumeInstallPath());
+ }
+ }
+
private void migrateVm(final MigrateStruct s, final Completion completion) {
final TaskProgressRange parentStage = getTaskStage();
final TaskProgressRange MIGRATE_VM_STAGE = new TaskProgressRange(0, 90);
diff --git a/storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotManagerImpl.java b/storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotManagerImpl.java
index b107b056fdc..090f29ca85d 100755
--- a/storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotManagerImpl.java
+++ b/storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotManagerImpl.java
@@ -75,6 +75,7 @@ public class VolumeSnapshotManagerImpl extends AbstractService implements
VolumeBeforeExpungeExtensionPoint,
ResourceOwnerAfterChangeExtensionPoint,
ReportQuotaExtensionPoint,
+ VolumeSnapshotDBSyncExtensionPoint,
AfterReimageVmInstanceExtensionPoint,
VmJustBeforeDeleteFromDbExtensionPoint,
VolumeJustBeforeDeleteFromDbExtensionPoint,
@@ -678,6 +679,12 @@ private VolumeSnapshotStruct saveIndividualTypeSnapshot(VolumeSnapshotVO vo) {
private void rollbackSnapshot(String uuid) {
VolumeSnapshotVO vo = dbf.getEntityManager().find(VolumeSnapshotVO.class, uuid);
+ if (vo.getStatus() == VolumeSnapshotStatus.Ready) {
+ logger.warn(String.format("volume snapshot[uuid:%s] has been marked Ready, skip rollback to keep control plane metadata consistent with hypervisor snapshot",
+ uuid));
+ return;
+ }
+
dbf.getEntityManager().remove(vo);
String sql = "delete from AccountResourceRefVO where resourceUuid = :vsUuid and resourceType = 'VolumeSnapshotVO'";
@@ -695,6 +702,42 @@ private void rollbackSnapshot(String uuid) {
}
}
+ @Override
+ @Transactional
+ public VolumeSnapshotInventory syncVolumeSnapshotDBAfterTakeSnapshot(VolumeInventory volume,
+ VolumeSnapshotInventory snapshot,
+ String volumeNewInstallPath) {
+ if (volumeNewInstallPath != null) {
+ VolumeVO latestVol = dbf.findByUuid(volume.getUuid(), VolumeVO.class);
+ latestVol.setInstallPath(volumeNewInstallPath);
+ dbf.update(latestVol);
+ }
+
+ VolumeSnapshotVO svo = dbf.findByUuid(snapshot.getUuid(), VolumeSnapshotVO.class);
+ if (svo == null) {
+ return null;
+ }
+
+ svo.setType(snapshot.getType());
+ svo.setPrimaryStorageUuid(snapshot.getPrimaryStorageUuid());
+ svo.setPrimaryStorageInstallPath(snapshot.getPrimaryStorageInstallPath());
+ svo.setStatus(VolumeSnapshotStatus.Ready);
+ svo.setSize(snapshot.getSize());
+ if (snapshot.getFormat() != null) {
+ svo.setFormat(snapshot.getFormat());
+ }
+ svo = dbf.updateAndRefresh(svo);
+
+ markSnapshotTreeCompleted(VolumeSnapshotInventory.valueOf(svo));
+ if (svo.getParentUuid() == null) {
+ VolumeSnapshotReferenceUtils.updateReferenceAfterFirstSnapshot(svo);
+ }
+
+ logger.debug(String.format("synced volume snapshot[uuid:%s] metadata after hypervisor snapshot succeeded",
+ svo.getUuid()));
+ return VolumeSnapshotInventory.valueOf(svo);
+ }
+
private void handle(final AskVolumeSnapshotStructMsg msg) {
AskVolumeSnapshotStructReply reply = new AskVolumeSnapshotStructReply();
CreateVolumeSnapshotMsg cmsg = new CreateVolumeSnapshotMsg();
@@ -982,33 +1025,14 @@ public void run(MessageReply reply) {
done(new FlowDoneHandler(msg) {
@Override
public void handle(Map data) {
- markSnapshotTreeCompleted(snapshot);
- if (volumeNewInstallPath != null) {
- vol.setInstallPath(volumeNewInstallPath);
- dbf.update(vol);
- }
-
- VolumeSnapshotVO svo = dbf.findByUuid(snapshot.getUuid(), VolumeSnapshotVO.class);
- svo.setType(snapshot.getType());
- svo.setPrimaryStorageUuid(snapshot.getPrimaryStorageUuid());
- svo.setPrimaryStorageInstallPath(snapshot.getPrimaryStorageInstallPath());
- svo.setStatus(VolumeSnapshotStatus.Ready);
- svo.setSize(snapshot.getSize());
- if (snapshot.getFormat() != null) {
- svo.setFormat(snapshot.getFormat());
- }
- svo = dbf.updateAndRefresh(svo);
-
- if (struct.isNewChain()) {
- VolumeSnapshotReferenceUtils.updateReferenceAfterFirstSnapshot(svo);
- }
+ VolumeSnapshotInventory sp = syncVolumeSnapshotDBAfterTakeSnapshot(
+ vol.toInventory(), snapshot, volumeNewInstallPath);
new FireSnapShotCanonicalEvent().
fireSnapShotStatusChangedEvent(
VolumeSnapshotStatus.valueOf(snapshot.getStatus()),
- VolumeSnapshotInventory.valueOf(svo));
+ sp);
- VolumeSnapshotInventory sp = svo.toInventory();
callExtensionPoints(sp);
ret.setInventory(sp);
@@ -1025,8 +1049,18 @@ private void callExtensionPoints(VolumeSnapshotInventory sp) {
error(new FlowErrorHandler(msg) {
@Override
public void handle(ErrorCode errCode, Map data) {
- if (struct != null) {
- rollbackSnapshot(struct.getCurrent().getUuid());
+ if (snapshot != null) {
+ syncVolumeSnapshotDBAfterTakeSnapshot(vol.toInventory(), snapshot, volumeNewInstallPath);
+ logger.warn(String.format("volume snapshot[uuid:%s] has been created on primary storage, keep database record for recovery after error: %s",
+ snapshot.getUuid(), errCode));
+ } else if (struct != null) {
+ String snapshotUuid = struct.getCurrent().getUuid();
+ if (getCurrentSnapshotStatus(snapshotUuid) == VolumeSnapshotStatus.Ready) {
+ logger.warn(String.format("volume snapshot[uuid:%s] has been marked Ready, keep database record after error: %s",
+ snapshotUuid, errCode));
+ } else {
+ rollbackSnapshot(snapshotUuid);
+ }
}
ret.setError(errCode);
bus.reply(msg, ret);
@@ -1036,6 +1070,13 @@ public void handle(ErrorCode errCode, Map data) {
}).start();
}
+ private VolumeSnapshotStatus getCurrentSnapshotStatus(String uuid) {
+ return Q.New(VolumeSnapshotVO.class)
+ .select(VolumeSnapshotVO_.status)
+ .eq(VolumeSnapshotVO_.uuid, uuid)
+ .findValue();
+ }
+
private void handle(MarkRootVolumeAsSnapshotMsg msg) {
final MarkRootVolumeAsSnapshotReply ret = new MarkRootVolumeAsSnapshotReply();
VolumeInventory vol = msg.getVolume();
diff --git a/test/src/test/groovy/org/zstack/test/integration/storage/snapshot/CreateSnapshotRollbackAfterDataPlaneSuccessCase.groovy b/test/src/test/groovy/org/zstack/test/integration/storage/snapshot/CreateSnapshotRollbackAfterDataPlaneSuccessCase.groovy
new file mode 100644
index 00000000000..84d723b2464
--- /dev/null
+++ b/test/src/test/groovy/org/zstack/test/integration/storage/snapshot/CreateSnapshotRollbackAfterDataPlaneSuccessCase.groovy
@@ -0,0 +1,180 @@
+package org.zstack.test.integration.storage.snapshot
+
+import org.springframework.http.HttpEntity
+import org.zstack.core.Platform
+import org.zstack.core.componentloader.PluginRegistry
+import org.zstack.core.db.Q
+import org.zstack.header.core.Completion
+import org.zstack.header.storage.snapshot.VolumeSnapshotConstant
+import org.zstack.header.storage.snapshot.VolumeSnapshotStatus
+import org.zstack.header.storage.snapshot.VolumeSnapshotTreeStatus
+import org.zstack.header.storage.snapshot.VolumeSnapshotTreeVO
+import org.zstack.header.storage.snapshot.VolumeSnapshotInventory
+import org.zstack.header.storage.snapshot.VolumeSnapshotVO
+import org.zstack.header.storage.snapshot.VolumeSnapshotVO_
+import org.zstack.header.storage.snapshot.reference.VolumeSnapshotReferenceVO
+import org.zstack.header.storage.snapshot.reference.VolumeSnapshotReferenceVO_
+import org.zstack.header.volume.VolumeVO
+import org.zstack.header.volume.VolumeVO_
+import org.zstack.kvm.KVMAgentCommands
+import org.zstack.kvm.KVMConstant
+import org.zstack.sdk.VmInstanceInventory
+import org.zstack.storage.snapshot.AfterCreateVolumeSnapshotExtensionPoint
+import org.zstack.test.integration.ldap.Env
+import org.zstack.test.integration.storage.StorageTest
+import org.zstack.testlib.EnvSpec
+import org.zstack.testlib.SubCase
+import org.zstack.utils.gson.JSONObjectUtil
+
+class CreateSnapshotRollbackAfterDataPlaneSuccessCase extends SubCase {
+ EnvSpec env
+ VmInstanceInventory vm
+
+ int takeSnapshotCount = 0
+ String firstSnapshotInstallPath
+ String firstNewVolumeInstallPath
+ String secondVolumeInstallPath
+ String secondSnapshotInstallPath
+ String secondNewVolumeInstallPath
+ long snapshotSize = 1L
+
+ @Override
+ void clean() {
+ env.delete()
+ }
+
+ @Override
+ void setup() {
+ useSpring(StorageTest.springSpec)
+ }
+
+ @Override
+ void environment() {
+ env = Env.localStorageOneVmEnv()
+ }
+
+ @Override
+ void test() {
+ env.create {
+ vm = env.inventoryByName("vm") as VmInstanceInventory
+ testKeepSnapshotMetadataWhenControlPlaneFailsAfterDataPlaneSuccess()
+ }
+ }
+
+ void testKeepSnapshotMetadataWhenControlPlaneFailsAfterDataPlaneSuccess() {
+ String originalVolumeInstallPath = Q.New(VolumeVO.class)
+ .select(VolumeVO_.installPath)
+ .eq(VolumeVO_.uuid, vm.rootVolumeUuid)
+ .findValue()
+
+ env.simulator(KVMConstant.KVM_TAKE_VOLUME_SNAPSHOT_PATH) { HttpEntity e, EnvSpec espec ->
+ KVMAgentCommands.TakeSnapshotCmd cmd = JSONObjectUtil.toObject(e.body, KVMAgentCommands.TakeSnapshotCmd.class)
+ takeSnapshotCount++
+
+ if (takeSnapshotCount == 1) {
+ assert cmd.volumeInstallPath == originalVolumeInstallPath
+ firstSnapshotInstallPath = cmd.volumeInstallPath
+ firstNewVolumeInstallPath = cmd.installPath
+ } else if (takeSnapshotCount == 2) {
+ secondVolumeInstallPath = cmd.volumeInstallPath
+ secondSnapshotInstallPath = cmd.volumeInstallPath
+ secondNewVolumeInstallPath = cmd.installPath
+ }
+
+ KVMAgentCommands.TakeSnapshotResponse rsp = new KVMAgentCommands.TakeSnapshotResponse()
+ rsp.newVolumeInstallPath = cmd.installPath
+ rsp.snapshotInstallPath = cmd.volumeInstallPath
+ rsp.size = snapshotSize
+ return rsp
+ }
+
+ PluginRegistry pluginRegistry = bean(PluginRegistry.class)
+ List extensions = pluginRegistry.getExtensionList(AfterCreateVolumeSnapshotExtensionPoint.class)
+ FailAfterSnapshotCreatedExtension failureExtension = new FailAfterSnapshotCreatedExtension()
+ extensions.add(failureExtension)
+
+ try {
+ expectError {
+ createVolumeSnapshot {
+ name = "snapshot-fail-after-metadata-synced"
+ volumeUuid = vm.rootVolumeUuid
+ }
+ }
+
+ assert takeSnapshotCount == 1
+
+ VolumeSnapshotVO firstSnapshot
+ retryInSecs(10, 1) {
+ firstSnapshot = Q.New(VolumeSnapshotVO.class)
+ .eq(VolumeSnapshotVO_.volumeUuid, vm.rootVolumeUuid)
+ .find()
+ assert firstSnapshot != null
+ assert firstSnapshot.status == VolumeSnapshotStatus.Ready
+ }
+
+ assert firstSnapshot.latest
+ assert firstSnapshot.primaryStorageInstallPath == firstSnapshotInstallPath
+ assert firstSnapshot.type == VolumeSnapshotConstant.HYPERVISOR_SNAPSHOT_TYPE.toString()
+ assert firstSnapshot.size == snapshotSize
+
+ VolumeSnapshotTreeVO firstTree = dbFindByUuid(firstSnapshot.treeUuid, VolumeSnapshotTreeVO.class)
+ assert firstTree.current
+ assert firstTree.status == VolumeSnapshotTreeStatus.Completed
+
+ assert Q.New(VolumeVO.class)
+ .select(VolumeVO_.installPath)
+ .eq(VolumeVO_.uuid, vm.rootVolumeUuid)
+ .findValue() == firstNewVolumeInstallPath
+
+ VolumeSnapshotReferenceVO ref = Q.New(VolumeSnapshotReferenceVO.class)
+ .eq(VolumeSnapshotReferenceVO_.volumeUuid, vm.rootVolumeUuid)
+ .limit(1)
+ .find()
+ if (ref != null && ref.referenceInstallUrl == firstSnapshotInstallPath) {
+ assert ref.referenceUuid == firstSnapshot.uuid
+ assert ref.referenceType == VolumeSnapshotVO.class.simpleName
+ }
+
+ org.zstack.sdk.VolumeSnapshotInventory secondSnapshotInv = createVolumeSnapshot {
+ name = "snapshot-after-recovery"
+ volumeUuid = vm.rootVolumeUuid
+ } as org.zstack.sdk.VolumeSnapshotInventory
+
+ assert takeSnapshotCount == 2
+ assert secondVolumeInstallPath == firstNewVolumeInstallPath
+
+ VolumeSnapshotVO firstSnapshotAfterRecovery = dbFindByUuid(firstSnapshot.uuid, VolumeSnapshotVO.class)
+ VolumeSnapshotVO secondSnapshot = dbFindByUuid(secondSnapshotInv.uuid, VolumeSnapshotVO.class)
+ assert !firstSnapshotAfterRecovery.latest
+ assert secondSnapshot.latest
+ assert secondSnapshot.parentUuid == firstSnapshot.uuid
+ assert secondSnapshot.primaryStorageInstallPath == secondSnapshotInstallPath
+ assert secondSnapshot.status == VolumeSnapshotStatus.Ready
+
+ assert Q.New(VolumeSnapshotVO.class)
+ .eq(VolumeSnapshotVO_.volumeUuid, vm.rootVolumeUuid)
+ .count() == 2
+ assert Q.New(VolumeVO.class)
+ .select(VolumeVO_.installPath)
+ .eq(VolumeVO_.uuid, vm.rootVolumeUuid)
+ .findValue() == secondNewVolumeInstallPath
+ } finally {
+ extensions.remove(failureExtension)
+ }
+ }
+
+ private static class FailAfterSnapshotCreatedExtension implements AfterCreateVolumeSnapshotExtensionPoint {
+ boolean failAfterSnapshotCreated = true
+
+ @Override
+ void afterCreateVolumeSnapshot(VolumeSnapshotInventory snapshot, Completion completion) {
+ if (failAfterSnapshotCreated) {
+ failAfterSnapshotCreated = false
+ completion.fail(Platform.operr("TEST.ERROR", "fail after KVM snapshot has been created"))
+ return
+ }
+
+ completion.success()
+ }
+ }
+}