On 08/20/2012 07:49 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Add a new qemuMonitorGetVersion() method to support invocation of the 'query-version' JSON monitor command. No HMP equivalent is provided, since this will only be used for QEMU >= 1.2 --- src/qemu/qemu_monitor.c | 24 ++++++++++ src/qemu/qemu_monitor.h | 7 +++ src/qemu/qemu_monitor_json.c | 78 +++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 7 +++ tests/qemumonitorjsontest.c | 102 +++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 218 insertions(+)
+ if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1;
Given this error,
+++ b/src/qemu/qemu_monitor.h @@ -574,6 +574,13 @@ int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon,
int qemuMonitorSystemWakeup(qemuMonitorPtr mon);
+int qemuMonitorGetVersion(qemuMonitorPtr mon, + int *major, + int *minor, + int *micro, + char **package) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
why not ATTRIBUTE_NONNULL(1) as well?
+ if (virJSONValueObjectGetNumberInt(qemu, "micro", micro) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-version reply was missing 'micro' version")); + goto cleanup; + }
query-version has existed since 0.14.0, so we are safe using it if we assume JSON. Hmm, weren't there versions, like 1.1, which lacked micro? /me researches... That's true for the 'qemu -help' output, but it appears that the QMP command has always provided micro, even when it is 0. So this should be safe.
+ if (package) { + const char *tmp; + if (!(tmp = virJSONValueObjectGetString(data, "package"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-version reply was missing 'package' version")); + goto cleanup; + }
Why is it an error if package is non-NULL but package data was not present? Can't we just leave *package=NULL in that case, rather than erroring out? After all, when package is NULL, we don't care whether package data was present.
+ + if (qemuMonitorTestAddItem(test, "query-version", + "{ " + " \"return\":{ " + " \"qemu\":{ " + " \"major\":0, " + " \"minor\":11, " + " \"micro\":6 " + " }," + " \"package\":\"2.283.el6\"" + " }" + "}") < 0)
Shouldn't we test typical values in use by actual qemu? For example, with RHEL, I see something like "package":"(qemu-kvm-0.12.1.2)". Overall, I like the idea. But do we have any code that uses the new monitor command, besides the testsuite? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org