diff mbox series

[v3,1/6] block: add bitmap-populate job

Message ID 20200619195621.58740-2-eblake@redhat.com (mailing list archive)
State New, archived
Headers show
Series block: add block-dirty-bitmap-populate job | expand

Commit Message

Eric Blake June 19, 2020, 7:56 p.m. UTC
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_int.h |  21 ++++
 block/bitmap-populate.c   | 207 ++++++++++++++++++++++++++++++++++++++
 blockjob.c                |   3 +-
 MAINTAINERS               |   1 +
 block/Makefile.objs       |   1 +
 7 files changed, 285 insertions(+), 2 deletions(-)
 create mode 100644 block/bitmap-populate.c

Comments

Vladimir Sementsov-Ogievskiy June 20, 2020, 4:16 a.m. UTC | #1
19.06.2020 22:56, Eric Blake wrote:
> 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_int.h |  21 ++++
>   block/bitmap-populate.c   | 207 ++++++++++++++++++++++++++++++++++++++
>   blockjob.c                |   3 +-
>   MAINTAINERS               |   1 +
>   block/Makefile.objs       |   1 +
>   7 files changed, 285 insertions(+), 2 deletions(-)
>   create mode 100644 block/bitmap-populate.c
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 0e1c6a59f228..a1bcdba04423 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2211,6 +2211,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.1
> +##
> +{ '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.1
> +##
> +{ 'struct': 'BlockDirtyBitmapPopulate',
> +  'base': 'BlockDirtyBitmap',
> +  'data': { 'job-id': 'str',
> +            'pattern': 'BitmapPattern',
> +            '*on-error': 'BlockdevOnError',
> +            '*auto-finalize': 'bool',
> +            '*auto-dismiss': 'bool' } }
> +

Peter said about a possibility of populating several target bitmaps simultaneously.

What about such a generalized semantics:

Merge all sources to each target

@targets: list of bitmaps to be populated by the job
{ 'struct': 'BlockDirtyBitmapPopulate',
   'data': { <common job fields>,
             'targets': ['BlockDirtyBitmap'],
             'sources': ['BitmapPopulateSource'] } }


@bitmap: specify dirty bitmap to be merged to target bitamp(s)
@node: specify a node name, which allocation-map is to be merged to target bitmap(s)
{ 'alternate': 'BitmapPopulateSource',
   'data': { 'bitmap': 'BlockDirtyBitmap',
             'node': 'str' } }


- so, we can merge several bitmaps together with several allocation maps into several target bitmaps.
(I remember, we also said about a possibility of starting several populating jobs, populating into
  same bitmap, I think it may be substituted by one job with several sources. Still, it's not hard to
  allow to use target bitmaps in a several jobs simultaneously and this is not about the QAPI interface)

Will this simplify things in libvirt?
Eric Blake June 22, 2020, 9:44 p.m. UTC | #2
On 6/19/20 11:16 PM, Vladimir Sementsov-Ogievskiy wrote:
> 19.06.2020 22:56, Eric Blake wrote:
>> 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>
>> ---

>> +{ 'struct': 'BlockDirtyBitmapPopulate',
>> +  'base': 'BlockDirtyBitmap',
>> +  'data': { 'job-id': 'str',
>> +            'pattern': 'BitmapPattern',
>> +            '*on-error': 'BlockdevOnError',
>> +            '*auto-finalize': 'bool',
>> +            '*auto-dismiss': 'bool' } }
>> +
> 
> Peter said about a possibility of populating several target bitmaps 
> simultaneously.
> 
> What about such a generalized semantics:
> 
> Merge all sources to each target
> 
> @targets: list of bitmaps to be populated by the job
> { 'struct': 'BlockDirtyBitmapPopulate',
>    'data': { <common job fields>,
>              'targets': ['BlockDirtyBitmap'],
>              'sources': ['BitmapPopulateSource'] } }

We still need the 'pattern' argument (the idea being that if we have: 
Base <- Active, we want to be able to merge in the allocation map of 
Active into bitmaps stored in Base as part of a commit operation, 
whether that is active commit of a live guest or offline commit while 
the guest is offline).  Having an array for 'targets' to merge into is 
fine, but for 'sources', it's less a concern about selecting from 
multiple sources, and more a concern about selecting the allocation 
pattern to be merged in (libvirt wants to merge the same allocation 
pattern into each bitmap in Base).  Generalizing things to allow the 
merge of more than one source at once might not hurt, but I'm not sure 
we need it yet.

But there are other patterns that we may want to support: an all-ones 
pattern, or maybe a pattern that tracks known-zeros instead of allocation.

> 
> 
> @bitmap: specify dirty bitmap to be merged to target bitamp(s)
> @node: specify a node name, which allocation-map is to be merged to 
> target bitmap(s)
> { 'alternate': 'BitmapPopulateSource',
>    'data': { 'bitmap': 'BlockDirtyBitmap',
>              'node': 'str' } }

This design is clever in that it lets us merge in both existing bitmaps 
and using a node-name for merging in an allocation map instead of a 
bitmap; but it limits us to only one pattern.  Better might be something 
where we supply a union (hmm, we've had proposals in the past for a 
default value to the discriminator to allow it to be optional, so I'll 
proceed as if we will finally implement that):

{ 'enum': 'BitmapPattern', 'data': [ 'bitmap', 'allocation-top' ] }
{ 'union': 'BitmapPopulateSource',
   'base': { '*pattern': 'BitmapPattern' },
   'discriminator': { 'name': 'pattern', 'default': 'bitmap' },
   'data': { 'bitmap': 'BitmapPopulateSource',
             'allocation-top': { 'node': 'str' } } }

so that you can then do:

{ "execute": "block-dirty-bitmap-populate",
   "arguments": { "targets": [ { "node": "base", "name": "b1" },
                               { "node": "base", "name": "b2" } ],
         "sources": [ { "pattern": "allocation-top", "node": "top" } ]
   } }

to merge in the allocation information of top into multiple bitmaps of 
base at once, or conversely, do:

{ "execute": "block-dirty-bitmap-populate",
   "arguments": { "targets": [ { "node": "base", "name": "b1" } ],
         "sources": [ { "pattern": "bitmap",
                        "node": "top", "name": "b1" } ]
   } }
{ "execute": "block-dirty-bitmap-populate",
   "arguments": { "targets": [ { "node": "base", "name": "b2" } ],
         "sources": [ { "node": "top", "name": "b2" } ]
   } }

and of course, wrap this in a "transaction" to ensure that it all 
succeeds or fails as a unit, rather than messing up one bitmap if 
another fails, while also allowing future extension for additional patterns.

> 
> 
> - so, we can merge several bitmaps together with several allocation maps 
> into several target bitmaps.
> (I remember, we also said about a possibility of starting several 
> populating jobs, populating into
>   same bitmap, I think it may be substituted by one job with several 
> sources. Still, it's not hard to
>   allow to use target bitmaps in a several jobs simultaneously and this 
> is not about the QAPI interface)
> 
> Will this simplify things in libvirt?

Peter, in your preliminary experiments with block-dirty-bitmap-populate, 
did you ever need to start more than one job to a single bitmap 
destination, or was it merely starting multiple jobs because you had 
multiple destinations but always just a single source?
Vladimir Sementsov-Ogievskiy June 23, 2020, 7:01 a.m. UTC | #3
23.06.2020 00:44, Eric Blake wrote:
> On 6/19/20 11:16 PM, Vladimir Sementsov-Ogievskiy wrote:
>> 19.06.2020 22:56, Eric Blake wrote:
>>> 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>
>>> ---
> 
>>> +{ 'struct': 'BlockDirtyBitmapPopulate',
>>> +  'base': 'BlockDirtyBitmap',
>>> +  'data': { 'job-id': 'str',
>>> +            'pattern': 'BitmapPattern',
>>> +            '*on-error': 'BlockdevOnError',
>>> +            '*auto-finalize': 'bool',
>>> +            '*auto-dismiss': 'bool' } }
>>> +
>>
>> Peter said about a possibility of populating several target bitmaps simultaneously.
>>
>> What about such a generalized semantics:
>>
>> Merge all sources to each target
>>
>> @targets: list of bitmaps to be populated by the job
>> { 'struct': 'BlockDirtyBitmapPopulate',
>>    'data': { <common job fields>,
>>              'targets': ['BlockDirtyBitmap'],
>>              'sources': ['BitmapPopulateSource'] } }
> 
> We still need the 'pattern' argument (the idea being that if we have: Base <- Active, we want to be able to merge in the allocation map of Active into bitmaps stored in Base as part of a commit operation, whether that is active commit of a live guest or offline commit while the guest is offline).  Having an array for 'targets' to merge into is fine, but for 'sources', it's less a concern about selecting from multiple sources, and more a concern about selecting the allocation pattern to be merged in (libvirt wants to merge the same allocation pattern into each bitmap in Base).  Generalizing things to allow the merge of more than one source at once might not hurt, but I'm not sure we need it yet.
> 
> But there are other patterns that we may want to support: an all-ones pattern, or maybe a pattern that tracks known-zeros instead of allocation.
> 
>>
>>
>> @bitmap: specify dirty bitmap to be merged to target bitamp(s)
>> @node: specify a node name, which allocation-map is to be merged to target bitmap(s)
>> { 'alternate': 'BitmapPopulateSource',
>>    'data': { 'bitmap': 'BlockDirtyBitmap',
>>              'node': 'str' } }
> 
> This design is clever in that it lets us merge in both existing bitmaps and using a node-name for merging in an allocation map instead of a bitmap; but it limits us to only one pattern.

Ah, yes, we can't discriminate by type node-name from 'all-ones' or something like this.

>Better might be something where we supply a union (hmm, we've had proposals in the past for a default value to the discriminator to allow it to be optional, so I'll proceed as if we will finally implement that):
> 
> { 'enum': 'BitmapPattern', 'data': [ 'bitmap', 'allocation-top' ] }
> { 'union': 'BitmapPopulateSource',
>    'base': { '*pattern': 'BitmapPattern' },
>    'discriminator': { 'name': 'pattern', 'default': 'bitmap' },
>    'data': { 'bitmap': 'BitmapPopulateSource',
>              'allocation-top': { 'node': 'str' } } }

Yes, this is better, of course.

> 
> so that you can then do:
> 
> { "execute": "block-dirty-bitmap-populate",
>    "arguments": { "targets": [ { "node": "base", "name": "b1" },
>                                { "node": "base", "name": "b2" } ],
>          "sources": [ { "pattern": "allocation-top", "node": "top" } ]
>    } }
> 
> to merge in the allocation information of top into multiple bitmaps of base at once, or conversely, do:
> 
> { "execute": "block-dirty-bitmap-populate",
>    "arguments": { "targets": [ { "node": "base", "name": "b1" } ],
>          "sources": [ { "pattern": "bitmap",
>                         "node": "top", "name": "b1" } ]
>    } }
> { "execute": "block-dirty-bitmap-populate",
>    "arguments": { "targets": [ { "node": "base", "name": "b2" } ],
>          "sources": [ { "node": "top", "name": "b2" } ]
>    } }
> 
> and of course, wrap this in a "transaction" to ensure that it all succeeds or fails as a unit, rather than messing up one bitmap if another fails, while also allowing future extension for additional patterns.
> 
>>
>>
>> - so, we can merge several bitmaps together with several allocation maps into several target bitmaps.
>> (I remember, we also said about a possibility of starting several populating jobs, populating into
>>   same bitmap, I think it may be substituted by one job with several sources. Still, it's not hard to
>>   allow to use target bitmaps in a several jobs simultaneously and this is not about the QAPI interface)
>>
>> Will this simplify things in libvirt?
> 
> Peter, in your preliminary experiments with block-dirty-bitmap-populate, did you ever need to start more than one job to a single bitmap destination, or was it merely starting multiple jobs because you had multiple destinations but always just a single source?
> 
>
Kevin Wolf June 23, 2020, 11:50 a.m. UTC | #4
Am 19.06.2020 um 21:56 hat Eric Blake geschrieben:
> 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>

> +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)
> +{
> +    int64_t len;
> +    BitpopBlockJob *job = NULL;
> +    int64_t cluster_size;
> +    BdrvDirtyBitmap *new_bitmap = NULL;
> +
> +    assert(bs);
> +    assert(target_bitmap);
> +
> +    if (!bdrv_is_inserted(bs)) {
> +        error_setg(errp, "Device is not inserted: %s",
> +                   bdrv_get_device_name(bs));
> +        return NULL;
> +    }
> +
> +    if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
> +        return NULL;
> +    }

