-
Notifications
You must be signed in to change notification settings - Fork 167
Add notification post-livemigration #2792
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds support for post-live-migration notifications in the Guest Emulation Transport (GET) protocol. It introduces a new guest notification type that will allow the host to notify the guest when a live migration has completed, enabling the guest to take appropriate actions in response.
Changes:
- Adds
NOTIFY_POST_LIVE_MIGRATIONguest notification to the GET protocol - Implements callback infrastructure for handling post-live-migration notifications
- Integrates the notification callback setup in OpenHCL worker initialization
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| vm/devices/get/get_protocol/src/lib.rs | Defines new NOTIFY_POST_LIVE_MIGRATION guest notification constant |
| vm/devices/get/guest_emulation_transport/src/process_loop.rs | Adds message type, field storage, and notification handling for post-live-migration callback |
| vm/devices/get/guest_emulation_transport/src/client.rs | Adds public API method to set the post-live-migration notification callback |
| openhcl/underhill_core/src/worker.rs | Attempts to wire up the callback in OpenHCL initialization |
8df0c14 to
96f62f7
Compare
96f62f7 to
22b442f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| &mut self, | ||
| _notification: get_protocol::PostLiveMigrationNotification, | ||
| ) { | ||
| tracing::info!(CVM_ALLOWED, "notify_post_live_migration"); |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The callback stored in self.post_live_migration is never actually invoked. Looking at the similar handle_debug_interrupt_notification function (lines 1565-1587), the callback should be called if it exists. The handler should invoke the callback like this: if let Some(callback) = self.post_live_migration.as_ref() { callback() }
| tracing::info!(CVM_ALLOWED, "notify_post_live_migration"); | |
| tracing::info!(CVM_ALLOWED, "notify_post_live_migration"); | |
| if let Some(callback) = self.post_live_migration.as_ref() { | |
| callback(); | |
| } |
| .notify(msg::Msg::SetDebugInterruptCallback(callback.into())); | ||
| } | ||
|
|
||
| /// Set the the callback to handle PostLiveMigrationNotification. |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate word "the" in the comment. Should be "Set the callback to handle PostLiveMigrationNotification."
| /// Set the the callback to handle PostLiveMigrationNotification. | |
| /// Set the callback to handle PostLiveMigrationNotification. |
| /// Store the callback to trigger the debug interrupt. | ||
| // TODO: Consider a strategy that avoids LocalOnly here. | ||
| SetDebugInterruptCallback(LocalOnly<Box<dyn Fn(u8) + Send + Sync>>), | ||
| /// Notify that livemigration has finished. |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The term should be "live migration" (two words) instead of "livemigration" (one word) for consistency with standard terminology and the rest of the codebase.
| /// Notify that livemigration has finished. | |
| /// Notify that live migration has finished. |
This pull request adds a notification post-livemigration.
Guests are not generally meant to be aware of live migration, but there are or soon will be exceptions, and this will support them.