On 19.06.2013, at 23:43, Anthony Liguori wrote:
> Alexander Graf <ag...@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 <aligu...@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 <aligu...@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.
Ok, works for me :). There really should be a more clever logic here
eventually. Maybe you could construct a QMP call object in several steps until
you send it out, to reflect the tree structure of JSON.
>
>>> +
>>> + 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.
I only realized that later in the series. I think I got confused on this one
when it got introduced already. What a terrific interface ;).
Alex
