Re: [Qemu-devel] [PATCH v6 11/16] block: simplify drive-backup

[Eric Blake <eblake@redhat.com>]

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

On 07/21/2016 01:40 PM, Denis V. Lunev wrote:
> From: Pavel Butsykin <pbutsykin@virtuozzo.com>
> 
> Now that we can support boxed commands, use it to greatly reduce the
> number of parameters (and likelihood of getting out of sync) when
> adjusting drive-backup parameters..

s/.././

> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Jeff Cody <jcody@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> ---
>  blockdev.c           | 115 +++++++++++++++++----------------------------------
>  hmp.c                |  29 ++++++++-----
>  qapi/block-core.json |   3 +-
>  3 files changed, 58 insertions(+), 89 deletions(-)
> 

> +++ b/hmp.c
> @@ -1109,8 +1109,24 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict)
>      const char *format = qdict_get_try_str(qdict, "format");
>      bool reuse = qdict_get_try_bool(qdict, "reuse", false);
>      bool full = qdict_get_try_bool(qdict, "full", false);
> -    enum NewImageMode mode;
>      Error *err = NULL;
> +    DriveBackup backup = {
> +        .device = (char *)device,
> +        .target = (char *)filename,
> +        .has_format = !!format,
> +        .format = (char *)format,
> +        .sync = full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
> +        .has_mode = true,
> +        .mode = reuse ? NEW_IMAGE_MODE_EXISTING : NEW_IMAGE_MODE_ABSOLUTE_PATHS,

Good.

> +        .has_speed = false,
> +        .speed = 0,
> +        .has_bitmap = false,
> +        .bitmap = NULL,
> +        .has_on_source_error = false,
> +        .on_source_error = 0,
> +        .has_on_target_error = false,
> +        .on_target_error = 0,

I'd drop these. C guarantees that they'll be zero-initialized, and
unless HMP has a way to set them to non-default values, it's not worth
wasting the lines of code.

With those fixups,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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