RE: [PATCH v6 2/5] KVM: selftests: Put command line options in alphabetical order in dirty_log_perf_test

From: Wang, Wei W
Date: Tue Oct 25 2022 - 22:10:10 EST


On Saturday, October 22, 2022 5:18 AM, Vipin Sharma wrote:
> There are 13 command line options and they are not in any order. Put them in
> alphabetical order to make it easy to add new options.
>
> No functional change intended.
>
> Signed-off-by: Vipin Sharma <vipinsh@xxxxxxxxxx>
> ---
> .../selftests/kvm/dirty_log_perf_test.c | 36 ++++++++++---------
> 1 file changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> index 56e08da3a87f..5bb6954b2358 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -406,50 +406,52 @@ int main(int argc, char *argv[])
>
> guest_modes_append_default();
>
> - while ((opt = getopt(argc, argv, "eghi:p:m:nb:f:v:os:x:")) != -1) {
> + while ((opt = getopt(argc, argv, "b:ef:ghi:m:nop:s:v:x:")) != -1) {
> switch (opt) {
> + case 'b':
> + guest_percpu_mem_size = parse_size(optarg);
> + break;

"break" wasn't there.
This is kind of a functional change (i.e. bug fixing), and would be better to be
moved to patch 1.

Other parts look good to me.

Reviewed-by: Wei Wang <wei.w.wang@xxxxxxxxx>


> case 'e':
> /* 'e' is for evil. */
> run_vcpus_while_disabling_dirty_logging = true;
> break;
> + case 'f':
> + p.wr_fract = atoi(optarg);
> + TEST_ASSERT(p.wr_fract >= 1,
> + "Write fraction cannot be less than one");
> + break;
> case 'g':
> dirty_log_manual_caps = 0;
> break;
> + case 'h':
> + help(argv[0]);
> + break;
> case 'i':
> p.iterations = atoi(optarg);
> break;
> - case 'p':
> - p.phys_offset = strtoull(optarg, NULL, 0);
> - break;
> case 'm':
> guest_modes_cmdline(optarg);
> break;
> case 'n':
> perf_test_args.nested = true;
> break;
> - case 'b':
> - guest_percpu_mem_size = parse_size(optarg);
> + case 'o':
> + p.partition_vcpu_memory_access = false;
> break;
> - case 'f':
> - p.wr_fract = atoi(optarg);
> - TEST_ASSERT(p.wr_fract >= 1,
> - "Write fraction cannot be less than one");
> + case 'p':
> + p.phys_offset = strtoull(optarg, NULL, 0);
> + break;
> + case 's':
> + p.backing_src = parse_backing_src_type(optarg);
> break;
> case 'v':
> nr_vcpus = atoi(optarg);
> TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
> "Invalid number of vcpus, must be between 1 and %d",
> max_vcpus);
> break;
> - case 'o':
> - p.partition_vcpu_memory_access = false;
> - break;
> - case 's':
> - p.backing_src = parse_backing_src_type(optarg);
> - break;
> case 'x':
> p.slots = atoi(optarg);
> break;
> - case 'h':
> default:
> help(argv[0]);
> break;
> --
> 2.38.0.135.g90850a2211-goog