So did we decide that we'll keep the legacy op blocker with the type of
another block job even though nobody could tell what it's good for?

Kevin
Eric Blake Sept. 2, 2020, 3:58 p.m. UTC | #5
[reviving an old thread]

On 6/22/20 4:44 PM, Eric Blake wrote:
> On 6/19/20 11:16 PM, Vladimir Sementsov-Ogievskiy wrote:
>> 19.06.2020 22:56, Eric Blake wrote:
>>> 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>
>>> ---
> 
>>> +{ 'struct': 'BlockDirtyBitmapPopulate',
>>> +  'base': 'BlockDirtyBitmap',
>>> +  'data': { 'job-id': 'str',
>>> +            'pattern': 'BitmapPattern',

As written, "pattern":"allocate-top" is rather limited - it can only 
grab allocation from the top node.  Being able to grab the allocation 
from a specific node may indeed be more useful.  Another bitmap patterns 
that might be useful would be an all-one pattern (create a bitmap that 
treats the entire disk as dirty).  I also remember brainstorming with 
John the question of whether we want bitmap-populate to have different 
mask modes: does the population perform an overwrite (the bitmap now 
matches the source pattern exactly, even if some bits were set and 
others cleared), a union merge (any bits already set in the bitmap 
remain set, additional bits are set according to pattern), or even a 
difference (any bits already cleared in the bitmap remain clear, while 
bits in the pattern can also clear additional bits in the bitmap).

If I understand Peter's goals, the initial libvirt use is a union mode 
(keep bits in the bitmap that are already set, but set additional bits 
according to the population pattern).

>>> +            '*on-error': 'BlockdevOnError',
>>> +            '*auto-finalize': 'bool',
>>> +            '*auto-dismiss': 'bool' } }
>>> +
>>
>> Peter said about a possibility of populating several target bitmaps 
>> simultaneously.
>>
>> What about such a generalized semantics:
>>
>> Merge all sources to each target
>>
>> @targets: list of bitmaps to be populated by the job
>> { 'struct': 'BlockDirtyBitmapPopulate',
>>    'data': { <common job fields>,
>>              'targets': ['BlockDirtyBitmap'],
>>              'sources': ['BitmapPopulateSource'] } }
> 
> We still need the 'pattern' argument (the idea being that if we have: 
> Base <- Active, we want to be able to merge in the allocation map of 
> Active into bitmaps stored in Base as part of a commit operation, 
> whether that is active commit of a live guest or offline commit while 
> the guest is offline).  Having an array for 'targets' to merge into is 
> fine, but for 'sources', it's less a concern about selecting from 
> multiple sources, and more a concern about selecting the allocation 
> pattern to be merged in (libvirt wants to merge the same allocation 
> pattern into each bitmap in Base).  Generalizing things to allow the 
> merge of more than one source at once might not hurt, but I'm not sure 
> we need it yet.

