-
Notifications
You must be signed in to change notification settings - Fork 650
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
Fix stop
succeeding when instance isn't stopped
#3684
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3684 +/- ##
==========================================
+ Coverage 88.92% 88.94% +0.02%
==========================================
Files 254 254
Lines 14271 14276 +5
==========================================
+ Hits 12690 12698 +8
+ Misses 1581 1578 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Hey @Sploder12, thanks! Would you mind rebasing? Although there doesn't seem to be a conflict, GH still shows a diff to the old version here: |
bd62341
to
4729fe4
Compare
tests/qemu/test_qemu_backend.cpp
Outdated
|
||
machine->state = mp::VirtualMachine::State::running; | ||
|
||
EXPECT_THROW(machine->shutdown(), std::runtime_error); |
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.
You could look into our MP_EXPECT_THROW_THAT
macro, which we use to add a matcher for the exception message, so that it's more clear about what you actually expect, not just any random std::runtime_error
.
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.
Alright, code looks good and now the stop
command fails as it should. Thanks, @Sploder12!
Fix `stop` succeeding when instance isn't stopped
Fix `stop` succeeding when instance isn't stopped
resolves #2907
When running
stop
on an instance that could not be stopped no error would be reported. This was becauseQemuVirtualMachine::shutdown(force=false)
ignored the result ofvm_process->wait_for_finished(shutdown_timeout)
which indicates if a timeout occurred or not. This issue does not seem to be present in other VM backends.This PR makes it so the timeout is not ignored and the timeout is properly reported.