-
Notifications
You must be signed in to change notification settings - Fork 229
qemudriver: Allow machine-specific options and pre-start QMP commands #1753
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: master
Are you sure you want to change the base?
qemudriver: Allow machine-specific options and pre-start QMP commands #1753
Conversation
680d6cd to
6d45393
Compare
Emantor
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.
Rather than adding a pre-start hook, I think it is more clear to expose somthing like a prepare() function which sets up the qemu process, and than have on() only do the cont monitor command. For backwards compatibility, the class can check whether the self._child attribute exists and call prepare() in on() if that was not done yet.
6d45393 to
a778800
Compare
Emantor
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.
I think we can take the other fixes in this PR as well.
a778800 to
10d2786
Compare
|
Thanks for the review and sorry for force-pushing so often - it looked easier to me to fix the one linting issue I overlooked when implementing the proposal from Rouven. |
|
It's all good and thanks for keeping up with the PR even if it takes some time on our side 👍 |
|
I shortly glimpsed into the failing pipeline and now I'm wondering whether some environment changes happened? The test_qemudriver is not able to locate qemu-system-arm... |
Signed-off-by: Joschka Seydell <[email protected]>
…off the instance. Signed-off-by: Joschka Seydell <[email protected]>
Signed-off-by: Joschka Seydell <[email protected]>
Signed-off-by: Joschka Seydell <[email protected]>
In order to build the new tests for the qemu driver, the qemu-system-arm binary needs to be available on the system. Signed-off-by: Rouven Czerwinski <[email protected]>
1eb616e to
500d6e9
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1753 +/- ##
========================================
+ Coverage 45.3% 45.4% +0.1%
========================================
Files 172 172
Lines 13517 13523 +6
========================================
+ Hits 6130 6151 +21
+ Misses 7387 7372 -15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Prior to this, we've mocked qemu. Why do we need to use the real binary now? If we decide we need the real qemu, we should at least add a marker so the tests get skipped on systems without it. Something along the lines of: Lines 11 to 12 in f63dfd7
|
Description
Two minor features are added to the
QemuDriver:Support machine-specific options (e.g.
-machine virt,secure=on)Support interactions with the QEMU instance via QMP before releasing CPU(s)
For both features, straight forward unit tests are provided.
Lastly, the changeset includes minor documentation updates and the introduction of type hints for the
qemudriver.pyfile.Checklist