Re: [PATCH] qtest/hyperv: Introduce a simple hyper-v test

[Andrew Jones]

On Mon, Jul 19, 2021 at 11:30:41AM +0200, Vitaly Kuznetsov wrote:
> Andrew Jones <drjones@redhat.com> writes:
> 
> > On Fri, Jul 16, 2021 at 02:55:28PM +0200, Vitaly Kuznetsov wrote:
> >> For the beginning, just test 'hv-passthrough' and a couple of custom
> >> Hyper-V  enlightenments configurations through QMP. Later, it would
> >> be great to complement this by checking CPUID values from within the
> >> guest.
> >> 
> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> >> ---
> >> - Changes since "[PATCH v8 0/9] i386: KVM: expand Hyper-V features early":
> >>  make the test SKIP correctly when KVM is not present.
> >> ---
> >>  MAINTAINERS               |   1 +
> >>  tests/qtest/hyperv-test.c | 228 ++++++++++++++++++++++++++++++++++++++
> >>  tests/qtest/meson.build   |   3 +-
> >>  3 files changed, 231 insertions(+), 1 deletion(-)
> >>  create mode 100644 tests/qtest/hyperv-test.c
> >> 
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index 148153d74f5b..c1afd744edca 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -1576,6 +1576,7 @@ F: hw/isa/apm.c
> >>  F: include/hw/isa/apm.h
> >>  F: tests/unit/test-x86-cpuid.c
> >>  F: tests/qtest/test-x86-cpuid-compat.c
> >> +F: tests/qtest/hyperv-test.c
> >>  
> >>  PC Chipset
> >>  M: Michael S. Tsirkin <mst@redhat.com>
> >> diff --git a/tests/qtest/hyperv-test.c b/tests/qtest/hyperv-test.c
> >> new file mode 100644
> >> index 000000000000..2155e5d90970
> >> --- /dev/null
> >> +++ b/tests/qtest/hyperv-test.c
> >> @@ -0,0 +1,228 @@
> >> +/*
> >> + * Hyper-V emulation CPU feature test cases
> >> + *
> >> + * Copyright (c) 2021 Red Hat Inc.
> >> + * Authors:
> >> + *  Vitaly Kuznetsov <vkuznets@redhat.com>
> >> + *
> >> + * This work is licensed under the terms of the GNU GPL, version 2 or 
> >> later.
> >> + * See the COPYING file in the top-level directory.
> >> + */
> >> +#include <linux/kvm.h>
> >> +#include <sys/ioctl.h>
> >> +
> >> +#include "qemu/osdep.h"
> >> +#include "qemu/bitops.h"
> >> +#include "libqos/libqtest.h"
> >> +#include "qapi/qmp/qdict.h"
> >> +#include "qapi/qmp/qjson.h"
> >> +
> >> +#define MACHINE_KVM "-machine pc-q35-5.2 -accel kvm "
> >> +#define QUERY_HEAD  "{ 'execute': 'query-cpu-model-expansion', " \
> >> +                    "  'arguments': { 'type': 'full', "
> >> +#define QUERY_TAIL  "}}"
> >> +
> >> +static bool kvm_enabled(QTestState *qts)
> >> +{
> >> +    QDict *resp, *qdict;
> >> +    bool enabled;
> >> +
> >> +    resp = qtest_qmp(qts, "{ 'execute': 'query-kvm' }");
> >> +    g_assert(qdict_haskey(resp, "return"));
> >> +    qdict = qdict_get_qdict(resp, "return");
> >> +    g_assert(qdict_haskey(qdict, "enabled"));
> >> +    enabled = qdict_get_bool(qdict, "enabled");
> >> +    qobject_unref(resp);
> >> +
> >> +    return enabled;
> >> +}
> >> +
> >> +static bool kvm_has_cap(int cap)
> >> +{
> >> +    int fd = open("/dev/kvm", O_RDWR);
> >> +    int ret;
> >> +
> >> +    if (fd < 0) {
> >> +        return false;
> >> +    }
> >> +
> >> +    ret = ioctl(fd, KVM_CHECK_EXTENSION, cap);
> >> +
> >> +    close(fd);
> >> +
> >> +    return ret > 0;
> >> +}
> >> +
> >> +static QDict *do_query_no_props(QTestState *qts, const char *cpu_type)
> >> +{
> >> +    return qtest_qmp(qts, QUERY_HEAD "'model': { 'name': %s }"
> >> +                          QUERY_TAIL, cpu_type);
> >> +}
> >> +
> >> +static bool resp_has_props(QDict *resp)
> >> +{
> >> +    QDict *qdict;
> >> +
> >> +    g_assert(resp);
> >> +
> >> +    if (!qdict_haskey(resp, "return")) {
> >> +        return false;
> >> +    }
> >> +    qdict = qdict_get_qdict(resp, "return");
> >> +
> >> +    if (!qdict_haskey(qdict, "model")) {
> >> +        return false;
> >> +    }
> >> +    qdict = qdict_get_qdict(qdict, "model");
> >> +
> >> +    return qdict_haskey(qdict, "props");
> >> +}
> >> +
> >> +static QDict *resp_get_props(QDict *resp)
> >> +{
> >> +    QDict *qdict;
> >> +
> >> +    g_assert(resp);
> >> +    g_assert(resp_has_props(resp));
> >> +
> >> +    qdict = qdict_get_qdict(resp, "return");
> >> +    qdict = qdict_get_qdict(qdict, "model");
> >> +    qdict = qdict_get_qdict(qdict, "props");
> >> +
> >> +    return qdict;
> >> +}
> >> +
> >> +static bool resp_get_feature(QDict *resp, const char *feature)
> >> +{
> >> +    QDict *props;
> >> +
> >> +    g_assert(resp);
> >> +    g_assert(resp_has_props(resp));
> >> +    props = resp_get_props(resp);
> >> +    g_assert(qdict_get(props, feature));
> >> +    return qdict_get_bool(props, feature);
> >> +}
> >> +
> >> +#define assert_has_feature(qts, cpu_type, feature)                     \
> >> +({                                                                     \
> >> +    QDict *_resp = do_query_no_props(qts, cpu_type);                   \
> >> +    g_assert(_resp);                                                   \
> >> +    g_assert(resp_has_props(_resp));                                   \
> >> +    g_assert(qdict_get(resp_get_props(_resp), feature));               \
> >> +    qobject_unref(_resp);                                              \
> >> +})
> >> +
> >> +#define resp_assert_feature(resp, feature, expected_value)             \
> >> +({                                                                     \
> >> +    QDict *_props;                                                     \
> >> +                                                                       \
> >> +    g_assert(_resp);                                                   \
> >> +    g_assert(resp_has_props(_resp));                                   \
> >> +    _props = resp_get_props(_resp);                                    \
> >> +    g_assert(qdict_get(_props, feature));                              \
> >> +    g_assert(qdict_get_bool(_props, feature) == (expected_value));     \
> >> +})
> >> +
> >> +#define assert_feature(qts, cpu_type, feature, expected_value)         \
> >> +({                                                                     \
> >> +    QDict *_resp;                                                      \
> >> +                                                                       \
> >> +    _resp = do_query_no_props(qts, cpu_type);                          \
> >> +    g_assert(_resp);                                                   \
> >> +    resp_assert_feature(_resp, feature, expected_value);               \
> >> +    qobject_unref(_resp);                                              \
> >> +})
> >> +
> >> +#define assert_has_feature_enabled(qts, cpu_type, feature)             \
> >> +    assert_feature(qts, cpu_type, feature, true)
> >> +
> >> +#define assert_has_feature_disabled(qts, cpu_type, feature)            \
> >> +    assert_feature(qts, cpu_type, feature, false)
> >
> > All the code above looks like stuff we should share with other tests.
> > Shouldn't we factor that stuff out of those test(s) into some include?
> >
> 
> Probably yes but we'll need to come up with better names for all these
> functions to make it clear they're CPU specific.
>

Some can be given more general names, i.e. s/feature/bool/ as they're not
cpu specific. The ones that take a cpu_type parameter could just get a
cpu_ prefix, I guess.

Thanks,
drew