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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---
>  .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

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]