[libvirt] [PATCH 4/4] tests: Introduce qemucapabilitiestest

[Eric Blake]

On 09/23/2013 06:58 AM, Michal Privoznik wrote:
> This test is there to ensure that our capabilities detection code isn't
> broken somehow.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  .gitignore                                      |    1 +
>  tests/Makefile.am                               |   12 +-
>  tests/qemucapabilitiesdata/caps_default.caps    |  132 ++
>  tests/qemucapabilitiesdata/caps_default.replies | 2519 +++++++++++++++++++++++

Big, but it has to be.  Can you please document in the commit message
how you captured the contents of this file, so that next time we add a
file for another version, we can do the same sort of capturing?  Also,
does the file name need to be named after a particular qemu version?

> +++ b/tests/qemucapabilitiesdata/caps_default.replies
> @@ -0,0 +1,2519 @@
> +{
> +    "QMP": {
> +        "version": {
> +            "qemu": {
> +                "micro": 3,
> +                "minor": 5,
> +                "major": 1

So this is based on qemu 1.5.3; notice how we have named files under
tests/qemuhelpdata to make it obvious which version of qemu we captured
data from.

> +++ b/tests/qemucapabilitiestest.c

> +
> +static qemuMonitorTestPtr
> +testQemuFeedMonitor(char *replies,
> +                    virDomainXMLOptionPtr xmlopt)
> +{
> +    qemuMonitorTestPtr test = NULL;
> +    char *token, *saveptr = NULL;
> +    char *tmp = replies;
> +
> +    /* Our JSON parser is stupid. It doesn't ignore whitespaces even though it
> +     * should. Hence, a reply needs to be on a single line. However, the
> +     * replies in the file are separated by a single empty line. So we must
> +     * preprocess the file prior splitting it into set of replies. */

Yuck.  We should probably fix that as a pre-requisite patch.  There's no
guarantee that future qemu will always output single-line JSON objects.

> +    while ((tmp = strchr(tmp, '\n'))) {
> +        if (*(tmp + 1) != '\n') {
> +            *tmp = ' ';
> +            tmp++;
> +        } else {
> +            tmp += 2;
> +        }
> +    }

Why are you tokenizing the file yourself...

> +
> +    token = strtok_r(replies, "\n", &saveptr);
> +    while (token) {

...only to repeat the tokenization via strtok_r?  Since you already know
that your file consists of entries separated by two newlines, why not
turn the second \n into NUL and add the items yourself on your first
pass through?  Besides, if you fix the JSON parser to allow multiline
input, then strtok_r won't help you find the doubled newlines very
efficiently.

Overall, I like where this is headed.  Looking forward to v2 and to
subsequent captures from more than just qemu 1.5.3.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130923/a6c5731d/attachment-0001.sig>