On 08/20/2012 07:49 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> 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@xxxxxxxxxx +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