feat: domain resume and suspend #47
Conversation
WalkthroughThis pull request implements VM suspend and resume functionality by introducing new asynchronous methods in both the Domain and Hypervisor classes. Test cases have been added to verify the domain lifecycle transitions from RUNNING to PAUSED and back to RUNNING, followed by shutdown. At the C++ layer, worker classes are introduced to handle suspend/resume operations via libvirt. Additionally, the Docker testing script is modified to remove caching and simplify its build/run process. No public API changes are made. Changes
Sequence Diagram(s)sequenceDiagram
participant Test
participant Domain
participant Hypervisor
participant NativeHypervisor
Test->>Domain: call suspend()
Domain->>Hypervisor: domainSuspend(domain)
Hypervisor->>NativeHypervisor: invoke virDomainSuspend(domain)
NativeHypervisor-->>Hypervisor: return result
Hypervisor-->>Domain: promise returned (state: PAUSED)
Test->>Domain: call resume()
Domain->>Hypervisor: domainResume(domain)
Hypervisor->>NativeHypervisor: invoke virDomainResume(domain)
NativeHypervisor-->>Hypervisor: return result
Hypervisor-->>Domain: promise returned (state: RUNNING)
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (2)
scripts/test-docker.sh (1)
20-25: Simplifying build command removes error checking.The Docker build command has been simplified by removing the caching mechanism, which is good for reliability. However, make sure the build succeeds before proceeding to run tests.
Consider adding error checking:
docker buildx build \ --platform linux/"$arch" \ -t "$tag" \ --load \ . + + # Check if build was successful + if [ $? -ne 0 ]; then + echo -e "${RED}Build failed for architecture $arch${NC}" + return 1 + fi__tests__/domain-lifecycle.test.ts (1)
176-289: LGTM! Comprehensive test for the new suspend/resume functionality.The test thoroughly validates the lifecycle of a domain through:
- Creation and startup (verified RUNNING state)
- Suspension (verified PAUSED state)
- Resumption (verified RUNNING state)
- Shutdown and cleanup (verified SHUTOFF state)
The test properly exercises the new functionality and ensures the domain transitions through the expected states.
There's significant duplication between this test and the previous one in setting up the domain. Consider extracting the domain setup into a helper function to reduce duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
__tests__/domain-lifecycle.test.ts(1 hunks)lib/domain.spec.ts(5 hunks)lib/domain.ts(1 hunks)lib/hypervisor.ts(1 hunks)scripts/test-docker.sh(1 hunks)src/domain.h(1 hunks)src/hypervisor-domain.cc(2 hunks)src/hypervisor.cc(2 hunks)src/hypervisor.h(2 hunks)
🧰 Additional context used
🧬 Code Definitions (4)
lib/domain.spec.ts (1)
lib/hypervisor.ts (2)
domainSuspend(188-193)domainResume(200-205)
lib/hypervisor.ts (3)
lib/domain.ts (1)
Domain(8-139)lib/types.ts (1)
Domain(130-130)lib/error.ts (1)
wrapMethod(55-61)
__tests__/domain-lifecycle.test.ts (3)
lib/domain-desc.ts (1)
DomainDesc(103-150)__tests__/helpers.ts (2)
TEST_VM_NAME(6-6)DISK_IMAGE(7-7)lib/domain-xml.ts (1)
domainDescToXml(300-412)
src/hypervisor-domain.cc (2)
src/worker.cc (2)
SetVirError(12-17)SetVirError(12-12)src/hypervisor-connect.cc (2)
dummyCallback(15-19)dummyCallback(15-15)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Docker Integration Tests (amd64)
🔇 Additional comments (17)
src/domain.h (1)
48-49: LGTM: Friend class additions follow existing patternThese new friend class declarations allow the suspend and resume workers to access the private members of the Domain class, following the same pattern used for other domain operations.
src/hypervisor.cc (2)
9-12: Explicit header includes improve maintainabilityAdding these direct dependencies that were likely previously included indirectly helps make the code more maintainable by clearly documenting the module's dependencies.
46-47: LGTM: New method exposures follow existing patternThe new instance methods for domain suspend and resume operations follow the established pattern for exposing libvirt functionality to JavaScript.
lib/domain.ts (2)
50-56: LGTM: Suspend implementation is well-documentedThe suspend method is properly implemented with comprehensive JSDoc comments describing its purpose and potential error conditions.
58-65: LGTM: Resume implementation is well-documentedThe resume method is properly implemented with comprehensive JSDoc comments describing its purpose and potential error conditions.
lib/hypervisor.ts (2)
183-193: LGTM: Domain suspend implementation follows existing patternThe domainSuspend method correctly follows the established pattern for wrapping native hypervisor methods with proper error handling via the wrapMethod function.
195-205: LGTM: Domain resume implementation follows existing patternThe domainResume method correctly follows the established pattern for wrapping native hypervisor methods with proper error handling via the wrapMethod function.
src/hypervisor.h (1)
76-77: LGTM! Method signatures correctly follow existing pattern.The new methods
DomainSuspendandDomainResumefollow the same signature pattern as other domain methods, maintaining consistency in the codebase.scripts/test-docker.sh (1)
30-38: LGTM! Added necessary capabilities for VM testing.The additional capabilities (
SYS_ADMIN,NET_ADMIN), device mapping (/dev/kvm), network configuration (--network host), and volume mount (/sys/fs/cgroup) are appropriate for testing virtualization functionality, especially for the new suspend/resume operations.lib/domain.spec.ts (5)
4-4: LGTM! Using DomainState enum improves code readability.Using the
DomainStateenum instead of numeric values makes the code more readable and less error-prone.
19-19: LGTM! Using enum value improves clarity.Replacing the hardcoded value
1withDomainState.RUNNINGmakes the code more maintainable and self-documenting.
28-29: LGTM! New mock functions support suspend/resume testing.The new mock functions and their addition to the hypervisor object properly support testing the suspend and resume functionality.
Also applies to: 43-44
91-96: LGTM! Test verifies Domain.suspend calls hypervisor.domainSuspend.The test correctly verifies that
domain.suspend()callshypervisor.domainSuspend()with the domain as an argument.
98-103: LGTM! Test verifies Domain.resume calls hypervisor.domainResume.The test correctly verifies that
domain.resume()callshypervisor.domainResume()with the domain as an argument.src/hypervisor-domain.cc (3)
658-658: Good job on fixing the comment header casing.The casing correction in the comment header for the DomainShutdown function improves consistency.
856-898: Well-implemented domain suspend functionality.The DomainSuspend functionality is well-implemented following the established pattern in the codebase. The worker class correctly calls virDomainSuspend and properly handles errors via SetVirError(). The Hypervisor::DomainSuspend method includes appropriate input validation before unwrapping the domain object and queueing the worker.
900-942: Well-implemented domain resume functionality.The DomainResume implementation follows the same consistent pattern as other domain operations. The worker class correctly calls virDomainResume and handles errors appropriately. Input validation is properly done before unwrapping the domain object and queueing the worker.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #47 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 7 7
Lines 767 785 +18
Branches 154 154
=========================================
+ Hits 767 785 +18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🤖 I have created a release *beep* *boop* --- ## [2.1.0](v2.0.4...v2.1.0) (2025-03-30) ### Features * domain resume and suspend ([#47](#47)) ([2b96ed3](2b96ed3)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Tests
Chores