But when it comes to multiple destinations or multiple sources, while it 
seems like it might be a convenience factor, I also worry that it is 
over-engineering.  See more below...

> 
> But there are other patterns that we may want to support: an all-ones 
> pattern, or maybe a pattern that tracks known-zeros instead of allocation.
> 
>>
>>
>> @bitmap: specify dirty bitmap to be merged to target bitamp(s)
>> @node: specify a node name, which allocation-map is to be merged to 
>> target bitmap(s)
>> { 'alternate': 'BitmapPopulateSource',
>>    'data': { 'bitmap': 'BlockDirtyBitmap',
>>              'node': 'str' } }
> 
> This design is clever in that it lets us merge in both existing bitmaps 
> and using a node-name for merging in an allocation map instead of a 
> bitmap; but it limits us to only one pattern.  Better might be something 
> where we supply a union (hmm, we've had proposals in the past for a 
> default value to the discriminator to allow it to be optional, so I'll 
> proceed as if we will finally implement that):
> 
> { 'enum': 'BitmapPattern', 'data': [ 'bitmap', 'allocation-top' ] }
> { 'union': 'BitmapPopulateSource',
>    'base': { '*pattern': 'BitmapPattern' },
>    'discriminator': { 'name': 'pattern', 'default': 'bitmap' },
>    'data': { 'bitmap': 'BitmapPopulateSource',
>              'allocation-top': { 'node': 'str' } } }
> 
> so that you can then do:
> 
> { "execute": "block-dirty-bitmap-populate",
>    "arguments": { "targets": [ { "node": "base", "name": "b1" },
>                                { "node": "base", "name": "b2" } ],
>          "sources": [ { "pattern": "allocation-top", "node": "top" } ]
>    } }
> 
> to merge in the allocation information of top into multiple bitmaps of 
> base at once,

