Re: [Qemu-ppc] [PATCH 05/12] spapr-vty: add qtest test case

[Alexander Graf]

On 19.06.2013, at 22:40, Anthony Liguori wrote:

> Pretty basic for the moment but the interface is pretty simple.
> 
> Signed-off-by: Anthony Liguori <address@hidden>
> ---
> tests/Makefile         |  3 ++
> tests/spapr-vty-test.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 92 insertions(+)
> create mode 100644 tests/spapr-vty-test.c
> 
> diff --git a/tests/Makefile b/tests/Makefile
> index c107489..7c0ce1a 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -67,6 +67,8 @@ gcov-files-sparc64-y += hw/m48t59.c
> check-qtest-arm-y = tests/tmp105-test$(EXESUF)
> gcov-files-arm-y += hw/tmp105.c
> 
> +check-qtest-ppc64-y = tests/spapr-vty-test$(EXESUF)
> +
> GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h 
> tests/test-qmp-commands.h
> 
> test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
> @@ -133,6 +135,7 @@ tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o
> tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
> tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
> tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y)
> +tests/spapr-vty-test$(EXESUF): tests/spapr-vty-test.o
> 
> # QTest rules
> 
> diff --git a/tests/spapr-vty-test.c b/tests/spapr-vty-test.c
> new file mode 100644
> index 0000000..8e3908b
> --- /dev/null
> +++ b/tests/spapr-vty-test.c
> @@ -0,0 +1,89 @@
> +/*
> + * QTest testcase for the sPAPR VTY
> + *
> + * Copyright IBM, Corp. 2013
> + *
> + * Authors:
> + *  Anthony Liguori   <address@hidden>
> + *
> + * 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 "libqtest.h"
> +
> +#include <glib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +
> +#define H_GET_TERM_CHAR         0x54
> +#define H_PUT_TERM_CHAR         0x58
> +
> +static char *qmp_ringbuf_read(const char *device, int max_len)
> +{
> +    size_t len;
> +    char *ret;
> +    char *ptr;
> +
> +    ret = qtest_qmp(global_qtest,
> +                    "{ 'execute': 'ringbuf-read', "
> +                    "  'arguments': { 'device': '%s', 'size': %d } }",
> +                    device, max_len);
> +    len = strlen(ret);
> +
> +    /* Skip pass {"return" */
> +    ptr = strchr(ret, '"');
> +    g_assert(ptr != NULL);
> +    ptr = strchr(ptr + 1, '"');
> +    g_assert(ptr != NULL);
> +
> +    /* Start of data */
> +    ptr = strchr(ptr + 1, '"');
> +    g_assert(ptr != NULL);
> +    ptr += 1;
> +
> +    len -= ptr - ret;
> +    memmove(ret, ptr, len);
> +    ret[len] = 0;
> +
> +    ptr = strrchr(ret, '"');
> +    g_assert(ptr != NULL);
> +    *ptr = 0;

Can't this be moved to a more generic function? Something along the lines of

  char *qmp_execute(const char *device, int max_len, const char *func, const 
char *args)

and then call it like:

  return qmp_execute(device, max_len, "ringbuf-read", "");

here? Or let the caller do the argument gathering completely. But a function 
like surely occurs in other places too, no?

> +
> +    return ret;
> +}
> +
> +static void vty_ping(void)
> +{
> +    const char *greeting = "Hello, world!";
> +    char *data;
> +    int i;
> +
> +    for (i = 0; greeting[i]; i++) {
> +        spapr_hcall3(H_PUT_TERM_CHAR, 0, 1, (uint64_t)greeting[i] << 56);

This looks like endianness breakage? Hypercall arguments should go in native 
endianness. Registers don't have endianness. Only memory accesses do.


Alex

> +    }
> +
> +    save_restore();
> +
> +    data = qmp_ringbuf_read("ring0", 16);
> +    g_assert_cmpstr(data, ==, greeting);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    int ret;
> +
> +    g_test_init(&argc, &argv, NULL);
> +
> +    qtest_start("-display none -serial chardev:ring0 "
> +                "-machine pseries -chardev memory,id=ring0,save=on");
> +
> +    qtest_add_func("/vty/ping", vty_ping);
> +    ret = g_test_run();
> +
> +    qtest_quit(global_qtest);
> +
> +    return ret;
> +}
> -- 
> 1.8.0
>