From c18971fed7825890c061ce68ab4d4d6b7fcc2503 Mon Sep 17 00:00:00 2001 From: J M Date: Fri, 10 Jul 2026 17:53:04 +0800 Subject: [PATCH] [storage]: recover stale XInfini image cache Validate cached XInfini snapshots before reuse. Rebuild missing regular caches and preserve user-owned snapshot reuse sources. Resolves: ZSTAC-86038 Change-Id: I88b8ac1fa8740194d735aa2c0dd139cadbd119ff --- .../header/storage/addon/StorageResource.java | 44 +++++ .../primary/PrimaryStorageControllerSvc.java | 2 +- .../storage/snapshot/VolumeSnapshotStats.java | 19 +- .../org/zstack/header/volume/VolumeStats.java | 39 +--- .../zstack/expon/ExponStorageController.java | 20 +- .../org/zstack/xinfini/XInfiniApiHelper.java | 7 + .../xinfini/XInfiniStorageController.java | 20 +- .../storage/zbs/ZbsStorageController.java | 8 +- .../addon/primary/ExternalPrimaryStorage.java | 88 ++++++--- .../xinfini/XinfiniPrimaryStorageCase.groovy | 175 +++++++++++++++++- .../testlib/ExternalPrimaryStorageSpec.groovy | 12 +- 11 files changed, 337 insertions(+), 97 deletions(-) create mode 100644 header/src/main/java/org/zstack/header/storage/addon/StorageResource.java diff --git a/header/src/main/java/org/zstack/header/storage/addon/StorageResource.java b/header/src/main/java/org/zstack/header/storage/addon/StorageResource.java new file mode 100644 index 00000000000..d2a3f26b1e0 --- /dev/null +++ b/header/src/main/java/org/zstack/header/storage/addon/StorageResource.java @@ -0,0 +1,44 @@ +package org.zstack.header.storage.addon; + +public abstract class StorageResource { + protected String installPath; + protected Long size; + protected Long actualSize; + protected String parentUri; + + public String getInstallPath() { + return installPath; + } + + public void setInstallPath(String installPath) { + this.installPath = installPath; + } + + public Long getSize() { + return size; + } + + public void setSize(long size) { + this.size = size; + } + + public Long getResourceActualSize() { + return actualSize; + } + + public void setActualSize(Long actualSize) { + this.actualSize = actualSize; + } + + public void setActualSize(long actualSize) { + this.actualSize = actualSize; + } + + public String getParentUri() { + return parentUri; + } + + public void setParentUri(String parentUri) { + this.parentUri = parentUri; + } +} diff --git a/header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java b/header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java index feb4c5c11c7..92fb46ebb08 100644 --- a/header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java +++ b/header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java @@ -62,7 +62,7 @@ public interface PrimaryStorageControllerSvc { void flattenVolume(String installPath, ReturnValueCompletioncomp); // support uri or path - void stats(String installPath, ReturnValueCompletion comp); + void stats(String installPath, ReturnValueCompletion comp); void batchStats(Collection installPath, ReturnValueCompletion> comp); diff --git a/header/src/main/java/org/zstack/header/storage/snapshot/VolumeSnapshotStats.java b/header/src/main/java/org/zstack/header/storage/snapshot/VolumeSnapshotStats.java index 8b4e3778ef3..a3e1496ba7f 100644 --- a/header/src/main/java/org/zstack/header/storage/snapshot/VolumeSnapshotStats.java +++ b/header/src/main/java/org/zstack/header/storage/snapshot/VolumeSnapshotStats.java @@ -1,22 +1,9 @@ package org.zstack.header.storage.snapshot; -public class VolumeSnapshotStats { - private String installPath; - private long actualSize; - - public String getInstallPath() { - return installPath; - } - - public void setInstallPath(String installPath) { - this.installPath = installPath; - } +import org.zstack.header.storage.addon.StorageResource; +public class VolumeSnapshotStats extends StorageResource { public long getActualSize() { - return actualSize; - } - - public void setActualSize(long actualSize) { - this.actualSize = actualSize; + return actualSize == null ? 0L : actualSize; } } diff --git a/header/src/main/java/org/zstack/header/volume/VolumeStats.java b/header/src/main/java/org/zstack/header/volume/VolumeStats.java index 2931945e7ed..a123c135105 100644 --- a/header/src/main/java/org/zstack/header/volume/VolumeStats.java +++ b/header/src/main/java/org/zstack/header/volume/VolumeStats.java @@ -1,14 +1,9 @@ package org.zstack.header.volume; -public class VolumeStats { - protected String installPath; +import org.zstack.header.storage.addon.StorageResource; + +public class VolumeStats extends StorageResource { protected String format; - protected Long actualSize; - protected Long size; - /** - * The parent uri of the volume, vendor://pool/path@snapshot or snapshot://uuid - */ - protected String parentUri; // TODO(shenjin): remove it @Deprecated @@ -29,30 +24,10 @@ public VolumeStats(String installPath, Long actualSize, Long size) { public VolumeStats() { } - public String getInstallPath() { - return installPath; - } - - public void setInstallPath(String installPath) { - this.installPath = installPath; - } - public Long getActualSize() { return actualSize; } - public void setActualSize(Long actualSize) { - this.actualSize = actualSize; - } - - public Long getSize() { - return size; - } - - public void setSize(long size) { - this.size = size; - } - public void setFormat(String format) { this.format = format; } @@ -61,14 +36,6 @@ public String getFormat() { return format; } - public void setParentUri(String parentUri) { - this.parentUri = parentUri; - } - - public String getParentUri() { - return parentUri; - } - @Deprecated public String getRunStatus() { return runStatus; diff --git a/plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java b/plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java index ae0ca47de4c..d3d6fca016e 100644 --- a/plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java +++ b/plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java @@ -1143,18 +1143,32 @@ public void fail(ErrorCode errorCode) { @Override public void flattenVolume(String installPath, ReturnValueCompletion comp) { // TODO flatten snapshot - stats(installPath, comp); + comp.success(getVolumeStats(installPath)); } @Override - public void stats(String installPath, ReturnValueCompletion comp) { + public void stats(String installPath, ReturnValueCompletion comp) { + if (installPath.contains("@")) { + VolumeSnapshotModule snap = apiHelper.getVolumeSnapshot(getSnapIdFromPath(installPath)); + VolumeSnapshotStats stats = new VolumeSnapshotStats(); + stats.setInstallPath(installPath); + stats.setSize(snap.getSnapSize()); + stats.setActualSize(snap.getDataSize()); + comp.success(stats); + return; + } + + comp.success(getVolumeStats(installPath)); + } + + private VolumeStats getVolumeStats(String installPath) { VolumeModule vol = apiHelper.getVolume(getVolIdFromPath(installPath)); VolumeStats stats = new VolumeStats(); stats.setInstallPath(installPath); stats.setSize(vol.getVolumeSize()); stats.setActualSize(vol.getDataSize()); stats.setFormat(VolumeConstant.VOLUME_FORMAT_RAW); - comp.success(stats); + return stats; } @Override diff --git a/plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniApiHelper.java b/plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniApiHelper.java index b67bfa0da7e..813e6cee8c4 100644 --- a/plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniApiHelper.java +++ b/plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniApiHelper.java @@ -9,6 +9,7 @@ import org.zstack.core.retry.RetryCondition; import org.zstack.header.core.Completion; import org.zstack.header.errorcode.OperationFailureException; +import org.zstack.header.errorcode.SysErrors; import org.zstack.header.volume.VolumeConfigs; import org.zstack.header.volume.VolumeVO; import org.zstack.header.volume.VolumeVO_; @@ -36,6 +37,7 @@ import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; +import static org.zstack.core.Platform.err; import static org.zstack.core.Platform.operr; import static org.zstack.utils.clouderrorcode.CloudOperationsErrorCode.*; @@ -102,6 +104,11 @@ public void call(XInfiniRequest req, Completion comp } public void errorOut(XInfiniResponse rsp) { + if (rsp.resourceIsDeleted()) { + throw new OperationFailureException(err(ORG_ZSTACK_XINFINI_10002, SysErrors.RESOURCE_NOT_FOUND, + "xinfini request failed, message: %s.", rsp.getMessage())); + } + if (!rsp.isSuccess()) { throw new OperationFailureException(operr(ORG_ZSTACK_XINFINI_10002, "xinfini request failed, message: %s.", rsp.getMessage())); } diff --git a/plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageController.java b/plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageController.java index 187ee73ff22..96a38ee0433 100644 --- a/plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageController.java +++ b/plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageController.java @@ -851,7 +851,23 @@ public void flattenVolume(String installPath, ReturnValueCompletion } @Override - public void stats(String installPath, ReturnValueCompletion comp) { + public void stats(String installPath, ReturnValueCompletion comp) { + if (installPath.contains("@")) { + VolumeSnapshotModule snapshot = apiHelper.getVolumeSnapshot(getSnapIdFromPath(installPath)); + VolumeSnapshotStats stats = new VolumeSnapshotStats(); + long size = SizeUnit.MEGABYTE.toByte(snapshot.getSpec().getSizeMb()); + stats.setInstallPath(installPath); + stats.setSize(size); + stats.setActualSize(size); + stats.setParentUri(getParentUri(apiHelper.getVolume(snapshot.getSpec().getBsVolumeId()))); + comp.success(stats); + return; + } + + comp.success(getVolumeStats(installPath)); + } + + private VolumeStats getVolumeStats(String installPath) { VolumeModule vol = apiHelper.getVolume(getVolIdFromPath(installPath)); VolumeStats stats = new VolumeStats(); stats.setInstallPath(installPath); @@ -859,7 +875,7 @@ public void stats(String installPath, ReturnValueCompletion comp) { stats.setActualSize(vol.getStatus().getAllocatedSizeByte()); stats.setFormat(VolumeConstant.VOLUME_FORMAT_RAW); stats.setParentUri(getParentUri(vol)); - comp.success(stats); + return stats; } private String getParentUri(VolumeModule vol) { diff --git a/plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java b/plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java index a723f79c12e..933c328dc19 100644 --- a/plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java +++ b/plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java @@ -1315,19 +1315,21 @@ public void fail(ErrorCode errorCode) { } @Override - public void stats(String installPath, ReturnValueCompletion comp) { + public void stats(String installPath, ReturnValueCompletion comp) { QueryVolumeCmd cmd = new QueryVolumeCmd(); cmd.setPath(installPath); httpCall(QUERY_VOLUME_PATH, cmd, QueryVolumeRsp.class, new ReturnValueCompletion(comp) { @Override public void success(QueryVolumeRsp returnValue) { - VolumeStats stats = new VolumeStats(); + StorageResource stats = installPath.contains("@") ? new VolumeSnapshotStats() : new VolumeStats(); stats.setInstallPath(installPath); stats.setSize(returnValue.getSize()); stats.setActualSize(returnValue.getActualSize()); - stats.setFormat(VolumeConstant.VOLUME_FORMAT_RAW); stats.setParentUri(ZbsHelper.normalizeToZbsPath(returnValue.getParentUri())); + if (stats instanceof VolumeStats) { + ((VolumeStats) stats).setFormat(VolumeConstant.VOLUME_FORMAT_RAW); + } comp.success(stats); } diff --git a/storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java b/storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java index 6e222dd4959..bd487ab8a6c 100644 --- a/storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java +++ b/storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java @@ -28,6 +28,7 @@ import org.zstack.header.errorcode.ErrorCode; import org.zstack.header.errorcode.ErrorCodeList; import org.zstack.header.errorcode.OperationFailureException; +import org.zstack.header.errorcode.SysErrors; import org.zstack.header.host.HostInventory; import org.zstack.header.host.HostState; import org.zstack.header.host.HostStatus; @@ -42,6 +43,7 @@ import org.zstack.header.storage.addon.RemoteTarget; import org.zstack.header.storage.addon.StorageCapacity; import org.zstack.header.storage.addon.StorageHealthy; +import org.zstack.header.storage.addon.StorageResource; import org.zstack.header.storage.addon.primary.*; import org.zstack.header.storage.backup.*; import org.zstack.header.storage.primary.*; @@ -1397,19 +1399,59 @@ private void doDownloadImageCache(ImageInventory image, String allocatedUrl, Ret .eq(ImageCacheVO_.imageUuid, image.getUuid()) .list(); + ImageCacheVO candidate = null; if (!controller.reportCapabilities().isSupportMultiSpace() && !caches.isEmpty()) { - // TODO check exists in ps - completion.success(caches.get(0).toInventory()); - return; + candidate = caches.get(0); + } else { + for (ImageCacheVO cache : caches) { + ImageCacheInventory inv = cache.toInventory(); + // TODO: suppose that path always starts with allocatedUrl + if (allocatedUrl != null && ImageCacheUtil.getImageCachePath(inv).startsWith(allocatedUrl)) { + candidate = cache; + break; + } + } } - for (ImageCacheVO cache : caches) { - ImageCacheInventory inv = cache.toInventory(); - // TODO: suppose that path always starts with allocatedUrl - if (allocatedUrl != null && ImageCacheUtil.getImageCachePath(inv).startsWith(allocatedUrl)) { - completion.success(inv); - return; - } + if (candidate != null) { + ImageCacheVO cache = candidate; + String cachePath = ImageCacheUtil.getImageCachePath(cache.getInstallUrl()); + ReturnValueCompletion statsCompletion = new ReturnValueCompletion(completion) { + @Override + public void success(StorageResource ignored) { + completion.success(cache.toInventory()); + } + + @Override + public void fail(ErrorCode errorCode) { + if (!errorCode.isError(SysErrors.RESOURCE_NOT_FOUND)) { + completion.fail(errorCode); + return; + } + + if (cache.getInstallUrl().startsWith(ImageConstant.SNAPSHOT_REUSE_IMAGE_SCHEMA)) { + completion.fail(operr("image cache[imageUuid:%s, primaryStorageUuid:%s] refers to a missing reused volume snapshot", + cache.getImageUuid(), cache.getPrimaryStorageUuid())); + return; + } + + controller.deleteVolumeAndSnapshot(cachePath, new Completion(completion) { + @Override + public void success() { + dbf.removeByPrimaryKey(cache.getId(), ImageCacheVO.class); + doDownloadImageCache(image, allocatedUrl, completion); + } + + @Override + public void fail(ErrorCode errorCode) { + completion.fail(errorCode); + } + }); + } + }; + + controller.stats(cachePath, statsCompletion); + return; } downloadImageTo(image, spec, ImageCacheVO.class.getSimpleName(), new ReturnValueCompletion(completion) { @@ -1649,13 +1691,13 @@ protected void handle(GetInstallPathForDataVolumeDownloadMsg msg) { protected void handle(GetVolumeBackingChainFromPrimaryStorageMsg msg) { GetVolumeBackingChainFromPrimaryStorageReply r = new GetVolumeBackingChainFromPrimaryStorageReply(); new While<>(msg.getRootInstallPaths()).each((installPath, compl) -> { - getParentChain(installPath, new ArrayList<>(), new ReturnValueCompletion>(compl) { + getParentChain(installPath, new ArrayList<>(), new ReturnValueCompletion>(compl) { @Override - public void success(List chains) { + public void success(List chains) { long tsize = 0; List chainPaths = new ArrayList<>(); - for (VolumeStats stats : chains) { - tsize += stats.getActualSize(); + for (StorageResource stats : chains) { + tsize += stats.getResourceActualSize(); chainPaths.add(stats.getInstallPath()); } @@ -1681,10 +1723,10 @@ public void done(ErrorCodeList errorCodeList) { }); } - private void getParentChain(String installPath, List chains, ReturnValueCompletion> completion) { - controller.stats(installPath, new ReturnValueCompletion(completion) { + private void getParentChain(String installPath, List chains, ReturnValueCompletion> completion) { + controller.stats(installPath, new ReturnValueCompletion(completion) { @Override - public void success(VolumeStats stats) { + public void success(StorageResource stats) { chains.add(stats); if (stats.getParentUri() == null) { chains.remove(0); // do not contain self @@ -1886,10 +1928,10 @@ protected void handle(AskVolumeSnapshotCapabilityMsg msg) { @Override protected void handle(SyncVolumeSizeOnPrimaryStorageMsg msg) { SyncVolumeSizeOnPrimaryStorageReply reply = new SyncVolumeSizeOnPrimaryStorageReply(); - controller.stats(msg.getInstallPath(), new ReturnValueCompletion(msg) { + controller.stats(msg.getInstallPath(), new ReturnValueCompletion(msg) { @Override - public void success(VolumeStats stats) { - reply.setActualSize(stats.getActualSize()); + public void success(StorageResource stats) { + reply.setActualSize(stats.getResourceActualSize()); reply.setSize(stats.getSize()); bus.reply(msg, reply); } @@ -1905,10 +1947,10 @@ public void fail(ErrorCode errorCode) { @Override protected void handle(EstimateVolumeTemplateSizeOnPrimaryStorageMsg msg) { EstimateVolumeTemplateSizeOnPrimaryStorageReply reply = new EstimateVolumeTemplateSizeOnPrimaryStorageReply(); - controller.stats(msg.getInstallPath(), new ReturnValueCompletion(msg) { + controller.stats(msg.getInstallPath(), new ReturnValueCompletion(msg) { @Override - public void success(VolumeStats stats) { - reply.setActualSize(stats.getActualSize()); + public void success(StorageResource stats) { + reply.setActualSize(stats.getResourceActualSize()); reply.setSize(stats.getSize()); bus.reply(msg, reply); } diff --git a/test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/xinfini/XinfiniPrimaryStorageCase.groovy b/test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/xinfini/XinfiniPrimaryStorageCase.groovy index 2b31ba85be7..2cf54e1305c 100644 --- a/test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/xinfini/XinfiniPrimaryStorageCase.groovy +++ b/test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/xinfini/XinfiniPrimaryStorageCase.groovy @@ -13,13 +13,16 @@ import org.zstack.xinfini.XInfiniPathHelper import org.zstack.xinfini.sdk.vhost.BdcModule import org.zstack.xinfini.sdk.vhost.BdcBdevModule import org.zstack.header.core.Completion +import org.zstack.header.core.ReturnValueCompletion import org.zstack.header.errorcode.ErrorCode import org.zstack.header.errorcode.OperationFailureException import org.zstack.header.host.HostConstant import org.zstack.header.host.HostVO import org.zstack.header.host.HostVO_ import org.zstack.header.host.PingHostMsg +import org.zstack.header.image.ImageConstant import org.zstack.header.message.MessageReply +import org.zstack.header.storage.addon.StorageResource import org.zstack.header.storage.backup.DownloadImageFromRemoteTargetMsg import org.zstack.header.storage.backup.DownloadImageFromRemoteTargetReply import org.zstack.header.storage.backup.UploadImageToRemoteTargetReply @@ -30,6 +33,7 @@ import org.zstack.header.storage.primary.ImageCacheVO import org.zstack.header.storage.primary.ImageCacheVO_ import org.zstack.header.storage.primary.PrimaryStorageHostRefVO import org.zstack.header.storage.primary.PrimaryStorageHostRefVO_ +import org.zstack.header.storage.snapshot.VolumeSnapshotStats import org.zstack.header.vm.VmBootDevice import org.zstack.header.vm.VmInstanceState import org.zstack.header.vm.VmInstanceVO @@ -37,6 +41,7 @@ import org.zstack.header.vm.VmInstanceVO_ import org.zstack.header.vm.VmStateChangedOnHostMsg import org.zstack.header.vm.devices.DeviceAddress import org.zstack.header.vm.devices.VirtualDeviceInfo +import org.zstack.header.volume.VolumeStats import org.zstack.header.volume.VolumeVO import org.zstack.header.volume.VolumeVO_ import org.zstack.kvm.KVMAgentCommands @@ -195,6 +200,8 @@ class XinfiniPrimaryStorageCase extends SubCase { testCreateDataVolume() testCreateSnapshot() testCreateTemplate() + testRecreateVmWhenImageCacheSnapshotMissing() + testMissingReusedSnapshotDoesNotDeleteSourceVolume() testClean() testImageCacheClean() testDeletePs() @@ -283,12 +290,7 @@ class XinfiniPrimaryStorageCase extends SubCase { assert result.getRootVolumePrimaryStorages().size() == 1 - env.message(UploadImageToRemoteTargetMsg.class) { UploadImageToRemoteTargetMsg msg, CloudBus bus -> - UploadImageToRemoteTargetReply r = new UploadImageToRemoteTargetReply() - assert msg.getRemoteTargetUrl().startsWith(exportProtocol) - assert msg.getFormat() == "raw" - bus.reply(msg, r) - } + mockUploadImageToRemoteTarget() env.afterSimulator(KVMConstant.KVM_START_VM_PATH) { rsp, HttpEntity e -> def cmd = JSONObjectUtil.toObject(e.body, KVMAgentCommands.StartVmCmd.class) @@ -351,6 +353,125 @@ class XinfiniPrimaryStorageCase extends SubCase { deleteVm(vm2.uuid) } + void mockUploadImageToRemoteTarget() { + env.message(UploadImageToRemoteTargetMsg.class) { UploadImageToRemoteTargetMsg msg, CloudBus bus -> + UploadImageToRemoteTargetReply r = new UploadImageToRemoteTargetReply() + assert msg.getRemoteTargetUrl().startsWith(exportProtocol) + assert msg.getFormat() == "raw" + bus.reply(msg, r) + } + } + + void testRecreateVmWhenImageCacheSnapshotMissing() { + List imageCaches = Q.New(ImageCacheVO.class) + .eq(ImageCacheVO_.imageUuid, image.uuid) + .eq(ImageCacheVO_.primaryStorageUuid, ps.uuid) + .list() + assert imageCaches.size() == 1 : + "image cache precondition is invalid: expected=1 actual=${imageCaches.size()} imageUuid=${image.uuid} primaryStorageUuid=${ps.uuid}" + + ImageCacheVO oldImageCache = imageCaches[0] + int oldSnapshotId = XInfiniPathHelper.getSnapIdFromPath(oldImageCache.installUrl) + int backingVolumeId = XInfiniPathHelper.getVolIdFromPath(oldImageCache.installUrl) + assert ExternalPrimaryStorageSpec.XinfiniSimulators.snapshots.containsKey(oldSnapshotId) : + "image cache snapshot precondition is invalid: expected snapshotId=${oldSnapshotId} to exist actualSnapshotIds=${ExternalPrimaryStorageSpec.XinfiniSimulators.snapshots.keySet()} installUrl=${oldImageCache.installUrl}" + + ExternalPrimaryStorageSpec.XinfiniSimulators.snapshots.remove(oldSnapshotId) + long retainedImageCacheCount = Q.New(ImageCacheVO.class) + .eq(ImageCacheVO_.imageUuid, image.uuid) + .eq(ImageCacheVO_.primaryStorageUuid, ps.uuid) + .count() + assert !ExternalPrimaryStorageSpec.XinfiniSimulators.snapshots.containsKey(oldSnapshotId) : + "stale image cache setup must remove only its snapshot: expected snapshotId=${oldSnapshotId} to be absent actualSnapshotIds=${ExternalPrimaryStorageSpec.XinfiniSimulators.snapshots.keySet()}" + assert retainedImageCacheCount == 1 : + "stale image cache setup must retain its database record: expected=1 actual=${retainedImageCacheCount} imageUuid=${image.uuid} primaryStorageUuid=${ps.uuid}" + assert ExternalPrimaryStorageSpec.XinfiniSimulators.volumes.containsKey(backingVolumeId) : + "stale image cache setup must retain its backing volume: expected volumeId=${backingVolumeId} to exist actualVolumeIds=${ExternalPrimaryStorageSpec.XinfiniSimulators.volumes.keySet()} installUrl=${oldImageCache.installUrl}" + + mockUploadImageToRemoteTarget() + + def recreatedVm = createVmInstance { + name = "vm-from-stale-image-cache" + instanceOfferingUuid = instanceOffering.uuid + imageUuid = image.uuid + l3NetworkUuids = [l3.uuid] + hostUuid = host1.uuid + primaryStorageUuidForRootVolume = ps.uuid + } as VmInstanceInventory + + List refreshedImageCaches = Q.New(ImageCacheVO.class) + .eq(ImageCacheVO_.imageUuid, image.uuid) + .eq(ImageCacheVO_.primaryStorageUuid, ps.uuid) + .list() + assert refreshedImageCaches.size() == 1 : + "stale image cache must be replaced in place: expected=1 actual=${refreshedImageCaches.size()} imageUuid=${image.uuid} primaryStorageUuid=${ps.uuid}" + + int newSnapshotId = XInfiniPathHelper.getSnapIdFromPath(refreshedImageCaches[0].installUrl) + assert newSnapshotId != oldSnapshotId : + "stale image cache snapshot must be recreated: expectedSnapshotId!=${oldSnapshotId} actualSnapshotId=${newSnapshotId} installUrl=${refreshedImageCaches[0].installUrl}" + assert ExternalPrimaryStorageSpec.XinfiniSimulators.snapshots.containsKey(newSnapshotId) : + "refreshed image cache must reference an existing snapshot: expected snapshotId=${newSnapshotId} to exist actualSnapshotIds=${ExternalPrimaryStorageSpec.XinfiniSimulators.snapshots.keySet()} installUrl=${refreshedImageCaches[0].installUrl}" + assert !ExternalPrimaryStorageSpec.XinfiniSimulators.volumes.containsKey(backingVolumeId) : + "stale image cache backing volume must be deleted before redownload: expected volumeId=${backingVolumeId} to be absent actualVolumeIds=${ExternalPrimaryStorageSpec.XinfiniSimulators.volumes.keySet()}" + + deleteVm(recreatedVm.uuid) + } + + void testMissingReusedSnapshotDoesNotDeleteSourceVolume() { + ImageCacheVO cache = Q.New(ImageCacheVO.class) + .eq(ImageCacheVO_.imageUuid, image.uuid) + .eq(ImageCacheVO_.primaryStorageUuid, ps.uuid) + .find() + assert cache != null + + def sourceSnapshot = createVolumeSnapshot { + name = "snapshot-reuse-source" + volumeUuid = vol.uuid + } as VolumeSnapshotInventory + int sourceVolumeId = XInfiniPathHelper.getVolIdFromPath(sourceSnapshot.primaryStorageInstallPath) + int sourceSnapshotId = XInfiniPathHelper.getSnapIdFromPath(sourceSnapshot.primaryStorageInstallPath) + String originalInstallUrl = cache.installUrl + String reuseInstallUrl = ImageConstant.SNAPSHOT_REUSE_IMAGE_SCHEMA + sourceSnapshot.uuid + + try { + SQL.New(ImageCacheVO.class) + .set(ImageCacheVO_.installUrl, reuseInstallUrl) + .eq(ImageCacheVO_.id, cache.id) + .update() + ExternalPrimaryStorageSpec.XinfiniSimulators.snapshots.remove(sourceSnapshotId) + + CreateVmInstanceAction action = new CreateVmInstanceAction() + action.name = "vm-from-missing-reused-snapshot" + action.instanceOfferingUuid = instanceOffering.uuid + action.imageUuid = image.uuid + action.l3NetworkUuids = [l3.uuid] + action.hostUuid = host1.uuid + action.primaryStorageUuidForRootVolume = ps.uuid + action.sessionId = adminSession() + + CreateVmInstanceAction.Result result = action.call() + assert result.error != null + assert JSONObjectUtil.toJsonString(result.error).contains("missing reused volume snapshot") + + ImageCacheVO retainedCache = Q.New(ImageCacheVO.class).eq(ImageCacheVO_.id, cache.id).find() + assert retainedCache != null + assert retainedCache.installUrl == reuseInstallUrl + assert ExternalPrimaryStorageSpec.XinfiniSimulators.volumes.containsKey(sourceVolumeId) + assert Q.New(ImageCacheVO.class) + .eq(ImageCacheVO_.imageUuid, image.uuid) + .eq(ImageCacheVO_.primaryStorageUuid, ps.uuid) + .count() == 1 + } finally { + SQL.New(ImageCacheVO.class) + .set(ImageCacheVO_.installUrl, originalInstallUrl) + .eq(ImageCacheVO_.id, cache.id) + .update() + deleteVolumeSnapshot { + uuid = sourceSnapshot.uuid + } + } + } + // when the old host's storage client cannot be isolated, deactivate must report // a failure instead of silently succeeding, otherwise the VM gets started on a // new host while the old QEMU still holds the volume (split-brain). @@ -613,6 +734,26 @@ class XinfiniPrimaryStorageCase extends SubCase { volumeUuid = vol.uuid } as VolumeSnapshotInventory + StorageResource volumeStats = queryStorageResourceStats(vol.installPath) + assert volumeStats instanceof VolumeStats : + "volume path must return VolumeStats: expected=${VolumeStats.name} actual=${volumeStats.class.name} installPath=${vol.installPath}" + assert volumeStats.installPath == vol.installPath : + "volume stats installPath mismatch: expected=${vol.installPath} actual=${volumeStats.installPath}" + assert volumeStats.size != null && volumeStats.size > 0 : + "volume stats size must be positive: expected>0 actual=${volumeStats.size} installPath=${vol.installPath}" + assert volumeStats.actualSize != null && volumeStats.actualSize >= 0 : + "volume stats actualSize must be non-negative: expected>=0 actual=${volumeStats.actualSize} installPath=${vol.installPath}" + + StorageResource snapshotStats = queryStorageResourceStats(snapshot.primaryStorageInstallPath) + assert snapshotStats instanceof VolumeSnapshotStats : + "snapshot path must return VolumeSnapshotStats: expected=${VolumeSnapshotStats.name} actual=${snapshotStats.class.name} installPath=${snapshot.primaryStorageInstallPath}" + assert snapshotStats.installPath == snapshot.primaryStorageInstallPath : + "snapshot stats installPath mismatch: expected=${snapshot.primaryStorageInstallPath} actual=${snapshotStats.installPath}" + assert snapshotStats.size != null && snapshotStats.size > 0 : + "snapshot stats size must be positive: expected>0 actual=${snapshotStats.size} installPath=${snapshot.primaryStorageInstallPath}" + assert snapshotStats.actualSize != null && snapshotStats.actualSize > 0 : + "snapshot stats actualSize must be positive: expected>0 actual=${snapshotStats.actualSize} installPath=${snapshot.primaryStorageInstallPath}" + stopVmInstance { uuid = vm.uuid } @@ -630,6 +771,28 @@ class XinfiniPrimaryStorageCase extends SubCase { } } + StorageResource queryStorageResourceStats(String installPath) { + StorageResource[] stats = new StorageResource[1] + ErrorCode[] errors = new ErrorCode[1] + controller.stats(installPath, new ReturnValueCompletion(null) { + @Override + void success(StorageResource returnValue) { + stats[0] = returnValue + } + + @Override + void fail(ErrorCode errorCode) { + errors[0] = errorCode + } + }) + + assert errors[0] == null : + "storage resource stats query must succeed: expectedError=null actualError=${errors[0]} installPath=${installPath}" + assert stats[0] != null : + "storage resource stats query must return a result: actual=null installPath=${installPath}" + return stats[0] + } + void testCreateTemplate() { env.message(DownloadImageFromRemoteTargetMsg.class) { DownloadImageFromRemoteTargetMsg msg, CloudBus bus -> DownloadImageFromRemoteTargetReply r = new DownloadImageFromRemoteTargetReply() diff --git a/testlib/src/main/java/org/zstack/testlib/ExternalPrimaryStorageSpec.groovy b/testlib/src/main/java/org/zstack/testlib/ExternalPrimaryStorageSpec.groovy index 98b3f94a24e..acb6786a2dd 100644 --- a/testlib/src/main/java/org/zstack/testlib/ExternalPrimaryStorageSpec.groovy +++ b/testlib/src/main/java/org/zstack/testlib/ExternalPrimaryStorageSpec.groovy @@ -1090,16 +1090,14 @@ class ExternalPrimaryStorageSpec extends PrimaryStorageSpec { simulator("/afa/v1/bs-volumes/:clone") { HttpServletRequest req, HttpEntity e, EnvSpec spec -> def body = JSONObjectUtil.toObject(e.body, LinkedHashMap.class) def specData = body?.spec ?: body - int volId = volumeCounter.incrementAndGet() - String volName = specData?.name ?: "clone-${volId}" int bsSnapId = specData?.bs_snap_id ?: 0 - - // Get size from source snapshot if available - long sizeMb = 1024 def srcSnap = snapshots.get(bsSnapId) - if (srcSnap != null) { - sizeMb = srcSnap.spec.size_mb ?: 1024 + if (srcSnap == null) { + throw new HttpError(404, "BsSnap not found") } + int volId = volumeCounter.incrementAndGet() + String volName = specData?.name ?: "clone-${volId}" + long sizeMb = srcSnap.spec.size_mb ?: 1024 def volSpec = [ id: volId, name: volName, pool_id: POOL_ID, size_mb: sizeMb,