On Mon, Jun 22, 2020 at 08:57:51PM +0100, Peter Maydell wrote: > On Fri, 19 Jun 2020 at 12:41, Andrew Jones <drjones@xxxxxxxxxx> wrote: > > Can you also post the attached patch with this one (a two patch series)? > > This would be easier to review if you'd just posted it as > a patch with a Based-on: and a note that it needed to be OK, will do. > applied after the bugfix patch. Anyway: > > + /* Enabling and disabling kvm-no-adjvtime should always work. */ > assert_has_feature_disabled(qts, "host", "kvm-no-adjvtime"); > + assert_set_feature(qts, "host", "kvm-no-adjvtime", true); > + assert_set_feature(qts, "host", "kvm-no-adjvtime", false); > > if (g_str_equal(qtest_get_arch(), "aarch64")) { > bool kvm_supports_sve; > @@ -475,7 +497,11 @@ static void > test_query_cpu_model_expansion_kvm(const void *data) > char *error; > > assert_has_feature_enabled(qts, "host", "aarch64"); > + > + /* Enabling and disabling pmu should always work. */ > assert_has_feature_enabled(qts, "host", "pmu"); > + assert_set_feature(qts, "host", "pmu", true); > + assert_set_feature(qts, "host", "pmu", false); > > It seems a bit odd that we do the same "set true, then > set false" sequence whether the feature is enabled or not. > Shouldn't the second one be "set false, then set true" ? That would be better. Will do. Thanks, drew