Re: [Qemu-devel] [PATCH 1/2] blockdev-backup: add bitmap argument

[John Snow]

On 08/30/2018 05:16 PM, John Snow wrote:
> It is only an oversight that we don't allow incremental backup with
> blockdev-backup. Add the bitmap argument which enables this.
> 
> Signed-off-by: John Snow <address@hidden>
> ---
>  blockdev.c           | 16 +++++++++++++++-
>  qapi/block-core.json |  7 ++++++-
>  2 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 72f5347df5..b9f19afacc 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3491,6 +3491,7 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, 
> JobTxn *txn,
>      BlockDriverState *bs;
>      BlockDriverState *target_bs;
>      Error *local_err = NULL;
> +    BdrvDirtyBitmap *bmap = NULL;
>      AioContext *aio_context;
>      BlockJob *job = NULL;
>      int job_flags = JOB_DEFAULT;
> @@ -3541,6 +3542,19 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, 
> JobTxn *txn,
>              goto out;
>          }
>      }
> +    if (backup->has_bitmap) {
> +        bmap = bdrv_find_dirty_bitmap(bs, backup->bitmap);
> +        if (!bmap) {
> +            error_setg(errp, "Bitmap '%s' could not be found", 
> backup->bitmap);
> +            goto out;
> +        }
> +        if (bdrv_dirty_bitmap_qmp_locked(bmap)) {
> +            error_setg(errp,
> +                       "Bitmap '%s' is currently locked and cannot be used 
> for "
> +                       "backup", backup->bitmap);
> +            goto out;
> +        }
> +    }
>      if (!backup->auto_finalize) {
>          job_flags |= JOB_MANUAL_FINALIZE;
>      }
> @@ -3548,7 +3562,7 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, 
> JobTxn *txn,
>          job_flags |= JOB_MANUAL_DISMISS;
>      }
>      job = backup_job_create(backup->job_id, bs, target_bs, backup->speed,
> -                            backup->sync, NULL, backup->compress,
> +                            backup->sync, bmap, backup->compress,
>                              backup->on_source_error, backup->on_target_error,
>                              job_flags, NULL, NULL, txn, &local_err);
>      if (local_err != NULL) {
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 4c7a37afdc..d3ea281039 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1315,6 +1315,10 @@
>  # @speed: the maximum speed, in bytes per second. The default is 0,
>  #         for unlimited.
>  #
> +# @bitmap: the name of dirty bitmap if sync is "incremental".
> +#          Must be present if sync is "incremental", must NOT be present
> +#          otherwise. (Since 3.1)
> +#
>  # @compress: true to compress data, if the target format supports it.
>  #            (default: false) (since 2.8)
>  #
> @@ -1346,7 +1350,8 @@
>  ##
>  { 'struct': 'BlockdevBackup',
>    'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
> -            'sync': 'MirrorSyncMode', '*speed': 'int', '*compress': 'bool',
> +            'sync': 'MirrorSyncMode', '*speed': 'int',
> +            '*bitmap': 'str', '*compress': 'bool',
>              '*on-source-error': 'BlockdevOnError',
>              '*on-target-error': 'BlockdevOnError',
>              '*auto-finalize': 'bool', '*auto-dismiss': 'bool' } }
> 

I'll be staging this patch (and not the hack that follows it), if there
are no objections.

Thanks, applied to my bitmaps tree:

https://github.com/jnsnow/qemu/commits/bitmaps
https://github.com/jnsnow/qemu.git

--js