From a5a89527165af23c12957e6fd1a9c9c7f427ecef Mon Sep 17 00:00:00 2001
From: Madhu Rajanna <madhupr007@gmail.com>
Date: Mon, 12 Jul 2021 11:11:46 +0530
Subject: [PATCH] rbd: fix clone problem

This commit fixes a bug in checkCloneImage() which was caused
by checking cloned image before checking on temp-clone image snap
in a subsequent request which lead to stale images. This was solved
by checking temp-clone image snap and flattening temp-clone if
needed.
This commit also fixes comparison bug in flattenCloneImage().

Signed-off-by: Rakshith R <rar@redhat.com>
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
---
 internal/rbd/clone.go       | 42 +++++++++++++------------------------
 internal/rbd/rbd_journal.go |  1 +
 2 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/internal/rbd/clone.go b/internal/rbd/clone.go
index a3f57bc75..d47548c2f 100644
--- a/internal/rbd/clone.go
+++ b/internal/rbd/clone.go
@@ -52,31 +52,7 @@ func (rv *rbdVolume) checkCloneImage(ctx context.Context, parentVol *rbdVolume)
 	snap.RbdSnapName = rv.RbdImageName
 	snap.Pool = rv.Pool
 
-	// check if cloned image exists
-	err := rv.getImageInfo()
-	if err == nil {
-		// check if do we have temporary snapshot on temporary cloned image
-		sErr := tempClone.checkSnapExists(snap)
-		if sErr != nil {
-			if errors.Is(err, ErrSnapNotFound) {
-				return true, nil
-			}
-
-			return false, err
-		}
-		err = tempClone.deleteSnapshot(ctx, snap)
-		if err == nil {
-			return true, nil
-		}
-
-		return false, err
-	}
-	if !errors.Is(err, ErrImageNotFound) {
-		// return error if its not image not found
-		return false, err
-	}
-
-	err = tempClone.checkSnapExists(snap)
+	err := tempClone.checkSnapExists(snap)
 	if err != nil {
 		switch {
 		case errors.Is(err, ErrSnapNotFound):
@@ -92,6 +68,11 @@ func (rv *rbdVolume) checkCloneImage(ctx context.Context, parentVol *rbdVolume)
 			if err != nil {
 				return false, err
 			}
+			// check image needs flatten, if yes add task to flatten the clone
+			err = rv.flattenRbdImage(ctx, rv.conn.Creds, false, rbdHardMaxCloneDepth, rbdSoftMaxCloneDepth)
+			if err != nil {
+				return false, err
+			}
 
 			return true, nil
 		case !errors.Is(err, ErrImageNotFound):
@@ -116,6 +97,11 @@ func (rv *rbdVolume) checkCloneImage(ctx context.Context, parentVol *rbdVolume)
 
 			return false, err
 		}
+		// check image needs flatten, if yes add task to flatten the clone
+		err = rv.flattenRbdImage(ctx, rv.conn.Creds, false, rbdHardMaxCloneDepth, rbdSoftMaxCloneDepth)
+		if err != nil {
+			return false, err
+		}
 
 		return true, nil
 	}
@@ -162,7 +148,7 @@ func (rv *rbdVolume) createCloneFromImage(ctx context.Context, parentVol *rbdVol
 
 	err = rv.doSnapClone(ctx, parentVol)
 	if err != nil {
-		return status.Error(codes.Internal, err.Error())
+		return err
 	}
 
 	err = rv.getImageID()
@@ -284,10 +270,10 @@ func (rv *rbdVolume) flattenCloneImage(ctx context.Context) error {
 	softLimit := rbdSoftMaxCloneDepth
 	// choosing 2 so that we don't need to flatten the image in the request.
 	const depthToAvoidFlatten = 2
-	if rbdHardMaxCloneDepth < depthToAvoidFlatten {
+	if rbdHardMaxCloneDepth > depthToAvoidFlatten {
 		hardLimit = rbdHardMaxCloneDepth - depthToAvoidFlatten
 	}
-	if rbdSoftMaxCloneDepth < depthToAvoidFlatten {
+	if rbdSoftMaxCloneDepth > depthToAvoidFlatten {
 		softLimit = rbdSoftMaxCloneDepth - depthToAvoidFlatten
 	}
 	err := tempClone.getImageInfo()
diff --git a/internal/rbd/rbd_journal.go b/internal/rbd/rbd_journal.go
index 453c5cbc4..88e7f1dc4 100644
--- a/internal/rbd/rbd_journal.go
+++ b/internal/rbd/rbd_journal.go
@@ -311,6 +311,7 @@ func (rv *rbdVolume) Exists(ctx context.Context, parentVol *rbdVolume) (bool, er
 
 		return false, err
 	}
+	// TODO: check image needs flattening and completed?
 
 	err = rv.repairImageID(ctx, j)
 	if err != nil {