From e735c800a238ea902a8891c90f9ca01bfbd2e4fc Mon Sep 17 00:00:00 2001
From: Qiu Jian <qiujian@yunionyun.com>
Date: Mon, 25 Apr 2022 22:40:57 +0800
Subject: [PATCH] fix: fail to delete storages with fake-deleted snapshots

---
 pkg/compute/models/storages.go     | 29 +++++++++++++++++++++++++----
 pkg/compute/storagedrivers/base.go |  9 ++++++---
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/pkg/compute/models/storages.go b/pkg/compute/models/storages.go
index 8de9c22857..9bbab33a46 100644
--- a/pkg/compute/models/storages.go
+++ b/pkg/compute/models/storages.go
@@ -174,7 +174,28 @@ func (self *SStorage) AllowDeleteItem(ctx context.Context, userCred mcclient.Tok
 	return db.IsAdminAllowDelete(userCred, self)
 }
 
+func (self *SStorage) getFakeDeletedSnapshots() ([]SSnapshot, error) {
+	q := SnapshotManager.Query().Equals("storage_id", self.Id).IsTrue("fake_deleted")
+	snapshots := make([]SSnapshot, 0)
+	err := db.FetchModelObjects(SnapshotManager, q, &snapshots)
+	if err != nil {
+		return nil, errors.Wrap(err, "FetchModelObjects")
+	}
+	return snapshots, nil
+}
+
 func (self *SStorage) Delete(ctx context.Context, userCred mcclient.TokenCredential) error {
+	// delete all hidden snapshots
+	fakeDeletedSnapshots, err := self.getFakeDeletedSnapshots()
+	if err != nil {
+		return errors.Wrap(err, "getFakeDeletedSnapshots")
+	}
+	for i := range fakeDeletedSnapshots {
+		err := fakeDeletedSnapshots[i].Delete(ctx, userCred)
+		if err != nil {
+			return errors.Wrap(err, "fakeDeletedSnapshots.Delete")
+		}
+	}
 	DeleteResourceJointSchedtags(self, ctx, userCred)
 	return self.SEnabledStatusInfrasResourceBase.Delete(ctx, userCred)
 }
@@ -265,9 +286,9 @@ func (self *SStorage) ValidateDeleteCondition(ctx context.Context, info jsonutil
 	if cnt > 0 {
 		return httperrors.NewNotEmptyError("storage has disks")
 	}
-	cnt, err = self.GetSnapshotCount()
+	cnt, err = self.GetVisibleSnapshotCount()
 	if err != nil {
-		return httperrors.NewInternalServerError("GetSnapshotCount fail %s", err)
+		return httperrors.NewInternalServerError("GetVisibleSnapshotCount fail %s", err)
 	}
 	if cnt > 0 {
 		return httperrors.NewNotEmptyError("storage has snapshots")
@@ -400,8 +421,8 @@ func (self *SStorage) GetDisks() []SDisk {
 	return disks
 }
 
-func (self *SStorage) GetSnapshotCount() (int, error) {
-	return SnapshotManager.Query().Equals("storage_id", self.Id).CountWithError()
+func (self *SStorage) GetVisibleSnapshotCount() (int, error) {
+	return SnapshotManager.Query().Equals("storage_id", self.Id).IsFalse("fake_deleted").CountWithError()
 }
 
 func (self *SStorage) IsLocal() bool {
diff --git a/pkg/compute/storagedrivers/base.go b/pkg/compute/storagedrivers/base.go
index 0610a7b7b8..97ee972b31 100644
--- a/pkg/compute/storagedrivers/base.go
+++ b/pkg/compute/storagedrivers/base.go
@@ -58,9 +58,12 @@ func (self *SBaseStorageDriver) ValidateSnapshotDelete(ctx context.Context, snap
 	}
 
 	if !snapshot.OutOfChain && snapshot.FakeDeleted {
-		_, err := models.SnapshotManager.GetConvertSnapshot(snapshot)
-		if err != nil {
-			return httperrors.NewBadRequestError("disk need at least one of snapshot as backing file")
+		disk, _ := snapshot.GetDisk()
+		if disk != nil {
+			_, err := models.SnapshotManager.GetConvertSnapshot(snapshot)
+			if err != nil {
+				return httperrors.NewBadRequestError("disk need at least one of snapshot as backing file")
+			}
 		}
 	}
 	return nil