Virtwinkvm 2256#1029
Conversation
Adds CLI flags and setup-manager support for aio=native/aio=threads on virtual disks. Both modes also set cache=none. Signed-off-by: Elizabeth Ashurov <eashurov@redhat.com>
Sets drive_aio_state from the CLI flag on the functest platform. Signed-off-by: Elizabeth Ashurov <eashurov@redhat.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for configuring the drive AIO state (--aio-native and --aio-threads) for virtual disks, including CLI option parsing, validation to prevent conflicting configurations, and propagation of these settings to the platform options. Feedback is provided regarding a missing update to the merge! method in HLKPlatformClientsOptions to ensure the new drive_aio_state property is correctly merged.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| prop :vbs_state, T.nilable(T::Boolean) | ||
| prop :ctrl_net_device, T.nilable(String) | ||
| prop :fw_type, T.nilable(String) | ||
| prop :drive_aio_state, T.nilable(String) |
There was a problem hiding this comment.
The newly added drive_aio_state property is missing from the merge! method of HLKPlatformClientsOptions. This will cause drive_aio_state to be silently ignored when merging client options.
Please update the merge! method to include it:
def merge!(other)
self.viommu_state = other.viommu_state unless other.viommu_state.nil?
self.enlightenments_state = other.enlightenments_state unless other.enlightenments_state.nil?
self.vbs_state = other.vbs_state unless other.vbs_state.nil?
self.ctrl_net_device = other.ctrl_net_device unless other.ctrl_net_device.nil?
self.fw_type = other.fw_type unless other.fw_type.nil?
self.drive_aio_state = other.drive_aio_state unless other.drive_aio_state.nil?
endReferences
- Avoid replacing simple, explicit property assignments with dynamic metaprogramming (such as iterating over properties and using
public_send) solely to satisfy RuboCop metrics likeMetrics/AbcSize, as explicit code is often clearer and easier to understand.
|
|
||
| Models::HLKPlatform.from_json_file(platform_json, logger) | ||
| platform = Models::HLKPlatform.from_json_file(platform_json, logger) | ||
| platform.clients_options.drive_aio_state = options.test.drive_aio_state |
There was a problem hiding this comment.
I think about something like --fs-test-image-format but for aio. Is there any reason to make drive_aio_state as a state and add it to the client? What do you think?
| "drive_aio_state": { | ||
| "native": { | ||
| "drive_cache_options": [ | ||
| ",cache=none", |
There was a problem hiding this comment.
Why force cache = none?
There was a problem hiding this comment.
This matches the QEMU command from the Kar tests logs. And according to info from the internet, it is also recommended to use cache=none with aio=native
2 participants
Adds CLI flags and setup-manager support for aio=native/aio=threads on
virtual disks. Both modes also set cache=none.