-
Notifications
You must be signed in to change notification settings - Fork 19
feat: Add --timesync flag for guest clock synchronization
#89
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
tylerfanelli
left a comment
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.
Thanks for the contribution @vyasgun, I've added some comments. I see this is still a draft, so apologies if I address points you were already working on.
Can you add a description of how to test?
Cargo.toml
Outdated
| log = "0.4.0" | ||
| env_logger = "0.11.8" | ||
| regex = "1.11.1" | ||
| IOKit-sys = { git = "https://github.com/vyasgun/iokit-sys", branch = "master" } |
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.
This would likely need to be an official release before we accept it as a dependency.
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.
An official release is going to be unlikely because this repository was last updated in 2018 😓 I opened a PR there last month: dcuddeback/iokit-sys#14 but doubt it would be merged.
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.
I think we shouldn't rely on it then.
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.
I found another crate with all the bindings which is being actively maintained: https://crates.io/crates/objc2-io-kit
src/main.rs
Outdated
| mod sleep_notifier; | ||
| mod status; | ||
| mod timesync; |
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.
Should sleep_notifier perhaps be a part of the timesync module? If there's no other uses, I believe it should.
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.
thanks, I put all the related code in timesync module.
src/sleep_notifier.rs
Outdated
| kIOMessageSystemWillSleep => tx.send(Activity::Sleep).unwrap(), | ||
| kIOMessageSystemWillPowerOn => tx.send(Activity::Wake).unwrap(), | ||
| kIOMessageSystemHasPoweredOn => tx.send(Activity::Wake).unwrap(), |
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.
Should likely return some type of error code on failure, rather than unwrap(). Since it seems to be called by the IO system, perhaps a log message would suffice?
Add a --timesync option which uses vsock to send host time to qemu-guest-agent running in the guest on VM suspend/resume. Uses IOKit's power management APIs to detect system sleep/wake events. Since the official iokit-sys crate is incomplete, a custom fork is required. This also maintains feature parity between krunkit and vfkit. Assisted by: Claude (Anthropic AI) Signed-off-by: Gunjan Vyas <vyasgun20@gmail.com> Co-authored-by: Tyler Fanelli <tfanelli@redhat.com>
|
Thanks @vyasgun, code LGTM. I'm having some issues with my Mac Mini, so I just need to sort that out and manually test before I merge. |
Signed-off-by: Gunjan Vyas <vyasgun20@gmail.com>
|
thanks @jakecorrenti - I updated the usage doc as well. |
Add a --timesync option which uses vsock to send host time to qemu-guest-agent running in the guest on VM suspend/resume.
Uses IOKit's power management APIs to detect system sleep/wake events. Since the official iokit-sys crate is incomplete, a custom fork is required.
This also maintains feature parity between krunkit and vfkit.
Fixes: #88
To test this, start krunkit with the
--timesync <vsock-port>flag and other arguments as follows:Inside the guest,
qemu-guest-agentprocess should be running:This is run as a systemd service on CRC which can be configured using
cloud-init.After this, put the host to sleep and check the time in the VM when it wakes up.