<fix>[storage]: ZSTAC-81222 defer referenced reimage trash

header/src/main/java/org/zstack/header/storage/snapshot/reference/VolumeSnapshotReferenceTreeVO_.java

@@ -12,7 +12,7 @@ public class VolumeSnapshotReferenceTreeVO_ extends ResourceVO_ {
     public static volatile SingularAttribute<VolumeSnapshotReferenceTreeVO, String> rootVolumeSnapshotUuid;
     public static volatile SingularAttribute<VolumeSnapshotReferenceTreeVO, String> rootVolumeUuid;
     public static volatile SingularAttribute<VolumeSnapshotReferenceTreeVO, String> rootVolumeSnapshotTreeUuid;
-    public static volatile SingularAttribute<VolumeSnapshotReferenceTreeVO, String> rootVolumeSnapshotInstallUrl;
+    public static volatile SingularAttribute<VolumeSnapshotReferenceTreeVO, String> rootInstallUrl;
 
     public static volatile SingularAttribute<VolumeSnapshotReferenceTreeVO, String> primaryStorageUuid;
     public static volatile SingularAttribute<VolumeSnapshotReferenceTreeVO, String> hostUuid;

storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java

@@ -56,6 +56,7 @@
 import org.zstack.resourceconfig.ResourceConfigFacade;
 import org.zstack.storage.backup.BackupStorageSystemTags;
 import org.zstack.storage.primary.*;
+import org.zstack.storage.snapshot.reference.VolumeSnapshotReferenceUtils;
 import org.zstack.storage.volume.VolumeSystemTags;
 import org.zstack.utils.Utils;
 import org.zstack.utils.gson.JSONObjectUtil;
@@ -2099,7 +2100,8 @@ public void fail(ErrorCode errorCode) {
                     @Override
                     public boolean skip(Map data) {
                         return controller.reportCapabilities().getSnapshotCapability()
-                                .getPlacementType() != VolumeSnapshotCapability.VolumeSnapshotPlacementType.INTERNAL;
+                                .getPlacementType() != VolumeSnapshotCapability.VolumeSnapshotPlacementType.INTERNAL
+                                || VolumeSnapshotReferenceUtils.isVolumeDirectlyReferenceByOthers(msg.getVolume());
                     }
 
                     @Override

storage/src/main/java/org/zstack/storage/snapshot/reference/VolumeSnapshotReferenceTreeBase.java

@@ -110,10 +110,7 @@ private void deleteSnapshotRefLeaf(DeleteVolumeSnapshotReferenceLeafMsg msg, Com
             return;
         }
 
-        boolean rootDeleted = msg.getLeaf().getParentId() == null && !Q.New(VolumeVO.class)
-                .eq(VolumeVO_.uuid, msg.getLeaf().getVolumeUuid())
-                .eq(VolumeVO_.primaryStorageUuid, msg.getTree().getPrimaryStorageUuid())
-                .isExists();
+        boolean rootDeleted = msg.getLeaf().getParentId() == null && isRootDeleted(msg);
         String endPath = rootDeleted ? self.getRootInstallUrl() : msg.getLeaf().getVolumeSnapshotInstallUrl();
         String startPath = msg.getLeaf().getDirectSnapshotInstallUrl();
         if (startPath.equals(endPath) && !rootDeleted) {
@@ -231,6 +228,15 @@ public void handle(Map data) {
         }).start();
     }
 
+    private boolean isRootDeleted(DeleteVolumeSnapshotReferenceLeafMsg msg) {
+        VolumeVO rootVolume = Q.New(VolumeVO.class)
+                .eq(VolumeVO_.uuid, msg.getLeaf().getVolumeUuid())
+                .eq(VolumeVO_.primaryStorageUuid, msg.getTree().getPrimaryStorageUuid())
+                .find();
+        return rootVolume == null || (self.getRootVolumeSnapshotTreeUuid() == null &&
+                !self.getRootInstallUrl().equals(rootVolume.getInstallPath()));
+    }
+
     private boolean hasSameVolumeResource(String snapshotInstallUrl, Set<String> snapshotInstallUrls) {
         if (!snapshotInstallUrl.contains("@")) {
             return snapshotInstallUrls.contains(snapshotInstallUrl);

storage/src/main/java/org/zstack/storage/snapshot/reference/VolumeSnapshotReferenceUtils.java

@@ -258,6 +258,7 @@ protected VolumeSnapshotReferenceTreeVO scripts() {
                 VolumeSnapshotReferenceTreeVO tree = Q.New(VolumeSnapshotReferenceTreeVO.class)
                         .eq(VolumeSnapshotReferenceTreeVO_.rootVolumeUuid, baseSnapshot.getVolumeUuid())
                         .eq(VolumeSnapshotReferenceTreeVO_.primaryStorageUuid, baseSnapshot.getPrimaryStorageUuid())
+                        .eq(VolumeSnapshotReferenceTreeVO_.rootInstallUrl, vol.getInstallPath())
                         .find();
                 if (tree != null) {
                     return tree;
@@ -494,7 +495,17 @@ protected void scripts() {
     }
 
     private static void deleteBitsOnPs(VolumeSnapshotReferenceTreeVO treeVO, VolumeSnapshotReferenceVO ref) {
-        List<VolumeSnapshotReferenceVO> treeRefs = Q.New(VolumeSnapshotReferenceVO.class).eq(VolumeSnapshotReferenceVO_.treeUuid, treeVO.getUuid()).list();
+        List<VolumeSnapshotReferenceVO> treeRefs;
+        if (ref.getParentId() == null && treeVO.getRootVolumeSnapshotTreeUuid() == null) {
+            treeRefs = Q.New(VolumeSnapshotReferenceVO.class)
+                    .in(VolumeSnapshotReferenceVO_.treeUuid, Q.New(VolumeSnapshotReferenceTreeVO.class)
+                            .select(VolumeSnapshotReferenceTreeVO_.uuid)
+                            .eq(VolumeSnapshotReferenceTreeVO_.primaryStorageUuid, treeVO.getPrimaryStorageUuid())
+                            .eq(VolumeSnapshotReferenceTreeVO_.rootInstallUrl, treeVO.getRootInstallUrl()))
+                    .list();
+        } else {
+            treeRefs = Q.New(VolumeSnapshotReferenceVO.class).eq(VolumeSnapshotReferenceVO_.treeUuid, treeVO.getUuid()).list();
+        }
 
         List<VolumeSnapshotReferenceInventory> otherLeafs;
         if (ref.getParentId() == null) {
@@ -532,7 +543,8 @@ private static void deleteSnapshotRefLeafInTree(VolumeSnapshotReferenceTreeVO tr
         boolean backingVolumeDeletedInDb = SQL.New("select vol.uuid from VolumeVO vol, VolumeSnapshotReferenceTreeVO tree" +
                         " where vol.uuid = :volUuid" +
                         " and tree.uuid = :treeUuid" +
-                        " and vol.primaryStorageUuid = tree.primaryStorageUuid", String.class)
+                        " and vol.primaryStorageUuid = tree.primaryStorageUuid" +
+                        " and (tree.rootVolumeSnapshotTreeUuid is not null or vol.installPath = tree.rootInstallUrl)", String.class)
                 .param("volUuid", ref.getVolumeUuid())
                 .param("treeUuid", ref.getTreeUuid())
                 .find() == null;

test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsPrimaryStorageCase.groovy

@@ -6,12 +6,21 @@ import org.zstack.core.cloudbus.EventCallback
 import org.zstack.core.cloudbus.EventFacade
 import org.zstack.core.db.DatabaseFacade
 import org.zstack.core.db.Q
+import org.zstack.core.trash.TrashType
 import org.zstack.header.errorcode.ErrorCode
 import org.zstack.header.errorcode.OperationFailureException
+import org.zstack.header.core.trash.InstallPathRecycleVO
+import org.zstack.header.core.trash.InstallPathRecycleVO_
 import org.zstack.header.message.MessageReply
+import org.zstack.header.storage.backup.UploadImageToRemoteTargetMsg
+import org.zstack.header.storage.backup.UploadImageToRemoteTargetReply
 import org.zstack.header.storage.primary.GetVolumeBackingChainFromPrimaryStorageMsg
 import org.zstack.header.storage.primary.GetVolumeBackingChainFromPrimaryStorageReply
 import org.zstack.header.storage.primary.PrimaryStorageConstant
+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.BatchSyncVolumeSizeOnPrimaryStorageMsg
 import org.zstack.header.volume.BatchSyncVolumeSizeOnPrimaryStorageReply
 import org.zstack.header.storage.addon.primary.ExternalPrimaryStorageVO
@@ -40,8 +49,10 @@ import org.zstack.storage.zbs.Config
 import org.zstack.storage.zbs.ZbsAgentUrl
 import org.zstack.storage.zbs.ZbsConstants
 import org.zstack.storage.zbs.ZbsGlobalProperty
+import org.zstack.storage.zbs.ZbsHelper
 import org.zstack.storage.zbs.ZbsPrimaryStorageMdsBase
 import org.zstack.storage.zbs.ZbsStorageController
+import org.zstack.storage.volume.VolumeSystemTags
 import org.zstack.test.integration.storage.StorageTest
 import org.zstack.testlib.EnvSpec
 import org.zstack.testlib.HttpError
@@ -194,6 +205,7 @@ class ZbsPrimaryStorageCase extends SubCase {
             testAttachPrimaryStorageFailsWhenActivatingHeartbeatVolumeFails()
             testMdsConnectFailed()
             testLifecycle()
+            testReimageReferencedRootCleanup()
             testDataVolumeLifecycle()
             testMdsPing()
             testCheckHostStorageConnection()
@@ -756,6 +768,114 @@ class ZbsPrimaryStorageCase extends SubCase {
         }
     }
 
+    void testReimageReferencedRootCleanup() {
+        attachPrimaryStorageToCluster {
+            primaryStorageUuid = ps.uuid
+            clusterUuid = cluster.uuid
+        }
+
+        env.message(UploadImageToRemoteTargetMsg.class) { UploadImageToRemoteTargetMsg msg, CloudBus bus ->
+            bus.reply(msg, new UploadImageToRemoteTargetReply())
+        }
+
+        def instanceOffering = env.inventoryByName("instanceOffering") as InstanceOfferingInventory
+        def image = env.inventoryByName("image") as ImageInventory
+        def l3 = env.inventoryByName("l3") as L3NetworkInventory
+        def vm = createVmInstance {
+            name = "reimage-referenced-root"
+            imageUuid = image.uuid
+            l3NetworkUuids = [l3.uuid]
+            instanceOfferingUuid = instanceOffering.uuid
+        } as VmInstanceInventory
+        def oldRoot = queryVolume {
+            conditions = ["uuid=${vm.rootVolumeUuid}"]
+        }[0] as VolumeInventory
+        def snapshot = createVolumeSnapshot {
+            name = "referenced-root-snapshot"
+            volumeUuid = oldRoot.uuid
+        } as VolumeSnapshotInventory
+        def child = createDataVolumeFromVolumeSnapshot {
+            name = "referenced-root-child"
+            volumeSnapshotUuid = snapshot.uuid
+            systemTags = [VolumeSystemTags.FAST_CREATE.tagFormat]
+        } as VolumeInventory
+
+        assert Q.New(VolumeSnapshotReferenceVO.class)
+                .eq(VolumeSnapshotReferenceVO_.referenceVolumeUuid, child.uuid).isExists() :
+                "FAST_CREATE child must retain the backing reference: childUuid=${child.uuid}"
+
+        List<String> deleteAttempts = Collections.synchronizedList(new ArrayList<>())
+        env.simulator(ZbsStorageController.QUERY_VOLUME_PATH) { HttpEntity<String> e, EnvSpec spec ->
+            def cmd = JSONObjectUtil.toObject(e.body, ZbsStorageController.QueryVolumeCmd.class)
+            def rsp = new ZbsStorageController.QueryVolumeRsp()
+            rsp.size = oldRoot.size
+            rsp.actualSize = oldRoot.actualSize
+            if (ZbsHelper.normalizeToZbsPath(cmd.path) == snapshot.primaryStorageInstallPath) {
+                rsp.parentUri = oldRoot.installPath
+            }
+            return rsp
+        }
+        env.simulator(ZbsStorageController.DELETE_VOLUME_PATH) { HttpEntity<String> e, EnvSpec spec ->
+            def cmd = JSONObjectUtil.toObject(e.body, ZbsStorageController.DeleteVolumeCmd.class)
+            String path = ZbsHelper.normalizeToZbsPath(cmd.path)
+            deleteAttempts.add(path)
+            return new ZbsStorageController.DeleteVolumeRsp()
+        }
+
+        stopVmInstance {
+            uuid = vm.uuid
+        }
+        reimageVmInstance {
+            vmInstanceUuid = vm.uuid
+        }
+
+        assert !deleteAttempts.contains(oldRoot.installPath) :
+                "reimage must not delete an old root with downstream references: oldRoot=${oldRoot.installPath} attempts=${deleteAttempts}"
+        assert !Q.New(InstallPathRecycleVO.class)
+                .eq(InstallPathRecycleVO_.storageUuid, ps.uuid)
+                .eq(InstallPathRecycleVO_.installPath, oldRoot.installPath)
+                .eq(InstallPathRecycleVO_.trashType, TrashType.ReimageVolume.toString())
+                .isExists() : "reimage must not create trash for referenced old root: oldRoot=${oldRoot.installPath}"
+
+        destroyVmInstance {
+            uuid = vm.uuid
+        }
+        expungeVmInstance {
+            uuid = vm.uuid
+        }
+        assert !Q.New(VolumeSnapshotVO.class).eq(VolumeSnapshotVO_.volumeUuid, oldRoot.uuid).isExists() :
+                "old root snapshots must be removed after VM expunge: volumeUuid=${oldRoot.uuid}"
+        assert Q.New(VolumeSnapshotReferenceVO.class)
+                .eq(VolumeSnapshotReferenceVO_.referenceVolumeUuid, child.uuid).isExists() :
+                "VM expunge must retain the FAST_CREATE child reference: childUuid=${child.uuid}"
+
+        int cleanupStartIndex = deleteAttempts.size()
+        deleteDataVolume {
+            uuid = child.uuid
+        }
+        expungeDataVolume {
+            uuid = child.uuid
+        }
+
+        retryInSecs {
+            List<String> cleanupAttempts = deleteAttempts.drop(cleanupStartIndex)
+            int snapshotIndex = cleanupAttempts.lastIndexOf(snapshot.primaryStorageInstallPath)
+            int oldRootIndex = cleanupAttempts.indexOf(oldRoot.installPath)
+            assert snapshotIndex >= 0 && oldRootIndex > snapshotIndex :
+                    "cleanup must delete leaf-to-root: snapshot=${snapshot.primaryStorageInstallPath} oldRoot=${oldRoot.installPath} attempts=${cleanupAttempts}"
+        }
+
+        assert !Q.New(VolumeSnapshotReferenceVO.class)
+                .eq(VolumeSnapshotReferenceVO_.referenceVolumeUuid, child.uuid).isExists() :
+                "reference must be removed after child expunge: childUuid=${child.uuid}"
+
+        detachPrimaryStorageFromCluster {
+            primaryStorageUuid = ps.uuid
+            clusterUuid = cluster.uuid
+        }
+        env.cleanSimulatorHandlers()
+    }
+
     void testMdsPing() {
         ExternalPrimaryStorageCanonicalEvent.AddonInfoChangedData data = null
         long count = 0