Re: [Qemu-devel] [PATCH 06/12] qmp: Expose qemu_announce_self as a qmp command

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1519 bytes --]

On 05/24/2017 01:05 PM, Vladislav Yasevich wrote:
> Add a qmp command that can trigger guest announcements.
> 
> Based on work of Germano Veit Michel <germano@redhat.com>
> 
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
>  migration/savevm.c | 14 ++++++++++++++
>  qapi-schema.json   | 19 +++++++++++++++++++
>  2 files changed, 33 insertions(+)
> 

> +void qmp_announce_self(bool has_params, AnnounceParameters *params,
> +                       Error **errp)
> +{
> +    AnnounceParameters announce_params;
> +
> +    memcpy(&announce_params, qemu_get_announce_params(),
> +           sizeof(announce_params));

Shallow copies of a QAPI type happen to work when the type is all scalar
(as AnnounceParameters currently is), but it's more robust to use
QAPI_CLONE() or QAPI_CLONE_MEMBERS() so that any future non-scalar
additions to the parameters type will be correctly copied without
introducing memory leaks or double frees.

Even this might be better:
AnnounceParameters announce_params = { 0 };
qemu_set_announce_parameters(&announce_params, qemu_get_announce_params());

> +
> +    if (has_params)
> +        qemu_set_announce_parameters(&announce_params, params);
> +
> +    qemu_announce_self(&announce_params);
> +}
The QMP changes look okay.  Do you have a testsuite covering the new QMP
command somewhere in the series?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]