[libvirt] [PATCH 5/5] Add a qemuMonitorGetMachines() method for query-machines command

[Eric Blake]

On 08/20/2012 07:49 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at 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 at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120820/df887eb8/attachment-0001.sig>