Conversation
Signed-off-by: Pavel Boldyrev <pavel@bpg.sh>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical issue where virtual machines would not reboot after a disk resize if the disk was not hotpluggable, preventing the guest OS from recognizing the increased storage. The changes ensure that the provider correctly identifies hotpluggable features based on Proxmox VE's defaults and triggers a necessary reboot when disk hotplug is disabled, allowing the guest OS to properly process the size change. It also refines the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This PR provides a well-implemented fix for the issue where a non-hotpluggable disk resize didn't trigger a required reboot. The changes correctly adjust the default hotplug logic and add a post-resize reboot, ensuring the guest OS recognizes the new disk size. The accompanying acceptance tests are thorough, covering the fix, the double-reboot scenario, and preventing regressions. The documentation updates are also clear and helpful. I have one suggestion to improve maintainability by refactoring some duplicated code.
What does this PR do?
Fixes an issue where resizing a disk on a running VM did not trigger a reboot when
diskwas excluded from thehotplugsetting. After the hotplug fix in #2412, CPU/memory/network changes are hotplugged without reboot. If the only change is a disk size increase and disk is not hotpluggable,rebootRequiredstaysfalse, no reboot occurs, and cloud-init'sgrowpartnever runs — leaving the guest filesystem at the old size.This PR makes two changes:
isHotpluggable()default — whenhotplugis not explicitly configured, PVE defaults tonetwork,disk,usb. PreviouslyisHotpluggable()returnedfalsefor all features in this case.reboot_after_update = falseby emitting a warning instead. In the combined case (config change + disk resize + non-hotpluggable), the VM reboots twice: once before resize (for config) and once after (for the guest to see the new size).Contributor's Note
make lintand fixed any issues.make docs; SDK: manual/docs/edits).!in PR title).make exampleto verify the change works (mainly for SDK / provider config changes).Proof of Work
TDD RED phase — test fails without fix (no reboot after disk resize):
TDD GREEN phase — test passes with fix (VM rebooted after resize, including double-reboot step):
Test verifies reboot via uptime check: captures VM uptime after step 1, asserts uptime reset (reboot occurred) after step 2 resize. Step 3 tests AIO change + resize (double reboot: pre-resize for config + post-resize for disk size).
Default hotplug test — no unnecessary reboot when disk is hotpluggable by default:
Test verifies NO reboot via uptime check: asserts uptime kept increasing (no reboot) after resize with PVE default hotplug.
Regression tests — no regressions:
Community Note
Closes #2684