Re: [PATCH RFC v2 1/5] block: add bitmap-populate job

[Eric Blake]

On 5/13/20 10:49 PM, John Snow wrote:
> This job copies the allocation map into a bitmap. It's a job because
> there's no guarantee that allocation interrogation will be quick (or
> won't hang), so it cannot be retrofit into block-dirty-bitmap-merge.

retrofitted

> It was designed with different possible population patterns in mind,
> but only top layer allocation was implemented for now.
>
> Signed-off-by: John Snow <address@hidden>
> ---

> +++ b/qapi/block-core.json
> @@ -2202,6 +2202,54 @@
>         { 'command': 'block-dirty-bitmap-merge',
>           'data': 'BlockDirtyBitmapMerge' }
>   
> +##
> +# @BitmapPattern:
> +#
> +# An enumeration of possible patterns that can be written into a bitmap.
> +#
> +# @allocation-top: The allocation status of the top layer
> +#                  of the attached storage node.

Presumably, allocated => bits are set.

> +#
> +# Since: 5.0

5.1, now

> +##
> +{ 'enum': 'BitmapPattern',
> +  'data': ['allocation-top'] }
> +
> +##
> +# @BlockDirtyBitmapPopulate:
> +#
> +# @job-id: identifier for the newly-created block job.
> +#
> +# @pattern: What pattern should be written into the bitmap?
> +#
> +# @on-error: the action to take if an error is encountered on a bitmap's
> +#            attached node, default 'report'.
> +#            'stop' and 'enospc' can only be used if the block device supports
> +#            io-status (see BlockInfo).
> +#
> +# @auto-finalize: When false, this job will wait in a PENDING state after it 
> has
> +#                 finished its work, waiting for @block-job-finalize before
> +#                 making any block graph changes.
> +#                 When true, this job will automatically
> +#                 perform its abort or commit actions.
> +#                 Defaults to true.
> +#
> +# @auto-dismiss: When false, this job will wait in a CONCLUDED state after it
> +#                has completely ceased all work, and awaits @block-job-dismiss.
> +#                When true, this job will automatically disappear from the 
> query
> +#                list without user intervention.
> +#                Defaults to true.
> +#
> +# Since: 5.0

5.1

> +##
> +{ 'struct': 'BlockDirtyBitmapPopulate',
> +  'base': 'BlockDirtyBitmap',
> +  'data': { 'job-id': 'str',
> +            'pattern': 'BitmapPattern',
> +            '*on-error': 'BlockdevOnError',
> +            '*auto-finalize': 'bool',
> +            '*auto-dismiss': 'bool' } }
> +
>   ##
>   # @BlockDirtyBitmapSha256:
>   #
> diff --git a/qapi/job.json b/qapi/job.json
> index 5e658281f5..5f496d4630 100644
> --- a/qapi/job.json
> +++ b/qapi/job.json
> @@ -22,7 +22,7 @@
>   # Since: 1.7
>   ##
>   { 'enum': 'JobType',
> -  'data': ['commit', 'stream', 'mirror', 'backup', 'create'] }
> +  'data': ['commit', 'stream', 'mirror', 'backup', 'create', 
> 'bitmap-populate'] }

Missing docs that 'bitmap-populate' is since 5.1.

> +++ b/block/bitmap-alloc.c
> @@ -0,0 +1,207 @@

New file, but no MAINTAINERS update.  It is covered by 'Block layer 
core', but shouldn't it also be covered by 'Dirty Bitmaps'?


> +BlockJob *bitpop_job_create(
> +    const char *job_id,
> +    BlockDriverState *bs,
> +    BdrvDirtyBitmap *target_bitmap,
> +    BitmapPattern pattern,
> +    BlockdevOnError on_error,
> +    int creation_flags,
> +    BlockCompletionFunc *cb,
> +    void *opaque,
> +    JobTxn *txn,
> +    Error **errp)
> +{

> +
> +    /* NB: new bitmap is anonymous and enabled */
> +    cluster_size = bdrv_dirty_bitmap_granularity(target_bitmap);
> +    new_bitmap = bdrv_create_dirty_bitmap(bs, cluster_size, NULL, errp);
> +    if (!new_bitmap) {
> +        return NULL;
> +    }

This means if the guest writes to the disk while the job is ongoing, the 
bitmap will be updated to mark that portion of the bitmap as set, even 
if it was not allocated at the time the job started.  But then again, 
the guest writes are causing allocation, so this seems like the right 
thing to do.

Do we need to worry about the converse case where the job started with 
something allocated but runs in parallel with the guest trimming, such 
that our bitmap marks something as set even though at the conclusion of 
our job it is no longer allocated?

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