Skip to content

Commit e39ea3a

Browse files
committed
rbd: avoid GetTrashList in ensureImageCleanup by using ImageID
ensureImageCleanup() previously called librbd.GetTrashList() and matched the image by name to obtain its ID. Since ImageID is already populated from the journal at all call sites, use it directly and call trashRemoveImage() without the expensive list operation. ENOENT is treated as success for idempotency (image already cleaned up). The one path where ImageID may be absent is DeleteTempImage(): when Delete() fails with ErrImageNotFound because the temp-clone is already in trash (opened by name fails), ImageID is not set. A new helper findImageIDInTrash() performs the GetTrashList fallback only in that exceptional retry scenario and populates ImageID before calling ensureImageCleanup(). Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Niels de Vos <ndevos@ibm.com>
1 parent 978d106 commit e39ea3a

1 file changed

Lines changed: 44 additions & 15 deletions

File tree

internal/rbd/rbd_util.go

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -720,29 +720,47 @@ func isCephMgrSupported(ctx context.Context, clusterID string, err error) (bool,
720720
return true, nil
721721
}
722722

723-
// ensureImageCleanup finds image in trash and if found removes it
724-
// from trash.
723+
// ensureImageCleanup removes the image from trash using its ImageID.
724+
// Returns nil when the image is not found in trash (already cleaned up).
725725
func (ri *rbdImage) ensureImageCleanup(ctx context.Context) error {
726+
if ri.ImageID == "" {
727+
return fmt.Errorf("ImageID is not set for image %q", ri)
728+
}
729+
726730
err := ri.openIoctx()
727731
if err != nil {
728732
return err
729733
}
730734

731-
trashInfoList, err := librbd.GetTrashList(ri.ioctx)
732-
if err != nil {
733-
log.ErrorLog(ctx, "failed to list images in trash: %v", err)
735+
err = ri.trashRemoveImage(ctx)
736+
if errors.Is(err, librbd.ErrNotExist) {
737+
return nil
738+
}
734739

735-
return err
740+
return err
741+
}
742+
743+
// findImageIDInTrash searches the trash list by name and populates ImageID when found.
744+
// Used as a fallback when ImageID is not known (e.g. image already in trash from a prior attempt).
745+
func (ri *rbdImage) findImageIDInTrash() (bool, error) {
746+
if err := ri.openIoctx(); err != nil {
747+
return false, err
736748
}
737-
for _, val := range trashInfoList {
738-
if val.Name == ri.RbdImageName {
739-
ri.ImageID = val.Id
740749

741-
return ri.trashRemoveImage(ctx)
750+
trashList, err := librbd.GetTrashList(ri.ioctx)
751+
if err != nil {
752+
return false, err
753+
}
754+
755+
for _, t := range trashList {
756+
if t.Name == ri.RbdImageName {
757+
ri.ImageID = t.Id
758+
759+
return true, nil
742760
}
743761
}
744762

745-
return nil
763+
return false, nil
746764
}
747765

748766
// Delete deletes a ceph image with provision and volume options.
@@ -848,12 +866,23 @@ func (rv *rbdVolume) DeleteTempImage(ctx context.Context) error {
848866

849867
err = tempClone.Delete(ctx)
850868
if err != nil {
851-
if errors.Is(err, rbderrors.ErrImageNotFound) {
852-
return tempClone.ensureImageCleanup(ctx)
853-
} else {
854-
// return error if it is not ErrImageNotFound
869+
if !errors.Is(err, rbderrors.ErrImageNotFound) {
855870
return err
856871
}
872+
873+
if tempClone.ImageID == "" {
874+
// The image was not accessible by name (likely already in trash from a
875+
// previous attempt). Search the trash list by name to get its ID.
876+
found, idErr := tempClone.findImageIDInTrash()
877+
if idErr != nil {
878+
return idErr
879+
} else if !found {
880+
// image is not in the trash, no need to delete it
881+
return nil
882+
}
883+
}
884+
885+
return tempClone.ensureImageCleanup(ctx)
857886
}
858887

859888
return nil

0 commit comments

Comments
 (0)