From 07332d0a80d8dd07dcb70db3c3ede40beed711ca Mon Sep 17 00:00:00 2001
From: Rakshith R <rar@redhat.com>
Date: Tue, 11 Jul 2023 12:36:08 +0530
Subject: [PATCH] rbd: do not execute rbd sparsify when volume is in use

This commit makes sure sparsify() is not run when rbd
image is in use.
Running rbd sparsify with workload doing io and too
frequently is not desirable.
When a image is in use fstrim is run and sparsify will
be run only when image is not mapped.

Signed-off-by: Rakshith R <rar@redhat.com>
(cherry picked from commit 98fdadfde77adc16b578e550d4327b6d43ce0a6c)
(cherry picked from commit 60a86ab8772abfaf343c3cac6b476dfd2d70d89d)
---
 internal/csi-addons/rbd/reclaimspace.go |  9 +++++++++
 internal/rbd/diskusage.go               | 11 +++++++++++
 internal/rbd/errors.go                  |  2 ++
 3 files changed, 22 insertions(+)

diff --git a/internal/csi-addons/rbd/reclaimspace.go b/internal/csi-addons/rbd/reclaimspace.go
index ff9f1a8f45f..8c2153808de 100644
--- a/internal/csi-addons/rbd/reclaimspace.go
+++ b/internal/csi-addons/rbd/reclaimspace.go
@@ -18,11 +18,13 @@ package rbd
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	csicommon "github.com/ceph/ceph-csi/internal/csi-common"
 	rbdutil "github.com/ceph/ceph-csi/internal/rbd"
 	"github.com/ceph/ceph-csi/internal/util"
+	"github.com/ceph/ceph-csi/internal/util/log"
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
 	rs "github.com/csi-addons/spec/lib/go/reclaimspace"
@@ -68,6 +70,13 @@ func (rscs *ReclaimSpaceControllerServer) ControllerReclaimSpace(
 	defer rbdVol.Destroy()
 
 	err = rbdVol.Sparsify()
+	if errors.Is(err, rbdutil.ErrImageInUse) {
+		// FIXME: https://github.com/csi-addons/kubernetes-csi-addons/issues/406.
+		// treat sparsify call as no-op if volume is in use.
+		log.DebugLog(ctx, fmt.Sprintf("volume with ID %q is in use, skipping sparsify operation", volumeID))
+
+		return &rs.ControllerReclaimSpaceResponse{}, nil
+	}
 	if err != nil {
 		// TODO: check for different error codes?
 		return nil, status.Errorf(codes.Internal, "failed to sparsify volume %q: %s", rbdVol, err.Error())
diff --git a/internal/rbd/diskusage.go b/internal/rbd/diskusage.go
index efb12f37c73..444a5a604fa 100644
--- a/internal/rbd/diskusage.go
+++ b/internal/rbd/diskusage.go
@@ -23,7 +23,18 @@ import (
 // Sparsify checks the size of the objects in the RBD image and calls
 // rbd_sparify() to free zero-filled blocks and reduce the storage consumption
 // of the image.
+// This function will return ErrImageInUse if the image is in use, since
+// sparsifying an image on which i/o is in progress is not optimal.
 func (ri *rbdImage) Sparsify() error {
+	inUse, err := ri.isInUse()
+	if err != nil {
+		return fmt.Errorf("failed to check if image is in use: %w", err)
+	}
+	if inUse {
+		// if the image is in use, we should not sparsify it, return ErrImageInUse.
+		return ErrImageInUse
+	}
+
 	image, err := ri.open()
 	if err != nil {
 		return err
diff --git a/internal/rbd/errors.go b/internal/rbd/errors.go
index 2443b8797e7..50542d563f7 100644
--- a/internal/rbd/errors.go
+++ b/internal/rbd/errors.go
@@ -42,4 +42,6 @@ var (
 	ErrMissingImageNameInVolID = errors.New("rbd image name information can not be empty in volID")
 	// ErrDecodeClusterIDFromMonsInVolID is returned when mons hash decoding on migration volID.
 	ErrDecodeClusterIDFromMonsInVolID = errors.New("failed to get clusterID from monitors hash in volID")
+	// ErrImageInUse is returned when the image is in use.
+	ErrImageInUse = errors.New("image is in use")
 )