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

[Anthony Liguori <aliguori@us.ibm.com>]

Alexander Graf <agraf@suse.de> writes:

> 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 <aliguori@us.ibm.com>
>> ---
>> 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   <aliguori@us.ibm.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 "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", "");

Yeah, it's not that simple.  We need to pull in the JSON parsing code
etc too.

I'm going to look into that but I think it's outside the scope of this series.

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

qmp support is pretty new in qtest and not quite baked yet.  My
expectation is that this function will disappear over time.

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

Yes, you are correct re: hcall args but it's not a breakage.

VTY is weird.  The characters are packed into the arguments.  So sending
two chars would look like:

spapr_hcall3(H_PUT_TERM_CHAR, 0, 2,
            (uint64_t)greeting[0] << 56 |
            (uint64_t)greeting[1] << 48);

It can take an additional argument too for the next 8 bytes.

So even within the Linux kernel or SLOF on big endian, the code would
look like this too.

Regards,

Anthony Liguori

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