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(a)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