passthrough: add robustness test suite #5853
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@chunfuwen @legoater Please help review. |
|
Convert to draft as tests and regression coverage still running. |
|
Test runs The first test failure happened because I'm now also running existing tests that use the provider/vfio code to confirm no regression was introduced. |
smitterl
force-pushed
the
vfio
branch
3 times, most recently
from
26b97bf
to
79c9be5
Compare
The failing |
chunfuwen
reviewed
Aug 29, 2024
| start_vm = no | ||
| vms = avocado-vt-vm1 vm2 | ||
| variants: | ||
| - start_stop_ap: |
There was a problem hiding this comment.
Does each s390x machine have the same AP address, e.g ap-00.000e ?
There was a problem hiding this comment.
No. It can be replaced to be adapted to a different server. Given that these ids don't reveal any confidential information I prefer to use them instead of using placeholder values.
chunfuwen
reviewed
Aug 29, 2024
| cmds += vm_start;0, | ||
| cmds += vm_attach;0;roce-0002:00:00.0, | ||
| cmds += vm_check_present;0;roce-0002:00:00.0, | ||
| - vm_running_attach_shutdown_start_attach.all: |
There was a problem hiding this comment.
The whole cfg holdes too many hard-coded string :cmds += . I wonder whether we can dynamically generate in the code.
There was a problem hiding this comment.
@chunfuwen I don't agree. This is on purpose. The test suite introduces a way to easily script new scenarios in the cfg file. The lines with 'cmds +=' are IMO a very readable way to define a test scenario.
chunfuwen
reviewed
Aug 29, 2024
| xml.sync() | ||
|
|
||
|
|
||
| class NodeDevHandlers(object): |
There was a problem hiding this comment.
NodeDevHandlers -->NodeDevHandlerAggregator? it looks like the latter is more meaningful to hold mulitple NodeDevHandler instances
There was a problem hiding this comment.
I found that the plural '...Handlers' already conveyed well that it represents a set of '...Handler' but in terms of code design patterns you are right, Aggregator should be more precise so I updated this. Thanks.
chunfuwen
reviewed
Aug 29, 2024
| self.fid = None | ||
| self.previously_started = False | ||
|
|
||
| global device_address_counter |
There was a problem hiding this comment.
From object-oriented perspective, since NodeDevHandler is one class ,and it is need to be instanced when we use it each time. So it looks like it does't make sense that one class refer to one global variable: device_address_counter.(unless device_address_counter is constant variable) considering it bring Maintainability Challenges
There was a problem hiding this comment.
You are right. In this case I'll use a class variable instead.
smitterl
force-pushed
the
vfio
branch
3 times, most recently
from
6321ce2
to
f1c2d01
Compare
Add test suite that will execute configured actions on passthrough devices and VMs. | Scenarios (edge coverage) | | | |------------------------------------------------------------------------------ |------------ |------------------------- | | VM | | | | all should end with "check_present" if applicable | | | | edge coverage: 1 | | | | RUNNING -> attach -> pause resume | 1 dev | all dev | | RUNNING -> attach -> reboot | 1 dev | all dev | | RUNNING -> attach -> detach attach | 1 dev | all dev | | RUNNING -> attach -> detach attach -> reboot | 1 dev | all dev | | RUNNING -> attach -> shutdown start -> attach | 1 dev | all dev | | STOPPED -> attach -> start -> RUNNING -> detach attach | 1 dev | all dev | | STOPPED -> attach -> start -> RUNNING -> reboot | 1 dev | all dev | | STOPPED -> attach -> start -> RUNNING -> shutdown start | 1 dev | all dev | | STOPPED -> attach -> start -> RUNNING -> destroy start | 1 dev | all dev | | STOPPED -> attach -> start -> RUNNING -> pause resume | 1 dev | all dev | | RUNNING -> attach -> 10x detach attach | 1 dev | all dev | | STOPPED -> attach -> start -> 10x detach attach reboot | 1 dev | all dev | | RUNNING -> attach -> attach | 1 dev type | all supported dev types | | RUNNING -> attach -> attach -> reboot | 1 dev type | all supported dev types | | RUNNING -> attach -> attach -> pause resume | 1 dev type | all supported dev types | | RUNNING -> attach -> attach -> detach attach -> detach attach | 1 dev type | all supported dev types | | RUNNING -> attach -> attach -> reboot -> detach attach -> detach attach | 1 dev type | all supported dev types | | Device | | | | Mediated devices need to be restarted, only possible when not used in VM | | | | RUNNING -> attach -> detach -> restart dev -> attach | 1 dev | all dev | | Migrate device between VMs | | | | (RUNNING0 -> attach detach) -> (RUNNING1 attach) | 1 dev | all dev | | (RUNNING0 -> attach detach) -> (RUNNING1 attach detach) -> (RUNNING0 attach) | 1 dev | all dev | The MdevHandler classes had to be updated to retain state, so that reusing them would create the same conditions as using the first time both for device creation (ids and driver overrides) and checks. Apply black, inspekt and isort tools for code quality. Signed-off-by: Sebastian Mitterle <smitterl@redhat.com>
|
additional single test run to cover the latest changes based on review |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add test suite that will execute configured actions on passthrough devices and VMs.
The MdevHandler classes had to be updated to retain state, so that reusing them would create the same conditions as using the first time both for device creation (ids and driver overrides) and checks.
Apply black, inspekt and isort tools for code quality.
Depends on avocado-framework/avocado-vt#3975