Skip to content

kvm: let libvirt remove RBD snapshots on volume delete - #13763

Open
wido wants to merge 1 commit into
apache:mainfrom
wido:kvm-rbd-delete-with-snapshots
Open

kvm: let libvirt remove RBD snapshots on volume delete#13763
wido wants to merge 1 commit into
apache:mainfrom
wido:kvm-rbd-delete-with-snapshots

Conversation

@wido

@wido wido commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Description

LibvirtStorageAdaptor#deletePhysicalDisk manually connected to Ceph via librados/librbd to unprotect and remove every snapshot of an RBD image before asking libvirt to delete the volume. libvirt's RBD storage backend has supported VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS since 1.2.20, which does the same unprotect/remove internally. Pass that flag instead and drop the manual cleanup.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

LibvirtStorageAdaptor#deletePhysicalDisk manually connected to Ceph via
librados/librbd to unprotect and remove every snapshot of an RBD image
before asking libvirt to delete the volume. libvirt's RBD storage
backend has supported VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS since
1.2.20, which does the same unprotect/remove internally. Pass that
flag instead and drop the manual cleanup.
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 19.65%. Comparing base (4f11707) to head (b2c5e04).

Files with missing lines Patch % Lines
.../hypervisor/kvm/storage/LibvirtStorageAdaptor.java 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               main   #13763   +/-   ##
=========================================
  Coverage     19.65%   19.65%           
+ Complexity    19792    19789    -3     
=========================================
  Files          6368     6368           
  Lines        574881   574843   -38     
  Branches      70351    70349    -2     
=========================================
- Hits         112970   112963    -7     
+ Misses       449639   449607   -32     
- Partials      12272    12273    +1     
Flag Coverage Δ
uitests 3.41% <ø> (ø)
unittests 20.92% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@weizhouapache

Copy link
Copy Markdown
Member

nice !

@wido

wido commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

nice !

Tnx! Fun is that I implemented this in Libvirt over 10y ago and totally forgot to fix it in CloudStack.

Simple change to reduce the amount of code, better to have Libvirt do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants