Re: [Qemu-devel] [PATCH v6 11/14] block/backup: support block job transactions

[Eric Blake <eblake@redhat.com>]

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

On 09/15/2015 12:11 AM, Fam Zheng wrote:
> From: Stefan Hajnoczi <stefanha@redhat.com>
> 
> Join the transaction when the 'transactional-cancel' QMP argument is
> true.
> 
> This ensures that the sync bitmap is not thrown away if another block
> job in the transaction is cancelled or fails.  This is critical so
> incremental backup with multiple disks can be retried in case of
> cancellation/failure.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/backup.c            |  25 +++++++--
>  blockdev.c                | 132 +++++++++++++++++++++++++++++++++++-----------
>  include/block/block_int.h |   3 +-
>  qapi-schema.json          |   4 +-
>  qapi/block-core.json      |  29 ++++++++++
>  5 files changed, 155 insertions(+), 38 deletions(-)
> 

> +++ b/qapi/block-core.json
> @@ -736,6 +736,11 @@
>  #                   default 'report' (no limitations, since this applies to
>  #                   a different block device than @device).
>  #
> +# @transactional-cancel: #optional whether failure or cancellation of other
> +#                        block jobs with @transactional-cancel true in the same
> +#                        transaction causes the whole group to cancel.
> +#                        (Since 2.5)
> +#

Spurious hunk here.

> @@ -750,6 +755,18 @@
>              '*on-target-error': 'BlockdevOnError' } }
>  
>  ##
> +# @DriveBackupTxn
> +#
> +# @transactional-cancel: #optional whether failure or cancellation of other
> +#                        block jobs with @transactional-cancel true in the same
> +#                        transaction causes the whole group to cancel.
> +#

Maybe mention default false here...

> +# Since: 2.5
> +##
> +{ 'struct': 'DriveBackupTxn', 'base': 'DriveBackup',
> +  'data': {'*transactional-cancel': 'bool' } }
> +
> +##
>  # @BlockdevBackup
>  #
>  # @device: the name of the device which should be copied.
> @@ -785,6 +802,18 @@
>              '*on-target-error': 'BlockdevOnError' } }
>  
>  ##
> +# @BlockdevBackupTxn
> +#
> +# @transactional-cancel: #optional whether failure or cancellation of other
> +#                        block jobs with @transactional-cancel true in the same
> +#                        transaction causes the whole group to cancel.

and again here.

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

(and we can simplify it later when more qapi patches land)

-- 
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 --]