On 08/20/2012 07:49 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Add a new qemuMonitorGetMachines() method to support invocation of the 'query-machines' JSON monitor command. No HMP equivalent is required, since this will only be present for QEMU >= 1.2 --- src/qemu/qemu_monitor.c | 30 +++++++++++++ src/qemu/qemu_monitor.h | 15 +++++++ src/qemu/qemu_monitor_json.c | 101 +++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 ++ tests/qemumonitorjsontest.c | 76 ++++++++++++++++++++++++++++++++ 5 files changed, 226 insertions(+)
query-machines is brand new to the upcoming 1.2 release; but since it _did_ make the rc0 candidate release, I have no problem pushing this libvirt patch now. However, without someone actually using the new command, it feels like this series is incomplete.
+int qemuMonitorGetMachines(qemuMonitorPtr mon, + qemuMonitorMachineInfoPtr **machines) +{ + VIR_DEBUG("mon=%p machines=%p", + mon, machines); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + }
Another instance where having ATTRIBUTE_NONNULL(1) might make more sense.
+++ b/tests/qemumonitorjsontest.c @@ -224,6 +224,81 @@ cleanup: }
static int +testQemuMonitorJSONGetMachines(const void *data) +{ + virCapsPtr caps = (virCapsPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNew(true, caps); + int ret = -1; + qemuMonitorMachineInfoPtr *info; + int ninfo; + const char *null = NULL;
Why did you need this?
+ if (STRNEQ_NULLABLE(info[i]->alias, (wantalias))) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "alias %s is not %s", \ + info[i]->alias, NULLSTR(wantalias)); \ + goto cleanup; \ + } \ + } while (0) + + CHECK(0, "pc-1.0", false, null);
Can't you just s/null/NULL/ and avoid the intermediate variable? Other than that, looks reasonable. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org