Re: [PATCH] Add unit tests for timer validation

[Peter Krempa <pkrempa@xxxxxxxxxx>]

On Tue, Sep 15, 2020 at 19:01:36 +0000, Sebastian Mitterle wrote:
> Add minimal coverage for non-x86_64 timer validation
> from commit 2f5d8ffebe5d3d00e16a051ed62ce8a703f18e7c
> 
> Signed-off-by: Sebastian Mitterle <smitterl@xxxxxxxxxx>
> ---
>  tests/meson.build        |   1 +
>  tests/qemuvalidatetest.c | 107 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 108 insertions(+)
>  create mode 100644 tests/qemuvalidatetest.c

[...]

> diff --git a/tests/qemuvalidatetest.c b/tests/qemuvalidatetest.c
> new file mode 100644
> index 0000000000..617669dae0
> --- /dev/null
> +++ b/tests/qemuvalidatetest.c
> @@ -0,0 +1,107 @@
> +/*
> + * qemuvalidatetest.c: Test the qemu domain validation
> + *
> + * Copyright (C) 2010-2020 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <config.h>
> +
> +#include "qemu/qemu_validate.c"
> +#include "testutils.h"
> +#include "internal.h"
> +#include "src/conf/virconftypes.h"
> +#include "src/conf/domain_conf.h"
> +#include "src/qemu/qemu_capabilities.h"
> +#include "src/util/virarch.h"
> +
> +static virDomainDefPtr
> +getDefWithUnsupportedTimerPresent(void)
> +{
> +    virDomainTimerDefPtr timer;
> +    virDomainDefPtr def;
> +
> +    if (VIR_ALLOC(timer) < 0)
> +        return NULL;
> +
> +    timer->present = 1;
> +    timer->name = VIR_DOMAIN_TIMER_NAME_TSC;
> +
> +    def = virDomainDefNew();
> +    def->virtType = VIR_DOMAIN_VIRT_QEMU;
> +    def->os = (virDomainOSDef) {
> +        .arch = VIR_ARCH_S390X,
> +        .machine = (char *)"m"
> +    };
> +    def->emulator = (char *)"e";
> +    def->clock = (virDomainClockDef) {
> +        .ntimers = 1,
> +    };
> +    if (VIR_ALLOC_N(def->clock.timers, 1) < 0)
> +        return NULL;
> +    def->clock.timers[0] = timer;

IMO the timers can be tested via the qemuxml2argvtest via a
DO_TEST_FAILURE case.


> +
> +    return def;
> +}
> +
> +static int
> +errorTimersNotOnX86(const void *unused G_GNUC_UNUSED)
> +{
> +    int ret = 0;
> +    char *log;
> +    char expectedError[75] = "'tsc' timer is not supported for virtType=qemu arch=s390x machine=m guests";

This is extreme overkill. We don't really validate error messages and in
this particular case I don't really see much value.

IMO a minimal XML with the timer in the qemuxml2argvtest will do just
fine.