7:17 a.m.
On Wed, Apr 05, 2017 at 07:37:34 -0400, John Ferlan wrote:
On 03/17/2017 12:36 PM, Jiri Denemark wrote:
> The test takes
>
> x86-cpuid-Something-guest.xml CPU (the CPU libvirt would use for
> host-model on a CPU described by x86_64-cpuid-Something.xml without
> talking to QEMU about what it supports on the host)
>
> and updates it according to CPUID data from QEMU:
>
> x86_64-cpuid-Something-enabled.xml (reported as "feature-words"
> property of the CPU device)
>
> and
>
> x86_64-cpuid-Something-disabled.xml (reported as "filtered-features"
> property of the CPU device).
>
> The result is compared to
>
> x86_64-cpuid-Something-json.xml (the CPU libvirt would use as
> host-model based on the reply from query-cpu-model-expansion).
>
> The comparison is a bit tricky because the *-json.xml CPU contains fewer
> disabled features. Only the features which are included in the base CPU
> model, but listed as disabled in *.json will be disabled in *-json.xml.
> The CPU computed by virCPUUpdateLive from the test data will list all
> features present in the host's CPUID data and not enabled in *.json as
> disabled. The cpuTestUpdateLiveCompare function checks that the computed
> and expected sets of enabled features match.
>
> Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
> ---
> tests/cputest.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 149 insertions(+)
>
> diff --git a/tests/cputest.c b/tests/cputest.c
> index 4a4d427e1..2c64c2cd0 100644
> --- a/tests/cputest.c
> +++ b/tests/cputest.c
> @@ -523,6 +523,151 @@ cpuTestGuestCPUID(const void *arg)
> }
>
>
> +static int
> +cpuTestUpdateLiveCompare(virArch arch,
> + virCPUDefPtr actual,
> + virCPUDefPtr expected)
> +{
> + size_t i, j;
> + int ret = 0;
> +
> + if (virCPUExpandFeatures(arch, actual) < 0 ||
> + virCPUExpandFeatures(arch, expected) < 0)
> + return -1;
> +
> + if (STRNEQ(actual->model, expected->model)) {
> + VIR_TEST_VERBOSE("Actual CPU model '%s', expected
'%s'\n",
> + actual->model, expected->model);
> + return -1;
> + }
> +
> + i = j = 0;
> + while (i < actual->nfeatures || j < expected->nfeatures) {
> + virCPUFeatureDefPtr featAct = NULL;
> + virCPUFeatureDefPtr featExp = NULL;
> + int cmp;
> +
> + if (i < actual->nfeatures)
> + featAct = actual->features + i;
> +
> + if (j < expected->nfeatures)
> + featExp = expected->features + j;
> +
> + /*
> + * Act < Exp => cmp < 0 (missing entry in Exp)
> + * Act = Exp => cmp = 0
> + * Act > Exp => cmp > 0 (missing entry in Act)
> + *
> + * NULL > name for any name != NULL
> + */
> + if (featAct && featExp)
> + cmp = strcmp(featAct->name, featExp->name);
> + else
> + cmp = featExp ? 1 : -1;
> +
> + if (cmp <= 0)
> + i++;
> + if (cmp >= 0)
> + j++;
> +
> + /* Possible combinations of cmp, featAct->policy, and
featExp->policy:
> + * cmp Act Exp result
> + * ---------------------------------
> + * 0 dis dis ok
> + * 0 dis req missing
> + * 0 req dis extra
> + * 0 req req ok
> + * ---------------------------------
> + * - dis X ok # ignoring extra disabled features
> + * - req X extra
> + * ---------------------------------
> + * + X dis extra
> + * + X req missing
> + */
> + if ((cmp == 0 &&
> + featAct->policy == VIR_CPU_FEATURE_DISABLE &&
> + featExp->policy == VIR_CPU_FEATURE_REQUIRE) ||
> + (cmp > 0 &&
> + featExp->policy == VIR_CPU_FEATURE_REQUIRE)) {
> + VIR_TEST_VERBOSE("Actual CPU lacks feature '%s'\n",
> + featExp->name);
> + ret = -1;
> + continue;
> + }
> +
> + if ((cmp == 0 &&
> + featAct->policy == VIR_CPU_FEATURE_REQUIRE &&
> + featExp->policy == VIR_CPU_FEATURE_DISABLE) ||
> + (cmp < 0 &&
> + featAct->policy == VIR_CPU_FEATURE_REQUIRE) ||
> + (cmp > 0 &&
> + featExp->policy == VIR_CPU_FEATURE_DISABLE)) {
> + VIR_TEST_VERBOSE("Actual CPU has extra feature
'%s'\n",
> + featAct->name);
I know it's only a test, but this log message raised Coverity's
attention because it's not sure featAct is NULL at this point if "cmp >
0". So that I can at least mask it my local tree - could featAct be
NULL or should there be a different message if cmp > 0?
Oops, featAct should be used only if cmp <= 0, otherwise it will either
be NULL or irrelevant and featExp should be used instead.
Jirka