Hmm, I left out the mandatory "job-id" parameter here; one of the key 
points of the new command is that some patterns (like allocation) may 
involve potentially lengthy I/O, so we need a job-id (the existing 
block-dirty-bitmap-merge does not).  But since the existing 
block-dirty-bitmap-merge supports multiple sources to one destination, 
supporting multiple patterns to one destination tracked by a single job 
id does have some appeal.

> or conversely, do:
> 
> { "execute": "block-dirty-bitmap-populate",
>    "arguments": { "targets": [ { "node": "base", "name": "b1" } ],
>          "sources": [ { "pattern": "bitmap",
>                         "node": "top", "name": "b1" } ]
>    } }
> { "execute": "block-dirty-bitmap-populate",
>    "arguments": { "targets": [ { "node": "base", "name": "b2" } ],
>          "sources": [ { "node": "top", "name": "b2" } ]
>    } }
> 
> and of course, wrap this in a "transaction" to ensure that it all 
> succeeds or fails as a unit, rather than messing up one bitmap if 
> another fails, while also allowing future extension for additional 
> patterns.

We already have transactions that let us perform multiple destinations 
as a group.  So what is the difference in the end results between 
merging one source into two separate destinations in one command spelled 
this way:

# proposal with many:many bitmap populate
{ "execute": "block-dirty-bitmap-populate",
   "arguments": { "job-id": "job0",
                  "targets": [ { "node": "base", "name": "b1" },
                               { "node": "base", "name": "b2" } ],
                  "source": { "pattern": "allocation", "node": "top" } } }
