feat(resize): support growing Lightbits volumes via qm resize#9
Merged
Conversation
Implement volume_resize: 4 KiB-align the new size (PVE passes bytes, padded to a 1 KiB multiple), PUT it to the project-scoped volume endpoint, and poll until the cluster reports the new size. Resize runs regardless of $running — PVE follows up with QEMU block_resize on a running guest, which needs the backing volume already grown. After the cluster resize, issue an 'nvme ns-rescan' on the owning controller as a robustness backup: the NVMe controller normally refreshes namespace capacity via an async event, but the rescan forces a synchronous re-read so the new size is visible before block_resize, and covers kernels/targets that don't honor the event. Rescans the controller (not the namespace) so it works under NVMe multipath. Advertise resize in volume_has_feature. Add t/volume_resize.t covering the PUT shape and 4 KiB alignment. Verified end-to-end on PVE 9.2.2 + LightOS 3.15.3 (offline and activated resize, host device reflected the new size).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds grow-only volume resize to the Lightbits Proxmox plugin: the plugin now advertises resize support, implements ChangesVolume Resize Implementation
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@LightbitsPlugin.pm`:
- Around line 571-578: The polling loop that checks volume size/state using _api
and variables $cur and $state may exit via timeout without confirming success;
after the for loop that polls "/api/v2/volumes/$uuid?projectName=$project" add a
post-loop verification: re-fetch the volume (or use last fetched $cur/$state),
and if $cur < $bytes or $state ne 'Available' then fail fast (croak/die/return
an error) instead of proceeding to the NVMe rescan and returning $bytes; ensure
the error includes $uuid, expected $bytes, actual $cur and $state for debugging
so callers don’t assume the resize succeeded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b7e6d4e0-b36c-4193-9151-8bb78d109229
📒 Files selected for processing (4)
CHANGELOG.mdLightbitsPlugin.pmREADME.mdt/volume_resize.t
The poll loop declared $cur/$state inside the loop, so on timeout it fell through and proceeded to the rescan + returned $bytes — reporting success for an incomplete resize (PVE would then block_resize the guest to a size the volume doesn't have). Hoist $cur/$state and die after the loop, with the volume UUID, expected bytes, and last-seen size/state, if it never converged. Extend t/volume_resize.t with a timeout case (no-op sleep so it runs instantly).
Collaborator
Author
|
@robb-lb If you have a chance, please review this pr as well before merging. This is scoped specifically for the volume resize with some additional tests and resiliency improvements. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds volume resize (grow) to the Lightbits storage plugin, exposed through
qm resize/ the Proxmox UI.volume_resize: 4 KiB-aligns the new size (PVE passes bytes, pre-padded to a 1 KiB multiple),PUTs it to the project-scoped volume endpoint, and polls until the cluster reports the new size.$running— PVE follows up with QEMUblock_resizeon a running guest, which requires the backing volume to already be larger. (The file-based base plugin returns early when running; that's wrong for network block storage.)nvme ns-rescanon the owning controller as a robustness backup. The NVMe controller normally refreshes namespace capacity via an async event; the rescan forces a synchronous re-read so the new size is visible beforeblock_resize, and covers kernel/target combinations that don't honor the event. Rescanning the controller (not the namespace) keeps it correct under NVMe multipath, and avoidsblockdev --rereadpt(which only re-reads a partition table).resizeinvolume_has_feature.Testing
t/volume_resize.t— asserts the PUT path/body and 4 KiB alignment across representative sizes. Full suite: 73 tests pass.lb-storageto a VM and resized viaqm resize, offline and with the volume activated. The Lightbits volume grew (1→2→3 GiB) and the host block device (/dev/nvme0n1) reflected the new capacity each time.Notes
main(independent of other in-flight feature groups). Grow-only; shrink is not supported by the backend.Summary by CodeRabbit
New Features
Documentation
Tests