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

2020-09-08 Thread Markus Armbruster
Eric Blake  writes:

> From: John Snow 
>
> 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 
> Signed-off-by: Eric Blake 
> ---
>  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:
[...]




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

2020-09-02 Thread Eric Blake
From: John Snow 

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 
Signed-off-by: Eric Blake 
---
 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.
+#
+# 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)
+#
 # Since: 1.7
 ##
 { 'enum': 'JobType',
-  'data': ['commit', 'stream', 'mirror', 'backup', 'create', 'amend'] }
+  'data': ['commit', 'stream', 'mirror', 'backup', 'create', 'amend',
+   'bitmap-populate'] }

 ##
 # @JobStatus:
diff --git a/include/block/block.h b/include/block/block.h
index 6e36154061c1..f4b740857725 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -200,6 +200,7 @@ typedef struct BDRVReopenState {
 typedef enum BlockOpType {
 BLOCK_OP_TYPE_BACKUP_SOURCE,
 BLOCK_OP_TYPE_BACKUP_TARGET,
+BLOCK_OP_TYPE_BITMAP_POPULATE,
 BLOCK_OP_TYPE_CHANGE,
 BLOCK_OP_TYPE_COMMIT_SOURCE,
 BLOCK_OP_TYPE_COMMIT_TARGET,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 9da7a42927eb..c85a79666524 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1257,6 +1257,27 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 BlockCompletionFunc *cb, void *opaque,
 JobTxn *txn, Error **errp);

+/*
+ * bitpop_job_create: Create a new bitmap population job.
+ *
+ * @job_id: The id of the newly-created job.
+ * @bs: Block device associated with the @target_bitmap.
+ * @target_bitmap: The bitmap to populate.
+ * @on_error: What to do if an error on @bs is encountered.
+ * @creation_flags: Flags that control the behavior of the Job lifetime.
+ *  See @BlockJobCreateFlags
+ * @cb: Completion function for the job.
+ * @opaque: Opaque pointer value passed to @cb.
+ * @txn: Transaction that this job is part of (may be NULL).
+ */
+BlockJob *bitpop_job_create(const char *job_id, BlockDriverState *bs,
+BdrvDirtyBitmap *target_bitmap,
+BitmapPattern pattern,
+