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

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> From: John Snow <jsnow@redhat.com>
>
> 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 retrofitted into block-dirty-bitmap-merge.
>
> It was designed with different possible population patterns in mind,
> but only top layer allocation was implemented for now.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  qapi/block-core.json      |  48 +++++++++
>  qapi/job.json             |   6 +-
>  include/block/block.h     |   1 +
>  include/block/block_int.h |  21 ++++
>  block/bitmap-populate.c   | 207 ++++++++++++++++++++++++++++++++++++++
>  blockjob.c                |   3 +-
>  MAINTAINERS               |   1 +
>  block/meson.build         |   1 +
>  8 files changed, 286 insertions(+), 2 deletions(-)
>  create mode 100644 block/bitmap-populate.c
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index db08c58d788c..1cac9a9a8207 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2216,6 +2216,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.

Greek to me.

> +#
> +# Since: 5.2
> +##
> +{ '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.2
> +##
> +{ '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 280c2f76f136..fb0b606e868d 100644
> --- a/qapi/job.json
> +++ b/qapi/job.json
> @@ -22,10 +22,14 @@
>  #
>  # @amend: image options amend job type, see "x-blockdev-amend" (since 5.1)
>  #
> +# @bitmap-populate: drive bitmap population job type, see
> +#                   "block-dirty-bitmap-populate" (since 5.2)

Dangling reference, healed in PATCH 3.  Job appearing before the command
it wraps is unusual.

What's a "drive bitmap population job"?

The reference provides a clue:

    ##
    # @block-dirty-bitmap-populate:
    #
    # Creates a new job that writes a pattern into a dirty bitmap.

Do you mean "dirty bitmap population job"?

By the way, I think the doc comment would be easier to read with
s/job type/job/.

> +#
>  # Since: 1.7
>  ##
>  { 'enum': 'JobType',
> -  'data': ['commit', 'stream', 'mirror', 'backup', 'create', 'amend'] }
> +  'data': ['commit', 'stream', 'mirror', 'backup', 'create', 'amend',
> +           'bitmap-populate'] }
>
>  ##
>  # @JobStatus:
[...]