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

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



On 23.09.2013 18:57, Eric Blake wrote:
> 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 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.

This is kind of tricky; The qemu capabilities (of any version) rely on
configure options passed during the build. Hence, my 1.5.3 differs (in
general) to yours 1.5.3; But I think we can name the files like
caps_1.5.3-1, caps_1.5.3-2 (similar to RHEL package naming).

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

I am not sure. I mean, we would have to count the matching { } to cut
the reply into pieces. For instance, if we get a reply among with an
event on the QMP monitor, it looks like this currently:

{"return": ...}\n
{"timestamp": ...}\n

(I've added '\n' intentionally to stress the line break after each reply)

And here comes the tricky part - we would have to parse json in order to
parse json (I guess bare search for matching { } isn't enough -
consider: {"return": {"blah": "}"}} ). Moreover, if we find that JSON
snippet from qemu is not valid - there should be a restart character
that would reset our JSON parser and translate it to START state so we
can continue reading another reply. Something like we have for qemu-ga.

Michal


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