wait for job to complete

vs. spelled this way:

# patch as written with 1:1 bitmap-populate, but tweak to source
{ "execute": "block-dirty-bitmap-add",
   "arguments": { "node": "top", "name": "tmp" } }
{ "execute": "block-dirty-bitmap-populate",
   "arguments": { "job-id": "job0",
                  "node": "top", "name": "tmp",
                  "source": { "pattern": "allocation", "node": "top" } } }
wait for job to complete
{ "execute": "transaction",
   "arguments": { "actions": [
     { "type": "block-dirty-bitmap-merge", "node": "base", "name": "b1",
         "bitmaps": [ { "node": "top", "name": "tmp" } ] },
     { "type": "block-dirty-bitmap-merge", "node": "base", "name": "b2",
         "bitmaps": [ { "node": "top", "name": "tmp" } ] }
] } }



> 
>>
>>
>> - so, we can merge several bitmaps together with several allocation 
>> maps into several target bitmaps.
>> (I remember, we also said about a possibility of starting several 
>> populating jobs, populating into
>>   same bitmap, I think it may be substituted by one job with several 
>> sources. Still, it's not hard to
>>   allow to use target bitmaps in a several jobs simultaneously and 
>> this is not about the QAPI interface)
>>
>> Will this simplify things in libvirt?
> 
> Peter, in your preliminary experiments with block-dirty-bitmap-populate, 
> did you ever need to start more than one job to a single bitmap 
> destination, or was it merely starting multiple jobs because you had 
> multiple destinations but always just a single source?

I guess I'm struggling in posting a v4 in part because I don't have a 
good answer to what is easiest for Peter to use.
diff mbox series

