From 9b0383ce6c6b863dc260de172b580d505787d39f Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Mon, 16 Sep 2024 14:30:24 +0200 Subject: [PATCH 1/2] cleanup: remove unneeded `updateSnapshotDetails()` function `updateSnapshotDetails()` just calls `getImageInfo()` on an `rbdVolume` created from the `rbdSnapshot`. `getImageInfo()` is a function of the base `rbdImage` struct, so there really is no need for this indirection. Signed-off-by: Niels de Vos --- internal/rbd/rbd_util.go | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index b76187aa0f1..6a146d7431d 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -1080,7 +1080,7 @@ func genSnapFromSnapID( } } - err = updateSnapshotDetails(ctx, rbdSnap) + err = rbdSnap.getImageInfo() if err != nil { return rbdSnap, fmt.Errorf("failed to update snapshot details for %q: %w", rbdSnap, err) } @@ -1088,25 +1088,6 @@ func genSnapFromSnapID( return rbdSnap, err } -// updateSnapshotDetails will copy the details from the rbdVolume to the -// rbdSnapshot. example copying size from rbdVolume to rbdSnapshot. -func updateSnapshotDetails(ctx context.Context, rbdSnap *rbdSnapshot) error { - vol := rbdSnap.toVolume() - err := vol.Connect(rbdSnap.conn.Creds) - if err != nil { - return err - } - defer vol.Destroy(ctx) - - err = vol.getImageInfo() - if err != nil { - return err - } - rbdSnap.VolSize = vol.VolSize - - return nil -} - // generateVolumeFromVolumeID generates a rbdVolume structure from the provided identifier. func generateVolumeFromVolumeID( ctx context.Context, From ddd41d358f4b049a18ae6c26d9bbe65944842cb6 Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Fri, 1 Nov 2024 13:02:20 +0100 Subject: [PATCH 2/2] cleanup: drop rbdVol.storeImageID() in favor of rbdVol.repairImageID() Both `rbdVolume.repairImageID()` and `rbdVolume.storeImageID()` do the same thing. `repairImageID()` includes optional extra checking for an already set ImageID, which is not part of `storeImageID()`. Remove some unneeded `ImageID == ""` checking because `repairImageID()` does that, and remove the `storeImageID()` function. Signed-off-by: Niels de Vos --- internal/rbd/controllerserver.go | 2 +- internal/rbd/rbd_journal.go | 37 ++++++++------------------------ internal/rbd/rbd_util.go | 9 ++++---- 3 files changed, 14 insertions(+), 34 deletions(-) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 1745dbc3793..08bd4734f7c 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -790,7 +790,7 @@ func (cs *ControllerServer) createBackingImage( } } }() - err = rbdVol.storeImageID(ctx, j) + err = rbdVol.repairImageID(ctx, j, true) if err != nil { return status.Error(codes.Internal, err.Error()) } diff --git a/internal/rbd/rbd_journal.go b/internal/rbd/rbd_journal.go index 38f4c8c705c..3af6d7a8200 100644 --- a/internal/rbd/rbd_journal.go +++ b/internal/rbd/rbd_journal.go @@ -541,7 +541,7 @@ func undoVolReservation(ctx context.Context, rbdVol *rbdVolume, cr *util.Credent // The volume handler won't remain same as its contains poolID,clusterID etc // which are not same across clusters. // -//nolint:gocyclo,cyclop,nestif // TODO: reduce complexity +//nolint:gocyclo,cyclop // TODO: reduce complexity func RegenerateJournal( volumeAttributes map[string]string, claimName, @@ -623,12 +623,12 @@ func RegenerateJournal( rbdVol.ImageID = imageData.ImageAttributes.ImageID rbdVol.Owner = imageData.ImageAttributes.Owner rbdVol.RbdImageName = imageData.ImageAttributes.ImageName - if rbdVol.ImageID == "" { - err = rbdVol.storeImageID(ctx, j) - if err != nil { - return "", err - } + + err = rbdVol.repairImageID(ctx, j, false) + if err != nil { + return "", err } + if rbdVol.Owner != owner { err = j.ResetVolumeOwner(ctx, rbdVol.JournalPool, rbdVol.ReservedID, owner) if err != nil { @@ -675,30 +675,11 @@ func RegenerateJournal( log.DebugLog(ctx, "re-generated Volume ID (%s) and image name (%s) for request name (%s)", rbdVol.VolID, rbdVol.RbdImageName, rbdVol.RequestName) - if rbdVol.ImageID == "" { - err = rbdVol.storeImageID(ctx, j) - if err != nil { - return "", err - } - } - return rbdVol.VolID, nil -} - -// storeImageID retrieves the image ID and stores it in OMAP. -func (rv *rbdVolume) storeImageID(ctx context.Context, j *journal.Connection) error { - err := rv.getImageID() - if err != nil { - log.ErrorLog(ctx, "failed to get image id %s: %v", rv, err) - - return err - } - err = j.StoreImageID(ctx, rv.JournalPool, rv.ReservedID, rv.ImageID) + err = rbdVol.repairImageID(ctx, j, false) if err != nil { - log.ErrorLog(ctx, "failed to store volume id %s: %v", rv, err) - - return err + return "", err } - return nil + return rbdVol.VolID, nil } diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index 6a146d7431d..748309b88f8 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -1169,12 +1169,11 @@ func generateVolumeFromVolumeID( } } - if rbdVol.ImageID == "" { - err = rbdVol.storeImageID(ctx, j) - if err != nil { - return rbdVol, err - } + err = rbdVol.repairImageID(ctx, j, false) + if err != nil { + return rbdVol, err } + err = rbdVol.getImageInfo() return rbdVol, err