Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions lib/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,19 @@ class CliTestOptions < T::Struct
prop :latest_session, T::Boolean, default: false
prop :category, T.nilable(String)
prop :testcase, T.nilable(String)
prop :drive_aio_state, T.nilable(String)

def aio_native=(value)
raise(AutoHCKError, '--aio-native cannot be combined with --aio-threads') if value && drive_aio_state == 'threads'

self.drive_aio_state = 'native' if value
end

def aio_threads=(value)
raise(AutoHCKError, '--aio-threads cannot be combined with --aio-native') if value && drive_aio_state == 'native'

self.drive_aio_state = 'threads' if value
end

def create_parser
OptionParser.new do |parser|
Expand Down Expand Up @@ -277,6 +290,16 @@ def define_options(parser)
parser.on('--testcase <test_case_names>', String,
'Run specific functest test cases, comma-separated (used with functest engine)',
&method(:testcase=))

parser.on('--aio-native', TrueClass,
'Use aio=native for virtual disks (forces cache=none, cannot combine with --aio-threads)',
'(used with functest engine)',
&method(:aio_native=))

parser.on('--aio-threads', TrueClass,
'Use aio=threads for virtual disks (forces cache=none, cannot combine with --aio-native)',
'(used with functest engine)',
&method(:aio_threads=))
end
# rubocop:enable Metrics/AbcSize, Metrics/MethodLength
end
Expand Down
5 changes: 4 additions & 1 deletion lib/engines/functest/functest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ def self.platform(logger, options)

raise InvalidConfigFile, "Platform configuration not found: #{platform_json}" unless File.exist?(platform_json)

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?


platform
end

ENGINE_MODE = 'test'
Expand Down
1 change: 1 addition & 0 deletions lib/models/hlk_platform.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class HLKPlatformClientsOptions < T::Struct
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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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?
      end
References
  1. Avoid replacing simple, explicit property assignments with dynamic metaprogramming (such as iterating over properties and using public_send) solely to satisfy RuboCop metrics like Metrics/AbcSize, as explicit code is often clearer and easier to understand.


# rubocop:disable Metrics/AbcSize
# There is no way to reduce the ABC size of this method
Expand Down
1 change: 1 addition & 0 deletions lib/setupmanagers/qemuhck/qemu_machine.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"vhost_state": true,
"viommu_state": false,
"drive_unsafe_cache_state": false,
"drive_aio_state": false,
"tpm_state": false
}
}
14 changes: 14 additions & 0 deletions lib/setupmanagers/qemuhck/qemu_machine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
# AutoHCK module
module AutoHCK
# QemuMachine class
# rubocop:disable Metrics/ClassLength
class QemuMachine
extend T::Sig
extend AutoHCK::AutoloadExtension
Expand Down Expand Up @@ -305,10 +306,22 @@ def init_config

@states_config = Json.read_json(STATES_JSON, @logger)
@states_config.each { |name, state| apply_state name, state }
validate_drive_aio_state

apply_cpu_options_config
end

# Both native and threads drive_aio_state options set cache=none (cache.direct=on);
# cache=unsafe has cache.direct=off, so QEMU would fail to start with both cache
# fragments applied at once.
def validate_drive_aio_state
return unless %w[native threads].include?(option_config('drive_aio_state'))
return unless option_config('drive_unsafe_cache_state')

raise QemuHCKError, "drive_aio_state=#{option_config('drive_aio_state')} is incompatible with " \
'drive_unsafe_cache_state=true (both set a conflicting cache= mode)'
end

def apply_cpu_options_config
extra = option_config('cpu_options')
return if extra.nil? || extra.empty?
Expand Down Expand Up @@ -794,4 +807,5 @@ def run(scope, run_opts = nil)
end
end
end
# rubocop:enable Metrics/ClassLength
end
2 changes: 1 addition & 1 deletion lib/setupmanagers/qemuhck/qemuhck.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class QemuHCK

QEMUHCK_INFO_LOG_FILE = 'qemuhck.txt'
OPT_NAMES = %w[viommu_state s3_state s4_state enlightenments_state vhost_state machine_type fw_type cpu
ctrl_net_device vbs_state tpm_state].freeze
ctrl_net_device vbs_state tpm_state drive_aio_state].freeze

def initialize(project)
@platform = T.let(project.engine_platform, Models::HLKPlatform)
Expand Down
14 changes: 14 additions & 0 deletions lib/setupmanagers/qemuhck/states.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,20 @@
]
}
},
"drive_aio_state": {
"native": {
"drive_cache_options": [
",cache=none",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why force cache = none?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

",aio=native"
]
},
"threads": {
"drive_cache_options": [
",cache=none",
",aio=threads"
]
}
},
"tpm_state": {
"true": {
"devices_list": [
Expand Down
Loading