Skip to content

rbd: avoid GetTrashList in ensureImageCleanup by using ImageID - #6468

Closed
nixpanic wants to merge 1 commit into
ceph:develfrom
nixpanic:rbd/ImageID-vs-GetTrashList
Closed

rbd: avoid GetTrashList in ensureImageCleanup by using ImageID#6468
nixpanic wants to merge 1 commit into
ceph:develfrom
nixpanic:rbd/ImageID-vs-GetTrashList

Conversation

@nixpanic

Copy link
Copy Markdown
Member

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().

@nixpanic
nixpanic requested review from a team as code owners August 13, 2026 13:12
Copilot AI lite review requested due to automatic review settings August 13, 2026 13:12
@mergify mergify Bot added the component/rbd Issues related to RBD label Aug 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes RBD image trash cleanup by avoiding librbd.GetTrashList() in the common path and instead removing trashed images directly by ImageID, which is typically available from the journal. It also adds a targeted fallback for the DeleteTempImage() retry scenario where ImageID might be missing because the image can no longer be opened by name.

Changes:

  • Updated ensureImageCleanup() to remove images from trash by ImageID and treat ENOENT as success for idempotency.
  • Added findImageIDInTrash() to resolve an ImageID by name from the trash list for exceptional recovery paths.
  • Updated DeleteTempImage() to use the fallback only when Delete() fails with ErrImageNotFound and ImageID is absent.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/rbd/rbd_util.go
@nixpanic
nixpanic force-pushed the rbd/ImageID-vs-GetTrashList branch from dc74388 to 5614e94 Compare August 13, 2026 14:46
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>
@nixpanic
nixpanic force-pushed the rbd/ImageID-vs-GetTrashList branch from 5614e94 to e39ea3a Compare August 13, 2026 16:21
@nixpanic
nixpanic marked this pull request as draft August 13, 2026 16:21
@nixpanic

Copy link
Copy Markdown
Member Author

closed in favor of #6472

@nixpanic nixpanic closed this Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/rbd Issues related to RBD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants