Add minimal coverage for non-x86_64 timer validation
from commit 2f5d8ffebe5d3d00e16a051ed62ce8a703f18e7c

Signed-off-by: Sebastian Mitterle <smitterl@redhat.com>
---
 tests/meson.build        |   1 +
 tests/qemuvalidatetest.c | 107 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+)
 create mode 100644 tests/qemuvalidatetest.c

diff --git a/tests/meson.build b/tests/meson.build
index 0a204c46e4..dfdc1e8b93 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -460,6 +460,7 @@ if conf.has('WITH_QEMU')
     { 'name': 'qemumigparamstest', 'link_with': [ test_qemu_driver_lib, test_utils_qemu_monitor_lib ], 'link_whole': [ test_utils_qemu_lib ] },
     { 'name': 'qemumonitorjsontest', 'link_with': [ test_qemu_driver_lib, test_utils_qemu_monitor_lib ], 'link_whole': [ test_utils_qemu_lib ] },
     { 'name': 'qemusecuritytest', 'sources': [ 'qemusecuritytest.c', 'qemusecuritymock.c' ], 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_utils_qemu_lib ] },
+    { 'name': 'qemuvalidatetest', 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_file_wrapper_lib ] },
     { 'name': 'qemuvhostusertest', 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_file_wrapper_lib ] },
     { 'name': 'qemuxml2argvtest', 'link_with': [ test_qemu_driver_lib, test_utils_qemu_monitor_lib ], 'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] },
     { 'name': 'qemuxml2xmltest', 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] },
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;
+
+    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";
+    virDomainDefPtr def = getDefWithUnsupportedTimerPresent();
+    if (qemuValidateDomainDefClockTimers(def, NULL) != -1) {
+        ret = -1;
+    }
+    log = virTestLogContentAndReset();
+    if (!strstr(log, expectedError)) {
+        printf("expected : %s", expectedError);
+        printf("but got : %s", virTestLogContentAndReset());
+        ret = -1;
+    }
+    return ret;
+}
+
+static int
+onlyErrorTimersNotOnX86IfPresent(const void *unused G_GNUC_UNUSED)
+{
+    int ret = 0;
+    virDomainDefPtr def = getDefWithUnsupportedTimerPresent();
+    def->clock.timers[0]->present = 0;
+    if (qemuValidateDomainDefClockTimers(def, NULL) == -1) {
+        ret = -1;
+    }
+    return ret;
+}
+
+static int
+testsuite(void)
+{
+    int ret = 0;
+
+#define DO_TEST(NAME) \
+    if (virTestRun("Command Exec " #NAME " test", \
+                   NAME, NULL) < 0) \
+        ret = -1
+
+    DO_TEST(errorTimersNotOnX86);
+    DO_TEST(onlyErrorTimersNotOnX86IfPresent);
+
+    return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+VIR_TEST_MAIN(testsuite);
-- 
2.26.2

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@redhat.com>
> ---
>  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.

On Tue, Sep 15, 2020 at 22:47:46 +0200, Peter Krempa wrote:
> 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@redhat.com>
> > ---
> >  tests/meson.build        |   1 +
> >  tests/qemuvalidatetest.c | 107 +++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 108 insertions(+)
> >  create mode 100644 tests/qemuvalidatetest.c

[...]

> > +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.

I'd actually see more value if qemuxml2argv test supports validating
error messages so that they are not obscured.

The errors should be recorded in a file though so that
VIR_TEST_REGENERATE_OUTPUT=1 can be used to update them without copying
stuff into code.

Additionally, while validating errors we must make sure that the locale
for the errors is set to the default one so that it doesn't break
randomly on certain hosts.