Patch

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0e1c6a59f228..a1bcdba04423 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2211,6 +2211,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.1
+##
+{ '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.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 5e658281f5c4..33ff3500f794 100644
--- a/qapi/job.json
+++ b/qapi/job.json
@@ -19,10 +19,14 @@ 
 #
 # @create: image creation job type, see "blockdev-create" (since 3.0)
 #
+# @bitmap-populate: drive bitmap population job type, see
+#                   "block-dirty-bitmap-populate" (since 5.1)
+#
 # Since: 1.7
 ##
 { 'enum': 'JobType',
-  'data': ['commit', 'stream', 'mirror', 'backup', 'create'] }
+  'data': ['commit', 'stream', 'mirror', 'backup', 'create',
+           'bitmap-populate'] }

 ##
 # @JobStatus:
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 791de6a59c2c..93fb886e7e97 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1231,6 +1231,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,
+                            BlockdevOnError on_error,
+                            int creation_flags,
+                            BlockCompletionFunc *cb, void *opaque,
+                            JobTxn *txn, Error **errp);
+
 BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
                                   const char *child_name,
                                   const BdrvChildClass *child_class,
diff --git a/block/bitmap-populate.c b/block/bitmap-populate.c
new file mode 100644
index 000000000000..901b78b56c3e
--- /dev/null
+++ b/block/bitmap-populate.c
@@ -0,0 +1,207 @@ 
+/*
+ * Async Dirty Bitmap Populator
+ *
+ * Copyright (C) 2020 Red Hat, Inc.
+ *
+ * Authors:
+ *  John Snow <jsnow@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+
+#include "trace.h"
+#include "block/block.h"
+#include "block/block_int.h"
+#include "block/blockjob_int.h"
+#include "block/block_backup.h"
+#include "block/block-copy.h"
+#include "qapi/error.h"
+#include "qapi/qmp/qerror.h"
+#include "qemu/ratelimit.h"
+#include "qemu/cutils.h"
+#include "sysemu/block-backend.h"
+#include "qemu/bitmap.h"
+#include "qemu/error-report.h"
+
+typedef struct BitpopBlockJob {
+    BlockJob common;
+    BlockDriverState *bs;
+    BdrvDirtyBitmap *target_bitmap;
+    BdrvDirtyBitmap *new_bitmap;
+    BlockdevOnError on_error;
+    uint64_t len;
+} BitpopBlockJob;
+
+static const BlockJobDriver bitpop_job_driver;
+
+static void bitpop_commit(Job *job)
+{
+    BitpopBlockJob *s = container_of(job, BitpopBlockJob, common.job);
+
+    bdrv_dirty_bitmap_merge_internal(s->target_bitmap, s->new_bitmap,
+                                     NULL, true);
+}
+
+/* no abort needed; just clean without committing. */
+
+static void bitpop_clean(Job *job)
+{
+    BitpopBlockJob *s = container_of(job, BitpopBlockJob, common.job);
+
+    bdrv_release_dirty_bitmap(s->new_bitmap);
+    bdrv_dirty_bitmap_set_busy(s->target_bitmap, false);
+}
+
+static BlockErrorAction bitpop_error_action(BitpopBlockJob *job, int error)
+{
+    return block_job_error_action(&job->common, job->on_error, true, error);
+}
+
+static bool coroutine_fn yield_and_check(Job *job)
+{
+    if (job_is_cancelled(job)) {
+        return true;
+    }
+
+    job_sleep_ns(job, 0);
+
+    if (job_is_cancelled(job)) {
+        return true;
+    }
+
+    return false;
+}
+
+static int coroutine_fn bitpop_run(Job *job, Error **errp)
+{
+    BitpopBlockJob *s = container_of(job, BitpopBlockJob, common.job);
+    int ret = 0;
+    int64_t offset;
+    int64_t count;
+    int64_t bytes;
+
+    for (offset = 0; offset < s->len; ) {
+        if (yield_and_check(job)) {
+            ret = -ECANCELED;
+            break;
+        }
+
+        bytes = s->len - offset;
+        ret = bdrv_is_allocated(s->bs, offset, bytes, &count);
+        if (ret < 0) {
+            if (bitpop_error_action(s, -ret) == BLOCK_ERROR_ACTION_REPORT) {
+                break;
+            }
+            continue;
+        }
+
+        if (!count) {
+            ret = 0;
+            break;
+        }
+
+        if (ret) {
+            bdrv_set_dirty_bitmap(s->new_bitmap, offset, count);
+            ret = 0;
+        }
+
+        job_progress_update(job, count);
+        offset += count;
+    }
+
+    return ret;
+}
+
+static const BlockJobDriver bitpop_job_driver = {
+    .job_driver = {
+        .instance_size          = sizeof(BitpopBlockJob),
+        .job_type               = JOB_TYPE_BITMAP_POPULATE,
+        .free                   = block_job_free,
+        .user_resume            = block_job_user_resume,
+        .run                    = bitpop_run,
+        .commit                 = bitpop_commit,
+        .clean                  = bitpop_clean,
+    }
+};
+
+
+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)
+{
+    int64_t len;
+    BitpopBlockJob *job = NULL;
+    int64_t cluster_size;
+    BdrvDirtyBitmap *new_bitmap = NULL;
+
+    assert(bs);
+    assert(target_bitmap);
+
+    if (!bdrv_is_inserted(bs)) {
+        error_setg(errp, "Device is not inserted: %s",
+                   bdrv_get_device_name(bs));
+        return NULL;
+    }
+
+    if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
+        return NULL;
+    }
+
+    if (bdrv_dirty_bitmap_check(target_bitmap, BDRV_BITMAP_DEFAULT, errp)) {
+        return NULL;
+    }
+
+    if (pattern != BITMAP_PATTERN_ALLOCATION_TOP) {
+        error_setg(errp, "Unrecognized bitmap pattern");
+        return NULL;
+    }
+
+    len = bdrv_getlength(bs);
+    if (len < 0) {
+        error_setg_errno(errp, -len, "unable to get length for '%s'",
+                         bdrv_get_device_or_node_name(bs));
+        return NULL;
+    }
+
+    /* 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;
+    }
+
+    /* Take ownership; we reserve the right to write into this on-commit. */
+    bdrv_dirty_bitmap_set_busy(target_bitmap, true);
+
+    job = block_job_create(job_id, &bitpop_job_driver, txn, bs,
+                           BLK_PERM_CONSISTENT_READ,
+                           BLK_PERM_ALL & ~BLK_PERM_RESIZE,
+                           0, creation_flags,
+                           cb, opaque, errp);
+    if (!job) {
+        bdrv_dirty_bitmap_set_busy(target_bitmap, false);
+        bdrv_release_dirty_bitmap(new_bitmap);
+        return NULL;
+    }
+
+    job->bs = bs;
+    job->on_error = on_error;
+    job->target_bitmap = target_bitmap;
+    job->new_bitmap = new_bitmap;
+    job->len = len;
+    job_progress_set_remaining(&job->common.job, job->len);
+
+    return &job->common;
+}
diff --git a/blockjob.c b/blockjob.c
index 470facfd47a0..e6ee8376645d 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -56,7 +56,8 @@  static bool is_block_job(Job *job)
     return job_type(job) == JOB_TYPE_BACKUP ||
            job_type(job) == JOB_TYPE_COMMIT ||
            job_type(job) == JOB_TYPE_MIRROR ||
-           job_type(job) == JOB_TYPE_STREAM;
+           job_type(job) == JOB_TYPE_STREAM ||
+           job_type(job) == JOB_TYPE_BITMAP_POPULATE;
 }

 BlockJob *block_job_next(BlockJob *bjob)
diff --git a/MAINTAINERS b/MAINTAINERS
index 955cc8dd5cd0..99651abcac4a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2050,6 +2050,7 @@  S: Supported
 F: include/qemu/hbitmap.h
 F: include/block/dirty-bitmap.h
 F: block/monitor/bitmap-qmp-cmds.c
+F: block/bitmap-populate.c
 F: block/dirty-bitmap.c
 F: block/qcow2-bitmap.c
 F: migration/block-dirty-bitmap.c
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 96028eedcef7..55f5970f7625 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -36,6 +36,7 @@  block-obj-$(CONFIG_LIBSSH) += ssh.o
 block-obj-y += accounting.o dirty-bitmap.o
 block-obj-y += write-threshold.o
 block-obj-y += backup.o
+block-obj-y += bitmap-populate.o
 block-obj-$(CONFIG_REPLICATION) += replication.o
 block-obj-y += throttle.o copy-on-read.o
 block-obj-y += block-copy.o