qemudriver: Allow machine-specific options and pre-start QMP commands
Description
Two minor features are added to the QemuDriver:
Support machine-specific options (e.g. -machine virt,secure=on)
While raising an exception for unsupported machines/boards is reasonable, adding machine-specific options shall not break the parsing. Therefore, the check is adjusted to only account for the leading board name.
Support interactions with the QEMU instance via QMP before releasing CPU(s)
Especially for bad case testing, it can be useful to be able to interact with the prepared but not yet spun off instance - e.g. to corrupt the boot medium without the need to build a corrupted image only for that purpose.
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.py file.
Checklist
- [x] Documentation for the feature
- [x] Tests for the feature
- [x] PR has been tested
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 :+1:
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...
Codecov Report
:x: Patch coverage is 93.93939% with 2 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 45.4%. Comparing base (f63dfd7) to head (cdc68c3).
:white_check_mark: All tests successful. No failed tests found.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| labgrid/driver/qemudriver.py | 93.9% | 2 Missing :warning: |
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
| Flag | Coverage Δ | |
|---|---|---|
| 3.10 | 45.4% <93.9%> (+0.1%) |
:arrow_up: |
| 3.11 | 45.4% <93.9%> (+0.1%) |
:arrow_up: |
| 3.12 | 45.4% <93.9%> (+0.1%) |
:arrow_up: |
| 3.13 | 45.4% <93.9%> (+0.1%) |
:arrow_up: |
| 3.14 | 45.4% <93.9%> (+0.1%) |
:arrow_up: |
| 3.9 | 45.5% <93.9%> (+0.1%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
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:
https://github.com/labgrid-project/labgrid/blob/f63dfd70a5da2c7007ee8c320fa969df2c0bf16e/tests/test_sigrok.py#L11-L12
Hmm, I'm actually not sure where this dependency is now coming from. It's not like the tests I want add depend on it. Maybe I forgot to inject the qemu_mock in one of the test cases? Could that be the root cause? I'll try this as soon as time permits.
I think I found the culprit - a missing qemu_version_mock - I overlooked the dependency to this in the get_qemu_base_args. Unfortunately, I wasn't able to test the workflow locally - could you retrigger the pipeline, @Bastian-Krause or @Emantor ? I reverted the change to the docker container so it should be quickly visible whether I now identified all missing parts...
After looking at the QEMUDriver again, I'm not too happy about starting the qemu process in on() (or with this PR in prepare() and stopping it in off(). This is not the fault of this PR, though. This PR only makes it even more visible.
I'll create an alternative PR starting qemu during on_activate() and stopping it in on_deactivate(). on() and off() will interact with it via QMP only. This should cover your use case because you can call monitor_command() right after activation. If we agree that this is a good solution, you can rebase your other changes on top.
After looking at the QEMUDriver again, I'm not too happy about starting the qemu process in
on()(or with this PR inprepare()and stopping it inoff(). This is not the fault of this PR, though. This PR only makes it even more visible.I'll create an alternative PR starting qemu during
on_activate()and stopping it inon_deactivate().on()andoff()will interact with it via QMP only. This should cover your use case because you can callmonitor_command()right after activation. If we agree that this is a good solution, you can rebase your other changes on top.
See #1771.
Makes sense. I haven't expected a proposal so early, thanks!